Maya: Camera extra data - additional fix for #3304 #3386

antirotor · 2022-06-21T23:10:40Z

Fix

This is adding forgotten commits to already merged PR #3304. It solves issues of parented geometry inside camera instance.

Testing

Try to publish camera with parented geometry. Validator should trigger.

…2_maya-camera-content-validation

ynbot · 2022-06-21T23:10:43Z

Task linked: OP-3302 Camera with proxy geometry

…alidation

m-u-r-p-h-y

validate on:

validate off:

BigRoy · 2022-06-22T08:26:49Z

openpype/hosts/maya/plugins/publish/extract_camera_mayaScene.py

+                    for cam, (attr, value) in itertools.product(cmds.ls(
                            baked_camera_shapes, type="camera", dag=True,
-                            shapes=True, long=True):
-                        attrs = {"backgroundColorR": 0.0,
-                                 "backgroundColorG": 0.0,
-                                 "backgroundColorB": 0.0,
-                                 "overscan": 1.0}
-                        for attr, value in attrs.items():
-                            plug = "{0}.{1}".format(cam, attr)
-                            unlock(plug)
-                            cmds.setAttr(plug, value)
+                            shapes=True, long=True), attrs.items()):
+                        # the above call still pull in shapes that are not
+                        # cameras, so we filter them out here
+                        if cmds.nodeType(cam) != "camera":
+                            continue
+                        plug = "{0}.{1}".format(cam, attr)
+                        unlock(plug)
+                        cmds.setAttr(plug, value)


Is this really a sensible change?

The code is much harder to read.

Not sure why we need to check whether the type is camera if we're already filtering to camera in the ls query. Plus we're now doing that per attribute, not just per camera (so a lot of potential queries?)

that check there was because I've overlooked the shape flag and was wondering why the hell I still get other shapes than camera. Apart from that, the code is maybe harder to read but is getting rid of one loop and re-setting of attrs that is constant in each iteration.

BigRoy · 2022-06-22T08:32:40Z

openpype/hosts/maya/plugins/publish/extract_camera_alembic.py

+                for member in members:
                    transform = cmds.listRelatives(
-                        member, parent=True, fullPath=True)[0]
+                        member, parent=True, fullPath=True)
+                    transform = transform[0] if transform else member


Not entirely sure but this is something to be aware of. Solely looking at this code if a camera instance previously contained a parent group of the camera instead of the camera itself the old logic would still select the 'camera' for export.

I suspect this will now change the behavior for older camera instances that originally would not include the parent hierarchy.

group1/group2/camera

Would previously export as

camera

Even when group1 is in the camera instance (unless something else invalidates that?)

I suspect after this change the export is now:

group1/group2/camera

antirotor · 2022-06-23T15:26:37Z

This is now behaving as it was before - if baked, camera will be out of its hierarchy.

It has one limitation and that is if the hierarchy of camera contains geometry that we want out too, it will fail on parenting/ancestor relationship but that is clear limitation I can live with.

BigRoy · 2022-06-23T15:33:35Z

openpype/hosts/maya/plugins/publish/extract_camera_alembic.py

+                        member, ad=True, fullPath=True) or []
+                    # skip hierarchy if it contains only camera
+                    # `member_content` will contain camera + its parents
+                    if camera in member_content \


Please do not do a lookup per member whether the camera is in member_content while it is a set. This could be very slow, right? Should this be a set?

and len(member_content) == len(camera.split("|")) - 2

What's that trying to solve? This looks like an odd comparison to me.

What this is trying to achieve is to allow situations where you have camera inside some hierarchy and at the same time handle cases where you have hierarchy with camera AND something else.

alice_GRP |---> bob_GRP | \---> camera1_CAM \---> box_GEO

in that case you would expect alembic with:

camera1_CAM alice_GRP \---> box_GEO

but that will fail because alice_GRP and camera1_CAM have parent relationship.

So what it does is that it gets all descendants of the member and check if the count of them is exactly the same as the path to the camera (minus 2 for the camera transform and shape).

Camera is |alice_GRP|bob_GRP|camera1_CAM|camera1_CAMShape and all descendants for member |alice_GRP are ["|alice_GRP", "|alice_GRP|bob_GRP"]

if count of member_content is higher, it means there are more things there than camera. I think there should be warning logged before continue.

This could be very slow, right?

actually Maya is pretty fast for that one. I've just ran some benchmarks and for 1000 calls I get something aroung ~0.02 sec

Why wouldn't we just check whether this:

descendents = cmds.listRelatives(member, allDescendents=True, fullPath=True) or [] shapes = cmds.ls(descendents, shapes=True, noIntermediate=True, long=True) cameras = cmds.ls(shapes, type="camera", long=True) if set(shapes) - set(cameras): # There are other shapes than just cameras present ...

Bonus is that this would ignore empty transforms/groups and intermediate shapes. We could also e.g. ignore deformers as well that might be parented under the camera for who knows what reason (which wouldn't end up in an Alembic anyway so we can just skip them too, right?)

I have a feeling we're overthinking or over-engineering this concept somehow. The fact that this starts behaving differently in a certain edge also feels less consistent than we'd like for a publish. No?

damn you are right. Why is it the sets always eludes me even if the are the easiest solution.

BigRoy · 2022-06-23T15:37:44Z

openpype/hosts/maya/plugins/publish/extract_camera_mayaScene.py

                    # Fix PLN-178: Don't allow background color to be non-black
-                    for cam in cmds.ls(
+                    for cam, (attr, value) in itertools.product(cmds.ls(
                            baked_camera_shapes, type="camera", dag=True,
-                            shapes=True, long=True):
-                        attrs = {"backgroundColorR": 0.0,
-                                 "backgroundColorG": 0.0,
-                                 "backgroundColorB": 0.0,
-                                 "overscan": 1.0}
-                        for attr, value in attrs.items():
-                            plug = "{0}.{1}".format(cam, attr)
-                            unlock(plug)
-                            cmds.setAttr(plug, value)
+                            long=True), attrs.items()):
+                        plug = "{0}.{1}".format(cam, attr)
+                        unlock(plug)
+                        cmds.setAttr(plug, value)


I know it's functionally the same - but do we feel itertools.product is really that much more readable then the nested for loop? I'm fine with either but somehow I have a harder time reading this new code.

antirotor added 2 commits June 14, 2022 17:13

Merge remote-tracking branch 'origin/develop' into enhancement/OP-330…

3fffcf2

…2_maya-camera-content-validation

🐛 fix handling of extra geometry

45d228c

antirotor requested a review from m-u-r-p-h-y June 21, 2022 23:10

antirotor self-assigned this Jun 21, 2022

Merge branch 'develop' into enhancement/OP-3302_maya-camera-content-v…

97fcd08

…alidation

antirotor added host: Maya type: bug Something isn't working labels Jun 21, 2022

m-u-r-p-h-y approved these changes Jun 22, 2022

View reviewed changes

BigRoy reviewed Jun 22, 2022

View reviewed changes

antirotor added 2 commits June 22, 2022 14:13

♻️ remove unnecessary check

8d849b7

🐛 keep baked camera out of hierarchy

1885900

antirotor requested a review from m-u-r-p-h-y June 23, 2022 15:27

BigRoy reviewed Jun 23, 2022

View reviewed changes

♻️ use sets for member comparsion

0b07036

mkolar approved these changes Jul 1, 2022

View reviewed changes

antirotor merged commit 15b2c62 into develop Jul 4, 2022

antirotor deleted the enhancement/OP-3302_maya-camera-content-validation branch July 4, 2022 13:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Maya: Camera extra data - additional fix for #3304 #3386

Maya: Camera extra data - additional fix for #3304 #3386

antirotor commented Jun 21, 2022

ynbot commented Jun 21, 2022

m-u-r-p-h-y left a comment

BigRoy Jun 22, 2022

antirotor Jun 22, 2022

BigRoy Jun 22, 2022 •

edited

antirotor commented Jun 23, 2022 •

edited

BigRoy Jun 23, 2022 •

edited

antirotor Jun 24, 2022

BigRoy Jun 24, 2022 •

edited

antirotor Jun 24, 2022

BigRoy Jun 23, 2022

Maya: Camera extra data - additional fix for #3304 #3386

Maya: Camera extra data - additional fix for #3304 #3386

Conversation

antirotor commented Jun 21, 2022

Fix

Testing

ynbot commented Jun 21, 2022

m-u-r-p-h-y left a comment

Choose a reason for hiding this comment

BigRoy Jun 22, 2022

Choose a reason for hiding this comment

antirotor Jun 22, 2022

Choose a reason for hiding this comment

BigRoy Jun 22, 2022 • edited

Choose a reason for hiding this comment

antirotor commented Jun 23, 2022 • edited

BigRoy Jun 23, 2022 • edited

Choose a reason for hiding this comment

antirotor Jun 24, 2022

Choose a reason for hiding this comment

BigRoy Jun 24, 2022 • edited

Choose a reason for hiding this comment

antirotor Jun 24, 2022

Choose a reason for hiding this comment

BigRoy Jun 23, 2022

Choose a reason for hiding this comment

BigRoy Jun 22, 2022 •

edited

antirotor commented Jun 23, 2022 •

edited

BigRoy Jun 23, 2022 •

edited

BigRoy Jun 24, 2022 •

edited