Add opacity threshold to USD Preview Surface Shader #917
Conversation
| @classmethod | ||
| def setUpClass(cls): | ||
| cmds.loadPlugin('mayaUsdPlugin') | ||
| cls.temp_dir = tempfile.mkdtemp() |
There was a problem hiding this comment.
You might want to consider moving this test to test/lib/usd/translators
In that folder there are utilities that will help setup a full test environments in the build area where the output files will be preserved, allowing easy investigation in case of problems. Look for inputPath = fixturesUtils.setUpClass(__file__) in files like https://github.com/Autodesk/maya-usd/blob/dev/test/lib/usd/translators/testUsdExportRfMShaders.py
There was a problem hiding this comment.
@JGamache-autodesk Thanks! I'll switch over our tests to live there. Quick question regarding naming...
I've been calling our tests testMayaUsdxxx but the ones in the translator section are testUsdxxx or textUsdMayaxxx. Which naming should I be going with?
There was a problem hiding this comment.
We haven't really set any naming convention for tests other than it should start with test. For translators, please follow the naming convention you see in https://github.com/Autodesk/maya-usd/blob/dev/test/lib/usd/translators/, i.e. testUsdxxx.
There was a problem hiding this comment.
Okay moved this over.
dgovil
force-pushed
the
opacity-threshold
branch
from
f1fc385
to
78e8633
Compare
dgovil
force-pushed
the
opacity-threshold
branch
2 times, most recently
from
e7112a3
to
a4ba188
Compare
There was a problem hiding this comment.
Thanks for this @dgovil! Just two things I think we should address before it goes in:
- I don't think we need
opacityThresholdAttrto bepublic/static, especially since none of the other attributes are. - I think we want to use
<instead of<=when testing the threshold.
Other than that, what's here looks good to me!
Note though that this is only part of the story. What's in compute() really only comes into play for software renders, or in preparation for feeding values into the shading fragment graph, so there's another chunk of work to this where we'd want to add support for opacityThreshold in the viewport.
We used to have a mechanism for keeping USD's previewSurface.glslfx and the shading fragments used by pxrUsdPreviewSurface in sync (or at least flagging when they were out of sync) back when they were both in the core USD repo, but since the split we've lost that. There have been a bunch of other changes to USD's implementation recently and I've been meaning to take a pass at sync'ing the two back up again.
test/lib/usd/translators/testUsdExportImportRoundtripPreviewSurface.py
Outdated
Show resolved
Hide resolved
dgovil
force-pushed
the
opacity-threshold
branch
from
a4ba188
to
9578137
Compare
dgovil
force-pushed
the
opacity-threshold
branch
from
9578137
to
046532a
Compare
|
@mattyjams I fixed up your notes. Agreed, that this doesn't resolve the actual shader work. We hadn't really bothered on our end since shader transparency was broken till recently. If/when you do sync up the shaders, that would be great. |
kxl-adsk
added
the
ready-for-merge
Yes, we are aware of this too. It's a task on our end as well, but we haven't got a chance to prioritize it yet. |
Opacity Threshold is an attribute on the USD preview surface shader that wasn't previously exposed to Maya, however is useful for artists to define the cutoff level for their opacity maps.
This exposes it in the attribute editor, makes sure it serializes properly to USD and that it clips the opacity accordingly.
There's a very minimal test included here that we used to check after merging upstream commits, but I hope we can add more material serialization tests in the future so I'm including it here.