Add formal support for Usd <-> Alembic facesets

[aferrall]

Description of Change(s)

Modifications to pxrUsdInShipped to support not only facesets which contain UsdShade-based material assignments, but all facesets associated with a given mesh primitive. This also supports both facesets with a single facegroup and those with multiple facegroups. As a special case for single facegroups, the faceset name is used without modification, which allows Alembic facesets to come through into USD and into Katana unmodified.

Changes to usdAbc (which is responsible for translating Alembic data structures to USD ones, and vice-versa) so that facesets are properly considered and represented in USD and ALembic.

The changes to alembicReader.cpp means the facesets themselves are represented in USD (not any custom attributes which may be present on the Alembic faceset objects), but this is sufficient for common uses (particularly lookdev assignments implemented in other tools).

The changes to alembicWriter.cpp translate the basic facesets from USD to Alembic, supporting both single facegroup facesets and multi facegroup facesets. In the single case, the same special handling for names is done as in pxrUsdInShipped, which allows a proper roundtrip of an Alembic cache with facesets to go to USD then back to Alembic with the same structure.

Fixes Issue(s)

[aferrall]

As far as testing goes, I ran this through tests with a number of different Alembic and Usd inputs, checking round tripping and behavior in abcview, usdview, and Katana. When I looked at the tests setup for usdAbc, I didn't see much framework exercising this kind of behavior, so I didn't immediately write up unit tests.

If there's a specific way you all would like to validate this kind of behavior, I'm happy to provide test coverage.

[jtran56]

Filed as internal issue #147291.

[spitzak]

I can report that this patch successfully imported Dreamworks abc files into usd and then into Katana and preserved the facesets exactly the same as our previous patch to do the same thing.

Not really clear why Pixar does not want to make named face sets into actual child prims in USD. But this uses your scheme rather than a non-USD-standard layout that our previous patch did.

[spiffmon]

Hi Bill,
There were a couple of reasons we decided to make FaceSets in USD be more constrained/limited than Alembic:

1. There were long threads on the Alembic ggroup about how difficult round-tripping through Maya was
2. There is no precedent in UsdGeom for having prims "under" gprims in namespace, i.e. gprims are leaves of the scenegraph - this has consequences wrt our visibility model, for example: if a Mesh is invisible, there is absolutely no way to make any prims beneath it visible.
3. We were (and still are) a little uncertain about the notion of allowing FaceSets to become "arbitrary resources" (in the RiResource sense), that is, containers where you can "override" arbitrary properties of the Mesh under which it lives.

That said, as we've worked to accommodate MaterialX in USD, the fact that FaceSets aren't prims does present a point of friction for "collection expressions". We are willing to reconsider this encoding decision, and are eager to hear other perspectives on the issues above. Please also feel free to take this discussion to the usd-interest group for greater exposure!

Cheers,
--spiff

[sunyab]

Hi @aferrall, sorry for the delay, I'm looking at merging this change in now.

Regarding testing, I think it'd be great to have a test like testUsdAbcConversionSubdiv that took a representative .abc file with facesets, opened it up on a UsdStage and verified that the expected prims and attributes exist. testUsdAbcInstancing is a similar test that converts the .abc file to a .usda file, then compares that file against a baseline that has been verified as correct. Please let us know if you need any help with this -- thanks!

[aferrall]

Hey @sunyab, sorry I haven't responded sooner, but I've been pulled onto other things recently. Thanks for considering the change, and I agree that a test would be great. I don't think I'll have time to get back to it for a week or two, but when I do, I'll look to the existing tests as reference and add one for faceset translation.

[aferrall]

Hey @sunyab -- I've added a small test which does a roundtrip conversion from source USD -> Alembic -> validation when reopening the Alembic on a UsdStage. While not exhaustive of the various scenarios which are possible, hopefully it provides a good start for core validation of my previous change.

Let me know if this is sufficient or if there is additional testing you'd like -- thanks.

[aferrall]

Hey @sunyab -- I was just checking in on this request (since it's been open for a while now even with the confidence tests) and noticed the new conflicts related to changes in pxrUsdInShipped/mesh.cpp. Looking at that code (and the surrounding changes), it seems as though the overall approach to managing facesets in USD has changed, from geometry property-based approach with the USDGeomFaceSetAPI to a more Alembic-like GeomSubset prim approach.

With that in mind, most of this pull request is no longer valid, as it will have to be reworked with the new data representation. I don't seem GeomSubset related changes to the usdAbc plugin, so I think this is still work that needs to be done to properly support Alembic interop with USD. If you agree, let me know so I can close out this pull request and let folks here know that there's work that will need to be scheduled.

Thanks.

[spiffmon]

Hi @aferrall, Your assessment is correct, and our apologies for not yet officially announcing the change in faceSet management (there is discussion of it on the ggroup, but no talk of timing). We plan on sending out a set of announcements about the changes/additions that will be available in 0.82, sometime in November. The good news is that the new approach makes the translation from Alembic much easier, but apologies for invalidating the PR.

…

--SpiffiPhone

On Oct 26, 2017, at 3:28 PM, aferrall ***@***.***> wrote: Hey @sunyab -- I was just checking in on this request (since it's been open for a while now even with the confidence tests) and noticed the new conflicts related to changes in pxrUsdInShipped/mesh.cpp. Looking at that code (and the surrounding changes), it seems as though the overall approach to managing facesets in USD has changed, from geometry property-based approach with the USDGeomFaceSetAPI to a more Alembic-like GeomSubset prim approach. With that in mind, most of this pull request is no longer valid, as it will have to be reworked with the new data representation. I don't seem GeomSubset related changes to the usdAbc plugin, so I think this is still work that needs to be done to properly support Alembic interop with USD. If you agree, let me know so I can close out this pull request and let folks here know that there's work that will need to be scheduled. Thanks. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[aferrall]

Hey @spiffmon -- thanks for the confirmation. I think the new direction for facesets will certainly be more flexible and will allow an easier implementation of what my changes were doing, so I'll close my PR now and we'll see about making changes to support Alembic that are compatible with the new API.