Merge pull request #1918 from Autodesk/t_gamaj/MAYA-114979/adjust_nor…

…mal_texture

MAYA-114979 - Validate normal maps for correctness

lib/usd/pxrUsdPreviewSurface/usdPreviewSurface.cpp

@@ -15,12 +15,14 @@
 //
 #include "usdPreviewSurface.h"
 
+#include <pxr/base/tf/envSetting.h>
 #include <pxr/base/tf/staticTokens.h>
 #include <pxr/base/tf/stringUtils.h>
 #include <pxr/pxr.h>
 
 #include <maya/MDataBlock.h>
 #include <maya/MDataHandle.h>
+#include <maya/MFileIO.h>
 #include <maya/MFloatVector.h>
 #include <maya/MFnDependencyNode.h>
 #include <maya/MFnNumericAttribute.h>
@@ -42,6 +44,17 @@ TF_DEFINE_PUBLIC_TOKENS(
     PxrMayaUsdPreviewSurfaceTokens,
     PXRUSDPREVIEWSURFACE_USD_PREVIEW_SURFACE_TOKENS);
 
+TF_DEFINE_ENV_SETTING(
+    USDMAYA_FIX_PREVIEW_SURFACE_CORRECTNESS_ON_LOAD,
+    false,
+    "If true, Color Space on the file node will be set to Raw when driving normals and "
+    "monochromatic attributes. We will also adjust Color Gain to (2, 2, 2) and Color Offset to "
+    "(-1, -1, -1) on normal maps.");
+
+namespace {
+static const MString normalAttrShortName = "nrm";
+}
+
 /* static */
 void* PxrMayaUsdPreviewSurface::creator() { return new PxrMayaUsdPreviewSurface(); }
 
@@ -177,7 +190,7 @@ MStatus PxrMayaUsdPreviewSurface::initialize()
 
     normalAttr = numericAttrFn.create(
         PxrMayaUsdPreviewSurfaceTokens->NormalAttrName.GetText(),
-        "nrm",
+        normalAttrShortName,
         MFnNumericData::k3Float,
         0.0,
         &status);
@@ -487,6 +500,64 @@ MStatus PxrMayaUsdPreviewSurface::compute(const MPlug& plug, MDataBlock& dataBlo
     return status;
 }
 
+/* virtual */
+MStatus
+PxrMayaUsdPreviewSurface::connectionMade(const MPlug& plug, const MPlug& otherPlug, bool asSrc)
+{
+    // Skip any adjustements on load, unless explicitly requested.
+    if (MFileIO::isReadingFile()
+        && !TfGetEnvSetting(USDMAYA_FIX_PREVIEW_SURFACE_CORRECTNESS_ON_LOAD)) {
+        return MPxNode::connectionMade(plug, otherPlug, asSrc);
+    }
+
+    // If we receive a connection on the "normal" input, and the connection is from a "file" node,
+    // then we want to adjust the Space, Gain, and Offset of that file node so they match the
+    // expected normal range of UsdPreviewSurface.
+    if (plug.partialName() == normalAttrShortName && otherPlug.node().hasFn(MFn::kFileTexture)) {
+        MFnDependencyNode otherDepNode(otherPlug.node());
+
+        MPlug sourceColorSpace = otherDepNode.findPlug("colorSpace");
+        if (!sourceColorSpace.isNull()) {
+            sourceColorSpace.setString("Raw");
+        }
+
+        auto setPlugValue = [&otherDepNode](const auto& plugName, const auto& plugValue) {
+            MPlug numericPlug = otherDepNode.findPlug(plugName);
+            if (!numericPlug.isNull()) {
+                numericPlug.setDouble(plugValue);
+            }
+        };
+
+        setPlugValue("colorGainR", 2);
+        setPlugValue("colorGainG", 2);
+        setPlugValue("colorGainB", 2);
+        setPlugValue("colorOffsetR", -1);
+        setPlugValue("colorOffsetG", -1);
+        setPlugValue("colorOffsetB", -1);
+        setPlugValue("alphaGain", 1);
+        setPlugValue("alphaOffset", 0);
+    }
+
+    // Similarly, if the connection is on a single channel attribute, like metalness, roughness, or
+    // opacity, and the source is a color channel, then we expect the file node to use the "Raw"
+    // colorspace:
+    if (!plug.isChild() && plug.attribute().hasFn(MFn::kNumericAttribute)
+        && otherPlug.node().hasFn(MFn::kFileTexture)
+        && (otherPlug.partialName() == "ocr" || otherPlug.partialName() == "ocg"
+            || otherPlug.partialName() == "ocb")) {
+        MFnNumericAttribute numericAttrFn(plug.attribute());
+        if (numericAttrFn.unitType() == MFnNumericData::kFloat) {
+            MFnDependencyNode otherDepNode(otherPlug.node());
+            MPlug             sourceColorSpace = otherDepNode.findPlug("colorSpace");
+            if (!sourceColorSpace.isNull()) {
+                sourceColorSpace.setString("Raw");
+            }
+        }
+    }
+
+    return MPxNode::connectionMade(plug, otherPlug, asSrc);
+}
+
 PxrMayaUsdPreviewSurface::PxrMayaUsdPreviewSurface()
     : MPxNode()
 {

lib/usd/pxrUsdPreviewSurface/usdPreviewSurface.h

@@ -77,6 +77,9 @@ class PxrMayaUsdPreviewSurface : public MPxNode
     PXRUSDPREVIEWSURFACE_API
     MStatus compute(const MPlug& plug, MDataBlock& dataBlock) override;
 
+    PXRUSDPREVIEWSURFACE_API
+    MStatus connectionMade(const MPlug& plug, const MPlug& otherPlug, bool asSrc) override;
+
 private:
     PxrMayaUsdPreviewSurface();
     ~PxrMayaUsdPreviewSurface() override;

lib/usd/pxrUsdPreviewSurface/usdPreviewSurfaceWriter.cpp

@@ -37,6 +37,7 @@
 #include <pxr/usdImaging/usdImaging/tokens.h>
 
 #include <maya/MFnDependencyNode.h>
+#include <maya/MGlobal.h>
 #include <maya/MObject.h>
 #include <maya/MPlug.h>
 #include <maya/MStatus.h>
@@ -51,6 +52,89 @@ REGISTER_SHADING_MODE_EXPORT_MATERIAL_CONVERSION(
     PxrMayaUsdPreviewSurfaceTokens->niceName,
     PxrMayaUsdPreviewSurfaceTokens->exportDescription);
 
+namespace {
+
+void _ValidateRawColorSpace(const MPlug& shadingNodePlug)
+{
+    MPlug otherPlug = shadingNodePlug.source();
+
+    // If the source is not a texture, or is not connected via a color component output, then we do
+    // not need to check the color space.
+    if (!otherPlug.node().hasFn(MFn::kFileTexture)
+        || (otherPlug.partialName() != "ocr" && otherPlug.partialName() != "ocg"
+            && otherPlug.partialName() != "ocb")) {
+        return;
+    }
+
+    MFnDependencyNode otherDepNode(otherPlug.node());
+    MPlug             sourceColorSpace = otherDepNode.findPlug("colorSpace");
+    if (sourceColorSpace.asString() != "Raw") {
+        MString warning("File texture \"");
+        warning += otherDepNode.name() + "\" connected to \"" + shadingNodePlug.name()
+            + "\" should use the \"Raw\" source color space";
+        MGlobal::displayWarning(warning);
+    }
+}
+
+void _ValidateNormalMap(const MPlug& shadingNodePlug)
+{
+    MPlug otherPlug = shadingNodePlug.source();
+
+    // If the source is not a texture, or is not connected via a color output, then we do not need
+    // to check the color space.
+    if (!otherPlug.node().hasFn(MFn::kFileTexture) || otherPlug.partialName() != "oc") {
+        return;
+    }
+
+    MFnDependencyNode otherDepNode(otherPlug.node());
+    MPlug             sourceColorSpace = otherDepNode.findPlug("colorSpace");
+    if (sourceColorSpace.asString() != "Raw") {
+        MString warning("File texture \"");
+        warning += otherDepNode.name() + "\" connected to \"" + shadingNodePlug.name()
+            + "\" should use the \"Raw\" source color space";
+        MGlobal::displayWarning(warning);
+    }
+
+    auto invalidValue = [&otherDepNode](const auto& plugName, const auto& plugValue) {
+        MPlug numericPlug = otherDepNode.findPlug(plugName);
+        if (!numericPlug.isNull()) {
+            if (numericPlug.asDouble() == plugValue) {
+                return false;
+            }
+        }
+        return true;
+    };
+
+    if (invalidValue("colorGainR", 2) || invalidValue("colorGainG", 2)
+        || invalidValue("colorGainB", 2)) {
+        MString warning("File texture \"");
+        warning += otherDepNode.name() + "\" connected to \"" + shadingNodePlug.name()
+            + "\" should use a Color Gain of (2, 2, 2)";
+        MGlobal::displayWarning(warning);
+    }
+    if (invalidValue("colorOffsetR", -1) || invalidValue("colorOffsetG", -1)
+        || invalidValue("colorOffsetB", -1)) {
+        MString warning("File texture \"");
+        warning += otherDepNode.name() + "\" connected to \"" + shadingNodePlug.name()
+            + "\" should use a Color Offset of (-1, -1, -1)";
+        MGlobal::displayWarning(warning);
+    }
+    if (invalidValue("alphaGain", 1)) {
+        MString warning("File texture \"");
+        warning += otherDepNode.name() + "\" connected to \"" + shadingNodePlug.name()
+            + "\" should use an Alpha Gain of 1";
+        MGlobal::displayWarning(warning);
+    }
+    if (invalidValue("alphaOffset", 0)) {
+        MString warning("File texture \"");
+        warning += otherDepNode.name() + "\" connected to \"" + shadingNodePlug.name()
+            + "\" should use an Alpha Offset of 0";
+        MGlobal::displayWarning(warning);
+    }
+}
+
+} // namespace
+
 UsdMayaShaderWriter::ContextSupport
 PxrMayaUsdPreviewSurface_Writer::CanExport(const UsdMayaJobExportArgs& exportArgs)
 {
@@ -152,6 +236,15 @@ static bool _AuthorShaderInputFromShadingNodeAttr(
             }
 
             shaderInput.Set(value, usdTime);
+        } else {
+            // Do some sanity checks for texture-driven attributes:
+            if (shaderInputTypeName == SdfValueTypeNames->Float) {
+                _ValidateRawColorSpace(shadingNodePlug);
+            }
+
+            if (shaderInputTypeName == SdfValueTypeNames->Normal3f) {
+                _ValidateNormalMap(shadingNodePlug);
+            }
         }
     }
 

test/lib/usd/translators/testUsdExportUsdPreviewSurface.py

@@ -186,6 +186,9 @@ def generateConnectedTestScene(self, attrTuples):
             "%s.outColor" % file_node, "%s.diffuseColor" % shading_node, force=True
         )
 
+        # This file node should have stayed "sRGB":
+        self.assertEqual(cmds.getAttr(file_node + ".colorSpace"), "sRGB")
+
         cmds.defaultNavigation(
             createNew=True, destination="%s.roughness" % shading_node
         )
@@ -201,6 +204,9 @@ def generateConnectedTestScene(self, attrTuples):
             "%s.outColorR" % file_node, "%s.roughness" % shading_node, force=True
         )
 
+        # The monochrome file node should have been set to "Raw" automatically:
+        self.assertEqual(cmds.getAttr(file_node + ".colorSpace"), "Raw")
+
         cmds.defaultNavigation(
             createNew=True, destination="%s.clearcoatRoughness" % shading_node
         )
@@ -223,6 +229,17 @@ def generateConnectedTestScene(self, attrTuples):
             "%s.outColor" % file_node, "%s.normal" % shading_node, force=True
         )
 
+        # The file node should have been set to "NormalMap" automatically:
+        self.assertEqual(cmds.getAttr(file_node + ".colorSpace"), "Raw")
+        self.assertEqual(cmds.getAttr(file_node + ".colorGainR"), 2)
+        self.assertEqual(cmds.getAttr(file_node + ".colorGainG"), 2)
+        self.assertEqual(cmds.getAttr(file_node + ".colorGainB"), 2)
+        self.assertEqual(cmds.getAttr(file_node + ".colorOffsetR"), -1)
+        self.assertEqual(cmds.getAttr(file_node + ".colorOffsetG"), -1)
+        self.assertEqual(cmds.getAttr(file_node + ".colorOffsetB"), -1)
+        self.assertEqual(cmds.getAttr(file_node + ".alphaGain"), 1)
+        self.assertEqual(cmds.getAttr(file_node + ".alphaOffset"), 0)
+
         shading_engine = "%sSG" % shading_node
         cmds.sets(
             renderable=True, noSurfaceShader=True, empty=True, name=shading_engine