-
Notifications
You must be signed in to change notification settings - Fork 44
SdrShaderNode categorization #100
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
base: main
Are you sure you want to change the base?
SdrShaderNode categorization #100
Conversation
Shader TerminalsThere is one piece of information I see missing here which is crucial for shading, and that is how these shaders link up to the RenderDelegates and Material Terminal namespaces. Typically the render delegates specify the materialContext's they support (mtlx, prman, arnold, nsi, glslfx), and thus which terminals they are interested in, with a specified order. However, currently, and in the proposal, there is no link on the shader as to which terminal namespaces are expected (not enforced). To me, this makes sense(if sticking with the current terms) as the "target". Currently the proposal describes MaterialX specifying its target as "MaterialX". I would like to suggest that becomes This does cause some confusion for UsdPreviewSurface in particular; which tends to be supported by all. Typically defined with a terminal namespace/materialContext of It may be that we don't try to shoehorn this into "targets" and have a separate "preferredMaterialContext" or similar field which describes the preferred terminal namespace to have it understood by a renderer as a priority. CollectionsI am also interested to learn more about "collections", the single sentence not quite fully describing their behaviours/design ideas? Is this how a renderer would specify which shaders should appear to an artist when they want to see shaders related to their renderer? Registry method requestAdditionally it would make me happy if there are methods added to the SdrRegistry to "getAllBySubDomain(const TfToken subDomain)" which could then filter (and likely cached for performance reasons) all the shaders related to a certain context; since I imagine this being used frequently to populate user interfaces. (We can then group further by "target") API changesI am also not super keen on the change in behaviour for GetShaderNodeIdentifiers/GetShadeNodeNames. We typically use this to get all the shaders and it works well for returning us a list of all the shaders without filtering applied. These method names make sense for their action, and with the new proposal for family to be more utilised, I see this being cumbersome to have to request all the possible different families of shaders/lights? I may well be misunderstanding the change, but from our usage, we use these methods to get all the shaders, and then we loop through thouse shaders and look at the actual shader items in the registry and retain the ones we care about. If anything I'd prefer the removal of the optional filter altogether from |
|
Hey @JamesPedFoundry , thanks for the feedback! Re: Shader TerminalsReconsidering Here are the concepts at play.
It's becoming clear to me that
We're also re-evaluating More thoughts soon to come on this. Re: CollectionsCollections aren't meant to be renderer-based -- you could use them to specify renderers but I think "renderer" deserves more first-class treatment as discussed above.
Re: Registry method requestWe recently landed a SdrShaderNode Query API. An additional Re: API changes
Thanks for the usage example.
|
|
@JamesPedFoundry and anyone else interested -- I've updated the proposal with a few key modifications
What are your thoughts? |
|
I really like and support the new term Overall, I think it's a nice refresh of the SDR conventions. Also, I thought of another example where |
|
Hi @anwang2009, I think these changes are looking great and agree the clarifications made here will significantly improve the SdrRegistry and our ability to use it as a source of "ground truth" as to what plug-ins, shaders or otherwise are available; as well as being able to produce more user friendly UI's due to standardising the categorisation. I did have a couple of last clarification I wanted to check. The first is that I see the addition of In your examples of data in the registry, example_2 is a math node. It doesn't have the materialX ND_ prefix so am I correct in thinking this would be a separate definition from the ND_Add_float2 and that the ND_ prefixed node would be more like example 4 below?
This would be necessary for the case where we would utilise the registry to discover all rendering material shaders. Cheers, |
|
SdrRegistry has two stages: discovery and parse. Discovery finds shaders in the filesystem and Parse constructs SdrShaderNode from those discovery results. @JamesPedFoundry -- Yes, the math node I put in for example 2 doesn't exist currently, it's just there for illustrative purposes. I checked the SdrRegistry for the
So |
|
Thank you for the clarifications, the addition of ParseAll and ParseAndGetAll does sound like a great addition and thank you for the explanation there. I did wonder what the context would be as I couldn't determine from looking around the MaterialX code base what the However, saying that I wouldn't be adverse to having the function or name specified to group up similar nodes with different input/output types (since there are many "Add" nodes). I wouldn't necessarily block any current progress for this though, as I imagine this is something that can be extended in the future; as you mention in a follow up issue. Thank you again for the proposal descriptions, and answering my questions, I don't think I have any further questions and look forward to this coming into the release line! |
Description of Proposal
We'd like to propose more structured categorization of SdrShaderNodes for clarity, discoverability, and transferability between DCCs.
Link to Rendered Proposal