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

Skeletons without SkelBindingAPI do not import animation #3341

Closed
dgovil opened this issue Sep 21, 2023 · 5 comments
Closed

Skeletons without SkelBindingAPI do not import animation #3341

dgovil opened this issue Sep 21, 2023 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@dgovil
Copy link
Collaborator

dgovil commented Sep 21, 2023

Describe the bug
After we upgraded to USD 23.8, Maya USD no longer imports skeleton animation unless the SkelBindingAPI is applied to the Skeleton Prim.

I tried building the latest commit (b48151b) and I see this issue.

This seems like some kind of incompatibility with USD 23.8 that I wanted to flag. I think skeleton animation should always import regardless of the binding.

The docs on UsdSkelBindingAPI also suggest it's only meant to be relevant to the geometry bindings. It's late so I didn't do a deep dive into the code, but it looks like it early exits if it can't get the bindings.

I can understand if skinning/binding information didn't import (though I'd argue even in that case it should still import because it's not a runtime evaluation, and there are likely enough old files that would benefit from it)

Steps to reproduce
Steps to reproduce the behavior:

  1. Import the attached file, it will have animation on the single joint
  2. Edit the file and remove the SkelBindingAPI on line 46 and reimport. It will have no animation

Expected behavior
I think skeletons should still import animation regardless of whether SkelBindingAPI is applied or not.

Attachments
JumpingCube.usda.zip

@dgovil dgovil added the bug Something isn't working label Sep 21, 2023
@dgovil dgovil changed the title Skeletons without SkelBindignAPI do not import animation Skeletons without SkelBindingAPI do not import animation Sep 21, 2023
@dgovil
Copy link
Collaborator Author

dgovil commented Sep 21, 2023

@spiffmon I think this might be a regression in USD 23.8.
I'll have to try and make a sample case before I report it to the USD repo (I'm not super familiar with the SkelCache API), but the Maya USD plugin is basically doing the following (slight pseudocode because it's spread out)

UsdSkelSkeletonQuery skelQuery = UsdSkelCache.GetSkelQuery(skel);
UsdSkelAnimQuery animQuery = skelQuery.GetAnimQuery();
animQuery.GetJointTransformTimeSamplesInInterval(args.GetTimeInterval(), times);

If SkelBindingAPI is not present, it always seems to return 0, whereas if skelBindingAPI is present, it returns the right number of samples

@spiffmon
Copy link
Contributor

The documentation could and should be improved, @dgovil , but like all applied API schemas, no data encoded by the schema is considered "present" unless the API has been applied to the prim - all Hydra 2 behaviors are predicated on that. So, while this is a behavior regression from 23.05, the 23.05 (and earlier) usdchecker will have flagged it as an error, and fixbrokenpixarschemas will have fixed it. I'd argue that if Hydra won't render it, Maya should not import it?

@dgovil
Copy link
Collaborator Author

dgovil commented Sep 21, 2023

In this case though the skeleton has no binding to a mesh, so there's no imageable for binding.

Wouldn't that be a case where you don't need the schema? Though I understand what you mean since animation data is part of the bindingAPI. I wonder if there's room for a Boolean flag to query the animation without the API Schema being present.

In the meantime I'll patch our internal importer to implicitly apply the schema before querying.

@spiffmon
Copy link
Contributor

I'd still disagree, @dgovil : without the SkelBindingAPI applied, it should be as if the skel:animationSource does not exist. Hydra is an example client, but the expectation is one that all clients should be able to rely on. We don't want any processing code needing to check for N different properties on every prim without an explicit declaration by the author that that related packet of data should be considered present.

And skeletons can be imaged by Hydra also!

@dgovil
Copy link
Collaborator Author

dgovil commented Sep 21, 2023

Hmm fair enough. I'll close this issue out and maybe propose adding a warning message instructing users what to do when importing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Status: Done
Development

No branches or pull requests

3 participants