Add faceset support to usd dev

[abucior]

Description of Change(s)

This work is a continuation of Adam Ferrall-Nunge's work in #219, which added support for Alembic facesets to USD. At that time they were represented as Faceset data on the parent prim, which was subsequently read by pxrUsdIn and expanded back to faceset nodes. That PR was never accepted because UsdGeomSubset was introduced which became the more appropriate choice for representation of Alembic Facesets.
To a large extent this simplified the problem: The exact same hierarchy is maintained between Alembic and USD with only the type changing (Alembic "Faceset" to USD "GeomSubset"). pxrUsdIn already has support for GeomSubset, so there was no need to make any pxrUsdIn changes for Katana.

This is a rebasing of #754 onto dev.

Testing

I've added a new unit test that performs round-tripping of faceset data from USD to Alembic and back again, verifying that the data is preserved as best as possible. It's not lossless, due to schema differences between Alembic and USD, but it's as good as can be represented, in my opinion.

[jtran56]

Filed as internal issue #USD-5049.

[spiffmon]

Hi Alan,
Thanks for doing this! There is just one problem with the change, as it translates Abc "faceExclusivity" to UsdGeomSubset's familyName property, when it in fact needs to be translating it to familyType. I don't think Abc has any equivalent of familyName. So this will ripple some function/class name and API-use changes through the alembicReader.cpp and alembicWriter.cpp, and should also be corrected in the "original.usda" test file.

Cheers,
--spiff

[abucior]

Thanks, Spiff.
It seems I misunderstood how familyType and familyName are supposed to work. Sorry about that.
After taking a look at this file as an example, it looks like I should have something like the following on the geom:
uniform token subsetFamily:faceExclusivity:familyType = "<something>"
and then in the GeomSubset I should have:
uniform token familyName = "faceExclusivity"
I'm using "faceExclusivity" here because I need some sort of familyName, and it seems appropriate to match the Alembic naming in this case. Does that seem about right to you? If so I'll get that fixed up and update the PR.

[spiffmon]

*FamilyName* is supposed to be optional, so you should be able to leave it empty. That said, an “ideal” transcoding would have the alembicWriter authoring the prim’s *familyName* as metadata or a property on the abc object, and have the reader look for that, and if not present, then maybe use “faceExclusivity” or “abcFamily” as familyName. In our current codebase, the only place we’re creating Subsets is in the Maya exporter, for material assignment, using the UsdShadeMaterialBinding API which sets the Subsets’ familyName to “materialBinding”, I think. I can’t evaluate how important that level of roundtripping fidelity would be, though.

On Wed, Jan 30, 2019 at 5:52 PM Alan Bucior ***@***.***> wrote: Thanks, Spiff. It seems I misunderstood how *familyType* and *familyName* are supposed to work. Sorry about that. After taking a look at this file <https://github.com/PixarAnimationStudios/USD/blob/dev/pxr/usd/lib/usdGeom/testenv/testUsdGeomSubset/Sphere.usda> as an example, it looks like I should have something like the following on the geom: uniform token subsetFamily:faceExclusivity:familyType = "<something>" and then in the GeomSubset I should have: uniform token familyName = "faceExclusivity" I'm using "faceExclusivity" here because I need some sort of *familyName*, and it seems appropriate to match the Alembic naming in this case. Does that seem about right to you? If so I'll get that fixed up and update the PR. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#758 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AF7qaLe2K05fczl8e2-YSW5omKAIPXQgks5vIkxXgaJpZM4aYxVs> .

-- --spiffiPhone

[abucior]

I see. Thanks for the explanation!
As you say, I could leave familyName empty, but if that's the case then it seems I wouldn't be able to specify familyType, as that's attached to a family. As a test, I tried calling CreateGeomSubset() leaving the familyName empty and specifying the familyType, but the resulting USD file had no reference to my familyType. Given that familyType is even a little bit round-trippable (via faceExclusivity), I think it makes sense to specify a familyName in order to retain the familyType.
It looks like the UsdShadeMaterialBindingAPI is using UsdShadeTokens->materialBind as the familyName. Given that Alembic can only ever have one subset family (since there's only faceExclusivity and nothing else), and it's most likely that Alembic faceSets would be used for material binding, it feels appropriate to use materialBind as the familyName here. I expect it's the "least surprising" choice from a user point of view. Does that sound good to you?

[spitzak]

Sounds to me like there is a bug and a FamilyName of "" does not work correctly. In any case everybody agreeing on the default FamilyName is probably the best solution. If there is precedent for "materialBind" and "" cannot be made to work then everybody should agree on that.

…

On Thu, Jan 31, 2019 at 9:29 AM Alan Bucior ***@***.***> wrote: I see. Thanks for the explanation! As you say, I could leave familyName empty, but if that's the case then it seems I wouldn't be able to specify familyType, as that's attached to a family. As a test, I tried calling CreateGeomSubset() leaving the familyName empty and specifying the familyType, but the resulting USD file had no reference to my familyType. Given that familyType is even a little bit round-trippable (via faceExclusivity), I think it makes sense to specify a familyName in order to retain the familyType. It looks like the UsdShadeMaterialBindingAPI is using UsdShadeTokens->materialBind as the familyName. Given that Alembic can only ever have one subset family (since there's only faceExclusivity and nothing else), and it's most likely that Alembic faceSets would be used for material binding, it feels appropriate to use materialBind as the familyName here. I expect it's the "least surprising" choice from a user point of view. Does that sound good to you? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#758 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABLBEd5bM6p_Wcya8X4Qouu9RhrFio60ks5vIyf4gaJpZM4aYxVs> .

[spiffmon]

Agreed on all counts: let's go with materialBind, and there may well be a bug with empty familyName, as we haven't really exercised it (though we should at least be testing it in the unit tests).

The only remaining question, if we care, is what to do in alembicWriter if there are multiple families of subsets for the same Mesh. This is currently an academic question as we don't have a use-case for it, but one could imagine making one family of subsets for material binding, and another for illumination or trace-groups... It would be good to agree on what the translation should be, even if we decide it's OK for it to be lossy.

[abucior]

I've pushed up a new commit. Please take a look when you get a chance. I've implemented the "materialBind" default here, as discussed.
Because the conversion from Faceset to GeomSubset requires simultaneous changes to both the GeomSubset (for its indices, familyName) and the parent prim (for the familyName's familyType), I had to add a bit of extra code to the _PrimReaderContext and _PrimWriterContext to enable this. I couldn't see any easy way to do make it work otherwise. If there's a better way to do this, please advise and I'll fix it up.

[spiffmon]

Hi Allen,
This looks great, and the mapping of "exclusive" to nonOverlapping seems right.

• There's a copypasta error in the comment at alembicReader.cpp:3356
• The defaultFamilyName TfToken declared soon after should be made static, or added to USD_ABC_GPRIM_NAMES in alembicUtil.h since it can then be shared with the writer - this helps multi-threaded performance (e.g. multiple abc files being opened at once)
• I'd similarly declare subsetFamilyAttributeName simply as static or public token "subsetFamily:materialBind:familyType", as you don't need the dynamism that constructing from a vector gives you
• Since neither USD nor (I think?) ALembic prevent you from creating a Faceset/Subset as a root-level prim -- even though it's meaningless to do so --, it might be good to add a simple check at the top of _ReadFaceSet that ensures the parent prim or parentPrimCOntext exists and is not the pseudoroot (which cannot be authored to). I guess in that case you could just not author the FamilyType - that way you're still translating as much as you can without generating a nasty error.
• The canonical sampling time when you don't know what time you're looking for is UsdTimeCode::EarliestTime().GetValue() (rather than 0.0f)

Very nice test!

Thanks, and looking forward to getting this in for 19.05,
--spiff