pxrUsdPreviewSurface opacity support broken following PR #574

[mattyjams]

Describe the bug
Following the merge of #574, the pxrUsdPreviewSurface shading node now uses the fragment graph XML from the VP 2.0 render delegate instead of its own version. One important difference there is that the render delegate version uses an opacity property instead of a transparency property. There were no changes to pxrUsdPreviewSurface or its shading node override to account for this, so as a result the opacity control on pxrUsdPreviewSurface is no longer working.

This was uncovered by running the testPxrUsdPreviewSurfaceDraw test internally at Pixar after merging the PR. This test is unfortunately not being run in the open source repo.

I was able to fix the issue by reverting the UsdPreviewSurface fragment graph in vp2ShaderFragments to use transparency instead as it did before it was copied there from pxrUsdPreviewSurface. That change can be seen in mattyjams@1a6628a.

I'm not exactly sure how or if that fragment graph is used by the VP2.0 render delegate, but I would guess that we don't actually want to fix this that way. It would seem then that we need to modify the shading node override for pxrUsdPreviewSurface, and/or modify the shading node itself. I remember running into problems though when writing the node because of the inversion from opacity to transparency, and the fact that the attribute declared in transparencyParameter() of the override seems to be expected to have "transparency" semantics (zero is fully opaque and one is fully transparent).

Steps to reproduce
Open the test scene here:
https://github.com/Autodesk/maya-usd/blob/dev/test/lib/usd/pxrUsdPreviewSurface/PxrUsdPreviewSurfaceDrawTest/PxrUsdPreviewSurfaceDrawTest.ma

You should notice that in the third "column" from the right, the spheres are all opaque and ramp from solid white at the front to middle gray in the back.

This is the image being produced when I run the test:

Expected behavior
The spheres in the third "column" from the right should ramp from fully transparent to fully opaque, matching the baseline image here:
https://github.com/Autodesk/maya-usd/blob/dev/test/lib/usd/pxrUsdPreviewSurface/PxrUsdPreviewSurfaceDrawTest/baseline/PxrUsdPreviewSurfaceDrawTest_value_ramps.png

Specs

• OS: Linux
• Compiler: gcc 6.3.1
• Maya version: 2019 Update 2
• Maya USD commit SHA: 57af31a
• Pixar USD commit SHA: PixarAnimationStudios/OpenUSD@7c888ac

[huidong-chen]

@mattyjams Thanks for reporting the issue.

The intention of the change to the render delegate version of UsdPreviewSurface.xml is to generate the opacity vertex stream requirement, which we can directly fill with the floating point primvars:displayOpacity data. Then in CPU side we can avoid a loop to invert the displayOpacity array to transparency.

Instead of your proposal, can we do it this way?

• Extract the core of UsdPreviewSurface shader fragment graph as UsdPreviewSurfaceCore, which will match the Pixar version. The Pixar shading node override will need a minor change to use UsdPreviewSurfaceCore.
• UsdPreviewSurface connects opacityToTransparency as input to the core and wraps both with a fragment graph.

I think in this way we can ensure both sides can work as expected. Let me know what you think.

By the way, the render delegate version has had another change to support multiple Maya lights, so the Pixar shading node override will also benefit from the merge of PR #574 .

[huidong-chen]

I forgot that the more important reason of the change is that we try to match the shader fragment parameter names with the UsdPreviewSurface property names, in order to update shader parameters without manual conversion.

My above proposal is not changed.

[mattyjams]

Thanks for the notes here, @HdC-adsk. I just implemented your proposed solution in #626. Let me know what you think!

[mattyjams]

Addressed by #626. Thanks again, @HdC-adsk!