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

Maya: abc options for Pointcache/Animation family - OP-5920 #5173

Conversation

tokejepsen
Copy link
Member

@tokejepsen tokejepsen commented Jun 22, 2023

Changelog Description

Add all options for alembic extraction on pointcache and animation families.

Testing notes:

  1. Create pointcache and/or animation (by loading a rig) in Maya
  2. Experiment with different publishing options.

Dependencies:

#5297
#5303

@ynbot
Copy link
Contributor

ynbot commented Jun 22, 2023

@ynbot ynbot added size/M Denotes a PR changes 500-999 lines, ignoring general files host: Maya type: documentation type: enhancement Enhancements to existing functionality labels Jun 22, 2023
Copy link
Collaborator

@BigRoy BigRoy left a comment

Choose a reason for hiding this comment

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

Are we sure about exposing ALL of this to the artist? I believe @mkolar recommended against this some time ago?

Additionally, I'd say this effort is better spent doing this against the new publisher too since this will basically break / be lost once that's merged.

@tokejepsen
Copy link
Member Author

Are we sure about exposing ALL of this to the artist? I believe @mkolar recommended against this some time ago?

Its been requested by a client.

Additionally, I'd say this effort is better spent doing this against the new publisher too since this will basically break / be lost once that's merged.

I agree, but I dont think we can wait for the merge of the new publisher.

@BigRoy
Copy link
Collaborator

BigRoy commented Jun 22, 2023

It's also backwards incompatible. I care less about existing project settings (an admin can fix that once) but all legacy instances using the old attribute names for write_color_sets or the other legacy attributes will now use the new name and lose their original (possibly tweaked) value of that scene for that option.

I understood from @antirotor that there's a big push to get maya on the new publisher for OP 3.16 so it might be coming soon too?

@tokejepsen
Copy link
Member Author

It's also backwards incompatible. I care less about existing project settings (an admin can fix that once) but all legacy instances using the old attribute names for write_color_sets or the other legacy attributes will now use the new name and lose their original (possibly tweaked) value of that scene for that option.

Cool, we can account for that.

@mkolar mkolar added the sponsored Client endorsed or requested label Jun 23, 2023
@tokejepsen
Copy link
Member Author

#5173 (comment)
@mkolar @antirotor @BigRoy @Minkiu bumping this as we should make a decision on it now before too much testing happens.

@LiborBatek LiborBatek self-requested a review March 7, 2024 14:25
@LiborBatek
Copy link
Member

LiborBatek commented Mar 7, 2024

So I have made another test round and seems ok when speaking of core functionality - no any errors shown and being able to produce animation and pointcache products without any issues (also tested loading em into workfile)

This time I have also tested passing of custom attributes and prepared some of those on my geometry (transform shape) as follows

Screenshot 2024-03-07 152346

Then setup my OP Settings like this:

Screenshot 2024-03-07 152322

However after publishing of animation and pointcache no any custom attributes were written if not explicitly set on Publish Instance (aka if left empty no attribs were written to Alembic)

so I have also tried to define those explicitly on Publish instances for animation and pointcache

Screenshot 2024-03-07 152424

This time all worked well and I have tested different combinations with prefixes and without, no issues at all and all good!

So my question being, if those custom attribs and custom prefixes should work if explicitly set or not. Seems like malfunction to me atm.
Edited: ok I can think of that the occasion Im having those custom attribute and custom prefix in Exposed Overrides it override those predefined by me above by empty values on Publish instance, leading to no any defined attribs in the end... am I right?? (see my screen on the top with my OP Settings and the combo of options)

@tokejepsen
Copy link
Member Author

Edited: ok I can think of that the occasion Im having those custom attribute and custom prefix in Exposed Overrides it override those predefined by me above by empty values on Publish instance, leading to no any defined attribs in the end... am I right?? (see my screen on the top with my OP Settings and the combo of options)

I'm not 100% sure what you are asking, but I think the issue might be overridden values in the publisher. When you have existing values in the publisher that is saved to the workfile, it wont get updated by the values from the settings.
For example I have mycustom;somecustom as Custom Attributes in the settings. When I open the publisher and generate a instance pointcacheMain, I get the correct mycustom;somecustom as Custom Attributes in the publisher. If I save this pointcacheMain to the workfile or publish it, it'll bake these Custom Attributes. If you then change the Custom Attributes in the settings, it wont update pointcacheMain in the publisher. If you create a new instance, the Custom Attributes will be updated though.

@LiborBatek does this resolve your issue?

@LiborBatek
Copy link
Member

Well I was more refering to Export defaults and if no any user changes being made...no any of my custom attribs got written.
I had to define those explicitly later by myself on the publish instance...then it started to work. Which to me is a bit odd (I have expected that when defined in the Project Settings then it should output those without need of any user interaction when in host app.

Does this make sense now?

@tokejepsen
Copy link
Member Author

Does this make sense now?

So this is what I've tested successfully:

  1. Setup the custom attributes default in Export Defaults and expose it in Exposed Overrides.
    Capture
  2. Load a rig. Only new instances will get the custom attributes as mentioned, cause existing attributes already have their custom attributes set.
  3. Add the custom attributes from the settings to an output node.
    Capture
  4. Publish and load the animation back in to validate the custom attributes have been published.

@LiborBatek could you take me through what you are doing and expecting to happen?

@LiborBatek
Copy link
Member

My experience so far being as follows:

  1. I define some custom attribs and prefix (as defaults)
Screenshot 2024-03-07 152322
  1. I define manually some attribs on my meshes in Rig asset (matching those from step 1)
Screenshot 2024-03-07 152346
  1. I publish Rig via publish rig instance (newly created)

  2. Use that Rig asset in animation and produce animation and/or pointcache (creating new publish instances and not touching any settings)

  3. load that animation/pointcache into empty scene and check for custom attribs are present

  4. unfortunatelly not any of custom attribs present!

  5. Going back to the animation and tweak pointcache and or animation publish instance and manually add those custom attirbs matching those on my rig meshes...

Screenshot 2024-03-07 152424
  1. Now all custom attribs got written into Alembic products

I would expect it would work for step 6) automatically because they are already setup in the OP Settings and should be picked up automatically during publishing process...

I still probably dont get the workflow correctly or I dont know whats goin on here :) sry

@tokejepsen
Copy link
Member Author

Use that Rig asset in animation and produce animation and/or pointcache (creating new publish instances and not touching any settings)

@LiborBatek in step 4, do you have the custom attributes from the project settings in the publisher?

load that animation/pointcache into empty scene and check for custom attribs are present

@LiborBatek in step 5, what loader are you using?

@LiborBatek
Copy link
Member

Use that Rig asset in animation and produce animation and/or pointcache (creating new publish instances and not touching any settings)

@LiborBatek in step 4, do you have the custom attributes from the project settings in the publisher?

My Publish Instance looks like this, just without those custom attribs marked in red (when kept empty as in step 4, no custom attribs written)....

Screenshot 2024-03-07 152424b ...so when performing step 7 (filling in those custom attribs manually) it looks exactly like this scrngrab

load that animation/pointcache into empty scene and check for custom attribs are present

@LiborBatek in step 5, what loader are you using?

Im using Reference loader for both animation and pointcache

@tokejepsen
Copy link
Member Author

My Publish Instance looks like this, just without those custom attribs marked in red (when kept empty as in step 4, no custom attribs written)....

Could you try loading in another version of ray_rigMain?

Newly created instance should get the custom attributes from the settings, so it should not be empty.

@tokejepsen
Copy link
Member Author

After discussing with @LiborBatek and the troubles he's had with keeping with the logic, I've implemented a optional validator that'll keep the instance alembic attributes in sync with the settings.

The case this should solve is when you have a rig in production which you need a certain Maya attribute published along with the alembic. You can publish the rig with the Maya attribute, but because the animation instances have already "baked" their alembic export options into the workfiles, the new required attribute setting wont get exported.

@BigRoy
Copy link
Collaborator

BigRoy commented Mar 28, 2024

After discussing with @LiborBatek and the troubles he's had with keeping with the logic, I've implemented a optional validator that'll keep the instance alembic attributes in sync with the settings.

The case this should solve is when you have a rig in production which you need a certain Maya attribute published along with the alembic. You can publish the rig with the Maya attribute, but because the animation instances have already "baked" their alembic export options into the workfiles, the new required attribute setting wont get exported.

Hmm well - but that would mean that by default anything that's set as "user editable" would essentially be 'blocked' from editing by default by the validator. So whenever they are deviating from the defaults on purpose they are also required to disable the validator?

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.

All works as expected and now also Validate Alembic Options Defaults kicks in when attribs not propagated or set by user and differ.

Repair works well too

When matching all publish ok. Also tested with asset without any custom attribs and vice versa. Also tested when loaded in the blank workfile ...all good!

Screenshot 2024-03-28 131801 Screenshot 2024-03-28 131817

@LiborBatek
Copy link
Member

@antirotor @mkolar still having request changes or can be merged now? all seems working well from my testing

@tokejepsen
Copy link
Member Author

Hmm well - but that would mean that by default anything that's set as "user editable" would essentially be 'blocked' from editing by default by the validator. So whenever they are deviating from the defaults on purpose they are also required to disable the validator?

@BigRoy yeah, that was the only way I could see how we could be able to propagate changes to existing instances. Otherwise artists wont be notified of any changes to the settings and because the instances publish attributes are baked into the workfile, they never receive the updated settings.
Open for suggestions but since the publisher UI is the last place a user can modify the settings, I could only see using a validator working.
It could also visually help if the settings were different when overridden from the settings like; ynput/ayon-core#146

@tokejepsen
Copy link
Member Author

still having request changes or can be merged now? all seems working well from my testing

@LiborBatek this will need an AYON port first before merging.

@BigRoy
Copy link
Collaborator

BigRoy commented Mar 29, 2024

@BigRoy yeah, that was the only way I could see how we could be able to propagate changes to existing instances. Otherwise artists wont be notified of any changes to the settings and because the instances publish attributes are baked into the workfile, they never receive the updated settings.
Open for suggestions but since the publisher UI is the last place a user can modify the settings, I could only see using a validator working.
It could also visually help if the settings were different when overridden from the settings like; ynput/ayon-core#146

I would likely disable the validator on our end but I see your standpoint. Being able to see what's non default would greatly help - 100%.

Copy link
Member

@antirotor antirotor left a comment

Choose a reason for hiding this comment

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

Looks good, port to AYON needed as stated.

@tokejepsen
Copy link
Member Author

Looks good, port to AYON needed as stated.

@antirotor port PR is already done, so this can be merged; ynput/ayon-core#336

@antirotor antirotor merged commit 9bf8528 into ynput:develop Apr 2, 2024
1 check passed
@ynbot ynbot added this to the next-patch milestone Apr 2, 2024
@tokejepsen tokejepsen deleted the enhancement/OP-5920_abc-options-for-Pointcache-Animation-family branch April 2, 2024 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
host: Maya host: UE port to AYON size/M Denotes a PR changes 500-999 lines, ignoring general files sponsored Client endorsed or requested target: AYON target: OpenPype type: enhancement Enhancements to existing functionality
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

9 participants