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.

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

Delaunay3D/LightmapGI: Improve triangulation #90701

Merged

Conversation

permelin
Copy link
Contributor

Fixes #90138
Fixes #83972

A note about scope

The Delaunay3D class is only used for LightmapGI probes internally, but it is also exposed to GDScript. My focus here is the internal use for probe triangulation.

The issues

  1. Overlapping tetrahedra.
  2. The pruning of thin/small tetrahedra removes too much.
  3. Tetrahedra are stretched out along one or two axes.

Overlapping tetrahedra

In a scene with a large enough volume and probes that are place just a few meters apart, triangulation can break down and generate overlapping tetrahedra.

I struggled to find a fix for this until @lawnjelly recommended quantization. I was skeptical, since quantization can flip a triangle around or obscure the fact that some points are coplanar. That would make things worse, not better. But I was also desperate, so I tried it.

After some tuning I found that with enough bits, it actually did help.

Even though I'm no longer able to provoke invalid triangulation, I don't expect the triangulation to be bullet proof. I'm sure there are still configurations of points that will break it.

Pruned tetrahedra

Once the triangulation is completed, a final pass discards any tetrahedra that are too thin or too small in terms of volume.

Having a fixed cut-off for volume is problematic since points are scaled down to the unit cube. In a 1000x1000 scene, tetrahedra that are several meters thick get pruned. This results in orphan probes that are not connected, and therefore not used. And after a lot of testing, I'm fairly convinced that this pruning serves no purpose. So have removed it, and instead made the thickness based pruning a bit more aggressive.

Stretched triangulation

If the point cloud dimensions are oblong rather than cube shaped, triangulation is proportionally stretched out.

The left shows the current triangulation of random points in a 1000x1000x50 volume. The right is after scaling has been corrected.

delaunay-axis-scaling

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally (with #90701 applied on top), it works as expected.

This testing project previously resulted in 200+ warnings being printed on every bake, now it doesn't result in any warning: test_lightmap_preview_bake_4.x.zip1

Code looks good to me at a glance. Thanks for commenting it thoroughly 🙂

Footnotes

  1. I intentionally placed multiple probes at the same location in the testing project to see if it breaks, but it appears to bake just fine and dynamic object lighting still behaves correctly.

@permelin permelin force-pushed the improve-delaunay3d-triangulation branch from b192f66 to 89d0934 Compare April 15, 2024 16:48
@lawnjelly
Copy link
Member

I haven't looked at this in detail as I'm not familiar, but can you not store the points as e.g. Vector3i (or 16 bit version) and take them out of the float domain completely?

@permelin
Copy link
Contributor Author

You probably could keep the points as integers. I tried it briefly at first but had some issue I couldn't explain.

Since dividing a float with a power of two only moves the binary point to the left by changing the exponent, the quantization is not lost by doing so. The subsequent math done with the points couldn't possibly be kept in the integer domain anyway. And some of the intermediate values when calculating the circumsphere is very, very large. So I thought that I might as well take full advantage of the R128 fixed point instead of using just half of it.

@lawnjelly
Copy link
Member

lawnjelly commented Apr 18, 2024

It is just a suggestion, as I say I haven't examined in detail so take with a pinch of salt.

Keeping as integer / fixed point as much as possible throughout can at least potentially exclude the possibly of more float bugs in other areas. I noticed e.g. you are using an epsilon in simplex_is_coplanar() and I'm wondering whether this is needed in integer space for instance. Likewise in the BSP this could potentially avoid bugs if epsilon is used (you can maybe do which side plane checks in integer math).

That said, I'm sure this is a significant improvement on the current state, so I'm all for incremental improvements too. After all, perfect is the enemy of good enough and all that. 👍

@permelin
Copy link
Contributor Author

I noticed e.g. you are using an epsilon in simplex_is_coplanar()

The input to simplex_is_coplanar() is the original, un-quantized points. Some points that were planar before quantization will then appear not to be, and without this final filter, degenerate tetrahedra would be returned since we're not actually moving the probes to quantized positions.

The point clouds that are triangulated can be nasty. This screenshot is from an MRP. There are probably a thousand edges connecting to the vertex at the bottom right, with a fraction of a degree between them. I just can't imagine how the math would work with keeping every intermediate value quantized and not get atrocious accuracy.

screenshot-24-04-13-00-25-39

@SpockBauru
Copy link

Thank you so much for this! Now I finally feel that #82642 is indeed closed. Speaking on which, I've backported all the 3 related PR's to 4.2.2 code, the files are at in my comment on #82642, hope it helps ^-^

@akien-mga akien-mga merged commit 4b66299 into godotengine:master Apr 22, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants