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

Fix a potential crash when updating skinned primitives. #2931

Merged
merged 1 commit into from
Mar 15, 2024

Conversation

cameronwhite
Copy link
Contributor

This fixes the crash reported in #2809, which contains the example file and steps to reproduce.
We've had this patch in Houdini for a couple weeks and haven't encountered any issues in testing

Description of Change(s)

With 6b3afc2, skinned primitives are now replaced by an entry with no type and no data source instead of being removed, since they have child prims (the skinning computation Sprims).
When doing so, the entry's cache of computed primvars was never cleared.

This could result in a crash in scenarios such as switching to displaying the result of a BakeSkinning operation. In this case, the skinned primitive changes type from e.g. a Mesh (with a computed primvar for the skinned positions) to empty, and then back to Mesh (which should have no computed primvars). Since the entry's primvar cache was never cleared, this incorrectly returns a stale list of computed primvars for the updated mesh which later produces a crash.

When the prim type of an existing entry is being changed, the cached primvars are now also cleared out while removing the old prim from the render index, to produce the same behaviour as if a new entry was inserted

Fixes Issue(s)

  • I have verified that all unit tests pass with the proposed changes
  • I have submitted a signed Contributor License Agreement

With 6b3afc2, skinned primitives are now replaced by an entry with
no type and no data source instead of being removed, since they have
child prims (the skinning computation Sprims).
When doing so, the entry's cache of computed primvars was never cleared.

This could result in a crash in scenarios such as switching to
displaying the result of a BakeSkinning operation. In this case, the
skinned primitive changes type from e.g. a Mesh (with a computed primvar
for the skinned positions) to empty, and then back to Mesh (which should
have no computed primvars). Since the entry's primvar cache was never cleared,
this incorrectly returns a stale list of computed primvars for the
updated mesh which later produces a crash.

When the prim type of an existing entry is being changed, the cached
primvars are now also cleared out while removing the old prim from the
render index.

Bug: PixarAnimationStudios#2809
@jesschimein
Copy link
Contributor

Filed as internal issue #USD-9255

❗ Please make sure that a signed CLA has been submitted!

@cameronwhite
Copy link
Contributor Author

❗ Please make sure that a signed CLA has been submitted!

I should already be registered under the SideFX CLA, and have done a few prior PRs already

@pixar-oss pixar-oss merged commit ea55daf into PixarAnimationStudios:dev Mar 15, 2024
3 of 5 checks passed
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

Successfully merging this pull request may close these issues.

3 participants