-
Notifications
You must be signed in to change notification settings - Fork 203
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
When selection changes mark the prims which are added/removed from the list dirty. #1838
When selection changes mark the prims which are added/removed from the list dirty. #1838
Conversation
@@ -572,6 +565,13 @@ void HdVP2BasisCurves::Sync( | |||
|
|||
const SdfPath& id = GetId(); | |||
|
|||
// Update the selection status if it changed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code was in initRepr, but it is not really any different from other code that has dirty flags so there is not a reason for it to be there.
@@ -1525,11 +1525,6 @@ void HdVP2BasisCurves::_UpdateDrawItem( | |||
*/ | |||
HdDirtyBits HdVP2BasisCurves::_PropagateDirtyBits(HdDirtyBits bits) const | |||
{ | |||
// Visibility and selection result in highlight changes: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DirtySelection is gone, it was never really used properly. There is now only DirtySelectionHighlight and DirtySelectionMode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
This may be a local issue, and I have yet to test it in our tree, but just running the MayaUSD tests I'm getting a crash when running testVP2RenderDelegateGeomSubset:
|
@dj-mcg I'm able to reproduce the crash too with USD 21.11. I had tried to run the test but I can see from my log I ran with 21.08 twice. I'll dig into it, thanks! |
@dj-mcg the crash is caused by an issue I am not sure how to solve. In HdVP2Mesh::Sync() we're casting the HdSceneDelegate* parameter into a UsdImagingDelegate to be able to call ConvertCachePathToIndexPath. In USD 21.11 I can see that the type of the HdSceneDelegate has changed to HdSceneIndexAdapterSceneDelegate. HdSceneIndexAdapterSceneDelegate has a list of HdSceneDelegates in it, and one of those is the UsdImagingDelegate. I don't see any interface on HdSCeneIndexAdapterSceneDelegate for me to access the HdSceneDelegates it holds, so I think I'm not going to be able to get a UsdImagingDelegate anymore. Okay, why do I need to call ConvertCachePathToIndexPath? In HdVP2Mesh we collect all the materials for the rprim to build the full set of geometry requirements necessary to draw. When we call HdRprim::GetMaterialId(), the returned SdfPath is an index path that can be passed into renderIndex.GetSprim(), which allows me to access the material required primvars. The problem comes with geom subsets. I access the geom subsets through the HdMeshTopology, and the materialIds stored there are cache paths. So I have to convert them to index paths before calling renderIndex.GetSprim(), so I need the UsdImagingDelegate. With the debugger attached I can see that the path in the material id is still a cache path, and that the render index holds a map based on the index path, so simply avoiding the call is not going to work. Is there any other way for me to get an index path to the geom subset material id? Or another way for me to get the UsdImagingDelegate? Maybe HdSceneIndexAdapterSceneDelegate::GetMeshTopology can convert the cache paths to index paths when it builds the topology? |
Tracing thought HdStMesh on how it supports geom subsets I eventually got to PrimUtils.pp HdStGetMaterialNetworkShader, which appears to be passing the cache path into renderIndex directly. But when I open a scene with geom subsets like fender_stratocaster.usdz from the AR kit I see all the geom subset materials. So clearly I am missing something. |
In UsdView the render index holds cache paths for sprims! That means it can loop up the geom subset materials directly, without having to convert to index paths. UsdImagingIndexProxy::InsertSprim converts the cache path to an index path before calling HdRenderIndex::InsertSprim. It makes a call to ConvertCachePathToIndexPath. When I run in MayaUSD, _cache2indexPath maps Ah I see. GetDelegateId() returns empty for usdImaging and so the cache path and index path always match! Maybe I can do that too. Or maybe I can write my own ConvertCachePathToIndexPath given that I know the results of GetDelegateId. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, and fixes the issue for us! Thanks!
I logged PixarAnimationStudios/OpenUSD#1687 for the cache path index path mismatch in HdStMesh. |
… Create a different back door for us to access the imaging delegate again, so that sub geom material ids can be converted from cache paths to index paths.
cccdc6f
The new commit uses a different back door to access the UsdImagingDelegate to work around the sub geom material Id issue. |
This change also removes the selected repr from proxyRenderDelegate because nothing actually needs it, and adds more Maya profiler events in mesh::Sync.