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

Hydra fails to update primvars on connection notification #2013

Closed
JGamache-autodesk opened this issue Aug 25, 2022 · 5 comments
Closed

Hydra fails to update primvars on connection notification #2013

JGamache-autodesk opened this issue Aug 25, 2022 · 5 comments

Comments

@JGamache-autodesk
Copy link
Contributor

Description of Issue

When connecting the first file texture to a UsdPreviewSurface shader, the viewport does not refresh correctly.

I have traced this back to UsdImagingDelegate::ApplyPendingUpdates() where a property path is not strong enough to cause a call to _ResyncUsdPrim(usdPath, &indexProxy) which correctly repopulates the mesh and fetches the new "st" primvar requirement.

Would this work as a potential fix? In UsdImagingDelegate::_OnUsdObjectsChanged(), loop on the _infoChanges and resync the prim path of any prim that has a didChangeAttributeConnection flag active in the SdfChangeList::Entry vector.

Steps to Reproduce

  1. Load the attached scene un usdview
  2. Run the following script in the interpreter window:
file_output = UsdShade.Shader(usdviewApi.stage.GetPrimAtPath("/Looks/usdPreviewSurface1SG/file1")).CreateOutput("rgb", Sdf.ValueTypeNames.Color3f)
surf_input = UsdShade.Shader(usdviewApi.stage.GetPrimAtPath("/Looks/usdPreviewSurface1SG/usdPreviewSurface1")).CreateInput("diffuseColor", Sdf.ValueTypeNames.Color3f)
surf_input.ConnectToSource(file_output)

EXPECTED: Viewport refreshes and shows texture on plane
BUG: Viewport refreshes, but plane turns yellow, which is the color at UV (0,0)
WORKAROUND: Deactivating then reactivating the plane prim will show correct texturing

The scene is a quad with a textured UsdPreviewSurface material that only requires connecting the file texture output to the surface to be complete. A graph editor component attached to that scene would expect the viewport to correctly refresh when doing the final connection.

System Information (OS, Hardware)

OS: Windows

Package Versions

USD: 22.08

Build Flags

@JGamache-autodesk
Copy link
Contributor Author

PrimVarIssue.zip

JGamache-autodesk added a commit to Autodesk/maya-usd that referenced this issue Aug 26, 2022
See PixarAnimationStudios/OpenUSD#2013

I do hope to get a better workaround either in the issue comments or on
this USD-interests thread:

https://groups.google.com/g/usd-interest/c/5LZT-IOaOTU/m/XwLzFTq7AgAJ

But, in the meantime, creating a dummy prim under the material is enough to
cause the resync to happen... Feel free to revoke my coding license.
This is ugly.
@tallytalwar
Copy link
Contributor

Filed as internal issue #USD-7602

@tcauchois
Copy link
Contributor

Since resync is potentially an expensive operation, ideally we could handle this by having UsdImaging or HdStorm mark geometry bound to the material with DirtyPrimvar or DirtyMaterialId. HdStorm already knows that if DirtyMaterialId is set it needs to re-compute the PSO of that object, in case the material requires quadrangulation or displacement (for example). Setting DirtyMaterialId could be done either in material sync (where we currently have a check, but extend that to check whether the primvar references have changed) or in UsdImaging, where we have the dependency list for that material and can call MarkMaterialDirty on all dependencies.

If setting DirtyPrimvar or DirtyMaterialId on the mesh doesn't work (and you've verified it's going through the update), the next step is to check the list of primvars the material claims to be looking at, which is retrieved here: https://github.com/PixarAnimationStudios/USD/blob/dev/pxr/imaging/hdSt/primUtils.cpp#L195; it's possible that's not getting updated properly?

@JGamache-autodesk
Copy link
Contributor Author

That is the correct way to find out the primvar info that is currently cached.

However, we need some way to tell the scene delegate that the cache needs to be updated before we start the render delegate Sync phase. One strategic location for a breakpoint is to see if the primvar descriptors are fetched from cache or recomputed. That would be here: https://github.com/PixarAnimationStudios/USD/blob/dev/pxr/usdImaging/usdImaging/delegate.cpp#L2706. We know the cache is refreshed when _GetPrimvarsForMaterial() ends up being called.

So we need a way to ask for a primvar cache refresh without having to flush all cached data for that GPrim. Is there some way to do that without forcing a resync?

@tcauchois
Copy link
Contributor

Ahh, I see what you mean I think. We need to make sure the usdImaging-side primvar filtering gets run again?

In that case, resyncing definitely works and might be the most straightforward. If you want to do something quicker, though... The code we have for resyncing material edits is here: https://github.com/PixarAnimationStudios/USD/blob/dev/pxr/usdImaging/usdImaging/delegate.cpp#L1536. On a material edit, we find the parent UsdShadeMaterial prim and send ProcessPropertyChange to all of its dependents, which should include both the hydra material and the hydra geometry. This is to say that on a material topology change, we should be running "GprimAdapter::ProcessPropertyChange", although it won't flag any of the attribute names as being interesting. You can throw in a line there: if (usdPrim.IsA() || usdPrim.IsA()) { return DirtyPrimvar; } and that should do the trick.

ymesh pushed a commit to ymesh/maya-usd that referenced this issue Nov 20, 2022
See PixarAnimationStudios/OpenUSD#2013

I do hope to get a better workaround either in the issue comments or on
this USD-interests thread:

https://groups.google.com/g/usd-interest/c/5LZT-IOaOTU/m/XwLzFTq7AgAJ

But, in the meantime, creating a dummy prim under the material is enough to
cause the resync to happen... Feel free to revoke my coding license.
This is ugly.
JGamache-autodesk added a commit to Autodesk/maya-usd that referenced this issue Jan 30, 2023
A workaround was created for
PixarAnimationStudios/OpenUSD#2013 which was fixed
in USD 23.02 via PixarAnimationStudios/OpenUSD@e3b963b

This makes sure the workaround ends once we reach that USD version.
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