Skip to content

Commit

Permalink
Changing 'pointIndices' attribute of BlendShape schema from uint[] to…
Browse files Browse the repository at this point in the history
… int[]

   This makes the property more consistent with other indexing properties in core schemas, at the expense of additional runtime boundary testing.

Fixes #858

(Internal change: 1979602)
  • Loading branch information
sgustafso authored and pixar-oss committed Jun 24, 2019
1 parent d65c08d commit 9a1bb17
Show file tree
Hide file tree
Showing 12 changed files with 82 additions and 49 deletions.
20 changes: 14 additions & 6 deletions pxr/usd/lib/usdSkel/blendShape.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ UsdAttribute
UsdSkelBlendShape::CreatePointIndicesAttr(VtValue const &defaultValue, bool writeSparsely) const
{
return UsdSchemaBase::_CreateAttr(UsdSkelTokens->pointIndices,
SdfValueTypeNames->UIntArray,
SdfValueTypeNames->IntArray,
/* custom = */ false,
SdfVariabilityUniform,
defaultValue,
Expand Down Expand Up @@ -265,12 +265,20 @@ UsdSkelBlendShape::ValidatePointIndices(TfSpan<const int> indices,
std::string* reason)
{
for (size_t i = 0; i < indices.size(); ++i) {
const unsigned pointIndex = indices[i];
if (pointIndex >= numPoints) {
const int pointIndex = indices[i];
if (pointIndex >= 0) {
if (ARCH_UNLIKELY(static_cast<size_t>(pointIndex) >= numPoints)) {
if (reason) {
*reason = TfStringPrintf(
"Index [%d] at element %td >= numPoints [%zu]",
pointIndex, i, numPoints);
}
return false;
}
} else {
if (reason) {
*reason = TfStringPrintf(
"Index [%d] at element %td >= numPoints [%zu]",
pointIndex, i, numPoints);
*reason = TfStringPrintf("Index [%d] at element %td < 0",
pointIndex, i);
}
return false;
}
Expand Down
4 changes: 2 additions & 2 deletions pxr/usd/lib/usdSkel/blendShape.h
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,8 @@ class UsdSkelBlendShape : public UsdTyped
/// authored, the number of elements must be equal to the number of elements
/// in the *offsets* array.
///
/// \n C++ Type: VtArray<unsigned int>
/// \n Usd Type: SdfValueTypeNames->UIntArray
/// \n C++ Type: VtArray<int>
/// \n Usd Type: SdfValueTypeNames->IntArray
/// \n Variability: SdfVariabilityUniform
/// \n Fallback Value: No Fallback
USDSKEL_API
Expand Down
59 changes: 42 additions & 17 deletions pxr/usd/lib/usdSkel/blendShapeQuery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,10 @@ UsdSkelBlendShapeQuery::GetBlendShapeIndex(size_t subShapeIndex) const



std::vector<VtUIntArray>
std::vector<VtIntArray>
UsdSkelBlendShapeQuery::ComputeBlendShapePointIndices() const
{
std::vector<VtUIntArray> indices(_blendShapes.size());
std::vector<VtIntArray> indices(_blendShapes.size());

WorkParallelForN(
_blendShapes.size(),
Expand All @@ -163,7 +163,27 @@ UsdSkelBlendShapeQuery::ComputeBlendShapePointIndices() const
// XXX: Some null blend shapes may be stored on _blendShapes
// to preserve the 'blendShapeTargets' order.
if (blendShape.shape) {
blendShape.shape.GetPointIndicesAttr().Get(&indices[i]);
VtValue val;
if (blendShape.shape.GetPointIndicesAttr().Get(&val)) {
if (val.IsHolding<VtIntArray>()) {
indices[i] = val.UncheckedGet<VtIntArray>();
} else if (val.IsHolding<VtUIntArray>()) {
// Backwards-compatibility:
// pointIndices used to be a uint[],
// but was changed to a int[].
// Convert the old value type.
const VtUIntArray& uindices =
val.UncheckedGet<VtUIntArray>();
indices[i].resize(uindices.size());

auto dst = TfMakeSpan(indices[i]);
for (size_t i = 0; i < dst.size(); ++i) {
const unsigned index = uindices[i];
dst[i] = index >= 0 ?
static_cast<int>(index) : 0;
}
}
}
}
}
});
Expand Down Expand Up @@ -346,7 +366,7 @@ UsdSkelBlendShapeQuery::ComputeDeformedPoints(
const TfSpan<const float> subShapeWeights,
const TfSpan<const unsigned> blendShapeIndices,
const TfSpan<const unsigned> subShapeIndices,
const std::vector<VtUIntArray>& blendShapePointIndices,
const std::vector<VtIntArray>& blendShapePointIndices,
const std::vector<VtVec3fArray>& subShapePointOffsets,
TfSpan<GfVec3f> points) const
{
Expand Down Expand Up @@ -418,30 +438,30 @@ _ComputeRangesFromCounts(const TfSpan<const unsigned>& counts,
/// sufficient to satisfy the given shapes.
size_t
_ComputeApproximateNumPointsForShapes(
const std::vector<VtUIntArray>& indicesPerBlendShape,
const std::vector<VtIntArray>& indicesPerBlendShape,
const std::vector<VtVec3fArray>& offsetsPerSubShape)
{
// Get the max index across all of the shapes.
unsigned maxIndex =
int maxIndex =
WorkParallelReduceN(
0, indicesPerBlendShape.size(),
[&indicesPerBlendShape](size_t start, size_t end, unsigned init) {
for (auto i = start; i < end; ++i) {
for (auto index : indicesPerBlendShape[i]) {
[&indicesPerBlendShape](size_t start, size_t end, int init) {
for (size_t i = start; i < end; ++i) {
for (int index : indicesPerBlendShape[i]) {
init = std::max(init, index);
}
}
return init;
},
[](unsigned lhs, unsigned rhs) {
[](int lhs, int rhs) {
return std::max(lhs, rhs);
});

// Also take the sizes of sub-shapes into account, for non-indexed shapes.
for (const auto& offsets : offsetsPerSubShape) {
maxIndex = std::max(maxIndex, static_cast<unsigned>(offsets.size()));
maxIndex = std::max(maxIndex, static_cast<int>(offsets.size()));
}
return maxIndex + 1;
return maxIndex > 0 ? maxIndex + 1 : 0;
}


Expand All @@ -462,7 +482,7 @@ UsdSkelBlendShapeQuery::ComputePackedShapeTable(
return false;
}

const std::vector<VtUIntArray> indicesPerBlendShape =
const std::vector<VtIntArray> indicesPerBlendShape =
ComputeBlendShapePointIndices();

const std::vector<VtVec3fArray> offsetsPerSubShape =
Expand Down Expand Up @@ -501,8 +521,10 @@ UsdSkelBlendShapeQuery::ComputePackedShapeTable(
}
} else {
// Blend shape is sparse. Only increment indexed points.
for (const unsigned index : indices) {
TF_AXIOM(index < numOffsetsPerPoint.size());
for (const int index : indices) {
TF_DEV_AXIOM(index >= 0 &&
static_cast<size_t>(index) <
numOffsetsPerPoint.size());
numOffsetsPerPoint[index] += numSubShapes;
}
}
Expand Down Expand Up @@ -545,7 +567,7 @@ UsdSkelBlendShapeQuery::ComputePackedShapeTable(
for (size_t pi = 0; pi < offsets.size(); ++pi) {
const GfVec3f& offset = offsets[pi];

TF_AXIOM(pi < nextOffsetIndexPerPoint.size());
TF_DEV_AXIOM(pi < nextOffsetIndexPerPoint.size());

const unsigned offsetIndex = nextOffsetIndexPerPoint[pi];
dst[offsetIndex] = GfVec4f(offset[0], offset[1], offset[2],
Expand All @@ -556,7 +578,10 @@ UsdSkelBlendShapeQuery::ComputePackedShapeTable(
} else {
// Must take indices into account.
for (size_t j = 0; j < indices.size(); ++j) {
const unsigned pointIndex = indices[j];
const int pointIndex = indices[j];
TF_DEV_AXIOM(pointIndex >= 0 &&
static_cast<size_t>(pointIndex) <
nextOffsetIndexPerPoint.size());

const GfVec3f& offset = offsets[j];

Expand Down
4 changes: 2 additions & 2 deletions pxr/usd/lib/usdSkel/blendShapeQuery.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ class UsdSkelBlendShapeQuery
/// ComputeSubShapes().
/// Since the _pointIndices_ property of blend shapes is optional,
/// some of the arrays may be empty.
USDSKEL_API std::vector<VtUIntArray>
USDSKEL_API std::vector<VtIntArray>
ComputeBlendShapePointIndices() const;

/// Compute an array holding the point offsets of all sub-shapes.
Expand Down Expand Up @@ -124,7 +124,7 @@ class UsdSkelBlendShapeQuery
const TfSpan<const float> subShapeWeights,
const TfSpan<const unsigned> blendShapeIndices,
const TfSpan<const unsigned> subShapeIndices,
const std::vector<VtUIntArray>& blendShapePointIndices,
const std::vector<VtIntArray>& blendShapePointIndices,
const std::vector<VtVec3fArray>& subShapePointOffsets,
TfSpan<GfVec3f> points) const;

Expand Down
2 changes: 1 addition & 1 deletion pxr/usd/lib/usdSkel/generatedSchema.usda
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ class BlendShape "BlendShape" (
doc = """**Required property**. Position offsets which, when added to the
base pose, provides the target shape."""
)
uniform uint[] pointIndices (
uniform int[] pointIndices (
doc = """**Optional property**. Indices into the original mesh that
correspond to the values in *offsets* and of any inbetween shapes. If
authored, the number of elements must be equal to the number of elements
Expand Down
2 changes: 1 addition & 1 deletion pxr/usd/lib/usdSkel/schema.usda
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ class BlendShape "BlendShape" (
base pose, provides the normals of the target shape."""
)

uniform uint[] pointIndices (
uniform int[] pointIndices (
doc = """**Optional property**. Indices into the original mesh that
correspond to the values in *offsets* and of any inbetween shapes. If
authored, the number of elements must be equal to the number of elements
Expand Down
Loading

0 comments on commit 9a1bb17

Please sign in to comment.