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

EMSUSD-489 fix reparenting crash #3364

Merged
merged 1 commit into from
Oct 5, 2023
Merged

Conversation

pierrebai-adsk
Copy link
Collaborator

There were multiple problems that triggered each others to cause a crash when using relative sub-layers. The problems were:

  • The relative sub-layer was not found.
  • This cause the code that applies the reparenting to all layers to do no work.
  • This would cause the prim to vanish.
  • This would cause the transform command to crash because it did not check that the scene item was valid.

The corrections done are:

  • Fix the getAllSubLayers helper function to use FindRelativeToLayer to find sub-layers, in case they are relative to their parent. Otherwise it would fail to find sub-layers.
  • Modify the applyToAllPrimSpecs, applyToAllLayersWithOpinions and applyToSomeLayersWithOpinions helper functions to return the number of layers that were affected.
  • This allows the caller to detect if no action was taken at all, indicating that an expected action failed.
  • Make the UsdSetMatrix4dUndoableCmd UFE command not crash when the scene item is not found.
  • Make the insert-child UFE command detect if the move to the new parent did no work at all and report an error instead of marching on and deleting the old location.
  • Otherwise, the prim would not be at the new location and would be deleted from its old location, so it owuld vanish.

Add a unit test that try to reparent a prim in a relative sub-layer. It failed without the fixes, now it works.

There were multiple problems that triggered each others to cause a crash when using relative sub-layers. The problems were:
- The relative sub-layer was not found.
- This cause the code that applies the reparenting to all layers to do no work.
- This would cause the prim to vanish.
- This would cause the transform command to crash because it did not check that the scene item was valid.

The corrections done are:
- Fix the getAllSubLayers helper function to use FindRelativeToLayer to find sub-layers, in case they are relative to their parent. Otherwise it would fail to find sub-layers.
- Modify the applyToAllPrimSpecs, applyToAllLayersWithOpinions and applyToSomeLayersWithOpinions helper functions to return the number of layers that were affected.
- This allows the caller to detect if no action was taken at all, indicating that an expected action failed.
- Make the UsdSetMatrix4dUndoableCmd UFE command not crash when the scene item is not found.
- Make the insert-child UFE command detect if the move to the new parent did no work at all and report an error instead of marching on and deleting the old location.
- Otherwise, the prim would not be at the new location and would be deleted from its old location, so it owuld vanish.

Add a unit test that try to reparent a prim in a relative sub-layer. It failed without the fixes, now it works.
@pierrebai-adsk pierrebai-adsk added bug Something isn't working adsk Related to Autodesk plugin labels Oct 3, 2023
Copy link
Collaborator

@seando-adsk seando-adsk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work. Thanks for adding the error checking and unit test.

@seando-adsk seando-adsk added ufe-usd Related to UFE-USD plugin in Maya-Usd and removed adsk Related to Autodesk plugin labels Oct 4, 2023
@pierrebai-adsk pierrebai-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Oct 4, 2023
@seando-adsk seando-adsk merged commit ca89f23 into dev Oct 5, 2023
12 checks passed
@seando-adsk seando-adsk deleted the bailp/EMSUSD-489/reparent-crash branch October 5, 2023 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ready-for-merge Development process is finished, PR is ready for merge ufe-usd Related to UFE-USD plugin in Maya-Usd
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants