OpenPype icon indicating copy to clipboard operation
OpenPype copied to clipboard

Houdini: New Publisher

Open antirotor opened this issue 3 years ago • 13 comments

Enhancement

This PR serves to convert old publishing system to the new one. It implements Houdini as a new host addon.

Developer notes

  • Context data is stored on root node. I am not sure if that is good idea or some custom node should be used for it.
  • use_selection creator attribute is defined on HoudiniCreator. If there is creator that doesn't need it, it has to override get_pre_create_attr_defs() - but I have a feeling that most creator will want to use use_selection.
  • imprint() now not only adds but also update data on instance. What to do with these parameters? I think they should be hidden for user or at least locked (but I couldn't find a way to set the lock directly in hou.ParmTemplate class and I don't feel comfortable to just locking all parameters in Extra folder on the node. We could store it directly as json in some hidden parameter not exposing it to user at all, or create OpenPype Data folder on the node and set everything there as locked? Or hide them and provide button to show when needed? I think editing those parameters manually should happen only as part of debugging process.

✏️ Note: It breaks currently old instances - backwards compatible collection of already existing instances must be added.

antirotor avatar Apr 08 '22 17:04 antirotor

Task linked: OP-3075 New publisher in Houdini

ynbot avatar Apr 08 '22 17:04 ynbot

This now works on single creator - Create PointCache. Publishing fails because more work has to be done on collectors, but it can serve as a base to rewrite other existing creators.

✏️ Note: It won't correctly display creator attributes in Python 2 variant of Houdini (#3782)

antirotor avatar Sep 02 '22 14:09 antirotor

Wanted to quickly test this PR. Getting this error when "resetting" in the UI after creating a pointcache instance I get this error:

Traceback (most recent call last):
  File "S:\openpype\OpenPype\openpype\tools\publisher\window.py", line 368, in _on_reset_clicked
    self.controller.reset()
  File "S:\openpype\OpenPype\openpype\tools\publisher\control.py", line 578, in reset
    self.save_changes()
  File "S:\openpype\OpenPype\openpype\tools\publisher\control.py", line 717, in save_changes
    self.create_context.save_changes()
  File "S:\openpype\OpenPype\openpype\pipeline\create\context.py", line 1121, in save_changes
    self._save_context_changes()
  File "S:\openpype\OpenPype\openpype\pipeline\create\context.py", line 1129, in _save_context_changes
    self.host.update_context_data(data, changes)
  File "S:\openpype\OpenPype\openpype\hosts\houdini\api\pipeline.py", line 137, in update_context_data
    lib.imprint(root_node, data)
  File "S:\openpype\OpenPype\openpype\hosts\houdini\api\lib.py", line 345, in imprint
    node.setParmTemplateGroup(parm_group)
  File "C:/PROGRA~1/SIDEEF~1/HOUDIN~1.303/houdini/python3.7libs\hou.py", line 15725, in setParmTemplateGroup
    return _hou.Node_setParmTemplateGroup(self, parm_template_group, rename_conflicting_parms)
hou.OperationFailed: The attempted operation failed.
Spare parameters are not alowed on the root node.

Re-opening the UI resolved it and then I was able to reset as expected.

Also note the typo in "allowed".

BigRoy avatar Sep 02 '22 15:09 BigRoy

Spare parameters are not alowed on the root node

That at least answers question about storing context data on the root node. We need to find better place.

antirotor avatar Sep 02 '22 16:09 antirotor

also I am wondering if this #3674 applies here too. I don't think so, because in new publisher all those parameters must have a value, but I need to think about it some more.

antirotor avatar Sep 02 '22 16:09 antirotor

That at least answers question about storing context data on the root node. We need to find better place

Potentially Houdini Variables. Those are per scene, get saved along and clear out when opening new scene. So it's persistent data with the scene.

  • Edit > Aliases and Variables (Alt + Shift + V)

Like in this image: IMG_3225

However, as you can see it is quite accessible to the user and variables become even accessible in Houdini nodes/wrangles, etc. So might not be what we want?


Otherwise this trick might do it to store it on the scene root.

hou.node(“/”).setUserData(“foo”, “bar”) 

Source

Instead of visible parameters store as user data that would just be hidden user data I believe.

BigRoy avatar Sep 02 '22 17:09 BigRoy

I have two questions.

First:

  • Does this PR mean that any existing scenes with old creators would now not work anymore since the creator identifiers have changed? That seems like quite a breaking change, no?

Second: afbeelding

It feels to me that members and instance_id don't seem like data that should persist in the scene. These values are "collected on the fly" in the Publisher window and for e.g. the Camera instances aren't actually the data that should be user-settable as an attribute like that.


For what it's worth, in houdini if we were ever to set members to node paths - why wouldn't the attribute be an Operator List parameter type? Should be much cleaner, no?

BigRoy avatar Sep 26 '22 10:09 BigRoy

t feels to me that members and instance_id don't seem like data that should persist in the scene. These values are "collected on the fly" in the Publisher window and for e.g. the Camera instances aren't actually the data that should be user-settable as an attribute like that.

Members could be created on the fly but instance_id is not. Instance id must persist on refresh or in any other process.

iLLiCiTiT avatar Sep 26 '22 11:09 iLLiCiTiT

Members could be created on the fly but instance_id is not.

Wouldn't the unique identifier in the scene be the node path in Houdini instead of that identifier? (It's just as unique?) Especially because it seems the OpenPypeContext or global data gets no unique_identifier. What's the reason we need this unique identifier to be that string attribute? Why could the Creator be able to return its own unique identifier for an instance, which could then be any string value?

BigRoy avatar Sep 26 '22 11:09 BigRoy

By the way, noticed a bug. If I change Validate Containers on the global Options then it persists that setting on the /obj/OpenPypeContext object. However, if I then reset the setting to its default value the publish_attributes will remain to be set to False. It seems that whenever publish_attributes was ever generated at least once on /obj/OpenPypeContext then changing the settings afterwards will not persist.

Also, the "Instance with id {} is already added to context." is logged so many times that I almost feel it's not worth being a warning log. With three instances in my scene this is the log on resetting the Publisher:

Instance with id 8acc2afe-9f8b-496c-b617-94180ebc183d is already added to context.
Instance with id 3b8282d9-17dc-45b1-be96-5d2d95298a2e is already added to context.
Instance with id 1be068f3-4a3a-46f3-8e40-3eda511dd6a5 is already added to context.
Instance with id 8acc2afe-9f8b-496c-b617-94180ebc183d is already added to context.
Instance with id 3b8282d9-17dc-45b1-be96-5d2d95298a2e is already added to context.
Instance with id 1be068f3-4a3a-46f3-8e40-3eda511dd6a5 is already added to context.
Instance with id 8acc2afe-9f8b-496c-b617-94180ebc183d is already added to context.
Instance with id 3b8282d9-17dc-45b1-be96-5d2d95298a2e is already added to context.
Instance with id 1be068f3-4a3a-46f3-8e40-3eda511dd6a5 is already added to context.
Instance with id 8acc2afe-9f8b-496c-b617-94180ebc183d is already added to context.
Instance with id 3b8282d9-17dc-45b1-be96-5d2d95298a2e is already added to context.
Instance with id 1be068f3-4a3a-46f3-8e40-3eda511dd6a5 is already added to context.
Instance with id 8acc2afe-9f8b-496c-b617-94180ebc183d is already added to context.
Instance with id 3b8282d9-17dc-45b1-be96-5d2d95298a2e is already added to context.
Instance with id 1be068f3-4a3a-46f3-8e40-3eda511dd6a5 is already added to context.
Instance with id 8acc2afe-9f8b-496c-b617-94180ebc183d is already added to context.
Instance with id 3b8282d9-17dc-45b1-be96-5d2d95298a2e is already added to context.
Instance with id 1be068f3-4a3a-46f3-8e40-3eda511dd6a5 is already added to context.
Instance with id 8acc2afe-9f8b-496c-b617-94180ebc183d is already added to context.
Instance with id 3b8282d9-17dc-45b1-be96-5d2d95298a2e is already added to context.
Instance with id 1be068f3-4a3a-46f3-8e40-3eda511dd6a5 is already added to context.
Instance with id 8acc2afe-9f8b-496c-b617-94180ebc183d is already added to context.
Instance with id 3b8282d9-17dc-45b1-be96-5d2d95298a2e is already added to context.
Instance with id 1be068f3-4a3a-46f3-8e40-3eda511dd6a5 is already added to context.
Instance with id 8acc2afe-9f8b-496c-b617-94180ebc183d is already added to context.
Instance with id 3b8282d9-17dc-45b1-be96-5d2d95298a2e is already added to context.
Instance with id 1be068f3-4a3a-46f3-8e40-3eda511dd6a5 is already added to context.

Which seems an awfully lot - and a lot of duplicates too?

BigRoy avatar Sep 26 '22 11:09 BigRoy

Does this PR mean that any existing scenes with old creators would now not work anymore since the creator identifiers have changed? That seems like quite a breaking change, no?

If it does not then it can be changed. Collection phase of creators gives option to use backwards compatibile approach.

Wouldn't the unique identifier in the scene be the node path in Houdini instead of that identifier? (It's just as unique?) Especially because it seems the OpenPypeContext or global data gets no unique_identifier. What's the reason we need this unique identifier to be that string attribute? Why could the Creator be able to return its own unique identifier for an instance, which could then be any string value?

There must be some unique identifier which is not host specific so you can point to it without knowing the host (or creator's) logic underhood. Also it helps to work with instances in publisher tool on change or refresh. If you want to do something with instance you don't have to know how it's unique in terms of creator but just know it's id and no matter when, you can have access to it even after restart or in different process.

Which seems an awfully lot - and a lot of duplicates too?

That's an easy fix PR.

iLLiCiTiT avatar Sep 26 '22 11:09 iLLiCiTiT

I have two questions. I am currently dealing with both these questions :) There should definitely be backwards compatibility with already created instances.

members were just a transient attribute I want to get rid of as it should be determined on-the-fly by creators. It's just that it was easier to make current plugins work until I'll figure a better way. For most of the current creators memeber == instance node itself so they can add that directly (without being it exposed as a visible attribute to the artist). For others it should be collected by its collectors.

antirotor avatar Sep 26 '22 11:09 antirotor

members were just a transient attribute I want to get rid of as it should be determined on-the-fly by creators.

So you could approve this PR to have it available? :)

iLLiCiTiT avatar Sep 26 '22 11:09 iLLiCiTiT

I have some trouble testing the new publisher as it is not opening:

Traceback (most recent call last):
  File "D:\REPO\OpenPype\openpype\tools\experimental_tools\dialog.py", line 171, in _on_btn_trigger
    tool.execute()
  File "D:\REPO\OpenPype\openpype\tools\experimental_tools\tools_def.py", line 61, in execute
    self.callback(*args, **kwargs)
  File "D:\REPO\OpenPype\openpype\tools\experimental_tools\tools_def.py", line 169, in _show_publisher
    self._publisher_tool = publisher.PublisherWindow(
AttributeError: module 'openpype.tools.publisher' has no attribute 'PublisherWindow'

In old publisher it complains about No registered families

image

Houdini 19.5.403 Aprentice

m-u-r-p-h-y avatar Oct 27 '22 07:10 m-u-r-p-h-y

I have some trouble testing the new publisher as it is not opening:

Traceback (most recent call last):
  File "D:\REPO\OpenPype\openpype\tools\experimental_tools\dialog.py", line 171, in _on_btn_trigger
    tool.execute()
  File "D:\REPO\OpenPype\openpype\tools\experimental_tools\tools_def.py", line 61, in execute
    self.callback(*args, **kwargs)
  File "D:\REPO\OpenPype\openpype\tools\experimental_tools\tools_def.py", line 169, in _show_publisher
    self._publisher_tool = publisher.PublisherWindow(
AttributeError: module 'openpype.tools.publisher' has no attribute 'PublisherWindow'

This was fixed yesterday in develop. I've merged it back in here so it should work.

In old publisher it complains about No registered families

This is right, there are no families for the old publisher. Old menu is there only because there might be someone with old creators still.

antirotor avatar Oct 27 '22 14:10 antirotor

I'm able to publish some basic families but not a whole lot of success overall. Even though I'm not sure if those are issues with me publishing wrong stuff, or problem with the new publisher.

nevertheless, when I delete an instance from the publisher, it's corresponding data is left in the scene, shouldn't we be cleaning up after ourselves a bit?

mkolar avatar Nov 07 '22 19:11 mkolar