Skip to content
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

Missing refresh after changing NodeGraph attribute value #2128

Closed
JGamache-autodesk opened this issue Dec 8, 2022 · 2 comments
Closed

Missing refresh after changing NodeGraph attribute value #2128

JGamache-autodesk opened this issue Dec 8, 2022 · 2 comments

Comments

@JGamache-autodesk
Copy link
Contributor

Description of Issue

Editing an attribute on a NodeGraph that is connected as source to a shader node will not cause a material sync.

Steps to Reproduce

  1. Load the following USD scene in usdview
  2. Programmatically change the value of /mtl/UsdPreviewSurface1/NodeGraph1.inputs:diffuseColor to red
  3. BUG: The viewport will not refresh

This one is actually a one line fix right here in usdImaging/delegate.cpp. The code in UsdImagingDelegate::_RefreshUsdObject() that refreshes on changes to UsdShadeShader attributes should also check for changes on UsdShadeNodeGraph attributes.

Scene:

#usda 1.0

def Capsule "Capsule1" (
    prepend apiSchemas = ["MaterialBindingAPI"]
)
{
    rel material:binding = </mtl/UsdPreviewSurface1>
}
def Scope "mtl"
{
    def Material "UsdPreviewSurface1" ()
    {
        token outputs:surface.connect = </mtl/UsdPreviewSurface1/NodeGraph1.outputs:surface>
        def NodeGraph "NodeGraph1" ()
        {
            color3f inputs:diffuseColor = (0.872, 0.5327, 0)
            token outputs:surface.connect = </mtl/UsdPreviewSurface1/NodeGraph1/UsdPreviewSurface1.outputs:surface>
            def Shader "UsdPreviewSurface1" ()
            {
                uniform token info:id = "UsdPreviewSurface"
                color3f inputs:diffuseColor.connect = </mtl/UsdPreviewSurface1/NodeGraph1.inputs:diffuseColor>
                token outputs:surface
            }
        }
    }
}

System Information (OS, Hardware)

Package Versions

Build Flags

@sunyab
Copy link
Contributor

sunyab commented Dec 9, 2022

Filed as internal issue #USD-7824

@ainaerco
Copy link
Contributor

I encountered the same issue and found that usdImaging material adapter could be another place for the fix:
https://github.com/PixarAnimationStudios/OpenUSD/blob/v21.05/pxr/usdImaging/usdImaging/materialAdapter.cpp#L98

if (child.IsA<UsdShadeShader>()) {
    index->AddDependency(cachePath, child);
}

Where we can add NodeGraph as material sprim dependency:

if (child.IsA<UsdShadeShader>() || child.IsA<UsdShadeNodeGraph>()) {
    index->AddDependency(cachePath, child);
}

I'm wondering why this was unnoticed for so long time, don't studios use nested NodeGraphs with interactive rendering?

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

No branches or pull requests

3 participants