PixarAnimationStudios
Physics parsing utilities
Description of Change(s)
Provides utility function for UsdPhysics parsing, see parsingUtils.dox file for more details.
Fixes Issue(s)
- [x] I have verified that all unit tests pass with the proposed changes
- [x] I have submitted a signed Contributor License Agreement
Filed as internal issue #USD-10246
:heavy_exclamation_mark: Please make sure that a signed CLA has been submitted!
![azure-pipelines[bot] avatar](https://avatars.githubusercontent.com/in/9426?v=4 "Published by a pub.dev verified publisher"){kind=small}
Filed as internal issue #USD-10246
❗ Please make sure that a signed CLA has been submitted!
yep Ales is on our corporate CLA 👍🏼
@jesschimein I did updated the code to latest dev and added some performance optimizations. The code should be ready for a review.
I didn't want to jump I until Pixar had their say, but I would add, as a potential consumer/user of this code, that this is not code SideFX is likely to use. To my eyes it translates and validates an existing (not directly usable) description of a physics sim into another (not directly usable) description, which we will then have to translate into our internal (simulateable) description. We would most likely look at this as "sample code" which we might use to help write our own direct translation from the USD representation to our internal representation.
I don't see any benefit to us that would offset paying the runtime and memory, and additional code complexity cost of doing this intermediate translation. A validator on the other hand (as mentioned in Pixar's comments) would make a lot of sense to us as something that could benefit us and our users.
Thanks for the feedback, so based on the discussions we have, I have these bullet points:
- adjust the code to coding rules
- take care of minor points in the review
- remove the custom traversal class, replace with input paths and exclude paths in the parsing parameters
- remove validation into separate code path
- document more
As looking at this as a sample code, this is perfectly fine. However I am not sure one can parse the data directly into the engine implementation, the parsing first translates into these descriptors, but later on works based on these descriptors to compute some values, like articulation roots, joint bodies etc. I am not sure one can just take the USD data as is and directly create data in the physics engine in general. But yeah I am happy if it serves as a sample code so that there is some consistency about how the physics data can be retrieved from the stage.
I am happy if it serves as a sample code
The intent of the suggestions was to keep this code practical, not to demote it to sample status :)
remove the custom traversal class, replace with input paths and exclude paths in the parsing parameters
To be sure we are on the same page, we talked about removing the parsePrimIterator and returning a usd primrange instead, and the exclude paths, which currently are used for pruning in the iterator would happen in the parser instead. I was under the impression this was a minimal change. In particular, I thought classes which are necessary for parsing, but not for end users, would be moved into the parser.cpp file and not publicly exposed.
the parsing first translates into these descriptors, but later on works based on these descriptors to compute some values, like articulation roots, joint bodies etc.
Are you saying that by doing the exclusion in the parsing, instead of the prim iterator, information required by someone trying to map into their own physics engine would become impossible to obtain, or they would have to resort to copying the code as an example? When we discussed it, I had the impression the impact of moving the iterator "under the hood" wouldn't impact the ability to use the parser in a practical way. I thought the computed values were all available in the descriptors as part of the parsing operation.
Sidebar,
Speaking of sample code, having some would make the answers to the above very obvious. I think it wouldn't have to be running sample code, just written documentation with brief code fragments to illustrate the principles and how one is meant to set up a joint and so on.
Yes, the parsePrimIterator would be gone, the signature of the parsing function would change to this:
USDPHYSICS_API bool LoadUsdPhysicsFromRange(const UsdStageWeakPtr stage, const std::vector<SdfPath>& includePath, const std::vector<SdfPath>& excludePath, UsdPhysicsReportFn reportFn, void* userData, const CustomUsdPhysicsTokens* customPhysicsTokens = nullptr, const std::vector<SdfPath>* simulationOwners = nullptr);
Then yes the traversals would get created in the parsing code, the returned descriptors would not change.
Thanks for the feedback @tallytalwar! I did updated the code and updated to latest develop.