From 96703418ce79948d774ab6399f8aa54da27827dd Mon Sep 17 00:00:00 2001 From: Alan Bucior Date: Mon, 11 Feb 2019 12:07:28 -0800 Subject: [PATCH] Updated as per comments on https://github.com/PixarAnimationStudios/USD/pull/758 - Removed copy paste error - Static token'ized some of the inline tokens and added them to alembicUtil.h - Checked for faceset's parent existing in alembicWriter - Also made some updates to the unit test. Some cleanups to make it more correct, and I think I'd made some updates there and failed to actually re-run the test, so a few things weren't working in there. Should be good now. --- pxr/usd/plugin/usdAbc/alembicReader.cpp | 18 +++++----- pxr/usd/plugin/usdAbc/alembicUtil.h | 2 ++ pxr/usd/plugin/usdAbc/alembicWriter.cpp | 15 +++------ .../usdAbc/testenv/testUsdAbcFaceset.py | 25 ++++++-------- .../testenv/testUsdAbcFaceset/original.usda | 33 +++++++------------ 5 files changed, 37 insertions(+), 56 deletions(-) diff --git a/pxr/usd/plugin/usdAbc/alembicReader.cpp b/pxr/usd/plugin/usdAbc/alembicReader.cpp index af6a4280dc..68ca7f02bc 100644 --- a/pxr/usd/plugin/usdAbc/alembicReader.cpp +++ b/pxr/usd/plugin/usdAbc/alembicReader.cpp @@ -3348,12 +3348,18 @@ _ReadFaceSet(_PrimReaderContext* context) return; } + // A FaceSet must be a child of another Alembic object. + if (!context->GetObject().getParent()) { + // No parent exists. + return; + } + Type object(context->GetObject(), kWrapExisting); // Add child properties under schema. context->SetSchema(Type::schema_type::info_type::defaultName()); - // Set prim type. This depends on the CurveType of the curve. + // Set prim type. context->GetPrim().typeName = UsdAbcPrimTypeNames->GeomSubset; context->AddProperty( @@ -3366,21 +3372,15 @@ _ReadFaceSet(_PrimReaderContext* context) SdfValueTypeNames->Token, _CopySynthetic(UsdGeomTokens->face)); - TfToken defaultFamilyName("materialBind"); context->AddUniformProperty( UsdGeomTokens->familyName, SdfValueTypeNames->Token, - _CopySynthetic(defaultFamilyName)); + _CopySynthetic(UsdAbcPropertyNames->defaultFamilyName)); _PrimReaderContext parentPrimContext = context->GetParentContext(); - TfToken subsetFamilyAttributeName = TfToken(TfStringJoin(std::vector{ - "subsetFamily", - defaultFamilyName.GetString(), - "familyType"}, ":")); - parentPrimContext.AddUniformProperty( - subsetFamilyAttributeName, + UsdAbcPropertyNames->defaultFamilyTypeAttributeName, SdfValueTypeNames->Token, _CopyFaceSetFamilyType(object)); diff --git a/pxr/usd/plugin/usdAbc/alembicUtil.h b/pxr/usd/plugin/usdAbc/alembicUtil.h index 1973aea884..4f85805b6d 100644 --- a/pxr/usd/plugin/usdAbc/alembicUtil.h +++ b/pxr/usd/plugin/usdAbc/alembicUtil.h @@ -102,6 +102,8 @@ TF_DECLARE_PUBLIC_TOKENS(UsdAbcPrimTypeNames, USD_ABC_PRIM_TYPE_NAMES); #define USD_ABC_GPRIM_NAMES \ (primvars) \ (userProperties) \ + ((defaultFamilyName, "materialBind")) \ + ((defaultFamilyTypeAttributeName, "subsetFamily:materialBind:familyType")) \ /* end */ #define USD_ABC_POINTBASED_NAMES \ ((uv, "primvars:uv")) \ diff --git a/pxr/usd/plugin/usdAbc/alembicWriter.cpp b/pxr/usd/plugin/usdAbc/alembicWriter.cpp index 6dc19d9892..d130130952 100644 --- a/pxr/usd/plugin/usdAbc/alembicWriter.cpp +++ b/pxr/usd/plugin/usdAbc/alembicWriter.cpp @@ -2923,15 +2923,9 @@ _WriteFaceSet(_PrimWriterContext* context) context->GetParent(), SdfAbstractDataSpecId(&parentPath)); - TfToken defaultFamilyName("materialBind"); - TfToken subsetFamilyAttributeName = TfToken(TfStringJoin(std::vector{ - "subsetFamily", - defaultFamilyName.GetString(), - "familyType"}, ":")); - - UsdSamples familyType = - parentPrimContext.ExtractSamples(subsetFamilyAttributeName, - SdfValueTypeNames->Token); + UsdSamples familyType = parentPrimContext.ExtractSamples( + UsdAbcPropertyNames->defaultFamilyTypeAttributeName, + SdfValueTypeNames->Token); // Copy all the samples. typedef Type::schema_type::Sample SampleT; @@ -2954,7 +2948,8 @@ _WriteFaceSet(_PrimWriterContext* context) FaceSetExclusivity faceSetExclusivity = kFaceSetNonExclusive; if (!familyType.IsEmpty()) { - const TfToken& value = familyType.Get(0.0f).UncheckedGet(); + double time = UsdTimeCode::EarliestTime().GetValue(); + const TfToken& value = familyType.Get(time).UncheckedGet(); if (!value.IsEmpty() && (value == UsdGeomTokens->partition || value == UsdGeomTokens->nonOverlapping)) { diff --git a/pxr/usd/plugin/usdAbc/testenv/testUsdAbcFaceset.py b/pxr/usd/plugin/usdAbc/testenv/testUsdAbcFaceset.py index 5e4b8a7102..ce57c9747c 100644 --- a/pxr/usd/plugin/usdAbc/testenv/testUsdAbcFaceset.py +++ b/pxr/usd/plugin/usdAbc/testenv/testUsdAbcFaceset.py @@ -64,17 +64,12 @@ def test_RoundTrip(self): for c, e in zip(faceIndices, expectedValue): self.assertEqual(c, e) - # Validate the indices for faceset2 (which was constant, but promoted - # to a single sample at 0 during conversion) + # Validate the indices for faceset2. indices = faceset2_1.GetIndicesAttr() timeSamples = indices.GetTimeSamples() - expectedTimeSamples = [0.0] - self.assertEqual(len(timeSamples), len(expectedTimeSamples)) - - for c, e in zip(timeSamples, expectedTimeSamples): - self.assertTrue(Gf.IsClose(c, e, 1e-5)) + self.assertEqual(len(timeSamples), 0) expectedFaceIndices = {0.0: [1, 2, 4]} for time, expectedValue in expectedFaceIndices.items(): @@ -104,28 +99,28 @@ def test_RoundTrip(self): faceset1_3 = UsdGeom.Subset(faceset1prim3) # Check the round-tripping of familyTypes. - imageable1 = UsdGeom.Imageable(prim1) imageable2 = UsdGeom.Imageable(prim2) imageable3 = UsdGeom.Imageable(prim3) - # In cube1 we've used "unrestricted" as the familyType. + # In cube1 we've used "partition" as the familyType. This should be + # converted to "nonOverlapping" as it more closely matches the Alembic + # definition of "partition". self.assertEqual(faceset1_1.GetFamilyNameAttr().Get(), 'materialBind') self.assertEqual(faceset2_1.GetFamilyNameAttr().Get(), 'materialBind') self.assertEqual(UsdGeom.Subset.GetFamilyType(imageable1, "materialBind"), - "unrestricted") + "nonOverlapping") - # In cube2 we've used "nonOverlapping" in order to ensure both - # familyTypes can be round-tripped. + # In cube2 we've used "unrestricted". This should come across directly. self.assertEqual(faceset1_2.GetFamilyNameAttr().Get(), 'materialBind') self.assertEqual(faceset2_2.GetFamilyNameAttr().Get(), 'materialBind') self.assertEqual(UsdGeom.Subset.GetFamilyType(imageable2, "materialBind"), - "nonOverlapping") - # We've also added another familyName in addition. We should see this + "unrestricted") + # We've also added another familyName in addition. We should see this # being lost in the round-trip and converted to "materialBind". self.assertEqual(faceset3_2.GetFamilyNameAttr().Get(), 'materialBind') - # In cube3, no familyName or familyType has been specified. Upon + # In cube3, no familyName or familyType has been specified. Upon # round-tripping, a default value of "unrestricted" will appear on the # the default "materialBind" familyName. self.assertEqual(faceset1_3.GetFamilyNameAttr().Get(), 'materialBind') diff --git a/pxr/usd/plugin/usdAbc/testenv/testUsdAbcFaceset/original.usda b/pxr/usd/plugin/usdAbc/testenv/testUsdAbcFaceset/original.usda index e44cc57d05..80d9cb1078 100644 --- a/pxr/usd/plugin/usdAbc/testenv/testUsdAbcFaceset/original.usda +++ b/pxr/usd/plugin/usdAbc/testenv/testUsdAbcFaceset/original.usda @@ -7,13 +7,12 @@ def Mesh "cube1" { float3[] extent = [(-0.5, -0.5, -0.5), (0.5, 0.5, 0.5)] - int[] faceVertexCounts = [4, 4, 4, 4, 4, 4] int[] faceVertexIndices = [2, 3, 1, 0, 6, 7, 3, 2, 4, 5, 7, 6, 0, 1, 5, 4, 3, 7, 5, 1, 6, 2, 0, 4] normal3f[] normals = [(0, 0, 1), (0, 0, 1), (0, 0, 1), (0, 0, 1), (0, 1, 0), (0, 1, 0), (0, 1, 0), (0, 1, 0), (0, 0, -1), (0, 0, -1), (0, 0, -1), (0, 0, -1), (0, -1, 0), (0, -1, 0), (0, -1, 0), (0, -1, 0), (1, 0, 0), (1, 0, 0), (1, 0, 0), (1, 0, 0), (-1, 0, 0), (-1, 0, 0), (-1, 0, 0), (-1, 0, 0)] ( interpolation = "faceVarying" ) - uniform token orientation = "leftHanded" + token orientation = "leftHanded" point3f[] points = [(-0.5, -0.5, 0.5), (0.5, -0.5, 0.5), (-0.5, 0.5, 0.5), (0.5, 0.5, 0.5), (-0.5, -0.5, -0.5), (0.5, -0.5, -0.5), (-0.5, 0.5, -0.5), (0.5, 0.5, -0.5)] ( interpolation = "vertex" ) @@ -21,10 +20,7 @@ def Mesh "cube1" interpolation = "faceVarying" ) uniform token subdivisionScheme = "none" - - int[] faceVertexCounts = [4, 4, 4, 4, 4, 4] - - uniform token subsetFamily:materialBind:familyType = "unrestricted" + uniform token subsetFamily:materialBind:familyType = "partition" def GeomSubset "faceset1" { @@ -40,67 +36,60 @@ def Mesh "cube1" { uniform token elementType = "face" uniform token familyName = "materialBind" - int[] indices = [1, 2, 4]; + int[] indices = [1, 2, 4] } - } def Mesh "cube2" { float3[] extent = [(-0.5, -0.5, -0.5), (0.5, 0.5, 0.5)] - int[] faceVertexCounts = [4, 4, 4, 4, 4, 4] int[] faceVertexIndices = [2, 3, 1, 0, 6, 7, 3, 2, 4, 5, 7, 6, 0, 1, 5, 4, 3, 7, 5, 1, 6, 2, 0, 4] - uniform token orientation = "leftHanded" + token orientation = "leftHanded" point3f[] points = [(-0.5, -0.5, 0.5), (0.5, -0.5, 0.5), (-0.5, 0.5, 0.5), (0.5, 0.5, 0.5), (-0.5, -0.5, -0.5), (0.5, -0.5, -0.5), (-0.5, 0.5, -0.5), (0.5, 0.5, -0.5)] ( interpolation = "vertex" ) uniform token subdivisionScheme = "none" - - int[] faceVertexCounts = [4, 4, 4, 4, 4, 4] - - uniform token subsetFamily:materialBind:familyType = "nonOverlapping" + uniform token subsetFamily:materialBind:familyType = "unrestricted" uniform token subsetFamily:someOtherFamily:familyType = "partition" def GeomSubset "faceset1" { uniform token elementType = "face" uniform token familyName = "materialBind" - int[] indices = [0, 3, 5]; + int[] indices = [0, 3, 5] } def GeomSubset "faceset2" { uniform token elementType = "face" uniform token familyName = "materialBind" - int[] indices = [1, 2, 4]; + int[] indices = [1, 2, 4] } def GeomSubset "faceset3" { uniform token elementType = "face" uniform token familyName = "someOtherFamily" - int[] indices = [0, 1, 2, 3, 4, 5]; + int[] indices = [0, 1, 2, 3, 4, 5] } } def Mesh "cube3" { float3[] extent = [(-0.5, -0.5, -0.5), (0.5, 0.5, 0.5)] - int[] faceVertexCounts = [4, 4, 4, 4, 4, 4] int[] faceVertexIndices = [2, 3, 1, 0, 6, 7, 3, 2, 4, 5, 7, 6, 0, 1, 5, 4, 3, 7, 5, 1, 6, 2, 0, 4] - uniform token orientation = "leftHanded" + token orientation = "leftHanded" point3f[] points = [(-0.5, -0.5, 0.5), (0.5, -0.5, 0.5), (-0.5, 0.5, 0.5), (0.5, 0.5, 0.5), (-0.5, -0.5, -0.5), (0.5, -0.5, -0.5), (-0.5, 0.5, -0.5), (0.5, 0.5, -0.5)] ( interpolation = "vertex" ) uniform token subdivisionScheme = "none" - int[] faceVertexCounts = [4, 4, 4, 4, 4, 4] - def GeomSubset "faceset" { uniform token elementType = "face" - int[] indices = [0, 3, 5]; + int[] indices = [0, 3, 5] } } +