[Apple] Support MaterialX UsdPrimvarReader in Storm#4091
Closed
ld-kerley wants to merge 1 commit into
Closed
Conversation
This is built on top of a corresponding MaterialX PR, that fixes nested geomprop nodes: AcademySoftwareFoundation/MaterialX#2957 Rendering a UsdPreviewSurface network that reads a primvar through an ND_UsdPrimvarReader_* node (for example varname="st" feeding a UsdUVTexture) failed in Hydra Storm with: HdSt_ApplyMaterialXFilter -- Unable to create the Glslfx Shader. MxException: No "geomprop" value found for geompropvalue node "primvar". That crash exposed a few different issues this PR fixes, all in how hdMtlx/hdSt build the document and emit the glslfx. 1. Preserve the topology-affecting primvar name. The topological classification (_IsTopologicalShader) only recognized the native geompropvalue family, so a UsdPrimvarReader varname was stripped from the anonymized network. 2. Recognize primvar-reader nodes generically (hdMtlx). A node is treated as a primvar reader when its implementation nodegraph wraps a geompropvalue or geompropvalueuniform node. New helpers HdMtlxIsPrimvarReaderNodeDef, HdMtlxGetPrimvarNameInputName and HdMtlxGetPrimvarDefaultInputName discover this and the input names that carry the primvar name and fallback (read from the interfacename on the wrapped geompropvalue inputs, for example varname and fallback for UsdPrimvarReader). These drive three call sites that previously only understood the native geompropvalue node by category or by the hardcoded geomprop/default parameter names: - _IsTopologicalShader now treats any primvar-reader node as topological. - _AddMaterialXNode now registers primvar-reader nodes in hdPrimvarNodes so the primvar is declared and bound. - _UpdatePrimvarNodes now extracts the primvar name and fallback from the correct inputs instead of assuming geomprop/default. Detecting by implementation structure rather than by family name keeps this robust for any future node that wraps a geompropvalue. Over-preserving a parameter is always correctness-safe; the only cost is slightly reduced shader reuse for such nodes, matching how the native geompropvalue node is already handled. 3. Emit glslfx-compatible primvar types (hdSt/materialXShaderGen.cpp). The primvar attributes block of the generated glslfx took its type name from the target shader language syntax. On Metal that produced names like float2, which HioGlslfxConfig does not understand (it accepts float, double, int, vec2, vec3, vec4), so the primvar declaration was rejected, st fell back to a default, and texture coordinates were lost (surfaces rendered untextured). The attributes type is now mapped to a glslfx type independent of the target language. This is a Metal-specific defect that became reachable only once a primvar actually reached shader generation.
ld-kerley
changed the title
Collaborator
|
Filed as internal issue #USD-12267 (This is an automated message. See here for more information.) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of Change(s)
This is built on top of a corresponding MaterialX PR, that fixes nested geomprop nodes:
AcademySoftwareFoundation/MaterialX#2957
Rendering a MaterialX UsdPreviewSurface network that reads a primvar through an ND_UsdPrimvarReader_* node (for example varname="st" feeding a UsdUVTexture) failed in Hydra Storm with:
That crash exposed a few different issues this PR fixes, all in how hdMtlx/hdSt build the document and emit the glslfx.
Preserve the topology-affecting primvar name. The topological classification (
_IsTopologicalShader) only recognized the native geompropvalue family, so theUsdPrimvarReadervarname was stripped from the anonymized network.Recognize primvar-reader nodes generically (hdMtlx). A node is treated as a primvar reader when its implementation nodegraph wraps a geompropvalue or geompropvalueuniform node. New helpers discover this and the input names that carry the primvar name and fallback. Detecting by implementation structure rather than by family name keeps this robust for any future node that wraps a geompropvalue.
Emit glslfx-compatible primvar types (hdSt/materialXShaderGen.cpp). The primvar attributes block of the generated glslfx took its type name from the target shader language syntax. On Metal that produced names like
float2, which HioGlslfxConfig does not understand (it acceptsvec2), so the primvar declaration was rejected. The attributes type is now mapped to a glslfx type independent of the target language.Unit test files are attached here - test_file.zip
Checklist
I have created this PR based on the dev branch
I have followed the coding conventions
I have added unit tests that exercise this functionality (Reference:
testing guidelines)
I have verified that all unit tests pass with the proposed changes
I have submitted a signed Contributor License Agreement (Reference:
Contributor License Agreement instructions)