LDrawLoader: Fix a corner case when smoothing normals

[gkjohnson]

Related issue: #23157 (comment)

Description

This PR fixes a small visual glitch is normal smoothing that has been bugging me for awhile and I finally took a moment to look into it. The way vertices are determined to be close enough to be merged is via vertex hashing which effectively quantizes all vertices into bins which should be expected to have artifacts at points that lie exactly on these quantization boundaries especially when the points being merged may have matrix operations applied to them incurring error. It's much faster than other more robust methods, though. This was happening in this dish element here:

The points on the top strip of the artifact have y set to 1.7 and the points on the bottom strip are set to 1.6999... which get quantized to 170 and 169 respectively meaning they won't get merged. The inclusion of multiplying the value by 1 + 1e-10 thereby adding a small epsilon to 1.6999... to offset the error and pushes it in to the appropriate value to be merged. This effectively moves the boundaries of the quantization by a very small amount meaning the error could still happen but given the nature of the data in the LDraw files it seems much more unlikely.

If there are other thoughts on how to address this let me know!

Before	After

[yomboprime]

This solution seems a bit hacky. Shouldn't be fine to just change the 1e2 constant to be higher?

I know it is the same, but I use the loader for another contruction set (TENTE) different from LEGO, and this adjustment seems ad hoc.

Perhaps the hashMultiplier should be a new LDrawLoader parameter (variable)?

[yomboprime]

Well, thinking better, forgive it. I guess the adjustment is acceptable as general solution for those small deltas.

[gkjohnson]

Yeah unfortunately it's a pretty finicky problem.

Increasing the multiplier to 1e3, 1e4, 1e5 etc doesn't help as there will be points that sit on the quantization boundary and those multipliers result in it being a round result. I'd considered using something like "Math.round" but that's significantly slower in Firefox (10-20x slower from what I can tell) and just changes the boundary to be at a different spot halfway between the current one which is still likely to have LDraw points specified on it.

Another option was to just add an epsilon to "shift" the boundary by some small amount that would be less likely to coincide with any points. But with the way floating point error works it's offset relative to the sign of the value. So if the value is less than 0 you need to add a negative epsilon and if it's above zero a positive one which this small 1e-10 multiplier does implicitly and without additional conditions. It also has the added bonus of scaling the offset by the scale of the number similar floating point error.

It does look like a magic number but there is some rationale behind it. I think if this seems to work for the models we have at hand we should commit to this internally and look into another solution or loader option if it fails at some point in the future.

[mrdoob]

Thanks!