gadomski / las-rs

Read and write ASPRS las files, Rust edition.
MIT License
73 stars 32 forks source link

Ensure a properly scaled bounding box #40

Closed Christopher22 closed 2 years ago

Christopher22 commented 2 years ago

First of all, thank you for the amazing work, @gadomski ! Currently, the bounding box of the LAS file may be invalid in some pathologic cases. Consider the points specified in the csv file attached. If the included coordinates are serialized with the default Transform and the resulting file is checked with lasinfo, the software will warn about multiple issues:

lasinfo (210720) report for 'D:\pathologic.las'
[...]
scale factor x y z:         0.001 0.001 0.001
  offset x y z:               0 0 0
  min x y z:                  -2.787 -0.932 -5.806
  max x y z:                  0.609 1.543 -0.094
WARNING: stored resolution of min_x not compatible with x_offset and x_scale_factor: -2.786861896514893
WARNING: stored resolution of min_y not compatible with y_offset and y_scale_factor: -0.932222902774811
WARNING: stored resolution of min_z not compatible with z_offset and z_scale_factor: -5.806345939636231
WARNING: stored resolution of max_x not compatible with x_offset and x_scale_factor: 0.60914021730423
WARNING: stored resolution of max_y not compatible with y_offset and y_scale_factor: 1.542856812477112
WARNING: stored resolution of max_z not compatible with z_offset and z_scale_factor: -0.094414718449116
[...]
[pathological.csv](https://github.com/gadomski/las-rs/files/7442998/pathological.csv)
WARNING: 1 points outside of header bounding box
[...]
WARNING: real max z larger than header max z by 0.000415

While the first warnings regarding the scaling of the bounding box may be neglectable the point outside of the bounding box is it clearly not. For example, it will seriously hinder the visualisation with potree.

This patch scales the bounding box accordingly to the transform, too. As a direct implication there are no longer any problems with the bounding box and floating-point issues.

Christopher22 commented 2 years ago

Amazing, thanks for review! May you upload a new version with the bugfix to crates.io? Such semantic version are so much cleaner than the direct git dependencies in a cargo.toml...

gadomski commented 2 years ago

Done: https://crates.io/crates/las/0.7.5.