-
Notifications
You must be signed in to change notification settings - Fork 1
Implementation of publishing vdb product #49
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
base: develop
Are you sure you want to change the base?
Implementation of publishing vdb product #49
Conversation
|
@antirotor guess the representation trait you implemented in core addon would be super helpful for the implementation of this product type as there should be different templates used for publishing if we have some different vdb extractors (other than tyflow one) included in the future. |
Why would it need different templates? |
| # make sure the tyflow vdb extractor would not be triggered | ||
| # when the non-tyflow vdb workflow is adopted by the user | ||
| # in the future | ||
| "is_tyflow": True, |
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.
If that ever happens, that would be a different product type? Because this one is tyflow_vdb? It just feels like we're overcomplicating code here for a non-existing use case yet (since there isn't any other VDB extractor 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.
If that ever happens, that would be a different product type? Because this one is
tyflow_vdb? It just feels like we're overcomplicating code here for a non-existing use case yet (since there isn't any other VDB extractor yet?)
I am thinking the approach to be less complicated. It's just that the tyflow vdb export(select the target exporter node from the tyflow editor) is totally different from the general vdb export(select the target asset from the max editor).
I am thinking if it is possible to just implement the vdb product type in the creator but with quite specific creator identifier(as if what we did in Houdini for abc/bgeo pointcache).
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 recently did something similar with 'render' creators in Houdini. Those are now all render product type, regardless of renderer. It's this one: ynput/ayon-houdini#214
| """Creator plugin for TyFlow VDB.""" | ||
| identifier = "io.ayon.creators.max.vdb" | ||
| label = "VDB (TyFlow)" | ||
| product_type = "tyflow_vdb" |
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.
Note that a lot of VDB based loaders currently target the vdbcache product type, and hence will not allow loading this product type.
For example, these all target vdbcache product type.
ayon_maya/plugins/load/load_vdb_to_vray.py
ayon_maya/plugins/load/load_vdb_to_redshift.py
ayon_maya/plugins/load/load_vdb_to_arnold.py
ayon_houdini/plugins/load/load_vdb.py
| "productName": f"{container_name}_{prod_name}", | ||
| # get the name of operator for the export | ||
| "operator": operator, | ||
| "productType": "vdb", |
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.
Pretty sure it's vdbcache in other hosts.
| "label": f"{container_name}_{prod_name}", | ||
| "family": "vdb", | ||
| "families": ["vdb"], | ||
| "productName": f"{container_name}_{prod_name}", |
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 do not override productName directly - this should really be targeting vdbcache product type from the get go... from the creator itself so that the product naming templates apply accordingly. @iLLiCiTiT thoughts?
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 do not override
productNamedirectly - this should really be targetingvdbcacheproduct type from the get go... from the creator itself so that the product naming templates apply accordingly. @iLLiCiTiT thoughts?
if that's the case, we need to also change the similar variable to not override productName in tycache collector.
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.
Don't know full flow, usually when this is needed then the product name template contains some key that is kept unfilled, e.g. {some_key} then create plugin returns the key value in get_dynamic_data as unfilled.
def get_dynamic_data(self, *args, **kwargs):
return {
"some_key": "{some_key}",
}With that you can fill the product name during collection phase.
| elif attribute == "ty_vdb_data": | ||
| node.modifiers[0].name = "AYON TyFlow VDB Data" | ||
| else: | ||
| node.modifiers[0].name = "OP Data" |
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 "OP Data"?
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.
It's the name of general container attributes for other product type. It would be renamed after the related PR for this issue as the backward compatibility needs to be implemented for this.
| # in the future | ||
| "is_tyflow": True, | ||
| "publish_attributes": { | ||
| "ValidateTyVDBFrameRange": { |
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 looks like out of the scope. Is the ValidateTyVDBFrameRange plugin ever showed in publisher UI? Because it looks like it is not, and you're using the "optional" plugin only to enabled/disable validation of instances based on "has_frame_range_validator".
If that's the true, then don't use Optional plugin, just add something like families = ["vdb_frame_validation"] filter there and add the family "vdb_frame_validation" to the instance (the family vdb_frame_validation is first random name that came to my mind).
| if attr_values.get("has_frame_range_validator", | ||
| self.validate_tyvdb_frame_range): | ||
|
|
||
| instance.data["families"] += "vdb_frame_validation" |
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.
| instance.data["families"] += "vdb_frame_validation" | |
| instance.data["families"].append("vdb_frame_validation") |
| class ValidateTyVDBFrameRange(ValidateFrameRange): | ||
| label = "Validate Frame Range (TyFlow VDB)" | ||
| families = ["vdb_frame_validation"] | ||
| optional = True |
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.
| optional = True |
server/settings/publishers.py
Outdated
| default_factory=BasicValidateModel, | ||
| title="Validate Frame Range (TyCache)" | ||
| ) | ||
| ValidateTyVDBFrameRange: BasicValidateModel = SettingsField( |
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 probably have only enabled in settings, the others will not affect anything.
| class CreateTyVDB(plugin.MaxTyflowVDBCacheCreator): | ||
| """Creator plugin for TyFlow VDB.""" | ||
| identifier = "io.ayon.creators.max.vdb" | ||
| identifier = "io.ayon.creators.max.vdbcache" |
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 change means all existing instances won't be collected.
| icon = "gear" | ||
|
|
||
| def get_publish_families(self): | ||
| return ["vdbcache", "tyflow_vdb"] |
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.
Shouldn't that "tyflow_vdb" be added during collection phase based on create plugin identifier instead?
Changelog Description
This PR is to implement the publishing aspect for vdb product type. It is only supported for the vdb publish from TyFlow.
Solve #48
TODO List:
Additional review information
Export VDBoperators in the tyflow editor, otherwises nothing shows up for publishing in the insrance containervdbas product type into theayon+settings://core/tools/publish/template_name_profiles/6/product_typesTesting notes: