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: optional shader stripping for model #444

Closed
wants to merge 9 commits into from

Conversation

antirotor
Copy link
Member

Changelog Description

Shaders are stripped from the model during extraction in Maya. This is adding option to disable that feature to retain shaders if needed.

Additional info

This is affecting OBJ extractor and Maya Scene model extractor. Default behavior is backwards compatible (stripping on), can be configured in creator settings.

Testing notes:

  1. create model in Maya
  2. disable shader stripping option
  3. publish
  4. load model - shaders should still be there

@antirotor antirotor added enhancement host: Maya sponsored This is directly sponsored by a client or community member labels Apr 23, 2024
@antirotor antirotor self-assigned this Apr 23, 2024
@ynbot ynbot added type: enhancement Improvement of existing functionality or minor addition size/XS labels Apr 23, 2024
@LiborBatek
Copy link
Member

@antirotor I did upload new maya addon package (0.1.16) and also removed older ones from the ayon-docker repo but I cant see any settings on the server

Seems there is some issue with the maya addon in this PR.

image

@antirotor
Copy link
Member Author

Seems there is some issue with the maya addon in this PR.

should be fixed

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.

Hmm no matter if turning Strip Shader On or Off it ends up the pretty much the same
Screenshot 2024-04-23 184038

...both versions (v002) with stripping On and (v003) without any Striping of shaders ends up with same files sizes and also when inspecting those assets....all having InitialShading group assigned.

Im not sure if only should be propagated to OBJ but even when inspecting those assets I cant see any difference in kilobytes and inside the file...

Screenshot 2024-04-23 184515

And when inspected inside (OBJ file)

image

@antirotor antirotor marked this pull request as ready for review May 13, 2024 14:55
@antirotor antirotor requested a review from LiborBatek May 13, 2024 14:55
@antirotor antirotor assigned LiborBatek and unassigned antirotor May 13, 2024
@antirotor
Copy link
Member Author

Should strip for OBJ too now

client/ayon_core/hosts/maya/plugins/publish/extract_obj.py Outdated Show resolved Hide resolved
client/ayon_core/hosts/maya/plugins/publish/extract_obj.py Outdated Show resolved Hide resolved
@@ -70,6 +70,9 @@ def process(self, instance):
noIntermediate=True,
long=True)

strip_shader = instance.data.get("strip_shaders", True)
self.log.debug("Stripping shaders: %s" % strip_shader)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this debug log too verbose? Does it help debug certain edge cases?

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 if you want to debug certain publish output - why your model comes without shaders, having this in the debug log can actually save you some time. But maybe it should be printed out only when strip shaders is enabled.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I also wonder whether "strip shaders" is better than reversing the terminology and naming the option "include shaders" or "export shaders" which is how usually export options are shown (whether something is included or not, instead of whether it's stripped?)

Copy link
Member Author

Choose a reason for hiding this comment

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

That term got there from the clients so I guess that is how they think about it. "Exporting models is stripping any shaders on them". What do you think @LiborBatek

Copy link
Collaborator

Choose a reason for hiding this comment

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

Totally understand it from the perspective of what model family has always been, etc. - But just looking at exporters in general and their terminology usage it's usually "what would you like to include"? That's why I wondered whether a export materials flag would be better?

Would love to know what Libor thinks as well. And maybe @dee-ynput from a product perspective.

@LiborBatek
Copy link
Member

Well to me its just really cosmetics...however the term strip could potentially cause some noise speaking of artists

Export Materials
Keep Materials
Include Materials
Remove Materials

any of these is clear enough for pretty much anyone imho.

Maybe the question is whether we should incorporate the new option in the Publisher as Enabled or Disabled by defaults and name it accordingly to it (remove OR keep materials etc) ...but maybe Im just thinking too much about it (backward compatibility and new option)

@iLLiCiTiT
Copy link
Member

Please move to https://github.com/ynput/ayon-maya

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
host: Maya size/XS 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.

5 participants