CollectLocalRenderInstances: use create_instances_for_aov

[MustafaJafar]

Changelog Description

• CollectLocalRenderInstances: prepare instance data first then create aov instances second

Additional review information

Stop relying on private methods like _get_legacy_product_name_and_group
This PR also follow reusing same functions used with deadline and adopted with local render in max and maya.

Personally, I'm not keen on using ayon_core.pipeline.farm.pyblish_functions.create_skeleton_instance for local render in Houdini as it can pollute the instance data adding irrelevant and data items that are not used in Houdini addon. which can make it hard to debug in the future.
I'm not against using it. and, I'm going to give it a try.

Testing notes:

1. local render should work as before.

[MustafaJafar]

I gave it a quick test, it seems to work as expected.
2 AOVs Beauty and N, only beauty gets a reviewable which follows my configuration.

[MustafaJafar]

.

[MustafaJafar]

chose wrong line number again.

[MustafaJafar]

Here's it. this should work with create_skeleton_instance.

[BigRoy]

Please use as much as possible from core - so that it uses the exact same logic. We are looking to generalize and unify so that we don't need to duplicate data.

Does it generate too much data on the instance? Possibly. But that may just mean that either in core - we're just redundantly adding too much data. But I'd rather have a key too much here - than have every DCC differ in the data they contain. The end goal is that as much as possible remains the same, even to such an extent that perhaps we'd need almost no changes per integration.

As explained in the issue #356 my preference is that we just take e.g. the Collector from 3dsmax PR, 1:1 and make a comment of the differences that are actually needed.

Then when we have it working between Maya, Max, Houdini and Blender I'd compare all of them - and see where they differ and why... and start generalizing as much as possible. This may mean that down the line we'd just have core CollectLocalRender plug-in one could e.g. inherit from or alike where each integration would only add minimal changes.

The issue we're currently facing is that all integrations are essentially doing their own thing. But preferably, it'd take <15 minutes to reliably add support for local rendering to an integration. Once we have this generalization I'd start rolling out to more integrations where it's currently not supported yet - etc.

In short, if you can use the core create_skeleton_data and it works, please do so. I'm fine with putting some slight logic directly after where we comment # houdini-specific or alike, etc.

[MustafaJafar]

I think this is the closer I can get to collect local render plugin from other Addons.
Let me outline my findings.

Standard Local Render Collector

We didn't agree on a standard yet. but, let's consider these (ynput/ayon-maya#383, ynput/ayon-3dsmax#99 and ynput/ayon-blender#217) as standard.

1. Create skeleton(base) instance data using create_skeleton_instance
2. Create instance data for each AOV using create_instances_for_aov
3. Add non-picklable instance data to AOV instances data.
4. for each representation within each AOV instance data fill in roots in stagingDir
5. Create AOV instance
6. Add necessary families e.g. review family or if the addon needs a specific family for render instances like render.local.hou in Houdini.
7. Skip integrating original instance.

Houdini local reneder collector

0. $${\color{orange}Houdin \space Specific: \space we \space have \space configurable \space aov \space filters \space in \space Houdini \space settings.}$$
1. Create skeleton(base) instance data using create_skeleton_instance
2. $${\color{purple}Add \space pickable \space data \space to \space skeleton(base) \space instance \space data \space e.g. \space families, \space instance\_node, \space creator\_attributes, \space publish\_attributes}$$
3. $${\color{orange}Houdinig \space Specific: \space Remove \space frame \space data}$$
4. Create instance data for each AOV using create_instances_for_aov
5. Add non-picklable instance data to AOV instances data.
6. for each representation within each AOV instance data fill in roots in stagingDir
7. Create AOV instance
8. $${\color{orange}set\_representation\_colorspace \space per \space representation \space}$$ this to preserve the existing logic before this PR. also, I checked create_skeleton_instance and create_instances_for_aov they don't do it.
9. Add necessary families e.g. review family or if the addon needs a specific family for render instances like render.local.hou in Houdini.
10. Skip integrating original instance.

I can remove purple point if you want but the code might set a false thought that families must be added per aov instance at the end although it can be done once at the beginning.
On the other hand, I can't do something to orange points.

@BigRoy Let me know your thoughts.

[BigRoy]

I can remove purple point if you want but the code might set a false thought that families must be added per aov instance at the end although it can be done once at the beginning.

I'm not seeing any purple or orange btw ;)

---

Thanks @MustafaJafar - to be entirely clear. The standard I want is:

• Define what files would be coming out of your current instance (e.g. expectedFiles data, even though I'd love that to be more structured down the line.)
  • Include colorspace info (and any other metadata we'd expect).

So that e.g. a DCC would solely need to e.g. (overly simplified)

expected_files = instance.data.setdefault('expectedFiles", {})
expected_files["beauty"] = ["/path/to/1", "/path/to/2"]

Or potentially keep the metadata close as well, instead of storing instance-wide?

expected_files = instance.data.setdefault('expectedFiles", [])
expected_files.append(
    RenderProduct(
        files=[],
        aov_name="beauty",
        colorspace="linear"
    )
)

Or whatever data structure is the minimal amount of inputs we'd need to go through publishing.
The whole calling create_skeleton and create_from_aov is actually what I want to end up abstracting away.

It'd be along the lines of perhaps a core plug-in that essentially does a 'split-into-other-instances' type of thing based on a list of outputs files on the original instance. (I do think we can actually make that work with e.g. caches as well instead of just image render outputs.) But for now it's focused on exactly trying to not need to have the create_skeleton calls, etc. per integration but just have it clear what data needs to live where - etc.

class CollectLocalRenderInstances(core.SplitInstanceCollector):
    # These will be safely deep copied
    transfer_keys = {"instance_node"}
    # These will transfer, but won't be a unique copy
    # and are passed by reference.
    transfer_transient_data_keys = {"transientData.x", "

    def post_process_instance(
	    self, 
        source_instance: pyblish.api.Instance, 
        child_instance: pyblish.api.Instance
    ):
        # Houdini-integration collects these later
        child_instance.data.pop("frameStart")
        child_instance.data.pop("frameEnd")
        child_instance.data.pop("handleStart")
        child_instance.data.pop("handleEnd")

    def get_render_products(self, instance) -> list[RenderProduct]:
        # Default implementation here could be the conversion
        # from `instance.data["expectedFiles"]` and other `instance.data`
        # for backwards compatibility
        # But preferably this may not need overriding per dcc
        ...

Where transfer keys would end up allowing both complex python objects

---

Re: set representation colorspace, etc. - these currently rely on all being the same across all of the instances and are taken from instance.data but should really be metadata per render product so each renderlayer can have its own colorspace, etc.

[MustafaJafar]

Let me update this: #358 (comment)

Houdini local reneder collector

0. $${\color{orange}Houdin \space Specific: \space we \space have \space configurable \space aov \space filters \space in \space Houdini \space settings.}$$
1. Create skeleton(base) instance data using create_skeleton_instance
2. $${\color{orange}Apply Houdini \space Specific \space logic}$$
3. Create instance data for each AOV using create_instances_for_aov
4. Transfer keys from instance data to AOV instances data.
5. Update representations: filling root and set colorspace.
6. Create AOV instance
7. Skip integrating original instance.

[MustafaJafar]

So that e.g. a DCC would solely need to e.g. (overly simplified)

expected_files = instance.data.setdefault('expectedFiles", {})
expected_files["beauty"] = ["/path/to/1", "/path/to/2"]

This is the current solution in Houdini.

---

I believe RenderProduct is something we want for a long time.
But I think it can turn into an EPIC as we have other products as well not just render products. e.g. pointcache product and etc.
Also, I wonder would it be more accurate to say Product Version ? anyways, I think this discussion should be in a dedicated place.

[moonyuet]

See comment above as the latest commit errored out the collector:

Traceback (most recent call last):
  File "C:\Users\Kayla\AppData\Local\Ynput\AYON\dependency_packages\ayon_2603121036_windows.zip\dependencies\pyblish\plugin.py", line 528, in __explicit_process
    runner(*args)
  File "D:\ayon-addon_template\ayon-houdini\client\ayon_houdini\plugins\publish\collect_local_render_instances.py", line 100, in process
    self.post_process_skeleton_data(instance, instance_skeleton_data)
TypeError: CollectLocalRenderInstances.post_process_skeleton_data() takes 2 positional arguments but 3 were given

[MustafaJafar]

I gave this PR some test runs. And, the result are the usual before the PR.

[moonyuet]

lgtm

[MustafaJafar]

@BigRoy Any further comments before merging this?