-
Notifications
You must be signed in to change notification settings - Fork 18
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
Define Oren-Nayar lobe explicitly as EON model (i.e. energy-compensated Fujii) #174
Define Oren-Nayar lobe explicitly as EON model (i.e. energy-compensated Fujii) #174
Conversation
Although I love the look of the energy-compensated Fujii model, there are some practical motivations for considering this a future update to OpenPBR, rather than a feature of the 1.0 version that is scheduled for release at FMX/SIGGRAPH. One of these motivations is the current implementations of Oren-Nayar in popular closure-based shading languages such as OSL and MDL, which matches the non-energy-compensated QON model. If it were possible to implement your new energy-compensated Fujii model as a pure MaterialX graph, with the existing Oren-Nayar closures used under the hood, then I would have no objection to including this new feature in OpenPBR. But any changes that require rewrites of closures across the industry should be discussed and agreed upon in an industry-wide forum before their inclusion in OpenPBR. |
I note that MaterialX doesn't itself define what Oren-Nayar model it uses (i.e. there are no formulas or references), there is just some code in OSL testrender and MDL. On what basis was the formula in that code chosen (and what model is it)? We do need to state some concrete form of the Oren-Nayar model. I strongly suspect that whatever model we choose now, will persist for the lifetime of OpenPBR. (Arguing to change it once it is used in the wild, will be exponentially harder). Forcing OpenPBR to match the current state of the OSL testrender code, just as a short term convenience rather than based on the merits of the model, seems wrong to me. |
Alas, it's both of the closure-based shading languages in our industry (OSL and MDL) that harmonize on QON in their open implementations, and MaterialX simply follows their example. So we'll need to reach consensus with both the OSL and MDL teams before proposing a change. I really do like the look of your energy-compensated Fujii model proposed above, but it seems far too late in the development of OpenPBR (one week before the FMX conference) to propose a change that will require industry agreement. With our earlier proposals for F82 tint and Zeltner/Burley sheen, we gave the industry more than 6 months to reach consensus and begin authoring their own implementations in preparations for OpenPBR, and I would strongly recommend that we give the industry similar lead time for proposing a new Oren-Nayar model in a future update to OpenPBR. |
But MaterialX spec doesn't say the model is QON, does it? What do you mean by MaterialX follows their example. |
As far as I know, neither of the major closure-based shading languages state the definition of Oren-Nayar in their specification, and it's true that MaterialX does not provide its own definition in its specification. But both OSL and MDL make explicit choices for Oren-Nayar in their open implementations on GitHub, and MaterialX follows their example in its GLSL/ESSL/MSL implementation of Oren-Nayar. |
Is this a possibility that has been explored? Specifically, is there any way we could describe the diffuse lobe as a multilple scattering model based on Lambertian microfacets, treat this energy-compensated Fujii model as the ground truth, and also provide an approximation based on QON? For the approximation, for example, could we for apply a novel scale factor to QON or mix it with a Lambertian lobe that accounts for the missing energy? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR looks good to me, but I'll let people more familiar with the model proof-read the equations.
I just noted a formatting typo though.
Co-authored-by: Julien Guertault <9511025+virtualzavie@users.noreply.github.com> Signed-off-by: Jamie Portsmouth <jamports@mac.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks really good.
As for the closure based languages, as Peter notes, isn't it possible to emulate the model with existing closures?
But even it isn't, choosing an energy conserving model seems the way forward, and like Jamie I'm concerned we will get stuck with an inadequate model otherwise.
@virtualzavie If this math can be accurately modeled using existing closures, then by all means let's make this change to our reference implementation, and we can make it available to the community for validation and visual testing. If it cannot be accurately modeled in this way, then we should move forward with a proposal for a new closure in OSL and MDL, so that OpenPBR can leverage this closure in its reference graph. |
No, I don't think it can be accurately modeled using the existing closures. Since the existing MaterialX spec doesn't define what Oren-Nayar model exactly it uses, but in practice in the OSL/GLSL implementations (and I assume MDL) it uses what I called the QON model, and there is no way to map from QON to FON by changing the input parameters. If we ignore that and just implement the energy compensation in the graph, then we would have some approximation, but it would be some sort of hybrid that doesn't quite pass the white furnace test, and also will be a huge mess in the XML graph. It would be much better to propose it as a new closure, probably via a flag on the existing Oren-Nayar closure (can just be a mode selection string I think, i.e. make the current one be "QON", and add "EON"). |
# Conflicts: # index.html
…/OpenPBR into oren_nayar_definition # Conflicts: # index.html
…/OpenPBR into oren_nayar_definition
I'm looking forward to today's MaterialX TSC meeting, where @portsmouth and @peterkutz plan to propose this functionality as a feature of the Once we have a sense of the future direction of this feature in the industry, we should document this in our OpenPBR specification, so that readers have an understanding of how this feature will be implemented in future versions of MaterialX/OSL/MDL, and giving a sense of what the corresponding changes to our reference implementation will look like. |
@jstone-lucasfilm anything else needed in this PR in order to be mergable, since I think we agreed to go ahead with it following the MaterialX TSC? |
@portsmouth I was imagining that we'd post the new energy-compensated diffuse model as an external whitepaper, so that both the MaterialX and OpenPBR specifications can reference it in a consistent way. This seems like a better precedent for us to follow, aligned with the other novel BSDF lobes that we're adding to both MaterialX and OpenPBR, and it's completely fine if the external whitepaper evolves in the build-up to SIGGRAPH 2024. |
I agree with that, but we are trying to get it published in a journal, so can't reference it immediately, and it would be confusing to informally publish and reference a half-baked whitepaper in the interim. As noted on Slack, i'd prefer to just move forward with this PR (with the concise summary of the formulas, so then the spec is logically consistent), and then as soon as the paper becomes available, swap out the formulas for a reference to the paper which will provide the formulas. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @portsmouth and @peterkutz, this looks good to me, and let's move forward with the merge to main.
67f0ea5
into
AcademySoftwareFoundation:main
Very cool, I derived this independently but never did anything with it, looks exactly the same, nice: https://www.desmos.com/calculator/extaj6cnzr. |
As discussed extensively on Slack, our specification of the diffuse lobe is incomplete since we don't make it clear what form of the Oren-Nayar model to take, and there are several options.
As argued on Slack, I think the model proposed by Fujii makes the most sense, as it:
It is also easy to add straightforward energy compensation so that:
This model is also very easy to implement (no tabulation) and efficient (even more-so than qualitative Oren-Nayar), via an accurate fit thanks to the analytical solution for the albedo.
NB, this model is fairly obvious, but apparently novel, so could be publishable. (Ideally, should be as a companion to the spec).