Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Houdini: Optimize collect inputs by caching scene containers once #305

Merged

Conversation

BigRoy
Copy link
Collaborator

@BigRoy BigRoy commented Mar 28, 2024

Changelog Description

Optimize collect inputs by caching scene containers once

Additional info

Behavior should be the same - it just doesn't list all scene containers and involved nodes per instance, but does it only once in the publish session, making it much faster when running over many instances.

Testing notes:

  1. Publish a few different Houdini products, preferably with loaded content used in the inputs - should work without errors.

@ynbot ynbot added type: enhancement Improvement of existing functionality or minor addition size/XS host: Houdini labels Mar 28, 2024
@MustafaJafar MustafaJafar added the community Issues and PRs coming from the community members label Apr 1, 2024
# down. As such, we cache it so we trigger it only once.
# todo: Instead of hidden cache make "CollectContainers" plug-in
cache_key = "__cache_containers"
scene_containers = instance.context.data.get(cache_key, None)
Copy link
Member

Choose a reason for hiding this comment

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

just a small question, is the cache_key always named as __cache_containers? It seems quite hard-coded to me but if it is always named like this, it is still okay.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good question. In the past I had sometimes prefixed it also with the class name to ensure better uniqueness. But yes, in a way it is hardcoded. Feel too unsafe? Should I prefix it more?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@moonyuet let me know what you'd like me to do here to make this ready for merging

Copy link
Member

Choose a reason for hiding this comment

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

@moonyuet let me know what you'd like me to do here to make this ready for merging

we can merge this first as I guess there would be enhancement when building the universal node for publishing.

@moonyuet
Copy link
Member

moonyuet commented Apr 5, 2024

Tested with houdini, looks good and please review comment above.

Copy link
Contributor

@MustafaJafar MustafaJafar left a comment

Choose a reason for hiding this comment

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

I think it works.
I tried many cases :

  • point directly to the sop node in loaded products.
  • point to the obj node that's parent of the loaded sop nodes.
  • point to an object merge node that combines different loaded nodes.

I'm not quite familiar with this plugin.
So, we trace of any loaded nodes that contribute to the output data and save their ids in "inputRepresentations" ?
if my description is correct, then I'm wondering how could we benefit from that piece of information ?

image

Co-authored-by: Kayla Man <64118225+moonyuet@users.noreply.github.com>
@BigRoy
Copy link
Collaborator Author

BigRoy commented Apr 5, 2024

I'm not quite familiar with this plugin.
So, we trace of any loaded nodes that contribute to the output data and save their ids in "inputRepresentations" ?
if my description is correct, then I'm wondering how could we benefit from that piece of information ?

That's correct - it detects what input loaded content was used in your output so they could turn into "Links" in your publish. I believe there's a IntegrateInputLinks plug-in that uses this data.

The data gets converted here:

class CollectInputRepresentationsToVersions(pyblish.api.ContextPlugin):

Then those turn into "generative links":

input_versions = instance.data.get("inputVersions")

It's so that from a particular publish you can see which input versions were used. So that with e.g. the (upcoming) links UI in the web frontend you could trace the inputs upstream and outputs downstream of each publish.

In OpenPype the loader also had a "Dependencies" tab bottom right hand side. This hasn't been ported to AYON loader (yet?) but could basically display the same info.

@moonyuet moonyuet merged commit 6c2aa22 into ynput:develop Apr 12, 2024
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Issues and PRs coming from the community members host: Houdini size/XS type: enhancement Improvement of existing functionality or minor addition
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants