Skip to content

Conversation

dvicini
Copy link
Member

@dvicini dvicini commented Aug 29, 2025

Description

This PR addresses the issue of the "shadow terminator" for triangle meshes with shading normals. Currently, shading normals might cause artifacts on shadow boundaries for curved surfaces with triangles that are larger than ~1px.

The problem is the inconsistency between shading normal and geometry: the cosine term due to the shading normal might produce a response that is > 0, but the shadow ray could already be occluded by the shape itself. This will manifest in artifacts along the shadow boundary.

Why not just subdivide the geometry? While these artifacts go away for high resolution meshes, in practice this might not always be easy to do (e.g., during differentiable rendering we might render lower resolution geometry). Moreover, Mitsuba itself does not implement any subdivision schemes. Finally, I've found that subdividing really only eliminates the issue once triangles reach the size of individual pixels. Before that, the artifacts are still very much visible.

Method

Algorithm. This PR provides an implementation of https://jo.dreggn.org/home/2021_terminator.pdf. This method offsets the shadow ray origin using a heuristic based on the local planes defined by the vertex normals. If shading and triangle normals are perfectly consistent, this is a no-op. However, if there is a mismatch the position of the shadow ray origin will get adjusted. This effectively increases the shadow ray offset and counteracts some of the most obvious issues. The downside is potential light leaking due to this larger offset. For that reason, it's likely best to allow some level of user override (e.g., this implementation allows to scale the offset).

Implementation. The implementation adds an additional field p_shadow to represent the origin of shadow rays to Interaction3f. The mesh.cpp's compute_surface_interaction computes this using the method of Hanika 2021. The other shapes simply set it to be equal to si.p. One caveat here is that setting this field might be confusing to users (in terms of API). E.g., one might forget to set it and get bad results. The code changes are small - the challenge will just be to avoid surprising or bad results that are hard to track down for users.

Results

Here a few example images that illustrate the problem (left) and improved version (right):

On teapot and matpreview we get a clear smoothing of the shadow boundaries:

teapot_0 000 teapot_1 000 matpreview_0 000 matpreview_1 000

On the bunny.ply and bunny_lowres.ply, the results are a bit disappointing and some artifacts remain:

bunny_0 000 bunny_1 000 bunny_lowres_0 000 bunny_lowres_1 000

Discussion

Curious to hear what others think and if there are better ways of achieving these improvements.

@dvicini
Copy link
Member Author

dvicini commented Sep 1, 2025

I simplified the code using @Speierers suggestions. The change is now a bit more self contained and uses a few instructions less.

@wjakob
Copy link
Member

wjakob commented Sep 3, 2025

Could you explain this new behavior in the documentation? Since m_shadow_offset_scale is a user-tunable parameter, it would also be good to explain what it does and when its value should be changed from the default.

@dvicini
Copy link
Member Author

dvicini commented Sep 4, 2025

Sounds good, will do. I will also try to come up with a unit test

@dvicini dvicini force-pushed the shadow_offset branch 3 times, most recently from 5b85107 to a1b48b3 Compare September 8, 2025 14:12
@dvicini
Copy link
Member Author

dvicini commented Sep 9, 2025

Added documentation, a basic unit test, and cleaned up the code. Just waiting for the CI to pass now to check if this is ready.

mag = dr::detach(dr::mulsign(mag, dr::dot(n, d)));
return dr::fmadd(mag, dr::detach(n), p + dr::detach(ray_offset));
Vector3f shading_offset = dr::select(mag > 0.f, dr::detach(ray_offset), 0.f);
return dr::fmadd(mag, dr::detach(n), p + shading_offset);
Copy link
Member

Choose a reason for hiding this comment

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

Probably it's fine to just do the calculation and then detach the final result. (Easier to read)

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