PixarAnimationStudios / OpenUSD

Universal Scene Description
http://www.openusd.org
Other
6.11k stars 1.22k forks source link

[katana] Looks group with intermediate hierarchy breaks assignment of specializing materials #1007

Closed nrusch closed 1 year ago

nrusch commented 5 years ago

Description of Issue

When reading in materials that are related via a specializes arc under a Looks scope which is a direct child of a "model" prim, the PxrUsdInCore_LooksGroupOp will create a hierarchicy of material locations, with the "specializing" material nested under the material it specializes.

Introducing an intermediate level of hierarchy below the Looks scope prevents the LooksGroupOp from running on the children of the intermediate group, so the base and specializing materials are created alongside each other. However, the logic used to determine the scene graph path to the specializing material for the purposes of setting materialAssign attributes still assumes the specializing material will be nested below its base material, which results in a broken materialAssign attribute.

Here is an example prim hierarchy:

MyModel    # kind = "model"
    Looks
        someGroup
            MasterMaterial
            ChildMaterial    # specializes `MasterMaterial`
    MyModel_meshGroup
        MyModel_mesh    # ChildMaterial bound here

I will attach a .usda file that demonstrates this issue using roughly the same structure as above.

Steps to Reproduce

  1. Read in the attached .usda file
  2. Expand the scene graph and observe that the materialAssign attribute on /MyModel/MyModel_meshGroup/myModel_mesh assumes that ChildMaterialA will be a child of MasterMaterial, when this is not the case.

Package Versions

nrusch commented 5 years ago

Here is the example stage (I had to add an additional .txt extension to get Github to accept it).

modelTest_specializedMaterial.usda.txt

nrusch commented 5 years ago

I think my instinct on this is that the LooksGroupOp should recursively traverse the hierarchy under the Looks scope (thus always constructing a parent-child relationship for specializ(ed|ing) materials, but I don't yet have my head around the potential negative implications of that.

jilliene commented 5 years ago

Filed as internal issue #USD-5645

nrusch commented 5 years ago

For completeness, it looks like this issue also affects specializing materials under "instance master" locations, even if there is no intermediate hierarchy under the Looks scope.

emeroo commented 5 years ago

Sorry for the delayed reply!

It is true that the LooksGroupOp operates on immediate children of the katanaLooksScopeName ("Looks") as opposed to walking recursively until it hits material prims. I don't know the full history of why this restriction was put into place, but I'm guessing there is a performance/complexity reason.

That being said, there is metadata that you can author in usd to create the nested structure that your usd scene is defining explicitly. I'm attaching a modified version of your example stage that defines both Materials as immediate children of the Looks group, but tacks on "ui" tokens that the LooksGroupOp respects when building out the Materials on the Katana side. The end result is a nested structure with correct material assignments.

I'm not sure if this will work for you, depending on whether or not "someGroup" is being used for reasons other than influencing the Katana-side scenegraph hierarchy. Let me know...

emeroo commented 5 years ago

Here's the reworked usd snippet with the "ui" metadata.

modelTest_specializedMaterial_restructured.usda.txt

nrusch commented 5 years ago

Thanks Emily.

Removing the intermediate hierarchy levels is not a big deal for us, and it fixes the majority of our use-cases. The display group/name attributes are helpful as well.

However, the broken materialAssign attributes are something that I think still should be looked into, since the cause is non-obvious.

emeroo commented 5 years ago

Additional details...

There was no performance/complexity reason for having the LooksGroupOp treat just the immediate children of "Looks" as Materials, it just happens to be the way that we organize our Materials... :woman_shrugging:

As to why the materialAssign attribute is broken in your case, when those "ui" hints aren't there, the utility that readPrim uses to generate a material assignment (ConvertUsdMaterialPathToKatLocation) assumes that specializing materials will be scenegraph children of their masters.

To go full circle, ConvertUsdMaterialPathToKatLocation is used by the LooksGroupOp too, to determine how to build out the Materials that it finds under the "Looks" scope. Which means, if the LooksGroupOp had properly processed MasterMaterial and ChildMaterialA as Materials, you would have ended up with ChildMaterialA as a child of MasterMaterial on the Katana side and the materialAssign value "/root/world/geo/MyModel/Looks/MasterMaterial/ChildMaterialA" would have indeed been valid. I'm attaching another variation on the original usda scene that removes someGroup, but does not add "ui" hints. When you read this new snippet into PxrUsdIn, you can see ChildMaterialA is nested underneath MasterMaterial.

All that is to say, proper materialAssign values rely on the LooksGroupOp running as expected on the Materials under "Looks".

I hope this helps clear things up a bit further.

modelTest_specializedMaterial_someGroupRemoved.usda.txt

nrusch commented 5 years ago

Thanks @emeroo. I understand the cause of the divergent behavior at this point, but because this feels like a fairly easy trap to fall into given that usdKatana can currently only have meaningful opinions about converting from USD to Katana (but not vice-versa), I would request that it still be considered a bug.

Exactly which aspect of the divergent logic should be considered buggy is something I think may need some discussion, although given the particular behaviors and limitations of the LooksGroupOp that are entirely hidden from the user, my instinct would be to point there.

emeroo commented 5 years ago

Makes sense. To be clear, I'm not defending the current behavior or saying this isn't a bug; just explaining how the pieces (LooksGroupOp not processing nested Materials, incorrect material assign values) are connected.

nrusch commented 5 years ago

Thanks for clarifying. I think it's my fault for misinterpreting your response... I see after reading over it again that you were leaving the door open for changes to the LooksGroupOp, but I think I initially glossed over that as something like: "the LooksGroupOp exists to serve our needs, and thus shall remain unsullied." ;)

emeroo commented 5 years ago

I had originally thought there was a performance reason for why it works the way it currently does, but I was totally wrong (the "Looks" sub-hierarchy gets traversed anyway). I bet the LooksGroupOp could be modified to walk over nested "Looks" scopes, as you originally suggested.

I'm glad you guys have a workaround in the meantime (and have a new one if you want, i.e. the "ui" hints).

And sorry on my part of explaining stuff you already knew! D'oh.

sunyab commented 1 year ago

Closing out old issue related to the retired usdKatana plugin.