Updated as per comments on #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

@@ -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<std::string>{
-        "subsetFamily",
-        defaultFamilyName.GetString(),
-        "familyType"}, ":"));
-
     parentPrimContext.AddUniformProperty(
-        subsetFamilyAttributeName,
+        UsdAbcPropertyNames->defaultFamilyTypeAttributeName,
         SdfValueTypeNames->Token,
         _CopyFaceSetFamilyType(object));
 

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")) \

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<std::string>{
-        "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<TfToken>();
+        double time = UsdTimeCode::EarliestTime().GetValue();
+        const TfToken& value = familyType.Get(time).UncheckedGet<TfToken>();
         if (!value.IsEmpty() && 
             (value == UsdGeomTokens->partition || 
              value == UsdGeomTokens->nonOverlapping)) {

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')

pxr/usd/plugin/usdAbc/testenv/testUsdAbcFaceset/original.usda

@@ -7,24 +7,20 @@
 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"
     )
     float2[] primvars:uv = [(0, 1), (1, 1), (1, 0), (0, 0), (0, 2), (1, 2), (1, 1), (0, 1), (0, 3), (1, 3), (1, 2), (0, 2), (0, 4), (1, 4), (1, 3), (0, 3), (1, 1), (2, 1), (2, 0), (1, 0), (-1, 1), (0, 1), (0, 0), (-1, 0)] (
         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]
     }
 }
+