Skip to content

Conversation

@erikaharrison-adsk
Copy link
Contributor

Description of Change(s)

Reduces the number of shader compilation across materials that differ only by a static color managed value.

There is no need to give color managed static colors to MaterialX as this will require adding CM nodes to convert constant values. Far better to compute the values once and set them into the shader already in the rendering color space.

Also note that MaterialX will not know how to handle a node with two color managed static colors when they differ in the color space name.

Link to proposal (if applicable)

  • N/A

Fixes Issue(s)

  • N/A

Checklist

Reduces the number of shader compilation across materials that
differ only by a static color managed value.

There is no need to give color managed static colors to MaterialX
as this will require adding CM nodes to convert constant values.
Far better to compute the values once and set them into the shader
already in the rendering color space.

Also note that MaterialX will not know how to handle a node with
two color managed static colors when they differ in the color
space name.
@JGamache-autodesk
Copy link
Contributor

@meshula A test case for this change is:
nanocolor_names_test.zip
With expected result:
image
Note that this file does not exercise the new ColorSpaceAPI, we might have a more extensive test coming at some point in the future. It does test a mix of MaterialX and Nanocolor names though.

@JGamache-autodesk
Copy link
Contributor

Tried adding ColorSpaceAPI to that example file:
nanocolor_names_test_with_CS_API.zip
And the color space does not seem to be currently transported by Hydra even though a call to ComputeColorSpaceName returns the expected result at the USD level.

I suppose that work is yet to be done, and requires deciding how the color space gets propagated to descendants. Will we end up with a colorSpace:__color_attr_name__ for each affected attribute, or will we get the equivalent of a colorSpace:__full_prim__ to tell that all GfColor and SdfAsset values in that prim are affected. To be confirmed.

@jesschimein
Copy link
Collaborator

Filed as internal issue #USD-11538

(This is an automated message. See here for more information.)

@spiffmon
Copy link
Member

Just confirming that the Hydra side of ColorSpaceAPI is planned but not yet available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants