Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

Maya: Implement iter_visible_nodes_in_range for extracting Alembics #3100

Merged
merged 12 commits into from
Jul 1, 2022

Conversation

BigRoy
Copy link
Collaborator

@BigRoy BigRoy commented Apr 25, 2022

Brief description

This fixes #3085 by implementing a per frame visibility check for all dag nodes in the pointcache instance and only returning those visible nodes for exporting.

Description

The visibleOnly attribute already existed on Animation and Pointcache instances in Maya in the Creators but was unused previously as far as I'm aware.

Additional info

Here's a TEST for just the iter_visible_nodes_in_range function. You can also try and publish these particular nodes in a pointcache to confirm the functionality in that regard:

from maya import cmds
from openpype.hosts.maya.api.lib import iter_visible_nodes_in_range


def cube(name):
    return cmds.ls(cmds.polyCube(name=name, constructionHistory=False), long=True)[0]


nodes = {
    name: cube(name) for name in [
        "on",
        "off",
        "animated_always_on",
        "animated_always_off",
        "animated_on_between_10_20",
        "connected_always_on",
        "connected_always_off",
        "connected_on_between_10_20",
        "expression_on",
        "expression_off",
        "expression_on_between_10_20"
    ]
}

cmds.setAttr(nodes["on"] + ".visibility", True)
cmds.setAttr(nodes["off"] + ".visibility", False)

# Animated
cmds.setKeyframe(nodes["animated_always_on"], attribute="visibility", time=1.0, value=True)
cmds.setKeyframe(nodes["animated_always_off"], attribute="visibility", time=1.0, value=False)
cmds.setKeyframe(nodes["animated_on_between_10_20"], attribute="visibility", time=0.0, value=False, itt="flat", ott="step")
cmds.setKeyframe(nodes["animated_on_between_10_20"], attribute="visibility", time=10.0, value=True, itt="flat", ott="step")
cmds.setKeyframe(nodes["animated_on_between_10_20"], attribute="visibility", time=20.0, value=False, itt="flat", ott="step")

# Connected
for src, dest in {
    "animated_always_on": "connected_always_on",
    "animated_always_off": "connected_always_off",
    "animated_on_between_10_20": "connected_on_between_10_20"
}.items():
    cmds.connectAttr(nodes[src] + ".visibility", 
                     nodes[dest] + ".visibility", 
                     force=True)
    
# Expressions
cmds.expression(name="visibilityExpressions", s= """
{0[expression_on]}.visibility = 1;
{0[expression_off]}.visibility = 0;
{0[expression_on_between_10_20]}.visibility = (time1.outTime >= 10) && (time1.outTime <= 20);
""".format(nodes))

# Expected test results
ALWAYS_ON = {
    nodes["on"],
    nodes["animated_always_on"],
    nodes["connected_always_on"],
    nodes["expression_on"],
}

ON_IN_10_20 = {
    nodes["animated_on_between_10_20"],
    nodes["connected_on_between_10_20"],
    nodes["expression_on_between_10_20"]
} | ALWAYS_ON

# Test `iter_visible_nodes_in_range`
node_names = list(nodes.values())
assert set(iter_visible_nodes_in_range(node_names, start=1, end=9)) == ALWAYS_ON
assert set(iter_visible_nodes_in_range(node_names, start=1, end=25)) == ON_IN_10_20
assert set(iter_visible_nodes_in_range(node_names, start=15, end=15)) == ON_IN_10_20
assert set(iter_visible_nodes_in_range(node_names, start=21, end=30)) == ALWAYS_ON

Testing notes:

  1. Test whether visibility is correctly detected in a wide range of scenarios. (e.g. Parenting relationships are not tested in the above example test case but can easily be tested against as well.)

- Functionality added in Maya 2018
- Usage without `MDGContext.makeCurrent()` is deprecated since Maya 2022
@BigRoy BigRoy self-assigned this Apr 26, 2022
Previously an empty string could be yielded for e.g. `"|cube"` splitting to `["", "cube"]`
Copy link
Member

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is some issue with sets containing hidden geo

// Error: pyblish.plugin : Traceback (most recent call last):
  File "D:\REPO\OpenPype\.venv\lib\site-packages\pyblish\plugin.py", line 522, in __explicit_process
    runner(*args)
  File "D:\REPO\OpenPype\openpype\hosts\maya\plugins\publish\extract_pointcache.py", line 93, in process
  File "D:\REPO\OpenPype\openpype\hosts\maya\api\lib.py", line 1241, in extract_alembic
    cmds.AbcExport(j=job_str, verbose=verbose)
  File "D:\REPO\OpenPype\openpype\hosts\maya\plugins\publish\extract_pointcache.py", line 2, in AbcExport
RuntimeError: -selection specified but nothing is actively selected.

Traceback (most recent call last):
  File "D:\REPO\OpenPype\.venv\lib\site-packages\pyblish\plugin.py", line 522, in __explicit_process
    runner(*args)
  File "<string>", line 93, in process
  File "D:\REPO\OpenPype\openpype\hosts\maya\api\lib.py", line 1241, in extract_alembic
    cmds.AbcExport(j=job_str, verbose=verbose)
  File "<string>", line 2, in AbcExport
RuntimeError: -selection specified but nothing is actively selected.

image

@BigRoy
Copy link
Collaborator Author

BigRoy commented Apr 26, 2022

You're trying to export a pointcache with NO geometry whatsoever due to it always being hidden and thus being excluded. :) I was actually expecting that to happen! I instead assumed user would use this for pointcaches of multiple pieces of geometry where at least one piece of geometry or transform is visible during the timeline.

Thus this should get validated for both scenarios, however this visibility check is relatively slow for large scenes so it could potentially mean the Validator would be slow. Plus, we'd have the plug-in likely "cache" the visibility states so the Extractor doesn't have to do it again but that also means a Validator is technically "collecting" information. Moving that to a Collector means that technically the collecting stage can be slow.

OR We'd just log a warning in the Extractor and do nothing instead? Thoughts?

Anyway, thanks - will look into this edge case.

@m-u-r-p-h-y
Copy link
Member

You are right, if I used only one pointCache instance for all geo it would pass.
But I didn't :)

@ynbot
Copy link
Contributor

ynbot commented Apr 29, 2022

@mkolar mkolar added the type: enhancement Enhancements to existing functionality label May 11, 2022
@mkolar
Copy link
Member

mkolar commented Jun 7, 2022

OR We'd just log a warning in the Extractor and do nothing instead? Thoughts?

I think, this might be the right approach in this situation. Probably more intuitive to export nothing if everything is hidden rather than crashing.

@mkolar
Copy link
Member

mkolar commented Jun 24, 2022

@BigRoy just a ping that if we do the last tweak mention in you comment, this will be good to merge

@BigRoy
Copy link
Collaborator Author

BigRoy commented Jun 28, 2022

just a ping that if we do the last tweak mention in you comment, this will be good to merge

@mkolar I've just pushed the change to the extractors so that the message is logged and the extractor continues. However, the integrator will currently still error on any instances that do not produce any files whatsoever.

See:
afbeelding

How would you like me to proceed?

@mkolar
Copy link
Member

mkolar commented Jun 28, 2022

How would you like me to proceed?

That's somewhat tricky. We should either figure this out in the validator, or this particular instance should never get to integration stage. No files, nothing to integrate.

It could get really confusing for the artist though, so I think validation should flag this, if we're able to catch it. If not, then the extractor, should figure out it produced no files, hence not create a representation and the integrator can then gracefully skip

@BigRoy
Copy link
Collaborator Author

BigRoy commented Jun 28, 2022

It could get really confusing for the artist though, so I think validation should flag this, if we're able to catch it. If not, then the extractor, should figure out it produced no files, hence not create a representation and the integrator can then gracefully skip

Agreed. This particular "validation" could run relatively slow - but I guess that's the side effect of this. Will add it.

@BigRoy
Copy link
Collaborator Author

BigRoy commented Jun 28, 2022

I've changed the function to an iterator to allow a much faster validation for 99% of the cases where at least a single node is detected to be visible. It's good to be aware that the validation allows any dag node to be visible. That also includes e.g. a group node which I felt makes sense to be the least restrictive and it also wouldn't fail on the extraction (which is what we were trying to solve anyway.)

Should be good to test now @mkolar

@BigRoy BigRoy requested a review from m-u-r-p-h-y June 28, 2022 19:33

if instance.data["family"] == "animation":
# Special behavior to use the nodes in out_SET
nodes = instance.data["out_hierarchy"]
Copy link
Collaborator Author

@BigRoy BigRoy Jun 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

It's good to be aware that this logic depends on THIS Collector plug-in.

The bug/issue is that if the instance in Maya is set to active=False on the objectSet/instance directly that that Collector does not run. This bug is present in many more areas actually so be aware.

There are two solutions to that:

  • Have the instance collector NOT collect the active state directly but instead collect as e.g. __active. Then have a very later Collector (CollectorOrder + 0.499) which sets instance.data["active"] = instance.data,pop("__active", True) - so that up to that point all collectors do run as expected.
  • Use/implement something like Post collectors like PYBLISH_QML_POST_COLLECT see: Implementing "Post Collect" pyblish/pyblish-qml#356 - that way we can have these collectors that should always run for an active instance in the post collection step after the user toggled states. Upside to this is that collecting will be MUCH faster since many collectors could move to the post collector order.

@mkolar Thoughts?

Copy link
Member

@iLLiCiTiT iLLiCiTiT Jun 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that during local publishing (artist publishing where he can toggle instance activity) we process during collection all instances no matter if they are active or not (until collector check that on it's own?).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(until collector check that on it's own?).

It's any collector that appends data into an instance that would fail to do so for inactive instances. So I suspect the issue will also exist for these:

Basically anything running over the instance would not actually run because Collect Instances already collects directly whether the instance should be considered "active" to run over the instance for. So the issue is much more widespread than just this particular one.

@iLLiCiTiT
Copy link
Member

I have stupid question. What does it return? I miss one word in the function name: "Get visible ... in frame range".

@BigRoy
Copy link
Collaborator Author

BigRoy commented Jun 29, 2022

I have stupid question. What does it return? I miss one word in the function name: "Get visible ... in frame range".

It's currently been renamed to iter_visible_in_frame_range - not sure if that's more descriptive for you.
But basically it yields the nodes from the input nodes that are visible at least once during the frame range.
Would you like me to refactor it to: iter_visible_nodes_in_frame_range?

@iLLiCiTiT
Copy link
Member

iLLiCiTiT commented Jun 29, 2022

Would you like me to refactor it to: iter_visible_nodes_in_frame_range?

I would say it's more important then it's from frame range :) (Maybe iter_visible_nodes_in_range would make it shorter?)

@ghost ghost changed the title Maya: Implement get_visible_in_frame_range for extracting Alembics Maya: Implement iter_visible_nodes_in_range for extracting Alembics Jun 29, 2022
Copy link
Member

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing with my old setup

validation for visibility works great

image

and publish as well

image

@BigRoy
Copy link
Collaborator Author

BigRoy commented Jun 30, 2022

Can we merge this?

@BigRoy BigRoy requested a review from iLLiCiTiT July 1, 2022 09:48
@mkolar
Copy link
Member

mkolar commented Jul 1, 2022

Merging. Thank you.

@mkolar mkolar merged commit f4e6a8a into ynput:develop Jul 1, 2022
@BigRoy BigRoy deleted the fix3085 branch March 20, 2024 15:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
host: Maya type: enhancement Enhancements to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Maya: animation publish option to not export hidden geometries to alembic
5 participants