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

Publisher: Create context has shared data for collection phase #3995

Merged

Conversation

iLLiCiTiT
Copy link
Member

@iLLiCiTiT iLLiCiTiT commented Oct 17, 2022

Brief description

Create context allow access to shared data for time during it's reset. This gives creat plugins ability to cache common data so they're not recollected in each creator again (e.g. nodes with avalon.pyblish.instance in scene).

Description

In most of host implementations is this benefitial for creators to run some data collection only once. Data can be stored under specific keys and are cleaned up when collection finishes. Out of collection phase are not data available.

This has been added to TrayPublisher's default creator as an example. In TrayPublisher it has almost none advantage.

Additional information

Has anyone better suggestion for the method names then collection_shared_data_contains, get_collection_shared_data and set_collection_shared_data? Outdated question

Testing notes

  • TrayPublisher should still work (even after refresh)
  • It should be possible to use this feature in one of hosts implementing publisher (Nuke, Houdini, ...)

Example usage

# NOTE: This is made up code based on Nuke knowledge
def _collect_instance_nodes():
    instance_data_with_node = []
    for node in nuke.allNodes(recurseGroups=True):
        if node.Class() in ["Viewer", "Dot"]:
            continue

        try:
            if node["disable"].value():
                continue
        except Exception as _exc:
            log.debug("Node {} have no disable knob - {}".format(
                node.fullName(), _exc))

        # get data from avalon knob
        instance_data = get_node_data(
            node, INSTANCE_DATA_KNOB)

        if (
            not instance_data
            or instance_data.get("id") != "pyblish.avalon.instance"
        ):
            continue

        instance_data_with_node.append((node, instance_data))

    return instance_data_with_node


def _cache_and_get_nodes(creator):
    """Cache instances in shared data.

    This function is out of creator's implementation because the logic is common
    for AutoCreator and Creator.

    Args:
        creator (Creator): Plugin which would like to get instances from host.
    Returns:
        List[nuke.Node]: Cached nodes with instances.
    """

    shared_key = "openpype.nuke.instance_data_with_node"
    if shared_key not in creator.collection_shared_data:
        
        creator.collection_shared_data[shared_key] = _collect_instance_nodes()
    return creator.collection_shared_data[shared_key]


class NukeAutoCreator(AutoCreator):
    def collect_instances(self):
        instance_data_with_node = _cache_and_get_nodes(self)
        for node, instance_data in instance_data_with_node :
            if instance_data["creator_identifier"] != self.identifier:
                continue
            ...


class NukeCreator(AutoCreator):
    def collect_instances(self):
        instance_data_with_node = _cache_and_get_nodes(self)
        for node, instance_data in instance_data_with_node :
            if instance_data["creator_identifier"] != self.identifier:
                continue
            ...

@ynbot
Copy link
Contributor

ynbot commented Oct 17, 2022

@iLLiCiTiT iLLiCiTiT self-assigned this Oct 17, 2022
@iLLiCiTiT iLLiCiTiT added the type: enhancement Enhancements to existing functionality label Oct 17, 2022
@jakubjezek001
Copy link
Member

jakubjezek001 commented Oct 17, 2022

Has anyone better suggestion for the method names then collection_shared_data_contains, get_collection_shared_data and set_collection_shared_data?

How about get_cached_instances_data and its comrades set_* and has_*

@jakubjezek001
Copy link
Member

jakubjezek001 commented Oct 17, 2022

I wonder whats gonna happen if user will add new instance during one session where the publisher window will be opened on second desktop for example. To be more specific, is it recached in case of altering the scene after the first caching?

@iLLiCiTiT
Copy link
Member Author

iLLiCiTiT commented Oct 17, 2022

How about get_cached_instances_data and its comrades set_* and has_*

I think I didn't explain it properly. It's not for caching instance data, it's for caching any data that may be common for multiple creators during CreateContext reset. During that time are creator plugins initialized and called collect_instances on them and also is called create on AutoCreators. So you can go through all nodes in scene and filter only those which contain pyblish.avalon.instance only once and store them to shared data for other creators so they don't have to do the same. But you can also store other, not instance specific, data. Also note that when reset of create context finishes and all creators collected intances and autocreators created, the access to shared data is not allowed anymore.

I wonder whats gonna happen if user will add new instance during one session where the publisher window will be opened on second desktop for example. To be more specific, is it recached in case of altering the scene after the first caching?

I think this question was raised because of misunderstanding the feature.

@BigRoy
Copy link
Collaborator

BigRoy commented Oct 18, 2022

I feel like just having a dict available "during" collection as a cache is a much more understandable API. I don't see a specific reason for this be so explicit with contains, get and set. It'll just make the code extremely verbose and long to write.

Each Creator would then automatically get that dict assigned like e.g.:

creator.collection_cache = cache

I feel like the example is much better as a parent class to be inherited from.
As such the example could become e.g.:

# NOTE: This is made up code based on Nuke knowledge
def _collect_instance_nodes():
    instance_data_with_node = []
    for node in nuke.allNodes(recurseGroups=True):
        if node.Class() in ["Viewer", "Dot"]:
            continue

        try:
            if node["disable"].value():
                continue
        except Exception as _exc:
            log.debug("Node {} have no disable knob - {}".format(
                node.fullName(), _exc))

        # get data from avalon knob
        instance_data = get_node_data(
            node, INSTANCE_DATA_KNOB)

        if (
                not instance_data
                or instance_data.get("id") != "pyblish.avalon.instance"
        ):
            continue

        instance_data_with_node.append((node, instance_data))

    return instance_data_with_node


class CachedNodeCreator(Creator):
    
    def get_nodes(self):
        key = "openpype.nuke.instance_data_with_node"
        if key in self.collection_cache:
            return self.collection_cache[key]
        
        nodes = _collect_instance_nodes()
        self.collection_cache[key] = nodes
        return nodes
    
    def iter_nodes_by_identifier(self, identifier=None):
        if identifier is None:
            identifier = self.identifier
        
        for node, instance_data in self.get_nodes():
            if instance_data["creator_identifier"] == identifier:
                yield node, instance_data
            

class NukeAutoCreator(CachedNodeCreator):
    def collect_instances(self):
        for node, instance_data in self.iter_nodes_by_identifier():
            ...


class NukeCreator(BaseCreator):
    def collect_instances(self):
        for node, instance_data in self.iter_nodes_by_identifier():
            ...

The cache could directly be cleared after "reset" has finished in the controller.

@iLLiCiTiT
Copy link
Member Author

iLLiCiTiT commented Oct 18, 2022

I feel like just having a dict available "during" collection as a cache is a much more understandable API.

The only reason to use methods was to raise specific exception in case someone would try access it out of collection phase. But I'm ok with it.

EDITED: Just realized I can do it anyway using property....

Copy link
Member

@antirotor antirotor left a comment

Choose a reason for hiding this comment

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

Tested it in Houdini and it seems to be working well.

@iLLiCiTiT iLLiCiTiT merged commit 6c199b7 into develop Oct 21, 2022
@iLLiCiTiT iLLiCiTiT deleted the feature/OP-4178_Add-Shared-data-for-collection-phase branch October 21, 2022 12:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: enhancement Enhancements to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants