Skip to content

Ellipsoids shape for volumetric primitives #1464

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

Merged
merged 23 commits into from
May 12, 2025
Merged

Conversation

Speierers
Copy link
Member

@Speierers Speierers commented Jan 22, 2025

This PR adds support for two new shapes in Mitsuba that can be used to efficiently render volumetric primitives (e.g. 3D Gaussians): ellipsoids and ellipsoidsmesh. Those shapes defines a point cloud of anisotropic ellipsoid primitives
using specified centers, scales, and quaternions. The former employs a closed-form ray-intersection formula with backface culling while the later uses a mesh-based representation to leverage hardware acceleration ray-tracing.

This PR also provides volprim_rf_basic, an example integrator that renders the set of ellipsoids as a radiance field, similar to 3D Gaussian splatting. The more advanced integrators described in the Don't Splat Your Gaussian paper as well as example scripts will be open-sourced in a seperate repository.

Other contributions included in this PR:

  • Add support for Shape::eval_attribute_X which returns a dynamic array of attributes
  • Add the ParamsFlag::ReadOnly
  • Add mitsuba.testing.RenderingRegressionTest for easily create regression tests similar to test_render.py
  • Add the infrastruture to support per-shape backface-culling (currently only used for the ellipsoidsmesh shape.

@Speierers Speierers marked this pull request as ready for review January 23, 2025 09:32
Copy link
Member

@wjakob wjakob left a comment

Choose a reason for hiding this comment

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

Hi Sébastien,

thank you for this PR, it looks extremely useful. I left some feedback.

High level questions: can you explain why backface culling is such an important feature when rendering gaussians, and why that excludes LLVM from using the mesh version of the plugin? Could there not be some workaround?

I noticed that there are various TODOs in the code related to backface culling.

* Taken from "Precision Improvements for Ray/Sphere Intersection", Ray Tracing Gems 2
* \return \c true if a solution could be found
*/
template <typename Value>
Copy link
Member

Choose a reason for hiding this comment

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

What is the role of this function compared to solve_quadratic above? That one also includes improvements to avoid floating point cancellation errors when resolving the two roots.

Copy link
Member Author

Choose a reason for hiding this comment

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

In our early experiments, the new function yield better accuracy. Although I should probably test this again now that the code was cleanedup and many other bugs were fixed.

Copy link
Member

Choose a reason for hiding this comment

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

If the new function is more accurate, then we should probably remove the old one.

Copy link
Member Author

Choose a reason for hiding this comment

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

@wjakob As described in this article, it is more precise, but seems to be specific to the ray-sphere intersection problem. Should we maybe name that function differently?

https://link.springer.com/content/pdf/10.1007/978-1-4842-4427-2_7.pdf

Copy link
Member

@njroussel njroussel Apr 30, 2025

Choose a reason for hiding this comment

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

I've read through the relevant pieces of code and the article. There's two numerical tricks:

  1. "If the sphere is small in relation to the distance to the ray origin": I had already implemented this in the sphere intersection routines, the idea is to first do a ray-plane intersection to move the ray origin on the same plane as the sphere center. This does not require any changes in solve_quadratic, it can be done by the caller. I imagine this one is the more important one for 3DGS-like scenes.
  2. "If the ray is close to a huge sphere": We already have this trick in the current solve_quadratic. It basically looks to find the solution with a larger magnitude, and then find the second solution thanks to the identity x1 * x2 = c / a.

(The tricks have some nice geometrical interpretations, but that doesn't mean that they are only applicable to ray-sphere intersections - they are both applicable to any quadratic solver as far as I can tell).

I'm going to push a few commits: remove improved_solve_quadratic, apply the first trick in the ellipsoid intersection routines. That should make the code cleaner and improve the numerical precision of any call to solve_quadratic. I'll ping @Speierers once that's done so we can double-check that it's just as robust.

Note: I think we should also be using true divisions, like it was already done in solve_quadratic, rather than a multiplication with a dr::rcp() as it's a numerical approximation with PTX.

@wjakob
Copy link
Member

wjakob commented Jan 31, 2025

Is mitsuba.testing.RenderingRegressionTest a replacement for test_render.py, i.e., does it make sense to migrate the other code to this new class? Duplication is always better to avoid, so I am wondering whether there is a strong reason to have both.

@Speierers
Copy link
Member Author

Is mitsuba.testing.RenderingRegressionTest a replacement for test_render.py, i.e., does it make sense to migrate the other code to this new class? Duplication is always better to avoid, so I am wondering whether there is a strong reason to have both.

Indeed this could be a replacement for test_render.py. I just didn't have the time to port of the test_render.py code to this new class.

@Speierers
Copy link
Member Author

High level questions: can you explain why backface culling is such an important feature when rendering gaussians, and why that excludes LLVM from using the mesh version of the plugin? Could there not be some workaround?

When working with volumetric primitives, you want to find them all along the ray, but you only want to account for them once (front face). Hence we rely on backface culling to avoid intersecting the primitives a second time.

Since Embree doesn't support per-shape backface culling for triangle meshes, the LLVM modes can't use the ellipsoidsmesh plugin.

A workaround would be to emulate backface culling at the integrator level, handling "null-interaction" when intersecting the backface of a volumetric primitive. This would add a lot of unwanted code complexity. Any other ideas are welcome!

@wjakob
Copy link
Member

wjakob commented Feb 10, 2025

Hi Sébastien -- regarding backface culling: I think that this can be done in Embree via the rtcSetGeometryIntersectFilterFunction function. Here is an example on how to do this in the OpenUSD repo: https://github.com/PixarAnimationStudios/OpenUSD/blob/release/pxr/imaging/plugin/hdEmbree/mesh.cpp#L716

@Speierers
Copy link
Member Author

Is mitsuba.testing.RenderingRegressionTest a replacement for test_render.py, i.e., does it make sense to migrate the other code to this new class? Duplication is always better to avoid, so I am wondering whether there is a strong reason to have both.

It could be a replacement, but I didn't make this change as test_render.py performs the Z-test in XYZ color space, while this new infrastructure works in RGB color space. This would require re-generating all the reference images, which could be done, but probably in a separate PR after this one lands 👍

@Speierers
Copy link
Member Author

Hi Sébastien -- regarding backface culling: I think that this can be done in Embree via the rtcSetGeometryIntersectFilterFunction function. Here is an example on how to do this in the OpenUSD repo: https://github.com/PixarAnimationStudios/OpenUSD/blob/release/pxr/imaging/plugin/hdEmbree/mesh.cpp#L716

Thanks @wjakob for this pointer. I managed to get it to work with Embree using the geometry intersection filters! I will update the PR with these changes sometime next week.

@Speierers Speierers force-pushed the ellipsoids_release branch from a1ba929 to 3ade6d8 Compare April 3, 2025 08:38
Copy link
Member

@njroussel njroussel left a comment

Choose a reason for hiding this comment

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

Hi @Speierers

I haven't finished reviewing everything quite yet, I still need to go trough the meat of it.

I think you made some slight mistakes in your last rebase. We've been using another branch that I rebased myself. It's mostly small fixes in the OptiX pipeline setup in scene_optix.inl. There's also a significant rewrite happening of that piece of code happening in #1561. (I want to get that PR merged before this one).

If it's ok with you, I can keep this branch up to date by re-doing & pushing the rebase (and the future ones). For any other changes or questions I have, I would tag you on the relevant parts of the code, of course.

We also noticed that some tests in test_volprim_rf_basic.py wasn't passing prior to the most recent rebase. I haven't dug into it at all yet, but do you remember something?

Relevant branches:

@wjakob wjakob force-pushed the master branch 5 times, most recently from ff60350 to 4504654 Compare April 15, 2025 04:19
@njroussel njroussel force-pushed the ellipsoids_release branch from 3ade6d8 to 9cb9df1 Compare April 28, 2025 11:54
Copy link
Member

@njroussel njroussel left a comment

Choose a reason for hiding this comment

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

I've rebased the branch and applied some of the changes we've discussed so far.

@njroussel njroussel force-pushed the ellipsoids_release branch 3 times, most recently from 4842138 to 7356409 Compare May 6, 2025 14:39
@njroussel njroussel force-pushed the ellipsoids_release branch from 7356409 to 1967396 Compare May 7, 2025 21:55
njroussel added 2 commits May 9, 2025 14:43
Newer version of pytest pickup all `__init__.py` files, which would also re-inmport the main mitsuba one. This produced errors when running the entire test suite. This commit moves the tests to another folder and explicitly excludes the python folder in pytest's configuration.
@njroussel njroussel force-pushed the ellipsoids_release branch from 1967396 to d7dd423 Compare May 9, 2025 13:54
@njroussel
Copy link
Member

Finally good to go. Thanks a lot @Speierers 🎉

@njroussel njroussel merged commit 43b0062 into master May 12, 2025
5 checks passed
@njroussel njroussel deleted the ellipsoids_release branch May 12, 2025 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants