Skip to content

Improvements to closure addition in HW shading#2351

Merged
jstone-lucasfilm merged 3 commits intoAcademySoftwareFoundation:mainfrom
jstone-lucasfilm:main
Apr 23, 2025
Merged

Improvements to closure addition in HW shading#2351
jstone-lucasfilm merged 3 commits intoAcademySoftwareFoundation:mainfrom
jstone-lucasfilm:main

Conversation

@jstone-lucasfilm
Copy link
Copy Markdown
Member

This changelist proposes improvements to the computation of throughput in the GLSL implementation of closure addition, deriving the math from our implementation of albedo scaling in vertical layering.

I'm very interested in feedback and review on this proposal, in order to make sure the assumptions I'm making here are correct.

This changelist proposes improvements to the computation of throughput in the GLSL implementation of closure addition, deriving the math from our implementation of albedo scaling in vertical layering.

I'm very interested in feedback and review on this proposal, in order to make sure the assumptions I'm making here are correct.
@jstone-lucasfilm
Copy link
Copy Markdown
Member Author

@lkerley @niklasharrysson @fpsunflower In thinking through this proposal, I found it helpful to work through a few concrete examples, and I'll share them here:

The sum of two light-admitting lobes is a light-admitting lobe:

throughput_1 = 1.0
throughput_2 = 1.0
throughput_sum = 1.0

The sum of two light-blocking lobes is a light-blocking lobe:

throughput_1 = 0.0
throughput_2 = 0.0
throughput_sum = 0.0

The sum of a light-admitting and light-blocking lobe is a light-blocking lobe:

throughput_1 = 1.0
throughput_2 = 0.0
throughput_sum = 0.0

The sum of two partially-blocking lobes is a more strongly-blocking lobe:

throughput_1 = 0.8
throughput_2 = 0.8
throughput_sum = 0.6

@jstone-lucasfilm
Copy link
Copy Markdown
Member Author

My sense is that this is a good step forward, but let me know if you have additional thoughts or recommendations on the change.

@ld-kerley
Copy link
Copy Markdown
Contributor

I think this could be an improvement, but Its a little hard for my head to wrap itself around - I'm slammed with other things right now, but I'd love to make some pictures and compare them to some sort of reference.

I did find a concrete example that felt like it might not be the most optimal - so its something I think we should look at pictures for (I'll make them when I get a chance if someone else doesn't first).

The sum of two lobes that block exactly half, end up blocking everything.

throughput_1 = 0.5
throughput_2 = 0.5
throughput_sum = 0.0

I think this is where my brain is trying to bring this back to some sort of physical material construction, but for add I'm not sure there is.

I'm not opposed to making this improvement, if we can make some pictures and demonstrate it does improve things (meaning bring them closer to a reference implementation). But I do wonder if we should revisit Niklas suggestion of retiring <add> in favor of something more like a <mix>.

@fpsunflower
Copy link
Copy Markdown

I don't exactly follow the derivation, but the final formula doesn't "feel" right to me either. Have you tried comparing with how layering is implemented in OSL's testrender?

@jstone-lucasfilm
Copy link
Copy Markdown
Member Author

Keep in mind that this logic is for closure addition via add, rather than for physically based layering via the layer node.

@fpsunflower If you have a good reference for closure addition in OSL that we could use, I'd be more than happy to refine our logic based on this.

@jstone-lucasfilm
Copy link
Copy Markdown
Member Author

jstone-lucasfilm commented Apr 22, 2025

@ld-kerley @fpsunflower In order to provide better framing for this proposal, it's worthwhile to highlight that closure addition is a deeply non-physical operator. In contrast to physically based operators such as closure layer and closure mix, the closure add operator allows the author of a shading model full control over the math by which two closures are combined, without needing to obey the rules of energy conservation in each intermediate computation.

Although one might propose that non-physical operators ought to be disallowed entirely in MaterialX and OSL, many industry shading models actually require this level of control, including public models such as PxrSurface and UnrealDefaultLit, as well as internal shading models used within film and game studios.

And although the math for combining reflection responses in closure addition is straightforward -- it's simple addition, after all -- the math for combining throughputs is less so, and it likely falls more in the domain of plausibility and intuition than the domain of physics. It's important that the sum of two closures have a throughput that is no greater than that of either of its two inputs, otherwise adding more and more specular layers would have the illogical effect of increasing the reflectivity of any diffuse layer beneath them in the stack.

This proposal represents one straightforward solution that meets this hard requirement, while providing more intuitive behavior than any of our previous implementations for the cases highlighted above, but I'm certainly open to other proposals that achieve this goal using different math.

@fpsunflower
Copy link
Copy Markdown

I see what you mean. In looking at how testrender handles this -- it may have gotten broken by the recent refactor we did to get testrender running on the GPU. But previously the logic would have just added the throughputs AFAICT. I think our newer code is not handling this case correctly (although maybe I'm missing something subtle).

I think the best course of action would be to come up with concrete unit tests so we can debug the behavior visually.

@jstone-lucasfilm
Copy link
Copy Markdown
Member Author

That makes sense, @fpsunflower, and I'll bet many renderers are in the same camp as testrender, handling this very unusual edge case in an incorrect way.

Although closure addition by itself is commonly used, I suspect it's very rare that a shading model would sum two closures and then perform a physically based layer of that sum over a third closure. It's only in that unusual edge case that the handling of throughput in closure addition would become visible to the artist.

Since this proposal represents one plausible solution to the problem, and it handles edge cases better than any of our previous implementations, my thinking is that we should move forward with this for now.

As we learn more about how other renderers handle this unusual case, we will likely find an even better solution, but I don't see a good reason to hold off on this improvement until we've fully completed that deep dive.

@niklasharrysson
Copy link
Copy Markdown
Contributor

I also have problems wrapping my head around what the physical meaning of throughput would be when doing unbounded addition of two BSDF's.

But since this is a non-physical operation anyway, it's probably not too important as long as the result looks feasible and doesn't explode in the edge case that someone would try to combine this with physical layering.

Your derivation seems logical to me - If two lobe reflectances are added unbounded their directional albedo could also be added (in the same unphysical anyway), so throughput_sum = 1 - (dir_albedo_1 + dir_albedo_2) and the following derivation seems ok to me.

But it would be interesting to see results, comparing this to a path traced reference of the case where an unbounded mix of two BSDFs are layered over a third BSDF.

@ld-kerley
Copy link
Copy Markdown
Contributor

I wonder if perhaps we should consider moving ND_add_bsdf in to the nprlib. It appears that consensus is that it doesn't have any physical basis, and it would allow people to continue using it, but perhaps might persuade people to exploring transitions to a more physically motivated mix based operation?

@jstone-lucasfilm
Copy link
Copy Markdown
Member Author

@ld-kerley That's a very principled suggestion, and certainly worth considering. The main holdup is that closure addition in MaterialX has inputs and outputs of type BSDF, and this data type is currently associated with our Physically Based Shading Node library.

Another option would be to clearly document that closure add is a non-physical operator, even though it may be applied to BSDF inputs that are themselves energy-conserving and physically strict.

Here's our current, very minimal documentation for closure add, and we don't yet mention this key detail at all:

https://github.com/AcademySoftwareFoundation/MaterialX/blob/main/documents/Specification/MaterialX.PBRSpec.md#node-add

@jstone-lucasfilm
Copy link
Copy Markdown
Member Author

Let's move forward for now, in the interest of not making the perfect the enemy of the good, but I'm very interested in ideas for improving this math in the future.

If we ultimately decide that closure addition should be replaced by a better concept in MaterialX and OSL, I'm very open to that as well, but that sounds like a much deeper research topic for our two teams.

@jstone-lucasfilm jstone-lucasfilm merged commit f9c3862 into AcademySoftwareFoundation:main Apr 23, 2025
32 checks passed
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.

4 participants