OpenPype
OpenPype copied to clipboard
Houdini: Enhance ABC publishing
Add automatic path attribute on creation, automatic output node connection.
- If
s@pathattribute does not exist, or not present on all primitives, generate automatic path where missing (on creation) - auto path would be constructed like this:{parent_node_name}/{output_node_name}(this behavior will have to be a bit more refined in the future) - if Output_SOP connector does not exist, create one, connecting it to the node with render flag on, and display it (display flag on).
[cuID:OP-1798]
If s@path attribute does not exist, or not present on all primitives, generate automatic path where missing (on creation) - auto path would be constructed like this: {parent_node_name}/{output_node_name} (this behavior will have to be a bit more refined in the future)
Would this be a Validator that adds in the extra nodes in Repair? Or when would this occur?
if Output_SOP connector does not exist, create one, connecting it to the node with render flag on, and display it (display flag on).
Could you elaborate slightly more about why this is needed?
Would this be a Validator that adds in the extra nodes in Repair? Or when would this occur?
Yeah, I think Repair action si the right place for it.
Could you elaborate slightly more about why this is needed?
This is currently required by validators (so again, it would be probably Repair action). I think Output is good to have to bring some order into what's going out - at least visually.
@BigRoy @moonyuet
Can we move the discussion here about the details of Houdini point cache enhancement ?
I'm sorry I should have done that earlier.
Here are the details I can think of : this code related to these files
houdini\plugins\create\create_pointcache.pyhoudini\plugins\publish\validate_primitive_hierarchy_paths.pyhoudini\plugins\publish\validate_sop_output_node.py
Current behavior
- on creation: it will set
sop_pathparm of the abc ROP to selection whether it's anObjNodeorSopNode - if selection is
SopNodesop_pathwill be set to aSopNodepath- on publishing: if no
pathattribute (path_attribof the abc ROP to be specific ), these validators willlog.errorvalidate_abc_primitive_to_detail.pyvalidate_primitive_hierarchy_paths.py
- if selection is
ObjNodeand it has a child of type"output"sop_pathwill be set to that"output" sop node pathwhich yields to the previous case
- if selection is
ObjNodesop_pathwill be set to anObjNodepath- on publishing: these validators will raise this error
AttributeError: 'ObjNode' object has no attribute 'geometry'validate_abc_primitive_to_detail.pyvalidate_primitive_hierarchy_paths.py
- After applying quick fix (checking
not hasattr(output_node, "geometry")) - on publishing:
validate_sop_output_node.pywilllog.error"Output node %s is not a SOP node. "- Note that it doesn't force a specific SOP node type
Desired behavior
Disclaimer: I personally don't know what could deliver a better user experience for artists, and I will appreciate any recommendations
- on creation:
- if selection is
SopNode- if it's not a node of type"output", an output node will be created and set its input to the selected node - on publishing: if nopathattribute (path_attribof the abc ROP to be specific ) - a repair action onvalidate_primitive_hierarchy_paths.pywill - create anamenode with the same attribute aspath_attribin ROP - add a default value{parent_node_name}/{output_node_name}or{parent_node_name}/{name_node_name}- it can be connected before or after the assigned node insop_path, it depends on the node whether it's of type "output" or any other type. - ANY RECOMMENDATIONS - if selection is
ObjNodeand it has a child of type"output"sop_pathwill be set to that"output" sop node pathwhich yields to the previous case
- if selection is
ObjNodesop_pathwill be set to anObjNodepath- Assuming I made the quick fix mentioned above
- on publishing:
validate_sop_output_node.pywilllog.error- a repair action to fix it, which will set thesop_pathof the ROP to the selected node (which is a general solution for all cases) - which yields to the first case
Conclusion
See things from this perspective, I can understand why a sop "output" node is not a mandatory.
Connecting the name node before won't be a good thing if the node in sop_path is a multi-input node like "merge" node
if it's a sop "output" node, then the name node must be connected before it
Using the 'output' sop-node and attaching the name-node before it satisfies the problem description above, which is what I was originally trying to achieve.
Let me know what do you think ?
For desired behavior:
Other than sopNode, it will be great to also include dopNode for selection although user usually cache as bgeo and use file cache to get the related bgeo into SOP level to do the alembic export. Adding dopNode for selection just in case that users may want to export the alembic within the DOP network. You can read this reddit post: https://www.reddit.com/r/Houdini/comments/f6oefh/i_have_a_simple_dop_simulation_that_i_want_to/
It can show that you can do alembic export for the DOP network.
Some comments on Output Node Not quite sure about the standard workflow but I personally dont use output nodes except for rendering. Usually I use null nodes at the end of all creation. If the purpose of the point cache are to export and do comparison between several same objects with different fracturing levels, it is okay to create output node and hook it into the node..(but you can preview these fracturing objects quickly with exploded view node for sure)
Path Attribute Repair Action As more I look at the issue (and PR), more questions come in my mind regarding to repair action(It can be that I overthink too). As mentioned in the PR, the path attributes can be created with different attribute-related nodes. Not only attribute-create node shown in the PR, you can create path attributes with string data by connectivity node.
Many options can be choosed for storing the attributes data in Houdini, meaning that the nodes and the parameter which links to the string datacan be varied. E.g. The relative reference of parm is chs("prefix") for connectivity, while chs("string1") in attribute create and chs("name1") in the name attribute.
In more complicated case, I am not sure if users use attribute promote (promoting point to primitive etc.) too. The repair action is only helpful for the occassion when there is no path attribute in the scene.
By this, it may be better idea to have one invalid options to show the user doesn't include string data in the path attribute, recommend what to use for the string data: i.e. 'opname("..")/opoutput(".", 0)'
As far as I understood, Here are the suggested edits:
-
houdini\plugins\create\create_pointcache.pyIt's better to leave it as it was, plus we may do this
- if selection is
ObjNode, if output node wasn't present, the node with the active display flag will be used
- if selection is
-
houdini\plugins\publish\validate_primitive_hierarchy_paths.pyadding a repair action to add a node with a default path value
-
node name
AUTO_PATHthere are 4 possibilities:name,attribute create,connectivity,vex wrangleconnectivityis better as it will incrementprefixand all connected primitives will share the same number -
attribute value
opname('..')/opname('..')Shape_ -
node comment
something similar to this, so that it will be clear for artistsauto path node created automatically by repair action Feel free to modify or replace it. -
node connection
connect before if the output sop node of typeoutputornullconnect afterwards for other types, setsop_pathto it
-
-
houdini\plugins\publish\validate_sop_output_node.pyadding a repair action to set
sop_pathto the selected node- if
SopNodeis selected, set its path tosop_path - if
ObjNodeis selected, follow same logic ascreate_pointcache
- if
-
applying quick fix (checking
not hasattr(output_node, "geometry"))validate_abc_primitive_to_detail.pyvalidate_primitive_hierarchy_paths.py
Regarding including dopNode
I think it can be possible during the creation process
if selected is dopNode then I would do this:
- create a new
geonode - create
dop import - create
unpacknode - use
unpackinstead of the selected node, continue default execution flow
Creator I agree that we should leave it as it is for current stage.
Validator
For validate_primitive_hierarchy_paths.py, there is repair action for sure but there are more than four possibilities to create primitive attribute. So we need to do accurate and more general checks on it.
If there is no primitive attribute named path in the network, it wont pass the validation. Rather than doing a check on just four certain nodes.
Repair action can't repair anything as the relative references of parameters to store attribute values vary from node to nodes.
But still it needs to be inside the validator for creating a new name node for storing the path attribute and connects to the node if the selected network doesn't have path attributes.
For current stage, maybe try not to touch validate_sop_output_node.py as it is not only pointcache using but other families like vdbcache involving.
DopNode
I dont have so much objection on that. but I think you can check the see if the dop nerwork from the dop import has the pack nodes.
Any thoughts? @BigRoy @antirotor
We are currently supporting SOPs level exports to Alembic (if we ignore the camera export that is). The alembic exporter additionally supports exporting Object level nodes but we do not support that yet in the publishing - adding support for that should be done but should definitely be an issue/PR of its own because it'll require different validators all together (e.g. path parm and use sop path parm must be disabled)
So we support the "Use SOP paths" enabled route only - that's what we are working to enhance with this issue and PR #5189
adding a repair action to set sop_path to the selected node if SopNode is selected, set its path to sop_path if ObjNode is selected, follow same logic as create_pointcache
I thought that whenever Use SOP Path is enabled on the ROP node you should not set it to an ObjNode. However it seems that this does work:
Which to me is confusing Houdini design because that's not a SOP path, hehe. Anyway, for our validations whenever and OBJ node is found as output node we'd likely need to isntead validate against:
- Output node with the lowest index (usually 0) (or maybe that's
ObjNode.outputForViewFlag?) I'm not sure what the code is to get the output node for anObjNode. It seems thatObjNode.subnetOutputs()outputs at least the output nodes so might be the way to go to find the lowest index one? - If no output node, then the node with the display flag set. ->
ObjNode.displayNode()
It doesn't change the validations itself, but only how we retrieve which node should be validated. I'm pretty sure that currently does not work during publish.
DopNode I dont have so much objection on that. but I think you can check the see if the dop nerwork from the dop import has the pack nodes.
I'm not too knowledgable about DOPs, but if the alembic rop doesn't support that natively with Use SOP path I'd say let's not try and automagically convert it.
As such, if this means we need to create extra nodes to import it to SOPs then I'd say, no. Let's not do that. It's up to the Houdini artist to create SOPs geometry for the alembic export to work. If they have something in DOPs, they convert it to SOPs and export the SOPs. If they have something in LOPs, they convert it to SOPs, etc. We're exposing the export feature to Alembic that they are familiar with from Houdini itself - we're not designing new export conversion features. Let's try and touch that as little as possible so we give the Houdini artist as much freedom as possible to do what they are great at.
Repair action can't repair anything as the relative references of parameters to store attribute values vary from node to nodes. But still it needs to be inside the validator for creating a new name node for storing the path attribute and connects to the node if the selected network doesn't have path attributes.
@moonyuet not sure what you're saying here. Why couldn't it?
I have to admit that I have a relatively hard time grasping what's being discussed here. There's a lot of information for something that initially seemed so trivial. I think we've lost somewhere what we are actually trying to solve and why. It would be great if for each mentioned changed that we want to do we could add a succinct explanation as to why that change is needed and what it solves.
@BigRoy,I admit that it is getting more and more confusing.
I am actually talking about the string value where stores the opname('..')/opname('..')Shape. So basically the nodes such as attribute_create or connectvity can store path primitive attributes with string value. But the relative references of the string value are different. e.g. the one from the attribute_create is chs("string1") while that from connectivity is chs("prefix"). Maybe the screenshot can explain more than words. Repair action may not fix all the errors as the relative references from nodes to nodes are different.
I am actually talking about the string value where stores the opname('..')/opname('..')Shape. So basically the nodes such as attribute_create or connectvity can store path primitive attributes with string value. But the relative references of the string value are different. e.g. the one from the attribute_create is chs("string1") while that from connectivity is chs("prefix"). Maybe the screenshot can explain more than words. Repair action may not fix all the errors as the relative references from nodes to nodes are different.
I still don't see the issue unfortunately. We never validate those particular nodes - we only validate the output and whether it's geometry is ok or not. If the geometry is invalid, then I believe currently this issue proposes to add repair actions which "insert" some nodes above the output node that add a path entry. That repair has full control over the nodes it generates and thus it should be trivial to set the right values. Right?
So we'd add repair actions like:
- Set single name
- Name using connectivity
Then whatever we use as node to set the actual name, either wrangle, name or attributecreate doesn't really matter. We just pick the one that we feel is the fastest node to use (which performs best), is the clearest for the artist in their graph visually and is easiest in code to maintain.
Let's take a few steps back: To clear up some misconceptions, correct me if I'm wrong the original objective of this issue was to:
- add
pathrepair action - create
outputsop node if not present
ouptut sop node + my ignorance, they added few complexity
my apologies.
let me summarize this discussion:
enhancement 1 : path repair action
if no path attribute, A connectivity node with
attribnamesame aspath_attribin the ROP nodeprefixlike thisopname('..')/opname('..')Shape_- Connected as follows
- before output node if the output node's is type
nulloroutput - otherwise after output node and set
sop_pathaccordingly
because output node may be a merge or copy node and connecting before will lead to un-desired results
- before output node if the output node's is type
P.S.
I prefer connectivity as it will increment for each connected piece
In maya it will be one object with many shapes inside it, we can discuss if this will be sufficient for now.
enhancement 2 : output repair action
because Validate Output Node requires a sop node set in sop_path in the ROP node
this validator will error for many reasons
a general repair action to set it to the selected sop node
I suggest to extend it allow selecting obj nodes as well and using the same logic to get the correct output node
this repair action will affect these families
pointcache,vdbcache
enhancement 3 : create point cache
bear in mind that Validate Output Node requires a sop node
this code doesn't correctly detect the output node whether it's an output node with the smallest outputidx
or the node with the display flag
also, it allows all node types e.g. I can create an alembic subset while selecting a dop node, an artist may wonder why it's allowed despite publisher will error
# try to find output node
for child in self.selected_nodes[0].children():
if child.type().name() == "output":
parms["sop_path"] = child.path()
break
bug fix : validators error (crash)
it happens when this line is executed as for many reasons, the output node may not have geometry
geo = output_node.geometryAtFrame(frame)
this bug happens in
validate_abc_primitive_to_detail.pyvalidate_primitive_hierarchy_paths.py
a quick fix to add a check like this: to skip if not hasattr(output_node, "geometry")
this fix may be irrelevant to this issue however I needed to fix it to test these enhancements
Let me know that do you think ? @moonyuet @BigRoy @antirotor
Thanks for the breakdown
enhancement 1 : path repair action
I think this can be a nice feature. Definitely a PR on its own... but there are so many edge cases that I'm still wondering whether it's worth it.
In many cases, Connectivity is NOT the way to do it. In many case, single name is NOT the way to do it. Etc. what if connectivity changes over time? etc.
The highest priority is that the Validator explains the issue to the artist in a way that is understandable. What data is missing, why and how can I fix it? Then having a repair is secondary to that. If you feel artists get stuck 90% of the time, our priority is improving the reports output so they know what to do.
Once that's done, then we can look into tooling that can provide some out of the box options which will be INVALID 90% of the time: Set path by connectivity, Set single path, Set path by attribute X, etc. but we will need to keep in mind that these are just "helpers" and not a full blown repair since there are so many cases where it's NOT the intended export design for your particular case in production.
enhancement 2 : output repair action
this repair action will affect these families
pointcache,vdbcache
Since this influences VDBCaches as well we can't use the same logic for both unfortunately - since afaik we should never set an object level path. Which actually makes me think allowing obj level path (as described in enhancement 3 below) should just be disallowed from OP point of view. It's just much more complex, and by still allowing explicit SOP level paths we still allow it to export the same things. (We're not losing features by requiring a SOP path instead - we're solely making it more explicit)
Having a repair action that updates the SOP path using the selection can be nice. However, we'll need to make sure that the action's label is not "Repair" but something like "Update sop path from selection" or alike because with "Repair" you wouldn't know it'll be using your selection. Which makes me think, isn't it much clearer if we just LOG very clearly what the user should end up doing (setting a valid SOP path in the SOP Path parameter) and maybe just having a "Select invalid" action on the plugin. The log then clearly mentions:
"The SOP Path is not set to an existing SOP path, make sure to update it on the ROP node." and then you can just select the ROP node to do it yourself. I'm not sure a "Set from selection" is going to be that much clearer for the artist especially compared to how much code might need to be written to write it and maintain it over time. Is it worth it?
enhancement 3 : create point cache
bear in mind that
Validate Output Noderequires a sop node
Yes.
this code doesn't correctly detect the output node whether it's an output node with the smallest
outputidxor the node with the display flag
This might be a language barrier in your wording, but it doesn't have to be an output node with the smallest id NOR does it need to have the display flag. The preferred way pipeline-wise would be to have it point to a SOP path on the ROP node, but apparently Houdini Alembic ROP supports putting an OBJ path in the SOP path parm - confusing, but hey, let's support it.
As such, output SOP node becomes:
output_node = = instance.data["output_node"]
if isinstance(sop_node, hou.SopNode):
# all ok
sop_node = output_node
elif isinstance(sop_node, hou.ObjNode):
# convert to relevant sop path
sop_node = get_obj_output(output_node)
if not sop_node:
sop_node = get_obj_display_output(output_node)
# Technically this could still be None if there are no nodes in the geometry node to begin with
However, as I mentioned in Enhancement 2 we're likely better of disallowing ObjNode paths to begin with and forcing SopNode paths.
As you can clearly see - even a simple case like this becomes way more complicated now ;) As such I'd recommend taking each of the enhancement you mention here into separate PRs since each likely involves code changes quite isolated plus already quite lengthy each.
also, it allows all node types e.g. I can create an alembic subset while selecting a dop node, an artist may wonder why it's allowed despite publisher will error
I think logic wise maybe it just makes sense to set any SOP path from selection if found, if not found set the OBJ node path (since apparently that works in Houdini) and if not found then set nothing from selection with maybe logging a warning that no valid selection was found.
selection = self.selected_nodes
path = None
preferred_node_types = [hou.SopNode, hou.ObjNode]
for type in preferred_node_types:
if path is None:
for node in selection:
if isinstance(node, type):
path = node.path()
break
Not the prettiest code, but it shows the idea. We don't need to redirect to "output" nodes, etc.
bug fix : validators error (crash)
Yes, let's add the fix. Could even be a PR of its own since it doesn't change much about the enhancements.
So, what about:
enhancement 1: Validate Prims Hierarchy Path(ABC)
It's already logs info to artist, I find it useful. however for this issue, I can add a create default path
which uses name node to make a single default path with a comment on the node to provide more guidance for artists.
Auto path node created automatically by "create default path" action
Feel free to modify or replace it.
enhancement 2 : Validate Output Node
log already mentions that SOP Path must point to a SOP node
I think adding actions like
Select Invalid ROP so that artist can fix sop_path themselves
Set selected SopNode as output node that only set sop_path if artists selected a SOP node
I like the first one more as it will be clear which ROP node to be fixed the second can be confusing when publishing many point caches
enhancement 3 : create point cache
I like forcing SopNode paths on creation which is going to set the sop_path to
- the selected sop node
- or the output of the select obj node
- or keep it empty + logging
bug fix : validators error (crash)
as discussed above
@BigRoy
Sounds good. Let's draft each of those and go from there.
There are many PRs related to this issue!
enhancement 1: Validate Prims Hierarchy Path(ABC)
✅ Enhancement/houdini add path action for abc validator
enhancement 2 : Validate Output Node
✅ Houdini: add select invalid action for ValidateSopOutputNode
enhancement 3 : create point cache
✅ Houdini: better selection on pointcache creation
bug fix : validators error (crash)
✅ Houdini: Add geometry check for pointcache family
✅ Houdini: Add geometry check for VDB family
it's not related to this issue however it shares the same bug as the point cache.