Fix introduction of decimal errors due to double decimal precision using BROTLI

[rboeters]

There is a problem in PotreeConverter that the coordinates (x, y and/or z) in the output differ in the last decimal from the input.

Consider the following example LAZ file (scale 0.001). It contains two points:

1. X: 1.000, Y: 2.000, Z: 3.000
2. X: 1.000, Y: 2.001, Z: 3.000

After conversion with PotreeConverter both points translate to X: 1.000, Y: 2.000, Z: 3.000.

This is due to the fact that Y of point 2 is read from LasZip as 2.0009999999.... Then with a cast to int32_t the last decimals are lost, and becomes 2.000.

decimal_error.zip

This pull request fixes that by applying some rounding.

[m-schuetz]

Thanks for the PR! Can you check the comments?

[rboeters]

Could you please provide a link to the comments? I am struggling to find them.

[m-schuetz]

Here at the bottom: https://github.com/potree/PotreeConverter/pull/611/files
(just one actually)

[m-schuetz]

Hi, can you tell me the reason for initializing min values with 0? As far as I can tell that would make the bounding box minimum 0, even if it is actually a large integer value.

Since min values are int32_t, shouldn't they be initialized with min.x = std::numeric_limits<int32_t>::max(); (instead of int64_t that it was before.

[rboeters]

To be honest this was trial and error.

I had tried what you suggested but then the whole result got scrambled. Like the higher the node level the more points were pulled towards the min xy of the complete dataset. Maybe because the viewer build upon this bug in decoding the Morton code.

Then I tried setting it 0 and the problem of coordinate differences got a little bit less.

The true solution was all the rounding that I introduced in stead of casting to int32_t.

[ollira]

The original initialization appears to be doing this:
min = { -1, -1, -1 };
...as the result from int64_t max value conversions to int32_t.

[ollira]

Perhaps this min struct could just be a constant, or totally eliminated. Point X, Y, Z values are relative to octree bounding box min, and always >= 0, are they not?

[rboeters]

Yes I think you're right. Just left it in there as Markus might want to throw in some other min values. But until now it was -1, -1, -1 indeed.

[Iisus]

Any update on this? It might also fix some issues we've seen with LAS files being rejected because of wrongly classified points as being outside the bbox.

[fq]

I have the same issue with a LAZ file being rejected due to the bounding box check. I did verified the bounding box using lasinfo which shows no point is outside bounding box.

The bounding box check code here did not compare xyz to the original min/max, instead it compared scaled ux/uy/uz to [0,1]. Due to the double decimal precision, a tiny error could be introduced and make ux>1 causing converter reject the LAZ file. Even the x is inside the original min/max, and no error reported by lasinfo.

I find this PR does fix my problem. Is there any update on when this will be merged?