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

Use product name templates for render products on farm publish #447

Closed
wants to merge 19 commits into from

Conversation

antirotor
Copy link
Member

Changelog Description

Product names for render instance were hardcoded in the Deadline logic, not following product name template use in other places. This PR adds ability to use product name templates with all necessary keys to influence how render products are named. It also collects information about renderlayer (ROP node name) to anatomy data along with AOV name so it can be used for template formatting.

Additional info

Product names for instances to be published were hardcoded directly in the script here:

product_type = skeleton["productType"]
if not s_product_name.startswith(product_type):
group_name = '{}{}{}{}{}'.format(
product_type,
task[0].upper(), task[1:],
s_product_name[0].upper(), s_product_name[1:])
else:
group_name = s_product_name
# if there are multiple cameras, we need to add camera name
expected_filepath = col[0] if isinstance(col, (list, tuple)) else col
cams = [cam for cam in cameras if cam in expected_filepath]
if cams:
for cam in cams:
if not aov:
product_name = '{}_{}'.format(group_name, cam)
elif not aov.startswith(cam):
product_name = '{}_{}_{}'.format(group_name, cam, aov)
else:
product_name = "{}_{}".format(group_name, aov)
else:
if aov:
product_name = '{}_{}'.format(group_name, aov)
else:
product_name = '{}'.format(group_name)

This was bypassing any template set in ayon+settings://core/tools/creator/product_name_profiles. With this change, one can set template like this:

{product[type]}<{Renderlayer}><_{Aov}>

resulting in:

renderMainlayer_normal

It is up to TD to set correctly such template to make sure, product name will be stable enough. As a side-effect, optional keys can be used now in product name templates adding flexibility (and perhaps some danger of misuse).

renderlayer is now set on instance along with aov and is collected to anatomy data, so it can be used in template formatting so you can use template for renders like:

{root[work]}/{project[name]}/{hierarchy}/{asset}/publish/{family}/{subset}/{@version}/<{renderlayer}/>

Note

Already collected anatomy data (when available) could be passed to get_product_name() to allow even more keys to be used, but is there any real use case for it?

Also, as a point for discussion - there is variant that is now ignored in this case. It could be reasonable to use it exactly for this and move templating to "variant templates" but maybe it is breaking settings to even smaller unmanagable bits.

Testing notes:

  1. Set product name template to use renderlayer or aov keys (can be optional using<{key}> syntax)
  2. Publish renders from DCC supporting setting of these keys (like Maya and Houdini)
  3. Use {renderlayer} or {aov} in anatomy templates

collect rop name to instance to align with the `renderlayer` in maya
if `aov` and `renderlayer` is set on instance, use it in anatomy data for formatting templates
allow using optional keys in formatting of product name templates
remove hardcoded instance names in favor of product name templates
@antirotor antirotor added enhancement sponsored This is directly sponsored by a client or community member labels Apr 24, 2024
@antirotor antirotor self-assigned this Apr 24, 2024
@ynbot ynbot added type: enhancement Improvement of existing functionality or minor addition host: Houdini size/S labels Apr 24, 2024
@moonyuet
Copy link
Member

Should we also test to see if it works with 3dsmax multicam renders?

@antirotor
Copy link
Member Author

Should we also test to see if it works with 3dsmax multicam renders?

definitely - there is issue with cameras that was even in original code and I've kept the logic to be fixed in some other PR. Problem with that logic is that it is trying to find camera name from the camera list in the file name and first camera found is used. But that is very fragile because it cannot deal with ambiguous names (what if your camera is named the same as AOV or other part) and so on.

Copy link
Member

@LiborBatek LiborBatek left a comment

Choose a reason for hiding this comment

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

I have setup product template using the renderlayer token but constantly getting errors when trying publish

Screenshot 2024-05-02 164502

Traceback (most recent call last):
  File "C:\Users\lbate\AppData\Local\Ynput\AYON\dependency_packages\ayon_2402141620_windows.zip\dependencies\pyblish\plugin.py", line 527, in __explicit_process
    runner(*args)
  File "C:\Work\REPO\ayon-core\client\ayon_core\plugins\publish\collect_anatomy_instance_data.py", line 56, in process
    self.fill_latest_versions(context, project_name)
  File "C:\Work\REPO\ayon-core\client\ayon_core\plugins\publish\collect_anatomy_instance_data.py", line 263, in fill_latest_versions
    product_entities = list(ayon_api.get_products(
  File "C:\Program Files\Ynput\AYON 1.0.2\dependencies\ayon_api\server_api.py", line 4248, in get_products
    parsed_data = query.query(self)
  File "C:\Program Files\Ynput\AYON 1.0.2\dependencies\ayon_api\graphql.py", line 343, in query
    raise GraphQlQueryFailed(response.errors, query_str, variables)
ayon_api.exceptions.GraphQlQueryFailed: GraphQl query Failed: Name 'LightingMain_{renderlayer}' does not match regex '^[a-zA-Z0-9_]([a-zA-Z0-9_\.\-]*[a-zA-Z0-9_])?$' on item 'project/products' (Line 3 Column 5)

Comment on lines +36 to +37
# to align with maya render layers
instance.data["renderlayer"] = rop.name()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, what? Why is the ROP node's name used as the renderlayer? That seems odd?

Isn't it much easier if a studio wants this for them to us $OS as part of the productName or subset attributes on the node itself so that it's editable by the studio, requires no additional changes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but then you can't use it in anatomy data. One thing is product name, the other is to use it somewhere else in path. I agree that render layer is somewhat arbitrary name in Houdini context, but most of the DCCs are using render layer is one way or the other and I wouldn't spam the anatomy data with every DCC specific term.

Comment on lines +355 to +364
# make render layer name (or ROP name) available in the
# templates
render_layer = instance.data.get("renderlayer")
if render_layer:
anatomy_data["renderlayer"] = render_layer

# make AOV name if present available for templates
aov = instance.data.get("aov")
if aov:
anatomy_data["aov"] = aov
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's "renderlayer" if not part of the product name already? This seems like an odd decision to allow renderlayer (which as far as I know doesn't really have a valid connotation in Houdini?) to be part of the anatomy template? Or?

Copy link
Member Author

Choose a reason for hiding this comment

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

you might want to have render layer as a part of the path (not the whole product name).

Copy link
Collaborator

Choose a reason for hiding this comment

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

you might want to have render layer as a part of the path (not the whole product name).

When doing so - what would you anticipate the product name be instead?

Coming back to this from your PR description:

This was bypassing any template set in ayon+settings://core/tools/creator/product_name_profiles. With this change, one can set template like this:

{product[type]}<{Renderlayer}><_{Aov}>

resulting in:

renderMainlayer_normal

It is up to TD to set correctly such template to make sure, product name will be stable enough. As a side-effect, optional keys can be used now in product name templates adding flexibility (and perhaps some danger of misuse).

renderlayer is now set on instance along with aov and is collected to anatomy data, so it can be used in template formatting so you can use template for renders like:

In practice a user should usually never see this. They see the product name or representation in the loader or the manager. Which raises the question - how come its this crucial in the published filepath which usually isn't something that is displayed to the artists anyway?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it is just an option to categorize things when you need to have access to them outside loaders/pipeline. We get those requests from time to time when you have part that is not integrated and you need to access the files directly. Then dropping the whole render layer folder to DCC with all of it's AOV sequences can speed things up.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Totally agree that's valuable - but doesn't that mean we should just expose these as data on the representations that can be picked up by the Delivery action instead? Because I believe this currently requires the template to actually use {aov} in the published filepaths just so one can use that also in the delivery template since I think it stores only the used context on publish, right?

If not and this is solely for "delivery" templates I think it could be nice.

Copy link
Member Author

Choose a reason for hiding this comment

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

You mean to "force" it to context on representation even without it being used in the template so it can be picked up later on by delivery action? Like we are storing udim here

# Explicitly store the full list even though template data might
# have a different value because it uses just a single udim tile
if repre.get("udim"):
repre_context["udim"] = repre.get("udim") # store list
(and also frame somewhere)

Copy link
Collaborator

@BigRoy BigRoy May 13, 2024

Choose a reason for hiding this comment

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

Yes, similar to that. Not sure if that's "good design" - but that was indeed what I meant.

I'd expect this to way more valuable for delivery templates than production templates since in production templates it may be likely you will have duplicity anyway since usually the AOV is part of the product name as well.

@MustafaJafar MustafaJafar self-requested a review May 21, 2024 09:02
antirotor and others added 4 commits May 22, 2024 14:31
Co-authored-by: Jakub Trllo <43494761+iLLiCiTiT@users.noreply.github.com>
Co-authored-by: Jakub Trllo <43494761+iLLiCiTiT@users.noreply.github.com>
…om:ynput/ayon-core into enhancement/render-product-names-templated
@MustafaJafar
Copy link
Contributor

Hey, I was trying to test this PR in Houdini and I'm confused.

which one should I change ? and what is the expected behavior ?

  • ayon+settings://core/tools/creator/product_name_profiles
  • ayon+anatomy://{my-project}/templates/publish/1 render template.

As far as I know updating product_name_profiles affect the product name in the creator tab. and how does that support renderLayer (rop name) before actually creating the rop ?

Also, I wonder how does that work with run-time instances that are created in https://github.com/ynput/ayon-core/blob/develop/client/ayon_core/hosts/houdini/plugins/publish/collect_local_render_instances.py

@LiborBatek
Copy link
Member

LiborBatek commented May 22, 2024

@MustafaJafar I guess you need to change both...one alters the product name (aka rendered file name) and the other alter or sets the folder path for the rendered sequence (again using tokens like <aov>) which wasnt possible before I guess.

@MustafaJafar
Copy link
Contributor

MustafaJafar commented May 22, 2024

I tested this PR furtherly and Here my results in Houdini:
On farm it works as expected.

Render job Render after Integration
image image

On local rendering, I think some work needs to be added to https://github.com/ynput/ayon-core/blob/develop/client/ayon_core/hosts/houdini/plugins/publish/collect_local_render_instances.py
I'll add my local edits from my testing as a PR to this PR.
Edit: Here it's #543

Copy link
Member

@LiborBatek LiborBatek left a comment

Choose a reason for hiding this comment

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

Finally I could properly test it and thanks to @antirotor setup correctly my templates so it successfully finnished my DL job.

All seems working well.

host_name=instance.context.data["hostName"],
product_type=skeleton["productType"],
dynamic_data=dynamic_data,
variant=instance.data.get('variant', ''),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we really want to fall back to an empty variant?

Comment on lines +580 to +589
product_name = get_product_name(
project_name=instance.context.data["projectName"],
task_name=task_name,
task_type=instance.data["taskEntity"]["taskType"],
host_name=instance.context.data["hostName"],
product_type=skeleton["productType"],
dynamic_data=dynamic_data,
variant=instance.data.get('variant', ''),
project_settings=instance.context.data.get("project_settings"),
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm a bit worried about the fact that this now seems to require templates to use aov to ensure multiple AOVs become a unique product? Does this mean that with 'older templates' this would now break? Or what is ensuring that they become separate products?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm a bit worried about the fact that this now seems to require templates to use aov to ensure multiple AOVs become a unique product? Does this mean that with 'older templates' this would now break? Or what is ensuring that they become separate products?

older templates were ignored anyway - product name was hardcoded here so no matter what you had in the settings, it wasn't used. Now this ofc rises interesting question about transition - because with this, the existing templates will be suddenly used and without change, it will probably break. 🤔

…emplated-local-houdini-render

Use product name templates for render products on local Houdini render
@@ -64,6 +64,8 @@ def process(self, instance):
expectedFiles = next(iter(instance.data["expectedFiles"]), {})

product_type = "render" # is always render
# TODO: Match the value of product_group with the value of
# group_name in farm pyblish functions.
product_group = get_product_name(
context.data["projectName"],
context.data["taskEntity"]["name"],
Copy link
Contributor

@MustafaJafar MustafaJafar Jun 21, 2024

Choose a reason for hiding this comment

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

for reference,
I gave this PR a test. and was going to mention an issue with the product group name in local Houdini render.
Then, I figured out there was a comment about it in the code.

Anyways, it doesn't work because it depends on the profile template name and the function call doesn't pass the necessary dynamic data.
Also, group name shouldn't include the {aov} token, therefore should it use a different template name ?
image

Copy link
Member

Choose a reason for hiding this comment

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

how come it was working for @LiborBatek ?

Copy link
Member

Choose a reason for hiding this comment

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

how come it was working for @LiborBatek ?

because tested for maya I guess...

@antirotor
Copy link
Member Author

This need some transitional option in the settings so default template can be configured.

@antirotor
Copy link
Member Author

This will be reopened in Houdini addon new repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
host: Houdini size/S sponsored This is directly sponsored by a client or community member type: enhancement Improvement of existing functionality or minor addition
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

8 participants