Hiding one scenegraph intance causes a separate instance to be drawn as if it was selected

[williamkrick]

Description of Issue

In USDView when I hide a scenegraph instance which is selected, a separate instance draws as selected even though it is not selected.

Steps to Reproduce

1. Unzip and open the attached file instanceAndNot.zip in USD View. In the scene the 5 cubes (1 hidden) are scenegraph instances and the one capsule is not an instance.
2. Select Capsule1 and choose the visibility property in the property window. It's value should be "inherited".
3. In the python interpreter modify Capsule1's visibility to be hidden: usdviewApi.property.Set('invisible')
4. Capsule1 correctly disappears.
5. Select ball_03 and choose the visibility property.
6. Modify the visibility to be hidden in the same way: usdviewApi.property.Set('invisible')
7. ball_03 disappears and a bounding box for ball_03 is drawn. The color of ball_04 incorrectly changes to the selection hilight color.

I can also reproduce this in MayaUSD using VP2RenderDelegate. If you choose to test MayaUSD & VP2RenderDelegate there are some additional issues (objects turn black) which I believe are a VP2RenderDelegate issue not a USD issue.

You've already fixed one similar issue #1461. This one differs in that the wrong selection is only requested immediately following the visibility change. If I change selection or otherwise refresh the object draw it gets the correct answer.

System Information (OS, Hardware)

Windows 10

Package Versions

USD v21.05
Python 3

Build Flags

[spitzak]

Probably related: #444

[jilliene]

Filed as internal issue #USD-6687

[williamkrick]

The Maya team has found another similar issue when modifying a prim's purpose:

To reproduce, use a similar workflow to the above, except instead of modifying the prim's visibility modify it's purpose such that first the prim is hidden (say by switching the purpose from 'default' to 'render'. Then switch the visibility back to 'default'. The selection is incorrectly displayed.

[williamkrick]

We have a workaround for this issue, which is to call populateSelection any time an instance has it's instance indices change. Changing the visibility of an instance changes the instance indices, but also changing if a given instance is instancable changes the instance indices. When I call populateSelection after the instance indices change then the behavior is correct.

I do have a performance issue with the workaround. I don't know when the instance indices change on a selected rprim, so I always have to call populateSelection. If it was safe to call populateSelection during execute then I could do it from Sync, but I believe that is not allowed. If something similar get GetVisibilityChangeCount() was implemented for DirtyInstanceIndex that would help (the DirtyInstanceIndex could still be for something not selected, but at least it would occur infrequently).

[tcauchois]

Yeah, this is all expected: changing visibility of native instances changes instance indices; similarly, changing purpose of native instances will repopulate a part of the scene (since we can't aggregate instances with different purposes), changing instance indices. In both cases, PopulateSelection needs to be called again: the selection buffer stores flattened indices (which change) instead of instance coordinates (which don't change, but can be many integers long).

The idea of adding HdChangeTracker::GetInstanceIndicesChangeCount (to mirror the visibility count) and then using that to gate when you call PopulateSelection seems great to me. I think it's correct to recompute the selection buffer whenever that happens, and aside from visibility changes it should be rare. If it works for you, we'd happily take the PR.

[williamkrick]

So it seems that something similar to GetInstanceIndicesChangeCount already almost exists. GetInstancerIndexVersion covers most of the cases that cause instance indices to change. If I combine the instancer index version and visibility change count then it covers all the workflows I am aware of that can create incorrect states. I'm working on a MayaUSD change now that uses these two trackers to control when I call populate selection.

[tcauchois]

GetInstancerIndexVersion confusingly tracks when instancers are added/removed from the render index. GetVisibilityChangeCount covers when we mark DirtyVisibility on geometry. I guess when the indices change, it's possible the same edits could mark DirtyVisibility on geometry, but in general I wouldn't expect those two counters to cover all of the cases; although I'm glad if they're working as an interim fix.

[williamkrick]

A bit more investigation has revealed that GetInstancerIndexVersion doesn't include enough information to handle all the workflows that modify instance indices, even when combined with GetVisibilityChangeCount. When I modify the Visibility attribute of an instanced rprim the visibility change count is not incremented, and neither is the GetInstanceIndexVersion. The only dirtying that goes on when an instanced prim's visibility changes is DirtyInstance and DirtyInstnaceIndex.

This leads me be believe that GetInstanceIndicesChangeCount is necessary. I'm working on a PR for it now.

One other question: functions like HdChangeTracker::RprimInserted have an initialDirtyState parameter. If that initial dirty state includes DirtyVisibility then _visChangeCount is not incremented. So to really know if visibility has changed you need to track both GetSceneStateVersion and GetVisibilityChangeCount.

This came up with my implementation of GetInstanceIndicesChangeCount. Some of the workflows I have don't dirty the existing instancer. Instead they remove the old instancer and add a new one. The new instancer has an initial dirty state that includes DirtyInstacceIndex. Then there is a MarkRprimDirty call with DirtyInstanceIndex, but that flag is already set and so my new counter is not incremented either. Should those RprimInserted type methods that include an initialDirtyState parameter be checking those flags for visibility changed (and DirtyInstanceIndex) and incrementing the relevant counters?

[tcauchois]

I think it's safest to recreate the selection set whenever you add/remove prims. The visibility change count is used to cache some stuff internally in hydra, but likewise I think those caches get discarded whenever you add/remove prims. To follow the existing pattern you'd keep those calls separate and the app would call both, but I don't think there's a functional difference between combining them vs keeping them separate.