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

Adding GetAll method to the codegenTemplates for multiple-apply schemas #1773

Closed

Conversation

robpieke
Copy link
Contributor

Description of Change(s)

Adding GetAll method to the codegenTemplates for multiple-apply schemas, both for C++ and Python.

This should enable a workflow such as:

from pxr import Usd

stage = Usd.Stage.CreateInMemory()
prim = stage.DefinePrim('/test')

MyMultipleApplySchema.Apply(prim, 'banana')
MyMultipleApplySchema.Apply(prim, 'apple')
MyMultipleApplySchema.Apply(prim, 'pear')

for api in MyMultipleApplySchema.GetAll(prim):
  api.GetAwesomenessAttr().Set(1.0)

This has been tested by rerunning usdGenSchema in the USD codebase and comparing the results of UsdCollectionAPI.GetAll(prim) to UsdCollectionAPI.GetAllCollections(prim) and verifying the entries in the resulting lists are the same.

Fixes Issue(s)

#1772

Note

I might not yet be covered by a CLA, but that should be addressed shortly :)

…chemas, both for C++ and Python.

This enables a workflow such as:
```
from pxr import Usd

stage = Usd.Stage.CreateInMemory()
prim = stage.DefinePrim('/test')

MyMultipleApplySchema.Apply(prim, 'banana')
MyMultipleApplySchema.Apply(prim, 'apple')
MyMultipleApplySchema.Apply(prim, 'pear')

for api in MyMultipleApplySchema.GetAll(prim):
  api.GetAwesomenessAttr().Set(1.0)
```
@spiffmon
Copy link
Member

@robpieke, could you please also rerun usdGenSchema in pxr/usd/usd to pick up the changes to UsdCollectionAPI, and also augment the doc for GetAllCollections() to to say that it is equivalent to GetAll()... and if SideFx is OK with updating some code eventually, maybe "\deprecate" GetAllCollections() ?

@robpieke
Copy link
Contributor Author

robpieke commented Feb 15, 2022

@robpieke, could you please also rerun usdGenSchema in pxr/usd/usd to pick up the changes to UsdCollectionAPI, and also augment the doc for GetAllCollections() to to say that it is equivalent to GetAll()... and if SideFx is OK with updating some code eventually, maybe "\deprecate" GetAllCollections() ?

Done! Thanks for the speedy feedback!

…sdGenSchema`) and deprecate the manually-crafted `GetAllCollections`.
@spiffmon
Copy link
Member

@robpieke , AFAICT, Pixar has three internal uses of GetAllCollections() that we'll be on the hook to update, but otherwise the HUSD is the only other place I see it in use. And thanks for updating UsdPhysics - I forgot that has multi-apply schemas!

@robpieke
Copy link
Contributor Author

the HUSD is the only other place I see it in use

@spiffmon Yeah, we have about ten usages split between C++ and Python that we'll swap over.

@jilliene
Copy link

Filed as internal issue #USD-7221

@robpieke
Copy link
Contributor Author

robpieke commented May 2, 2022

Just checking to make sure this isn't waiting on me for anything :)

@spiffmon
Copy link
Member

spiffmon commented May 2, 2022

Hi @robpieke , no it's not. We have a backlog of internal and external issues/PR's we're working our way through, and just haven't even gotten to this one yet.

sunyab added 4 commits May 9, 2022 14:03
could lead to invalid memory access.

(Internal change: 2228809)
(Internal change: 2230317)
(Internal change: 2230746)
(Internal change: 2237514)
@crevilla
Copy link

Hi @robpieke
I was asked to review this pull request here at Pixar and I think we'd benefit from having more of the implementation of GetAll centralized as opposed to written explicitly into every multiple apply API schema.
In particular, it'd be useful to have a static protected function in UsdAPISchemaBase:
static TfTokenVector _GetMultipleApplyInstances(const UsdPrim &prim, const TfType &schemaType)
to return all the instance names matching the schema type for a prim so that the generated code only has to populate the vector of schema of objects from this list.
For the shared function's impelementation, the UsdSchemaRegistry has the static functions GetAPISchemaTypeName and GetTypeNameAndInstance to help get the type name of the schema and to parse type name and instance name from an applied schema string respectively.

@robpieke
Copy link
Contributor Author

Hi @crevilla ... sorry it took so long to return to this, but I've taken a stab at centralising the logic as requested.

pixar-oss added a commit that referenced this pull request Sep 1, 2022
Adding `GetAll` method to the `codegenTemplates` for multiple-apply schemas

(Internal change: 2246433)
(Internal change: 2246516)
@sunyab
Copy link
Contributor

sunyab commented Sep 1, 2022

This PR was just merged into the dev branch, not sure why this PR didn't auto close. Closing this out.

@sunyab sunyab closed this Sep 1, 2022
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.

7 participants