Fix #96458: Add channel processing for UsdUVTexture #117901

Philipp Oeser · 2024-02-06T15:22:06+01:00

Philipp Oeser commented

2024-02-06 15:22:06 +01:00

This adds the processing required to import and export simple material graphs utilizing the UsdUVTexture outputs channels.

If only r, g or b are specified as output, we hook up a Separate Color node and connect the appropriate channel from there.
(if a is specified as output, the Alpha output of an image texture node was used already)

On the export side, we traverse from the socket to the image texture node, and if a Separate Color on the way, we are using the channel from there to put on the output.

https://openusd.org/release/spec_usdpreviewsurface.html#texture-reader

This adds the processing required to import and export simple material graphs utilizing the UsdUVTexture outputs channels. If only r, g or b are specified as output, we hook up a `Separate Color` node and connect the appropriate channel from there. (if a is specified as output, the Alpha output of an image texture node was used already) On the export side, we traverse from the socket to the image texture node, and if a `Separate Color` on the way, we are using the channel from there to put on the output. https://openusd.org/release/spec_usdpreviewsurface.html#texture-reader

🎉 2

Philipp Oeser added 3 commits 2024-02-06 15:22:15 +01:00

e85a9c50ed WIP

e622f3a816 Merge branch 'main' into 96458

e876e4e061 move channel handling fromconvert_usd_uv_texture to follow_connection

Philipp Oeser added this to the Pipeline, Assets & IO project 2024-02-06 15:22:38 +01:00

Philipp Oeser requested review from Jesse Yurkovich 2024-02-06 15:22:55 +01:00

Philipp Oeser requested review from Michael Kowalski 2024-02-06 15:23:15 +01:00

Philipp Oeser requested review from Charles Wardlaw 2024-02-06 15:23:22 +01:00

Philipp Oeser referenced this pull request

2024-02-06 15:24:33 +01:00

USD Importer - not respecting output channels #96458

brecht changed target branch from main to blender-v4.1-release

2024-02-07 13:01:27 +01:00

Brecht Van Lommel commented

2024-02-07 13:19:19 +01:00

Retargeting for 4.1.

👍 2

Michael Kowalski commented

2024-02-08 00:20:02 +01:00

The changes look good to me and work nicely in my tests. Very nice work!

My one concern is that on import, duplicate Separate Color nodes are created when multiple UsdUVTexture outputs are connected. (This is also an issue with the current code for converting texture scale/bias values, where duplicate Vector Math nodes are created for separate inputs.)

See the attached images for an example. When the original Blender shaders in original.png are exported to USD and the USD is re-imported into Blender, we get an excessive number of nodes, as shown in nodes_from_usd.png.

I have a proposed fix for this in https://projects.blender.org/makowalski/blender/src/branch/fix_duplicate_imported_shaders, in commit

5557f23fd1

The fix uses heuristics to cache and re-use nodes where possible. With this change the nodes are shared on import, as seen in nodes_from_usd_fixed.png.

However, I'm fine with this pull request getting committed as is, and am approving it. I can submit the changes in my fix_duplicate_imported_shaders branch as a separate pull request for review after the current changes go in.

The changes look good to me and work nicely in my tests. Very nice work! My one concern is that on import, duplicate `Separate Color` nodes are created when multiple `UsdUVTexture` outputs are connected. (This is also an issue with the current code for converting texture scale/bias values, where duplicate `Vector Math` nodes are created for separate inputs.) See the attached images for an example. When the original Blender shaders in `original.png` are exported to USD and the USD is re-imported into Blender, we get an excessive number of nodes, as shown in `nodes_from_usd.png`. I have a proposed fix for this in https://projects.blender.org/makowalski/blender/src/branch/fix_duplicate_imported_shaders, in commit https://projects.blender.org/makowalski/blender/commit/5557f23fd1cc903e8a175b30702ee1d790a1d233 The fix uses heuristics to cache and re-use nodes where possible. With this change the nodes are shared on import, as seen in `nodes_from_usd_fixed.png`. However, I'm fine with this pull request getting committed as is, and am approving it. I can submit the changes in my `fix_duplicate_imported_shaders` branch as a separate pull request for review after the current changes go in.

original.PNG

60 KiB

nodes_from_usd.PNG

107 KiB

nodes_from_usd_fixed.PNG

67 KiB

Michael Kowalski approved these changes 2024-02-08 00:21:17 +01:00

Jesse Yurkovich approved these changes 2024-02-08 03:51:22 +01:00

Jesse Yurkovich left a comment

Works well with the scenes and hand-crafted files I put together.

Philipp Oeser commented

2024-02-08 09:14:48 +01:00

My one concern is that on import, duplicate Separate Color nodes are created when multiple UsdUVTexture outputs are connected. (This is also an issue with the current code for converting texture scale/bias values, where duplicate Vector Math nodes are created for separate inputs.)

Yeah, I was mentioning that as well in #96458 (comment)
"it might make sense to cache the Separate Color nodes as well (so we dont create multiple of these for a single texture)"

The fix uses heuristics to cache and re-use nodes where possible. With this change the nodes are shared on import, as seen in nodes_from_usd_fixed.png.

This looks good to me.

However, I'm fine with this pull request getting committed as is, and am approving it. I can submit the changes in my fix_duplicate_imported_shaders branch as a separate pull request for review after the current changes go in.

OK, will merge the PR in a bit then. Thx already for the future followup "duplicate fix" commit!

> My one concern is that on import, duplicate `Separate Color` nodes are created when multiple `UsdUVTexture` outputs are connected. (This is also an issue with the current code for converting texture scale/bias values, where duplicate `Vector Math` nodes are created for separate inputs.) Yeah, I was mentioning that as well in https://projects.blender.org/blender/blender/issues/96458#issuecomment-1114436 "it might make sense to cache the Separate Color nodes as well (so we dont create multiple of these for a single texture)" > The fix uses heuristics to cache and re-use nodes where possible. With this change the nodes are shared on import, as seen in `nodes_from_usd_fixed.png`. This looks good to me. > However, I'm fine with this pull request getting committed as is, and am approving it. I can submit the changes in my `fix_duplicate_imported_shaders` branch as a separate pull request for review after the current changes go in. OK, will merge the PR in a bit then. Thx already for the future followup "duplicate fix" commit!

Philipp Oeser merged commit 228ee8f1c3 into blender-v4.1-release

2024-02-08 10:01:50 +01:00

Philipp Oeser referenced this issue from a commit

2024-02-08 10:01:51 +01:00

Fix #96458: Add channel processing for UsdUVTexture

Philipp Oeser deleted branch 96458

2024-02-08 10:01:56 +01:00

Michael Kowalski referenced this pull request

2024-02-08 16:53:15 +01:00

USD: Fix duplicate shader nodes on import #118002

Sign in to join this conversation.

No reviewers