-
Notifications
You must be signed in to change notification settings - Fork 6
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: Publish any ROP node (Generic Creator) #2
base: develop
Are you sure you want to change the base?
Conversation
Also, it would be great, if the node changed color or added a sticker on top when published, especially since this helps to identify the publish node easily in the scene. Not a very important one, but helps little. |
Interesting request - when would you like the 'clear' this color or sticker? I can imagine it otherwise always having the sticker after the first publish you've done from the workfile. |
Yes, when its done fitst publish sounds good. Another instance I can think is It helps a lot when the filecache node publishes, coz in sops its easy to mess around to find the publish node. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've a question about collect batch families type.
instance.data["family"] = "generic" | ||
# TODO: Do not add the dynamic 'rop' family in the collector? | ||
instance.data["families"] = ["generic", "rop"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it possible to use CreateHoudiniGeneric.collect_instances
instead of this collector?
would it be better to use CreateHoudiniGeneric.collect_instances
instead of this collector? (I've included it in this review, the other comment below)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually - I didn't do that, because I believe that'd make it so that also the instance anatomy data collected for the instance would be the "generic" product type (which may affect the names for your resulting products or output files (I'm not entirely sure).
I recall there being a reason for doing it 'later' in publishing - but I'll try to reproduce some issues and maybe add a comment. But if it does work, and there are no side effects (🥁) then I'd love to remove this and rely on the Creator, yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a look at this - unfortunately we can't currently.
This is the Collector that turns the CreateContext
's CreatedInstance
items into the pyblish instances. As you can see it sets the instance's family to the product type - which makes sense, but also means that we can't override the family explicitly separate from the product type.
We want to preserve e.g. product type "pointcache" because that's what the publisher attributes are also based on (like certain export options specific for a product type) whilst avoiding the usage of the regular product type plug-ins. We may need to redesign how the familie work for the 'original legacy rop instances' so that we maybe can differentiate which plug-ins are exposed where.
This is again, a CreateContext / Publisher system design issue or I'm missing how I could best redesign this to work around that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the code withTODO
explaining the issue. Whether there are better ways, not sure. @iLLiCiTiT @antirotor
render_target_items = { | ||
"local": "Local machine rendering", | ||
"local_no_render": "Use existing frames (local)", | ||
"farm": "Farm Rendering", | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to add some condition here to add more items based on the instance ?
I tried but I found out that it works per creator not per instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No - unfortunately not. The current Creator system design and the Publisher UI do not allow it unfortunately. @antirotor I believe is aware of it and it's part of the "Extended Publisher" epic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MustafaJafar it may be that this PR ynput/ayon-core#860 by @iLLiCiTiT may solve this. The question then becomes WHAT would define which of the targets would be available?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is use existing frames (farm) not supported yet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is use existing frames (farm) not supported yet?
I believe currently it is for some, not all, if I remember correctly. Not sure about it exactly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is use existing frames (farm) not supported yet?
Render targets
feature is not yet implemented for all product types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BigRoy I gave it a test and it actually works although it was hard for me to learn how to use it. I can't really tell if there are any alternative solutions but, we can discuss that on discord.
ynput/ayon-core#860 (review)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding use exiting frames using farm is essential IMHO, there is a dicussion happened in couple of days ago, about this feature.
Local publish is tkaing huge amount of time and leaving houdini freezed until the publish get done. This a time saving for artists.
…/houdini_generic_publish' of https://github.com/BigRoy/ayon-houdini into enhancement/houdini_generic_publish
Hey! Just testing this branch, we cannot publish anything from houdini(workfiles/caches etc), somehow its getting conflict, I've attatched the error when we tried to publish workfiles with this branch
|
…enhancement/houdini_generic_publish # Conflicts: # client/ayon_houdini/api/lib.py # client/ayon_houdini/api/plugin.py # client/ayon_houdini/plugins/publish/extract_rop.py
…render" to use existing frames
Thanks @krishnaavril - should be solved now. It will now log a warning on creation. But that would only have been the case if you triggered the generic creation on a node that was already a publishable instance. I've changed the code, but would still be lovely if you could share a screen capture of how you get it to the state that it breaks. |
Thanks, Yes I do have the self-publish button enabled in Ayon settings.
Will test these pushed changes today, thanks |
I've tested the code with the labs karma node, I think the issue is still there
after create was done and when I clicked on publish this error popped up! |
publish_button_parm = hou.ButtonParmTemplate( | ||
"AYON_self_publish", | ||
"Publish", | ||
help="Directly publishes only this node and any inputs with a " | ||
"comment entered in a popup prompt. All other instances will " | ||
"be disabled in the publish.", | ||
script_callback="from ayon_houdini.api.lib import " | ||
"self_publish; self_publish()", | ||
script_callback_language=hou.scriptLanguage.Python, | ||
) | ||
ayon_folder.addParmTemplate(publish_button_parm) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should fix the bug reported in #2 (comment)
publish_button_parm = hou.ButtonParmTemplate( | |
"AYON_self_publish", | |
"Publish", | |
help="Directly publishes only this node and any inputs with a " | |
"comment entered in a popup prompt. All other instances will " | |
"be disabled in the publish.", | |
script_callback="from ayon_houdini.api.lib import " | |
"self_publish; self_publish()", | |
script_callback_language=hou.scriptLanguage.Python, | |
) | |
ayon_folder.addParmTemplate(publish_button_parm) | |
publish_button_parm = parm_group.find("AYON_self_publish") | |
if not publish_button_parm: | |
publish_button_parm = hou.ButtonParmTemplate( | |
"AYON_self_publish", | |
"Publish", | |
help="Directly publishes only this node and any inputs with a " | |
"comment entered in a popup prompt. All other instances will " | |
"be disabled in the publish.", | |
script_callback="from ayon_houdini.api.lib import " | |
"self_publish; self_publish()", | |
script_callback_language=hou.scriptLanguage.Python, | |
) | |
ayon_folder.addParmTemplate(publish_button_parm) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, the above logic always creates self publish button as it doesn't take the value of the following setting into consideration:
ayon+settings://houdini/general/add_self_publish_button
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can also do this which leverage self.add_publish_button
that is set in apply_settings
.
publish_button_parm = hou.ButtonParmTemplate( | |
"AYON_self_publish", | |
"Publish", | |
help="Directly publishes only this node and any inputs with a " | |
"comment entered in a popup prompt. All other instances will " | |
"be disabled in the publish.", | |
script_callback="from ayon_houdini.api.lib import " | |
"self_publish; self_publish()", | |
script_callback_language=hou.scriptLanguage.Python, | |
) | |
ayon_folder.addParmTemplate(publish_button_parm) | |
publish_button_parm = parm_group.find("AYON_self_publish") | |
if not publish_button_parm and self.add_publish_button: | |
publish_button_parm = hou.ButtonParmTemplate( | |
"AYON_self_publish", | |
"Publish", | |
help="Directly publishes only this node and any inputs with a " | |
"comment entered in a popup prompt. All other instances will " | |
"be disabled in the publish.", | |
script_callback="from ayon_houdini.api.lib import " | |
"self_publish; self_publish()", | |
script_callback_language=hou.scriptLanguage.Python, | |
) | |
ayon_folder.addParmTemplate(publish_button_parm) |
But, this has two downsides:
# TODO: Move this choice of automatic 'imprint' to settings so studio can | ||
# configure which nodes should get automatically imprinted on creation | ||
# TODO: Do not import and reload the creator plugin file | ||
from ayon_houdini.plugins.create import create_generic |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, never import plugins. Shouldn't node_type_product_types
from the creator be moved here (or to different pipeline file)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, never import plugins. Shouldn't node_type_product_types from the creator be moved here (or to different pipeline file)?
It should be taken from settings instead - or stored elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about settings. Those values don't seem like something what TD can fill, they actually look like they are quite pre-determined (based only on observation and my understanding).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are - until you get into the realm of custom HDAs (custom nodes) a studio develops on their own inside of Houdini (or get shipped with addons/plugins for Houdini and are not native to Houdini). We may not know about ALL nodes from our codebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, basically, pipeline admins would need to expand that list as needed e.g. to add their own types or like what krishna did at early stage of this PR ynput/ayon-core#542 (comment) when he tested labs::karma::2.0
node that we don't usually support in the current status of the addon, correct?
Maybe, they would spend some time to figure out the correct values for product types.. but, is it the same case across different addons where we have product type and families filters?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does moving node_type_product_types
to settings resolve this?
# Read creator and publish attributes | ||
publish_attributes = {} | ||
creator_attributes = {} | ||
for key, value in dict(node_data).items(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why it's not stored as json string? (talking about "publish_attributes"
and "creator_attributes"
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So that the attributes are actually accessible as options on the node for the user to click. We want them to be like native houdini parms so houdini users can do with them using houdini logic whatever they want - that's the whole point. ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So "handleStart" = 0
will become "handleStart": True
(as an example).
BTW considering in progress feature on create context: "Value change based callbacks to update attribute definitions or values." will break this assumption that you can freely change create and publish attributes with no consequences.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does exposing these publisher UI attributes on the node actually make sense?
BTW considering in progress feature on create context: "Value change based callbacks to update attribute definitions or values." will break this assumption that you can freely change create and publish attributes with no consequences.
Yes, makes sense. There's just a plethora of reasons why one WOULD want to actually exploit this type of full control in Houdini but I'm not confident whether those are production use cases that really make sense over time.
@fabiaserra @krishnaavril @MustafaJafar how much use would you say there is in actually having all these publisher UI options exposed as separate Houdini parms? Like, would you really ever have those parms reference some other attribute in a production scenario? Or is setting those via the publisher UI sufficient (it would allow us to create more dynamic UI over time without 'unexpected changes' coming back from Houdini).
See here for a screenshot with some of those exposed attributes.
In this case in particular the Creator Attributes and Publish Attributes
So "handleStart" = 0 will become "handleStart": True (as an example).
Luckily those are not exposed like that in Houdini 🙈 - but yes, that definitely points to a crucial bug that would need solving. However, 'setting' them om the attribute definitions in publisher UI themselves would then automatically turn that back into 0 and 1 so it may be 'ok' but it's definitely illogical code. Thanks for point it out!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I understand the questions but let me try explain why:
how much use would you say there is in actually having all these publisher UI options exposed as separate Houdini parms?
What do you mean by "separate"? If you just mean if there's a point on having these options exposed on the Houdini nodes natively... yeah, there's a lot of use, it allows artists to use Houdini workflows and not be limited by another UI, making it very easy entry point for Houdini users (the advanced options are probably confusing and irrelevant for anyone though?)
Like, would you really ever have those parms reference some other attribute in a production scenario?
Potentially, that's the beauty of Houdini
Or is setting those via the publisher UI sufficient
Not as it is, but not because just of the UI setting parameters... the whole publish framework being coupled with the export of the nodes is very limited IMO as I have already explained a few times
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, the node parameters and the publisher attributes are synced.
From the node to the publisher and from the publisher to the node.
That logic is implemented in collect_instances
by creating the instance manually and adding it to the context and update_instances
by the imprint
method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does exposing these publisher UI attributes on the node actually make sense?
BTW considering in progress feature on create context: "Value change based callbacks to update attribute definitions or values." will break this assumption that you can freely change create and publish attributes with no consequences.
Yes, makes sense. There's just a plethora of reasons why one WOULD want to actually exploit this type of full control in Houdini but I'm not confident whether those are production use cases that really make sense over time.
@fabiaserra @krishnaavril @MustafaJafar how much use would you say there is in actually having all these publisher UI options exposed as separate Houdini parms? Like, would you really ever have those parms reference some other attribute in a production scenario? Or is setting those via the publisher UI sufficient (it would allow us to create more dynamic UI over time without 'unexpected changes' coming back from Houdini).
See here for a screenshot with some of those exposed attributes.
In this case in particular the Creator Attributes and Publish Attributes
So "handleStart" = 0 will become "handleStart": True (as an example).
Luckily those are not exposed like that in Houdini 🙈 - but yes, that definitely points to a crucial bug that would need solving. However, 'setting' them om the attribute definitions in publisher UI themselves would then automatically turn that back into 0 and 1 so it may be 'ok' but it's definitely illogical code. Thanks for point it out!
Does exposing these publisher UI attributes on the node actually make sense?
BTW considering in progress feature on create context: "Value change based callbacks to update attribute definitions or values." will break this assumption that you can freely change create and publish attributes with no consequences.
Yes, makes sense. There's just a plethora of reasons why one WOULD want to actually exploit this type of full control in Houdini but I'm not confident whether those are production use cases that really make sense over time.
@fabiaserra @krishnaavril @MustafaJafar how much use would you say there is in actually having all these publisher UI options exposed as separate Houdini parms? Like, would you really ever have those parms reference some other attribute in a production scenario? Or is setting those via the publisher UI sufficient (it would allow us to create more dynamic UI over time without 'unexpected changes' coming back from Houdini).
See here for a screenshot with some of those exposed attributes.
In this case in particular the Creator Attributes and Publish Attributes
So "handleStart" = 0 will become "handleStart": True (as an example).
Luckily those are not exposed like that in Houdini 🙈 - but yes, that definitely points to a crucial bug that would need solving. However, 'setting' them om the attribute definitions in publisher UI themselves would then automatically turn that back into 0 and 1 so it may be 'ok' but it's definitely illogical code. Thanks for point it out!
Hello @BigRoy ! I think it was completly fine having properties in ayon publisher window, Honestly Me and my teams uses only visualise purpose, there is no change scenario seen in production from our end. (I was writing this coz we tested this rop publish when houdini is in core)
Can we write a logic where ayon filters the rop node inside any studio specific otl?
This will help to identify what kind of files will be writting(pointcache/renders) by node or by the location of node we adding(obj -> pointcache/usd, stage-> Usd/USD render rop), this will help on creating generic for any otl(without we hardcoded in the script), this should essentially open the possiblity of adding generic for any custom otl with no code changes(or a ayon frontend parameter will be good too, where we mention our node names and parameters). we also know studio uses there own otls for render, we can put in the documentation that to add duplicate there parameter and name it as a picture parameter, thus it refers the path and publishes.
Let me know if this make sense.
Implement Generic ROP publish from ynput/ayon-core#542
Changelog Description
This PR implements an idea for "lower level publishing" in Houdini. This implement Generic ROP publishing. Just create any Houdini ROP node (or custom Rop node HDA) and publish your product types from it!
Additional info
As part of the Ynput Houdini Workgroup sessions I developed this quick prototype to expose a way to batch publish and ingest files. Consider it more of an exploration of what's possible then a "drop it in production now" ready-to-go solution.
Explainer
Yes, this requires some explanation. Here you go.
2024-05-22.20-48-11.mov
What I forgot to add is that it currently still relies on detecting what the output files are for a ROP node based on a "file" parm that is often unique in Houdini per node. If anyone knows a way to just query the expected output files for a ROP node (similar to what PDG/TOPs seems to do I'd love to know!) but otherwise we'll just need to expand that list.
However, I also played with the idea of having "custom file list" attributes on the Node that when enabled could override the "Collector" logic and would instead use that list of files as the publish instance's files. So that e.g. one instance could also publish multiple representations. For that, @MustafaJafar did this lovely non-functional 'prototype' but it does get the idea across:
Demo scene file
The demo scene file:
ynts_char_hero_pdg_v012.zip
TODO 📔 ✏️
$OS
as default variant name.ayon+settings://core/publish/collect_comment_per_instance
✅ - I guess this is fine for now.get_product_name
logic of the Creator and the product types so that they adhere to project templates correctly.Known issues / Blockers 📔 🔥
Testing notes: