I’m attaching a file that demonstrates this. In 23.05, one sphere is lit; in 23.08, both spheres are lit.
We believe the new behaviour is better, but it raises concerns for users who may have exported USD assets with light-linking baked-in via 23.05 (or earlier), and who then try to bring it into a 23.08 environment.
It felt premature to jump straight to a Github issue, so wanted to kick things off here in case anyone had interesting insights, or whether it’s “yep, that’s going to suck for a bit until everyone’s entirely on the other side of the fence”.
OK, after looking at link.usda (I had been testing with a simpler internal scene), I tested MembershipQuery itself in both releases, and there I did see a difference in the inclusion of /sphere2.
I don’t think we anticipated people redundantly manually applying collection:lightLink to lights as a shortcut(?) to disabling includeRoot. When I remove that application from the scene, I get the same answer in both releases, as expected. Is this a pattern your toolset has been promoting, or just something you came across in some user scene?
Apologies we didn’t consider this in making the change… it’s not obvious that this is something we should add to fixbrokenpixarschemas, because for any scene authored post 23.05, it would be an incorrect fix (i.e. uncommenting the explicit override in link.usda)
Thanks Spiff. This is the output we’ve currently been generating ourselves. For consistency (not specifically as a shortcut for disabling includeRoot) and perceived safety, we always explicitly call CollectionAPI::Apply when working with any collection.
While not applicable in the lightLink case, if there are multiple layers all manipulating a user-created collection (perhaps the collection is created on Layer1 to include /a and then it’s changed on Layer2 to include /a and /b) then we see the redundant Apply calls protecting the collection’s definition if the creation layer is muted/removed.
None of this is to propose a change in USD, just information sharing from this end. The change caught us by surprise, but we should be able to smooth over the transition now knowing the redundant Apply call is “to blame”.