Physics rigid body mass computation and tests #1677
Conversation
|
Filed as internal issue #USD-7017 |
pxr/usd/usdPhysics/massProperties.h
Outdated
|
|
||
| PXR_NAMESPACE_OPEN_SCOPE | ||
|
|
||
| GfQuatf IndexedRotation(uint32_t axis, float s, float c) |
There was a problem hiding this comment.
@AlesBorovicka , since this is a public header, our coding conventions require that every public function, type, and class, be prefixed with the ProperCase module name, so UsdPhysicsIndexedRotation() etc. We do not allow module namespaces, but do allow a nesting class of static methods, if you prefer. So UsdPhysicsMass::IndexedRotation() etc, if that seems more appealing. Thanks!
There was a problem hiding this comment.
I haven't checked out the test, but left a few things in the code itself to follow up on - thanks, @AlesBorovicka !
pxr/usd/usdPhysics/rigidBodyAPI.cpp
Outdated
|
|
||
| const float COMPARE_TOLERANCE = 1e-05f; | ||
|
|
||
| struct MassApiData |
There was a problem hiding this comment.
Conventions: non-public types prefixed with "_", e.g. _MassApiData
pxr/usd/usdPhysics/rigidBodyAPI.cpp
Outdated
| }; | ||
|
|
||
| // gather all the mass information for the gien prim, based on UsdPhysicsMassAPI | ||
| MassApiData ParseMassApi(const UsdPrim& usdPrim) |
There was a problem hiding this comment.
Conventions: non-public functions prefixed with "_", e.g. _ParseMassApi
pxr/usd/usdPhysics/rigidBodyAPI.cpp
Outdated
| // no sentinel value we need to check for authored value | ||
| if (comAttribute.HasAuthoredValue()) |
There was a problem hiding this comment.
We must have overlooked these properties in our epic discussion about sentinels last year. To recap, predicating computation results on "isAuthored" state is fragile, because simple (common, and unwitting) "breakdown" scene mutations can then change the outcome of a computation. Can we give centerOfMass and any other needful schema properties workable sentinel fallbacks, and then compare to sentinel here rather than HasAuthoredValue?
pxr/usd/usdPhysics/rigidBodyAPI.cpp
Outdated
| } | ||
|
|
||
| // custom get center of mass, using also transformation to apply scaling | ||
| bool GetCoM(const UsdPrim& usdPrim, GfVec3f& com, UsdGeomXformCache& xfCache) |
There was a problem hiding this comment.
Another coding convention: mutable objects passed by pointer rather than reference so that it's easy to tell at calling sites what's boing to be mutated. Of all the conventions I've called out, this is the one that there's some internal contention about, but it is still enforced.
pxr/usd/usdPhysics/rigidBodyAPI.cpp
Outdated
| } | ||
|
|
||
| // compute mass properties for given rigid body | ||
| float UsdPhysicsRigidBodyAPI::ComputeMassProperties(GfVec3f& _diagonalInertia, GfVec3f& _com, GfQuatf& _principalAxes, const MassInformationFn& massInfoFn) const |
pxr/usd/usdPhysics/rigidBodyAPI.cpp
Outdated
| const UsdPrim collisionPrim = *it; | ||
| if (collisionPrim && collisionPrim.HasAPI<UsdPhysicsCollisionAPI>()) | ||
| { | ||
| collisionPrims.push_back(collisionPrim); |
There was a problem hiding this comment.
If you remove the const above, which isn't buying you much, then you can std::move(collisionPrim) here, which does buy you several refcount increments.
pxr/usd/usdPhysics/rigidBodyAPI.cpp
Outdated
| if (!hasMaterials) | ||
| { | ||
| std::vector<UsdShadeMaterial> generalMaterials = UsdShadeMaterialBindingAPI::ComputeBoundMaterials(collisionPrims); | ||
| for (size_t i = 0; i < physicsMaterials.size(); i++) | ||
| { | ||
| if (!physicsMaterials[i]) | ||
| { | ||
| const UsdShadeMaterial& generalMaterial = generalMaterials[i]; | ||
| if (generalMaterial && generalMaterial.GetPrim().HasAPI<UsdPhysicsMaterialAPI>()) | ||
| { | ||
| physicsMaterials[i] = generalMaterial; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
Ok, weird. I added a comment in what must have been a forked branch version of this code that you don't need this second pass, here.
Description of Change(s)
Exposing UsdPhysicsCollisionAPI::GetCollisionPhysicsMaterialPrim that gathers material prim for the collision and added tests for it.
Exposing UsdPhysicsRigidBodyAPI::ComputeMassProperties that can compute mass properties for the rigid body and added tests for it.
Added massProperties.h that include basic mass computer.