ASPRSorg / LAS

LAS Specification
https://www.asprs.org/committee-general/laser-las-file-format-exchange-activities.html
146 stars 17 forks source link

Publish LAS 1.4 R14 #76

Closed esilvia closed 5 years ago

esilvia commented 5 years ago

LAS 1.4 R14 is ready for final comment before publication. Please post any comments on this thread and we'll do our best to address your feedback promptly.

Here's a summary of the changes with respect to R13, the current version (issue numbers included):

Noteworthy changes:

Clarification or usability improvements:

In accordance with the requirements for a Revision without incrementing the Version Number, there are no substantive changes that significantly impact the interpretation of the LAS format itself.

PDF here: https://s3.amazonaws.com/asprs-las/LAS-specification-5ae7567ae98f953674c09b078596f72e0a08a223.pdf

Line by line comparisons: https://github.com/ASPRSorg/LAS/pull/76/files

rapidlasso commented 5 years ago

Turns out FUGRO implemented the triples in the Extra Bytes. For more info see my comment here: (https://github.com/ASPRSorg/LAS/issues/1)

rapidlasso commented 5 years ago

For the "extra bytes" scale factor and offset, should we state that the scale factor (if not used) should be set to 1.0 and that the offset (if not used) should be set to 0.0. I came across a data set that left the scale factor at zero and merely indicated in the 'options' that the scale factor was not set. This broke my code that always multiplied. Having a default value of 1.0 may safe future implementation from having that bug.

esilvia commented 5 years ago

The spec states in its current form that unused scale, offset, and nodata values should be zero if unused as indicated in the options field, which it sounds like is the case for your sample dataset. Since I don't see any ambiguity in its current form, I'm not sure that I can or should change that in a revision. Do you feel differently @rapidlasso ?

rapidlasso commented 5 years ago

Not strongly. But the best "zero" for a scale factor is a value of "1.0". It was just a poor decision / oversight when I wrote that part of the "Extra Bytes" specification back then and I wish I had formulated it like that.

manfred-brands commented 5 years ago

If we do want to simplify, then in that case one could deprecate the flags for offset/scale factor. They are always there and set to 0.0/1.0 as default. We should not have both a flag that indicates not to use them and a default value for those that ignore the flag.

rapidlasso commented 5 years ago

Here I strongly disagree. That would really change the specification as this also affects used scale factors (that may no longer get used because the flag is zero). My suggested change only affects the zero setting of unused scale factors.

manfred-brands commented 5 years ago

I agree that from a compatibility stand my suggestion is not good. But dictating a useful value for fields that are not to be used will lead to developers ignoring that flag. Changing a spec is a whole new way to solve bugs in code.

esilvia commented 5 years ago

I've chewed on this for a bit and I think it's best to keep the scale defaulting to zero. When we implement a "Compact Extra Bytes" VLR then I think we should take this into consideration.

esilvia commented 5 years ago

Per a discussion on #1 regarding #4, I'm leaving this PR open for another day or two to ensure that we have consensus on R14. Thus far I believe we're clear to proceed with publication.

rapidlasso commented 5 years ago

The change from raw anytype to scaled and offset double values for the min / max values will require LASlib (and maybe other implementations) to make functional changes. How are we supposed to know whether an existing LAS/LAZ file uses the old anytype convention that is currently valid versus the new double convention that is soon to be valid once R14 is ratified? Here are the current functions used in the LASlib and LASzip APIs for this:

https://github.com/LAStools/LAStools/blob/master/LASzip/src/lasattributer.hpp#L119 https://github.com/LAStools/LAStools/blob/master/LASzip/src/lasattributer.hpp#L132 https://github.com/LAStools/LAStools/blob/master/LASzip/src/lasattributer.hpp#L277

manfred-brands commented 5 years ago

@rapidlasso comments repeat what I had previously said at #1

No matter the intention, the R14 changes the type of the min/max fields from R13 without anywhere an indication in the file that it is a different version. CARIS stores min/max values as the field type. LASLib interprets them as such as well.

rapidlasso commented 5 years ago

CARIS stores min max doubles or min max anytype? I don't think LASlib / LAStools actually use those values anywhere at the moment.

manfred-brands commented 5 years ago

anytype:

Value interpreted as anytype: Beam (UInt32) MinimumValue=1 MaximumValue=512 NoDataValue=4294967295 Profile (UInt32) MinimumValue=1 MaximumValue=576 NoDataValue=4294967295

Values interpreted as double: Beam (UInt32) MinimumValue=4.94065645841247E-324 MaximumValue=2.52961610670718E-321 NoDataValue=4294967295 Profile (UInt32) MinimumValue=4.94065645841247E-324 MaximumValue=2.84581812004558E-321 NoDataValue=4294967295

manfred-brands commented 5 years ago

@rapidlasso some lasreader code is calling set_min/max and update_min/max. (_ply/_qfit/_txt). In the current version it is calling lasattributer.cast which converts the stored value to anytype.

rapidlasso commented 5 years ago

Yes. LASlib / LASzip can set anytype min/max values when writing. But I have no tool that uses the written values for any post processing. So I would currently not notice when these values are broken.

esilvia commented 5 years ago

You're right. A revision shouldn't change the field type regardless of the original intent. I'll revert #4 for R14 and defer it to 1.5.

The original post now has updated PDF and commit comparison links.

esilvia commented 5 years ago

Now that the ExtraByte min/max objection has been resolved, this is the last call for comments before LAS 1.4 R14 gets released. If I receive no more objections, I'll merge the pull request, remove the DRAFT tags, and send the PDF to ASPRS for publication.