Autodesk: Fix vtarray unecessary or hidden copies #3549
Conversation
remove change of skeleton API
| @@ -2577,7 +2577,7 @@ HdSceneIndexAdapterSceneDelegate::GetInstancerId(SdfPath const &id) | |||
| } | |||
|
|
|||
| if (instancerIds.size() > 0) { | |||
| instancerId = instancerIds[0]; | |||
| instancerId = instancerIds.AsConst()[0]; | |||
There was a problem hiding this comment.
You could consider cfront as well.
https://openusd.org/dev/api/group___s_t_l___a_p_i.html#gaeea74d6d743836c57fef3d0d4e6c982b
| @@ -2665,7 +2668,7 @@ UsdSkelImagingSkeletonAdapter::_SkelData::ComputePoints( | |||
|
|
|||
| if(TF_VERIFY(_boneMeshPoints.size() == _boneMeshJointIndices.size())) { | |||
|
|
|||
| VtVec3fArray skinnedPoints(_boneMeshPoints); | |||
There was a problem hiding this comment.
I'm surprised this is triggering a detach. The copy constructor should be using a const VtArray&.
Line 281 in 6284755
There was a problem hiding this comment.
I think it's triggered later when skinnedPoints is used, not when it's constructed.
There was a problem hiding this comment.
So you're effectively moving the detach that would've happened here?
GfVec3f* points = skinnedPoints.data();
| for (VtTokenArray::reverse_iterator it = opOrderVec.rbegin() ; | ||
| it != opOrderVec.rend(); ++it) { | ||
| const VtTokenArray opOrderVecConst = opOrderVec.AsConst(); | ||
| for (VtTokenArray::const_reverse_iterator it = opOrderVecConst.rbegin() ; |
There was a problem hiding this comment.
If you're open to the suggestion, crbegin and crend would be less invasive changes.
| @@ -1155,7 +1155,7 @@ UsdGeomPointInstancer::_ComputeExtentAtTimePreamble( | |||
| } | |||
|
|
|||
| // verify that all the protoIndices are in bounds. | |||
| TF_FOR_ALL(iter, *protoIndices) { | |||
There was a problem hiding this comment.
Might be a good opportunity to transition to a ranged base for loop.
for (const int protoIndex : protoIndices->AsConst()){ ... }
There was a problem hiding this comment.
I don't know if it's going away, but there are several open issues / pull requests that are focused on replacing it.
|
Filed as internal issue #USD-10707 (This is an automated message. See here for more information.) |
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Description of Change(s)
The changes contained in this PR were done after checking VtArrays unintended copies with the environment variable
VT_LOG_STACK_ON_ARRAY_DETACH_COPYon several scene including Caldera from Activision and the Pixar usdskel examples.It also introduces a new function
GetJointInverseWorldBindTransformsto benefit from the caching done in the skel definition.This should reduce the number of allocations with scenes leveraging usdskel and instances.
Link to proposal (if applicable)
Fixes Issue(s)
Checklist
I have created this PR based on the dev branch
I have followed the coding conventions
I have added unit tests that exercise this functionality (Reference:
testing guidelines)
I have verified that all unit tests pass with the proposed changes
I have submitted a signed Contributor License Agreement (Reference:
Contributor License Agreement instructions)