Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ignore point padding in COPC/laz files #76

Merged
merged 2 commits into from
May 6, 2024
Merged

Ignore point padding in COPC/laz files #76

merged 2 commits into from
May 6, 2024

Conversation

jshrake
Copy link
Contributor

@jshrake jshrake commented May 4, 2024 •
edited
Loading

This PR addresses both #39 and #57:

  • Fixes the Header::offset_to_end_of_points calculation to properly account for the case that the legacy field may be 0.
  • Adds test coverage for a COPC file down sampled with PDAL and sourced from https://github.com/PDAL/data/blob/master/autzen/autzen-classified.copc.laz. These tests initially fail, reproducing the error reported in Test for EVLR offset is not valid for laz #39.
  • Ignores any potential point padding between the end of the point data and the start of the first elvr when the point format is compressed. It's not clear how this check could be supported if we're not able to calculate the uncompressed byte offset of the start of the first elvr.

jshrake added 2 commits May 4, 2024 08:10
@jshrake
Add failing COPC tests cases that reproduce gadomski#39
f5a8823 
- Fixes the raw::Header::offset_to_end_of_points calculation
  to handle the case where the legacy header field
  number_of_point_records is 0.
- Adds a COPC file and failing test cases that reproduce gadomski#39.
- The file is sourced from https://github.com/PDAL/data/blob/master/autzen/autzen-classified.copc.laz
  and downsampled with PDAL.
@jshrake
Ignore point padding for compressed files
5fc354d 
Ignores any potential point padding between the end of the point data and the start of the first elvr if the point format is compressed.

Closes gadomski#39
@gadomski gadomski linked an issue May 5, 2024 that may be closed by this pull request
Test for EVLR offset is not valid for laz #39
Closed
gadomski
gadomski reviewed May 5, 2024
View reviewed changes
Copy link
Owner

@gadomski gadomski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! I have one question regarding the test TODOs. And, to confirm, this PR will close #39, correct?

tests/autzen.rs Show resolved Hide resolved
@jshrake
Copy link
Contributor Author

jshrake commented May 5, 2024

Thanks for the contribution! I have one question regarding the test TODOs. And, to confirm, this PR will close #39, correct?

Correct, this will close #39.

@gadomski gadomski merged commit db6cc85 into gadomski:main May 6, 2024
3 checks passed
@gadomski
Copy link
Owner

gadomski commented May 6, 2024

Awesome, thanks so much for the contribution! Released: https://crates.io/crates/las/0.8.6

@jshrake jshrake deleted the issues/39-fix-reading-laz-with-elvr branch May 6, 2024 13:48
@gadomski gadomski mentioned this pull request Aug 12, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gadomski gadomski gadomski approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Test for EVLR offset is not valid for laz
2 participants
@jshrake @gadomski