LOOKDEVX-1847 - Fix rendering of gltf texture nodes #3295
Conversation
We previously were adding default geompropvalue nodes on dangling texcoord inputs in the source MaterialX document. We now let them be, and process them post shadergen as re-implementation of the texcoord node into geompropvalue code. This allows dealing with the texcoord nodes embedded in the glTf texture nodes added to MaterialX 1.38.6.
| // | ||
| // Copyright Contributors to the MaterialX Project | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
| // |
There was a problem hiding this comment.
Copied from MaterialX GeomPropValueNodeGlsl.cpp but adapted to read the index input of a texcoord node and convert it to USD "st" primvar naming scheme.
| // Copyright Contributors to the MaterialX Project | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
| // | ||
|
|
There was a problem hiding this comment.
Copied from MaterialX GeomPropValueNodeGlsl.h.
| // system. To make these graphs compatible, we will redirect any unconnected input that uses such an | ||
| // auto-connection scheme to instead read a texcoord geomprop called "st" which is the canonical | ||
| // name for UV at index zero. | ||
| void _AddMissingTexcoordReaders(mx::DocumentPtr& mtlxDoc) |
There was a problem hiding this comment.
No longer necessary since we handle that in the shadergen.
|
This allows getting proper VP2 shadergen with MaterialX glTf texture nodes, as described in AcademySoftwareFoundation/MaterialX#1486 |
Also handle USDMTLX_PRIMARY_UV_NAME env var.
| // functor instead of a hardcoded Token. This way we could add the equivalent of this code as a | ||
| // functor in the MaterialX parser, and the _ExtractPrimvarsFromNode function found in | ||
| // pxr\usdImaging\usdImaging\materialParamUtils.cpp would then be able to handle indexed values. | ||
| #if 0 |
There was a problem hiding this comment.
The old code used to try to use the index. With this #if 0, index is not considered at all. Doesn't this create a regression?
There was a problem hiding this comment.
The previous code would try and fail because Hydra would not have provided texcoord data. Here I have a grid with 2 UV sets.
From left to right:
- Grid with texture reading geompropvalue "st"
- Grid with texture reading texcoord at index 0
- Grid with texture reading geompropvalue "st1"
- Grid with texture reading texcoord at index 1
And this is before my changes. I started a discussion in AOUSD forums to see if I can fix that in USD, which would enable me to remove the #if 0.
With the changes in this PR, the last grid will look like the second one for texcoord at index 0, matching the results obtained with usdview.
I will add a unit test for the texcoord case so we can follow future developments on the texcoord 1 front.
Texcoord_1.zip
There was a problem hiding this comment.
Thanks @JGamache-autodesk for the explanation. This will do.
There was a problem hiding this comment.
This one is clearer than the MTLXUVStream test. We can clearly see that using UV1 will render using the "st" stream instead of "st1" as the bottom left one.
| def Shader "texcoord1" | ||
| { | ||
| uniform token info:id = "ND_texcoord_vector2" | ||
| int inputs:index = 1 |
There was a problem hiding this comment.
Will be ignored with current code. Fixes are being discussed.
| float outputs:out | ||
| } | ||
|
|
||
|
|
||
| def Shader "multiply10" |
There was a problem hiding this comment.
Adding the UV multiplier node makes the noise more visible.
There was a problem hiding this comment.
Original image had a noise that was varying so slowly that we could not actually see that it was not varying at all since UV1 was not read correctly. We can now see the UV0 noise and will be able to tell when UV1 gets read correctly.
JGamache-autodesk
added
the
ready-for-merge
|
@seando-adsk Ready for merge. |
seando-adsk
deleted the
gamaj/LOOKDEVX-1847/fix_rendering_of_gltf_texture_nodes
branch
Previous work in PR #3295 broke normal mapping by pessimistically using the arbitrary tangent generator. Bring back the texcoord tangents if we detect that implicit texcoords are going to be used.
We previously were adding default geompropvalue nodes on dangling texcoord inputs in the source MaterialX document.
We now let them be, and process them post shadergen as re-implementation of the texcoord node into geompropvalue code. This allows dealing with the texcoord nodes embedded in the glTf texture nodes added to MaterialX 1.38.6.