From b529fa4d9c78a0248a36799e7399ebaf3f5c50b2 Mon Sep 17 00:00:00 2001 From: Andy Date: Mon, 2 Dec 2024 17:29:20 -0500 Subject: [PATCH 1/5] feat: added an implementation for the NormalMapTexture validator - added all necessary files to move the NormalMapTexture validator from complianceChecker.py to the new Validation Framework --- .../usdShadeValidators/plugInfo.json | 5 +- .../testenv/testUsdShadeValidators.cpp | 214 +++++++++++++++- .../testUsdShadeValidators/normalMap.jpg | Bin 0 -> 1229 bytes .../usdShadeValidators/validatorTokens.h | 9 + .../usdShadeValidators/validators.cpp | 234 ++++++++++++++++++ 5 files changed, 460 insertions(+), 2 deletions(-) create mode 100644 pxr/usdValidation/usdShadeValidators/testenv/testUsdShadeValidators/normalMap.jpg diff --git a/pxr/usdValidation/usdShadeValidators/plugInfo.json b/pxr/usdValidation/usdShadeValidators/plugInfo.json index d16a95a1ed..2b4082956c 100644 --- a/pxr/usdValidation/usdShadeValidators/plugInfo.json +++ b/pxr/usdValidation/usdShadeValidators/plugInfo.json @@ -17,7 +17,10 @@ }, "MaterialBindingRelationships": { "doc": "All properties named 'material:binding' or in that namespace should be relationships." - }, + }, + "NormalMapTextureValidator" : { + "doc": "UsdUVTexture nodes that feed the _inputs:normals_ of a UsdPreviewSurface must ensure that the data is encoded and scaled properly. Specifically, since normals are expected to be in the range [(-1,-1,-1), (1,1,1)], the Texture node must transform 8-bit textures from their [0..1] range by setting its _inputs:scale_ to (2, 2, 2, 1) and _inputs:bias_ to (-1, -1, -1, 0). Normal map data is commonly expected to be linearly encoded. However, many image-writing tools automatically set the profile of three-channel, 8-bit images to SRGB. To prevent an unwanted transformation, the UsdUVTexture's _inputs:sourceColorSpace_ must be set to raw." + }, "ShaderSdrCompliance": { "doc": "Shader prim's input types must be conforming to their appropriate sdf types in the respective sdr shader.", "schemaTypes": [ diff --git a/pxr/usdValidation/usdShadeValidators/testenv/testUsdShadeValidators.cpp b/pxr/usdValidation/usdShadeValidators/testenv/testUsdShadeValidators.cpp index bc3b4506e1..3cd59edfee 100644 --- a/pxr/usdValidation/usdShadeValidators/testenv/testUsdShadeValidators.cpp +++ b/pxr/usdValidation/usdShadeValidators/testenv/testUsdShadeValidators.cpp @@ -14,6 +14,7 @@ #include "pxr/usd/usd/relationship.h" #include "pxr/usd/usd/stage.h" #include "pxr/usd/usdGeom/scope.h" +#include "pxr/usd/usdGeom/xform.h" #include "pxr/usd/usdShade/material.h" #include "pxr/usd/usdShade/materialBindingAPI.h" #include "pxr/usd/usdShade/shader.h" @@ -44,6 +45,7 @@ TestUsdShadeValidators() UsdShadeValidatorNameTokens->materialBindingApiAppliedValidator, UsdShadeValidatorNameTokens->materialBindingRelationships, UsdShadeValidatorNameTokens->materialBindingCollectionValidator, + UsdShadeValidatorNameTokens->normalMapTextureValidator, UsdShadeValidatorNameTokens->shaderSdrCompliance, UsdShadeValidatorNameTokens->subsetMaterialBindFamilyName, UsdShadeValidatorNameTokens->subsetsMaterialBindFamily }; @@ -59,8 +61,10 @@ TestUsdShadeValidators() UsdValidationValidatorMetadataVector metadata = registry.GetValidatorMetadataForPlugin( _tokens->usdShadeValidatorsPlugin); - TF_AXIOM(metadata.size() == 7); + TF_AXIOM(metadata.size() == 8); for (const UsdValidationValidatorMetadata &metadata : metadata) { + + std::string name = metadata.name.GetString(); validatorMetadataNameSet.insert(metadata.name); } @@ -554,6 +558,213 @@ TestUsdShadeEncapsulationRulesValidator() } } +void ValidateError(const UsdValidationErrorVector& errors, + const TfToken& expectedErrorIdentifier, + const SdfPath& expectedPrimPath, + const std::string& expectedErrorMsg, + UsdValidationErrorType expectedErrorType = UsdValidationErrorType::Error) +{ + TF_AXIOM(errors.size() == 1); + TF_AXIOM(errors[0].GetIdentifier() == expectedErrorIdentifier); + TF_AXIOM(errors[0].GetType() == expectedErrorType); + TF_AXIOM(errors[0].GetSites().size() == 1); + TF_AXIOM(errors[0].GetSites()[0].IsValid()); + TF_AXIOM(errors[0].GetSites()[0].IsPrim()); + TF_AXIOM(errors[0].GetSites()[0].GetPrim().GetPath() == + expectedPrimPath); + TF_AXIOM(errors[0].GetMessage() == expectedErrorMsg); +} + +void +TestUsdShadeNormalMapTextureValidator() +{ + UsdValidationRegistry ®istry = UsdValidationRegistry::GetInstance(); + const UsdValidationValidator *validator = registry.GetOrLoadValidatorByName( + UsdShadeValidatorNameTokens->normalMapTextureValidator); + TF_AXIOM(validator); + + // Create a Stage, Material, and Two Shaders (UsdPreviewSurface, + // UsdUVTexture). + UsdStageRefPtr usdStage = UsdStage::CreateInMemory(); + UsdShadeMaterial material = UsdShadeMaterial::Define(usdStage, + SdfPath("/RootMaterial")); + + const std::string usdPreviewSurfaceShaderPath = + "/RootMaterial/UsdPreviewSurface"; + UsdShadeShader usdPreviewSurfaceShader = UsdShadeShader::Define( + usdStage, SdfPath(usdPreviewSurfaceShaderPath)); + usdPreviewSurfaceShader.CreateIdAttr( + VtValue(TfToken("UsdPreviewSurface"))); + UsdPrim usdPreviewSurfaceShaderPrim = usdPreviewSurfaceShader.GetPrim(); + + UsdShadeShader usdUvTextureShader = UsdShadeShader::Define( + usdStage, SdfPath("/RootMaterial/NormalTexture")); + usdUvTextureShader.CreateIdAttr(VtValue(TfToken("UsdUVTexture"))); + + // Add initial valid file and sourceColorSpace input values. + std::string textureAssetPath = "./normalMap.jpg"; + UsdShadeInput fileInput = usdUvTextureShader.CreateInput( + TfToken("file"), SdfValueTypeNames->Asset); + fileInput.Set(SdfAssetPath(textureAssetPath)); + UsdShadeInput sourceColorSpaceInput = usdUvTextureShader.CreateInput( + TfToken("sourceColorSpace"), SdfValueTypeNames->Token); + const TfToken rawToken("raw"); + sourceColorSpaceInput.Set(rawToken); + + // Connect the output of the UsdUVTexture Shader to the normal of the + // UsdPreviewSurface Shader. + usdUvTextureShader.CreateOutput(TfToken("rgb"), SdfValueTypeNames->Float3); + UsdShadeInput normalInput = usdPreviewSurfaceShader.CreateInput( + TfToken("normal"), SdfValueTypeNames->Normal3f); + normalInput.ConnectToSource( + SdfPath("/RootMaterial/NormalTexture.outputs:rgb")); + + // Verify invalid bias & scale error, they should exists and do + // not exist at this point. + UsdValidationErrorVector errors = validator->Validate( + usdPreviewSurfaceShaderPrim); + TfToken expectedErrorIdentifier( + "usdShadeValidators:NormalMapTextureValidator.NonCompliantBiasAndScale"); + std::string expectedErrorMsg = + TfStringPrintf("UsdUVTexture prim <%s> reads 8 bit Normal Map " + "@./normalMap.jpg@, which requires that " + "inputs:scale be set to (2, 2, 2, 1) and " + "inputs:bias be set to (-1, -1, -1, 0) for proper " + "interpretation as per the UsdPreviewSurface and " + "UsdUVTexture docs.", + usdUvTextureShader.GetPath().GetText()); + ValidateError(errors, + expectedErrorIdentifier, + usdUvTextureShader.GetPath(), + expectedErrorMsg); + + // Add bias and scale, but add a non-compliant bias value. + UsdShadeInput biasInput = usdUvTextureShader.CreateInput( + TfToken("bias"), SdfValueTypeNames->Float4); + const GfVec4f compliantBias = GfVec4f(-1, -1, -1, 0); + const GfVec4f nonCompliantVector = GfVec4f(-9, -9, -9, -9); + biasInput.Set(nonCompliantVector); + UsdShadeInput scaleInput = usdUvTextureShader.CreateInput( + TfToken("scale"), SdfValueTypeNames->Float4); + const GfVec4f compliantScale = GfVec4f(2, 2, 2, 1); + scaleInput.Set(compliantScale); + + // Verify the non-compliant bias value error occurs. + errors = validator->Validate(usdPreviewSurfaceShaderPrim); + expectedErrorIdentifier = TfToken( + "usdShadeValidators:NormalMapTextureValidator.NonCompliantBiasValues"); + expectedErrorMsg = + TfStringPrintf("UsdUVTexture prim <%s> reads an 8 bit Normal " + "Map, but has non-standard inputs:bias value of " + "(%.6g, %.6g, %.6g, %.6g). inputs:bias must be set to " + "[-1,-1,-1,0] so as to fulfill the requirements " + "of the normals to be in tangent space of " + "[(-1,-1,-1), (1,1,1)] as documented in the " + "UsdPreviewSurface and UsdUVTexture docs.", + usdUvTextureShader.GetPath().GetText(), + nonCompliantVector[0], nonCompliantVector[1], + nonCompliantVector[2], nonCompliantVector[3]); + ValidateError(errors, + expectedErrorIdentifier, + usdUvTextureShader.GetPath(), + expectedErrorMsg); + + // Update to a compliant bias and a non-compliant scale value. + biasInput.Set(compliantBias); + scaleInput.Set(nonCompliantVector); + + // Verify the non-compliant scale value error occurs. + errors = validator->Validate(usdPreviewSurfaceShaderPrim); + expectedErrorIdentifier = TfToken( + "usdShadeValidators:NormalMapTextureValidator.NonCompliantScaleValues"); + expectedErrorMsg = + TfStringPrintf("UsdUVTexture prim <%s> reads an 8 bit Normal " + "Map, but has non-standard inputs:scale value " + "of (%.6g, %.6g, %.6g, %.6g). inputs:scale must " + "be set to (2, 2, 2, 1) so as fulfill the " + "requirements of the normals to be in tangent " + "space of [(-1,-1,-1), (1,1,1)] as documented in " + "the UsdPreviewSurface and UsdUVTexture docs.", + usdUvTextureShader.GetPath().GetText(), + nonCompliantVector[0], nonCompliantVector[1], + nonCompliantVector[2], nonCompliantVector[3]); + ValidateError(errors, + expectedErrorIdentifier, + usdUvTextureShader.GetPath(), + expectedErrorMsg, + UsdValidationErrorType::Warn); + + // Set a compliant scale value, and an invalid sourceColorSpace. + scaleInput.Set(compliantScale); + sourceColorSpaceInput.Set(TfToken("error")); + + // Verify the invalid sourceColorSpace error occurs. + errors = validator->Validate(usdPreviewSurfaceShaderPrim); + expectedErrorIdentifier = TfToken( + "usdShadeValidators:NormalMapTextureValidator.InvalidSourceColorSpace"); + expectedErrorMsg = + TfStringPrintf("UsdUVTexture prim <%s> that reads" + " Normal Map @%s@ should set " + "inputs:sourceColorSpace to 'raw'.", + usdUvTextureShader.GetPath().GetText(), + textureAssetPath.c_str()); + ValidateError(errors, + expectedErrorIdentifier, + usdUvTextureShader.GetPath(), + expectedErrorMsg); + + // Correct the sourceColorSpace, hook up the normal input of + // UsdPreviewSurface to a non-shader output. + sourceColorSpaceInput.Set(rawToken); + UsdGeomXform nonShaderPrim = UsdGeomXform::Define( + usdStage, SdfPath("/RootMaterial/Xform")); + UsdShadeConnectableAPI connectableNonShaderAPI(nonShaderPrim.GetPrim()); + UsdShadeOutput nonShaderOutput = connectableNonShaderAPI.CreateOutput( + TfToken("myOutput"), SdfValueTypeNames->Float3); + nonShaderOutput.Set(GfVec3f(1.0f, 2.0f, 3.0f)); + normalInput.ConnectToSource(nonShaderOutput); + + // Verify a non-shader connection error occurs. + errors = validator->Validate(usdPreviewSurfaceShaderPrim); + expectedErrorIdentifier = TfToken( + "usdShadeValidators:NormalMapTextureValidator.NonShaderConnection"); + expectedErrorMsg = + TfStringPrintf("UsdPreviewSurface.normal on prim <%s> is connected " + "to a non-Shader prim.", + usdPreviewSurfaceShaderPath.c_str()); + ValidateError(errors, + expectedErrorIdentifier, + usdPreviewSurfaceShader.GetPath(), + expectedErrorMsg); + + // Set the normal input back to a valid shader and update the file input + // to an invalid file path. + normalInput.ConnectToSource( + SdfPath("/RootMaterial/NormalTexture.outputs:rgb")); + fileInput.Set(SdfAssetPath("./doesNotExist.jpg")); + + // Verify the invalid input file error occurs. + errors = validator->Validate(usdPreviewSurfaceShaderPrim); + expectedErrorIdentifier = + TfToken("usdShadeValidators:NormalMapTextureValidator.InvalidFile"); + expectedErrorMsg = + TfStringPrintf("UsdUVTexture prim <%s> has invalid or unresolvable " + "inputs:file of @%s@", + usdUvTextureShader.GetPath().GetText(), + "./doesNotExist.jpg"); + ValidateError(errors, + expectedErrorIdentifier, + usdUvTextureShader.GetPath(), + expectedErrorMsg); + + // Reset the file to a valid path. + fileInput.Set(SdfAssetPath("./normalMap.jpg")); + + // Verify no errors exist. + errors = validator->Validate(usdPreviewSurfaceShaderPrim); + TF_AXIOM(errors.empty()); +} + int main() { @@ -561,6 +772,7 @@ main() TestUsdShadeMaterialBindingAPIAppliedValidator(); TestUsdShadeMaterialBindingRelationships(); TestUsdShadeMaterialBindingCollections(); + TestUsdShadeNormalMapTextureValidator(); TestUsdShadeShaderPropertyCompliance(); TestUsdShadeSubsetMaterialBindFamilyName(); TestUsdShadeSubsetsMaterialBindFamily(); diff --git a/pxr/usdValidation/usdShadeValidators/testenv/testUsdShadeValidators/normalMap.jpg b/pxr/usdValidation/usdShadeValidators/testenv/testUsdShadeValidators/normalMap.jpg new file mode 100644 index 0000000000000000000000000000000000000000..562ac9304ca25b07dab155cdbc4cf30d691c5a0b GIT binary patch literal 1229 zcmb7DO=uHA6#iy6+oZ`R-EB5Ch~O5Y2T4Ix#9K7cgh+}ZR%olB$>tZ5G;C6O5qc54 z7CiWKY1M-l51#ZY(yJf{+Ny|n^599)i`4PWrZyFl6yMJ7e7yI4Z{EC_gZsK<|cXC$Wc5# zK)>J7XGv8cRSgy?&SC)#~zc_c6Pg}0LZOg4wl6$kgvX7LA(AkeG%jUSQAQ2F&u{#< z@fXlNbj_K}bVxdrb;j3WbT7VW#I|{aSNvH28h-~>W{@;lqq}?LpkQhJSRT?cMOqdF uy+1)_EXIZrq#eS*B}QW1B?b~$Y;yT;(auS0t9z8B?Nw5jFl0p{Z2kcd-oi8h literal 0 HcmV?d00001 diff --git a/pxr/usdValidation/usdShadeValidators/validatorTokens.h b/pxr/usdValidation/usdShadeValidators/validatorTokens.h index cd16c3c82f..880bb07713 100644 --- a/pxr/usdValidation/usdShadeValidators/validatorTokens.h +++ b/pxr/usdValidation/usdShadeValidators/validatorTokens.h @@ -25,6 +25,8 @@ PXR_NAMESPACE_OPEN_SCOPE "usdShadeValidators:MaterialBindingRelationships")) \ ((materialBindingCollectionValidator, \ "usdShadeValidators:MaterialBindingCollectionValidator")) \ + ((normalMapTextureValidator, \ + "usdShadeValidators:NormalMapTextureValidator")) \ ((shaderSdrCompliance, "usdShadeValidators:ShaderSdrCompliance")) \ ((subsetMaterialBindFamilyName, \ "usdShadeValidators:SubsetMaterialBindFamilyName")) \ @@ -47,6 +49,13 @@ PXR_NAMESPACE_OPEN_SCOPE ((incompatShaderPropertyWarning, "IncompatShaderPropertyWarning")) \ ((mismatchPropertyType, "MismatchedPropertyType")) \ ((missingFamilyNameOnGeomSubset, "MissingFamilyNameOnGeomSubset")) \ + ((nonShaderConnection, "NonShaderConnection")) \ + ((invalidFile, "InvalidFile")) \ + ((invalidShaderPrim, "InvalidShaderPrim")) \ + ((invalidSourceColorSpace, "InvalidSourceColorSpace")) \ + ((nonCompliantBiasAndScale, "NonCompliantBiasAndScale")) \ + ((nonCompliantScale, "NonCompliantScaleValues")) \ + ((nonCompliantBias, "NonCompliantBiasValues")) \ ((invalidFamilyType, "InvalidFamilyType")) /// \def USD_SHADE_VALIDATOR_NAME_TOKENS diff --git a/pxr/usdValidation/usdShadeValidators/validators.cpp b/pxr/usdValidation/usdShadeValidators/validators.cpp index 137090ba76..7dcde4eaad 100644 --- a/pxr/usdValidation/usdShadeValidators/validators.cpp +++ b/pxr/usdValidation/usdShadeValidators/validators.cpp @@ -7,6 +7,7 @@ #include "pxr/base/tf/stringUtils.h" #include "pxr/base/tf/token.h" +#include "pxr/usd/ar/resolver.h" #include "pxr/usd/sdr/registry.h" #include "pxr/usd/sdr/shaderProperty.h" #include "pxr/usd/usd/prim.h" @@ -16,6 +17,7 @@ #include "pxr/usd/usdGeom/imageable.h" #include "pxr/usd/usdGeom/subset.h" #include "pxr/usd/usdGeom/tokens.h" +#include "pxr/usd/usdShade/connectableAPI.h" #include "pxr/usd/usdShade/materialBindingAPI.h" #include "pxr/usd/usdShade/shader.h" #include "pxr/usd/usdShade/tokens.h" @@ -538,6 +540,234 @@ _SubsetsMaterialBindFamily(const UsdPrim &usdPrim) return errors; } +static +UsdValidationErrorVector +_NormalMapTextureValidator(const UsdPrim& usdPrim) { + + if (!usdPrim.IsA()) { + return {}; + } + + const UsdShadeShader shader(usdPrim); + + TfToken shaderId; + TfToken UsdPreviewSurface("UsdPreviewSurface"); + + // We may have failed to fetch an identifier for asset/source-based + // nodes. OR, we could potentially be driven by a UsdPrimvarReader, + // in which case we'd have nothing to validate + if (!shader.GetShaderId(&shaderId) || shaderId != UsdPreviewSurface) { + return {}; + } + + const UsdShadeInput normalInput = shader.GetInput(TfToken("normal")); + if (!normalInput) { + return {}; + } + + const UsdShadeAttributeVector valueProducingAttributes = UsdShadeUtils::GetValueProducingAttributes(normalInput); + if (valueProducingAttributes.empty() || valueProducingAttributes[0].GetPrim() == usdPrim) { + return {}; + } + + const UsdPrim sourcePrim = valueProducingAttributes[0].GetPrim(); + UsdShadeShader sourceShader(sourcePrim); + if (!sourceShader) { + // In theory, could be connected to an interface attribute of a + // parent connectable... not useful, but not an error + const UsdShadeConnectableAPI& connectable = + UsdShadeConnectableAPI(sourcePrim); + + if (connectable){ + return {}; + } + + return { + UsdValidationError{ + UsdShadeValidationErrorNameTokens->nonShaderConnection, + UsdValidationErrorType::Error, + UsdValidationErrorSites{ + UsdValidationErrorSite(usdPrim.GetStage(), + usdPrim.GetPath()) + }, + TfStringPrintf("UsdPreviewSurface.normal on prim <%s> is connected to a" + " non-Shader prim.", + usdPrim.GetPath().GetText()) + } + }; + } + + TfToken sourceShaderId; + TfToken UsdUVTexture("UsdUVTexture"); + + bool gotShaderSourceId = sourceShader.GetShaderId(&sourceShaderId); + + // We may have failed to fetch an identifier for asset/source-based + // nodes. OR, we could potentially be driven by a UsdPrimvarReader, + // in which case we'd have nothing to validate + if (!gotShaderSourceId || sourceShaderId != UsdUVTexture) { + return {}; + } + + const auto getInputValue = [](const UsdShadeShader &inputShader, const TfToken &token, auto& outputValue) -> bool { + const UsdShadeInput input = inputShader.GetInput(token); + if (!input) { + return false; + } + + const UsdShadeAttributeVector valueProducingAttributes = + UsdShadeUtils::GetValueProducingAttributes(input); + + // Query value producing attributes for input values. + // This has to be a length of 1, otherwise no attribute is producing a value. + // We require an input parameter producing the value. + if (valueProducingAttributes.empty() || + valueProducingAttributes.size() != 1 || + !UsdShadeInput::IsInput(valueProducingAttributes[0])) { + return false; + } + + return valueProducingAttributes[0].Get(&outputValue, + UsdTimeCode::EarliestTime()); + }; + + SdfAssetPath textureAssetPath; + bool valueForFileExists = getInputValue(sourceShader, TfToken("file"), + textureAssetPath); + + UsdValidationErrorVector errors; + + if (!valueForFileExists || textureAssetPath.GetResolvedPath().empty()) { + std::string assetPath = !textureAssetPath.GetAssetPath().empty() + ? textureAssetPath.GetAssetPath() + : ""; + errors.emplace_back( + UsdShadeValidationErrorNameTokens->invalidFile, + UsdValidationErrorType::Error, + UsdValidationErrorSites{ + UsdValidationErrorSite(usdPrim.GetStage(), + sourcePrim.GetPath()) + }, + TfStringPrintf("UsdUVTexture prim <%s> has invalid or unresolvable " + "inputs:file of @%s@", + sourcePrim.GetPath().GetText(), assetPath.c_str())); + } + + auto textureIs8Bit = [](std::string resolvedPath) { + + std::string extension = ArGetResolver().GetExtension(resolvedPath); + extension = TfStringToLower(extension); + static const std::unordered_set eightBitExtensions = + {"bmp", "tga", "png", "jpg", "jpeg", "tif"}; + + return eightBitExtensions.find(extension) != eightBitExtensions.end(); + }; + + if (!textureIs8Bit(textureAssetPath.GetResolvedPath())) { + // Nothing more is required for image depths > 8 bits, which + // we assume FOR NOW, are floating point + return errors; + } + + TfToken colorSpace; + TfToken rawColorSpace("raw"); + bool valueForColorSpaceExists = getInputValue(sourceShader, TfToken("sourceColorSpace"), colorSpace); + if (!valueForColorSpaceExists || colorSpace != rawColorSpace) { + errors.emplace_back( + UsdShadeValidationErrorNameTokens->invalidSourceColorSpace, + UsdValidationErrorType::Error, + UsdValidationErrorSites{ + UsdValidationErrorSite(usdPrim.GetStage(), + sourcePrim.GetPath()) + }, + TfStringPrintf("UsdUVTexture prim <%s> that reads" + " Normal Map @%s@ should set " + "inputs:sourceColorSpace to 'raw'.", + sourcePrim.GetPath().GetText(), + textureAssetPath.GetAssetPath().c_str())); + } + + GfVec4f bias; + bool valueForBiasExists = getInputValue(sourceShader, TfToken("bias"), bias); + + GfVec4f scale; + bool valueForScaleExists = getInputValue(sourceShader, TfToken("scale"), scale); + + if (!(valueForBiasExists && valueForScaleExists)) + { + errors.emplace_back( + UsdShadeValidationErrorNameTokens->nonCompliantBiasAndScale, + UsdValidationErrorType::Error, + UsdValidationErrorSites{ + UsdValidationErrorSite(usdPrim.GetStage(), sourcePrim.GetPath()) + }, + TfStringPrintf("UsdUVTexture prim <%s> reads 8 bit Normal Map " + "@%s@, which requires that inputs:scale be set to " + "(2, 2, 2, 1) and inputs:bias be set to " + "(-1, -1, -1, 0) for proper interpretation as per " + "the UsdPreviewSurface and UsdUVTexture docs.", + sourcePrim.GetPath().GetText(), textureAssetPath.GetAssetPath().c_str()) + ); + return errors; + } + + // We still warn for inputs:scale not conforming to UsdPreviewSurface + // guidelines, as some authoring tools may rely on this to scale an + // effect of normal perturbations. + // don't really care about fourth components... + bool nonCompliantScaleValues = scale[0] != 2 || + scale[1] != 2 || scale[2] != 2; + + if (nonCompliantScaleValues) + { + errors.emplace_back( + UsdShadeValidationErrorNameTokens->nonCompliantScale, + UsdValidationErrorType::Warn, + UsdValidationErrorSites{ + UsdValidationErrorSite(usdPrim.GetStage(), sourcePrim.GetPath()) + }, + TfStringPrintf("UsdUVTexture prim <%s> reads an 8 bit Normal " + "Map, but has non-standard inputs:scale value " + "of (%.6g, %.6g, %.6g, %.6g). inputs:scale must be set to " + "(2, 2, 2, 1) so as fulfill the requirements " + "of the normals to be in tangent space of " + "[(-1,-1,-1), (1,1,1)] as documented in the " + "UsdPreviewSurface and UsdUVTexture docs.", + sourcePrim.GetPath().GetText(), + scale[0], scale[1], scale[2], scale[3]) + ); + } + + // Note that for a 8bit normal map, inputs:bias must be appropriately + // set to [-1, -1, -1, 0] to fulfill the requirements of the + // normals to be in tangent space of [(-1,-1,-1), (1,1,1)] as documented + // in the UsdPreviewSurface docs. Note this is true only when scale + // values are respecting the requirements laid in the + // UsdPreviewSurface / UsdUVTexture docs. We continue to warn! + if (!nonCompliantScaleValues && (bias[0] != -1 || bias[1] != -1 || + bias[2] != -1)) + { + errors.emplace_back( + UsdShadeValidationErrorNameTokens->nonCompliantBias, + UsdValidationErrorType::Error, + UsdValidationErrorSites{ + UsdValidationErrorSite(usdPrim.GetStage(), sourcePrim.GetPath()) + }, + TfStringPrintf("UsdUVTexture prim <%s> reads an 8 bit Normal " + "Map, but has non-standard inputs:bias value of " + "(%.6g, %.6g, %.6g, %.6g). inputs:bias must be set to " + "[-1,-1,-1,0] so as to fulfill the requirements " + "of the normals to be in tangent space of " + "[(-1,-1,-1), (1,1,1)] as documented in the " + "UsdPreviewSurface and UsdUVTexture docs.", + sourcePrim.GetPath().GetText(), + bias[0], bias[1], bias[2], bias[3]) + ); + } + + return errors; +} + TF_REGISTRY_FUNCTION(UsdValidationRegistry) { UsdValidationRegistry ®istry = UsdValidationRegistry::GetInstance(); @@ -554,6 +784,10 @@ TF_REGISTRY_FUNCTION(UsdValidationRegistry) UsdShadeValidatorNameTokens->materialBindingCollectionValidator, _MaterialBindingCollectionValidator); + registry.RegisterPluginValidator( + UsdShadeValidatorNameTokens->normalMapTextureValidator, + _NormalMapTextureValidator); + registry.RegisterPluginValidator( UsdShadeValidatorNameTokens->shaderSdrCompliance, _ShaderPropertyTypeConformance); From 5c7efc64f505d62d31be7f9a459a1a2b2f64c398 Mon Sep 17 00:00:00 2001 From: Andy Date: Mon, 2 Dec 2024 18:14:47 -0500 Subject: [PATCH 2/5] fix: making sure lines are less than 80 characters wide --- .../testenv/testUsdShadeValidators.cpp | 26 ++++---- .../usdShadeValidators/validatorTokens.h | 30 ++++----- .../usdShadeValidators/validators.cpp | 61 +++++++++++-------- 3 files changed, 65 insertions(+), 52 deletions(-) diff --git a/pxr/usdValidation/usdShadeValidators/testenv/testUsdShadeValidators.cpp b/pxr/usdValidation/usdShadeValidators/testenv/testUsdShadeValidators.cpp index 3cd59edfee..e8ace42895 100644 --- a/pxr/usdValidation/usdShadeValidators/testenv/testUsdShadeValidators.cpp +++ b/pxr/usdValidation/usdShadeValidators/testenv/testUsdShadeValidators.cpp @@ -562,7 +562,8 @@ void ValidateError(const UsdValidationErrorVector& errors, const TfToken& expectedErrorIdentifier, const SdfPath& expectedPrimPath, const std::string& expectedErrorMsg, - UsdValidationErrorType expectedErrorType = UsdValidationErrorType::Error) + UsdValidationErrorType expectedErrorType = + UsdValidationErrorType::Error) { TF_AXIOM(errors.size() == 1); TF_AXIOM(errors[0].GetIdentifier() == expectedErrorIdentifier); @@ -579,7 +580,8 @@ void TestUsdShadeNormalMapTextureValidator() { UsdValidationRegistry ®istry = UsdValidationRegistry::GetInstance(); - const UsdValidationValidator *validator = registry.GetOrLoadValidatorByName( + const UsdValidationValidator *validator = + registry.GetOrLoadValidatorByName( UsdShadeValidatorNameTokens->normalMapTextureValidator); TF_AXIOM(validator); @@ -624,7 +626,7 @@ TestUsdShadeNormalMapTextureValidator() UsdValidationErrorVector errors = validator->Validate( usdPreviewSurfaceShaderPrim); TfToken expectedErrorIdentifier( - "usdShadeValidators:NormalMapTextureValidator.NonCompliantBiasAndScale"); + "usdShadeValidators:NormalMapTextureValidator.NonCompliantBiasAndScale"); std::string expectedErrorMsg = TfStringPrintf("UsdUVTexture prim <%s> reads 8 bit Normal Map " "@./normalMap.jpg@, which requires that " @@ -652,15 +654,15 @@ TestUsdShadeNormalMapTextureValidator() // Verify the non-compliant bias value error occurs. errors = validator->Validate(usdPreviewSurfaceShaderPrim); expectedErrorIdentifier = TfToken( - "usdShadeValidators:NormalMapTextureValidator.NonCompliantBiasValues"); + "usdShadeValidators:NormalMapTextureValidator.NonCompliantBiasValues"); expectedErrorMsg = TfStringPrintf("UsdUVTexture prim <%s> reads an 8 bit Normal " "Map, but has non-standard inputs:bias value of " - "(%.6g, %.6g, %.6g, %.6g). inputs:bias must be set to " - "[-1,-1,-1,0] so as to fulfill the requirements " - "of the normals to be in tangent space of " - "[(-1,-1,-1), (1,1,1)] as documented in the " - "UsdPreviewSurface and UsdUVTexture docs.", + "(%.6g, %.6g, %.6g, %.6g). inputs:bias must be " + "set to [-1,-1,-1,0] so as to fulfill the " + "requirements of the normals to be in tangent " + "space of [(-1,-1,-1), (1,1,1)] as documented in " + "the UsdPreviewSurface and UsdUVTexture docs.", usdUvTextureShader.GetPath().GetText(), nonCompliantVector[0], nonCompliantVector[1], nonCompliantVector[2], nonCompliantVector[3]); @@ -676,7 +678,7 @@ TestUsdShadeNormalMapTextureValidator() // Verify the non-compliant scale value error occurs. errors = validator->Validate(usdPreviewSurfaceShaderPrim); expectedErrorIdentifier = TfToken( - "usdShadeValidators:NormalMapTextureValidator.NonCompliantScaleValues"); + "usdShadeValidators:NormalMapTextureValidator.NonCompliantScaleValues"); expectedErrorMsg = TfStringPrintf("UsdUVTexture prim <%s> reads an 8 bit Normal " "Map, but has non-standard inputs:scale value " @@ -701,7 +703,7 @@ TestUsdShadeNormalMapTextureValidator() // Verify the invalid sourceColorSpace error occurs. errors = validator->Validate(usdPreviewSurfaceShaderPrim); expectedErrorIdentifier = TfToken( - "usdShadeValidators:NormalMapTextureValidator.InvalidSourceColorSpace"); + "usdShadeValidators:NormalMapTextureValidator.InvalidSourceColorSpace"); expectedErrorMsg = TfStringPrintf("UsdUVTexture prim <%s> that reads" " Normal Map @%s@ should set " @@ -746,7 +748,7 @@ TestUsdShadeNormalMapTextureValidator() // Verify the invalid input file error occurs. errors = validator->Validate(usdPreviewSurfaceShaderPrim); expectedErrorIdentifier = - TfToken("usdShadeValidators:NormalMapTextureValidator.InvalidFile"); + TfToken("usdShadeValidators:NormalMapTextureValidator.InvalidFile"); expectedErrorMsg = TfStringPrintf("UsdUVTexture prim <%s> has invalid or unresolvable " "inputs:file of @%s@", diff --git a/pxr/usdValidation/usdShadeValidators/validatorTokens.h b/pxr/usdValidation/usdShadeValidators/validatorTokens.h index 880bb07713..9293b25465 100644 --- a/pxr/usdValidation/usdShadeValidators/validatorTokens.h +++ b/pxr/usdValidation/usdShadeValidators/validatorTokens.h @@ -16,21 +16,21 @@ PXR_NAMESPACE_OPEN_SCOPE -#define USD_SHADE_VALIDATOR_NAME_TOKENS \ - ((encapsulationValidator, \ - "usdShadeValidators:EncapsulationRulesValidator")) \ - ((materialBindingApiAppliedValidator, \ - "usdShadeValidators:MaterialBindingApiAppliedValidator")) \ - ((materialBindingRelationships, \ - "usdShadeValidators:MaterialBindingRelationships")) \ - ((materialBindingCollectionValidator, \ - "usdShadeValidators:MaterialBindingCollectionValidator")) \ - ((normalMapTextureValidator, \ - "usdShadeValidators:NormalMapTextureValidator")) \ - ((shaderSdrCompliance, "usdShadeValidators:ShaderSdrCompliance")) \ - ((subsetMaterialBindFamilyName, \ - "usdShadeValidators:SubsetMaterialBindFamilyName")) \ - ((subsetsMaterialBindFamily, \ +#define USD_SHADE_VALIDATOR_NAME_TOKENS \ + ((encapsulationValidator, \ + "usdShadeValidators:EncapsulationRulesValidator")) \ + ((materialBindingApiAppliedValidator, \ + "usdShadeValidators:MaterialBindingApiAppliedValidator")) \ + ((materialBindingRelationships, \ + "usdShadeValidators:MaterialBindingRelationships")) \ + ((materialBindingCollectionValidator, \ + "usdShadeValidators:MaterialBindingCollectionValidator")) \ + ((normalMapTextureValidator, \ + "usdShadeValidators:NormalMapTextureValidator")) \ + ((shaderSdrCompliance, "usdShadeValidators:ShaderSdrCompliance")) \ + ((subsetMaterialBindFamilyName, \ + "usdShadeValidators:SubsetMaterialBindFamilyName")) \ + ((subsetsMaterialBindFamily, \ "usdShadeValidators:SubsetsMaterialBindFamily")) #define USD_SHADE_VALIDATOR_KEYWORD_TOKENS (UsdShadeValidators) diff --git a/pxr/usdValidation/usdShadeValidators/validators.cpp b/pxr/usdValidation/usdShadeValidators/validators.cpp index 7dcde4eaad..97bfec1c1f 100644 --- a/pxr/usdValidation/usdShadeValidators/validators.cpp +++ b/pxr/usdValidation/usdShadeValidators/validators.cpp @@ -565,8 +565,10 @@ _NormalMapTextureValidator(const UsdPrim& usdPrim) { return {}; } - const UsdShadeAttributeVector valueProducingAttributes = UsdShadeUtils::GetValueProducingAttributes(normalInput); - if (valueProducingAttributes.empty() || valueProducingAttributes[0].GetPrim() == usdPrim) { + const UsdShadeAttributeVector valueProducingAttributes = + UsdShadeUtils::GetValueProducingAttributes(normalInput); + if (valueProducingAttributes.empty() || + valueProducingAttributes[0].GetPrim() == usdPrim) { return {}; } @@ -590,8 +592,8 @@ _NormalMapTextureValidator(const UsdPrim& usdPrim) { UsdValidationErrorSite(usdPrim.GetStage(), usdPrim.GetPath()) }, - TfStringPrintf("UsdPreviewSurface.normal on prim <%s> is connected to a" - " non-Shader prim.", + TfStringPrintf("UsdPreviewSurface.normal on prim <%s> is " + "connected to a non-Shader prim.", usdPrim.GetPath().GetText()) } }; @@ -609,7 +611,8 @@ _NormalMapTextureValidator(const UsdPrim& usdPrim) { return {}; } - const auto getInputValue = [](const UsdShadeShader &inputShader, const TfToken &token, auto& outputValue) -> bool { + const auto getInputValue = [](const UsdShadeShader &inputShader, + const TfToken &token, auto& outputValue) -> bool { const UsdShadeInput input = inputShader.GetInput(token); if (!input) { return false; @@ -619,7 +622,8 @@ _NormalMapTextureValidator(const UsdPrim& usdPrim) { UsdShadeUtils::GetValueProducingAttributes(input); // Query value producing attributes for input values. - // This has to be a length of 1, otherwise no attribute is producing a value. + // This has to be a length of 1, otherwise no attribute is producing + // a value. // We require an input parameter producing the value. if (valueProducingAttributes.empty() || valueProducingAttributes.size() != 1 || @@ -648,8 +652,8 @@ _NormalMapTextureValidator(const UsdPrim& usdPrim) { UsdValidationErrorSite(usdPrim.GetStage(), sourcePrim.GetPath()) }, - TfStringPrintf("UsdUVTexture prim <%s> has invalid or unresolvable " - "inputs:file of @%s@", + TfStringPrintf("UsdUVTexture prim <%s> has invalid or " + "unresolvable inputs:file of @%s@", sourcePrim.GetPath().GetText(), assetPath.c_str())); } @@ -671,7 +675,8 @@ _NormalMapTextureValidator(const UsdPrim& usdPrim) { TfToken colorSpace; TfToken rawColorSpace("raw"); - bool valueForColorSpaceExists = getInputValue(sourceShader, TfToken("sourceColorSpace"), colorSpace); + bool valueForColorSpaceExists = + getInputValue(sourceShader, TfToken("sourceColorSpace"), colorSpace); if (!valueForColorSpaceExists || colorSpace != rawColorSpace) { errors.emplace_back( UsdShadeValidationErrorNameTokens->invalidSourceColorSpace, @@ -688,10 +693,12 @@ _NormalMapTextureValidator(const UsdPrim& usdPrim) { } GfVec4f bias; - bool valueForBiasExists = getInputValue(sourceShader, TfToken("bias"), bias); + bool valueForBiasExists = getInputValue(sourceShader, TfToken("bias"), + bias); GfVec4f scale; - bool valueForScaleExists = getInputValue(sourceShader, TfToken("scale"), scale); + bool valueForScaleExists = getInputValue(sourceShader, TfToken("scale"), + scale); if (!(valueForBiasExists && valueForScaleExists)) { @@ -699,14 +706,16 @@ _NormalMapTextureValidator(const UsdPrim& usdPrim) { UsdShadeValidationErrorNameTokens->nonCompliantBiasAndScale, UsdValidationErrorType::Error, UsdValidationErrorSites{ - UsdValidationErrorSite(usdPrim.GetStage(), sourcePrim.GetPath()) + UsdValidationErrorSite(usdPrim.GetStage(), + sourcePrim.GetPath()) }, TfStringPrintf("UsdUVTexture prim <%s> reads 8 bit Normal Map " "@%s@, which requires that inputs:scale be set to " "(2, 2, 2, 1) and inputs:bias be set to " "(-1, -1, -1, 0) for proper interpretation as per " "the UsdPreviewSurface and UsdUVTexture docs.", - sourcePrim.GetPath().GetText(), textureAssetPath.GetAssetPath().c_str()) + sourcePrim.GetPath().GetText(), + textureAssetPath.GetAssetPath().c_str()) ); return errors; } @@ -724,15 +733,16 @@ _NormalMapTextureValidator(const UsdPrim& usdPrim) { UsdShadeValidationErrorNameTokens->nonCompliantScale, UsdValidationErrorType::Warn, UsdValidationErrorSites{ - UsdValidationErrorSite(usdPrim.GetStage(), sourcePrim.GetPath()) + UsdValidationErrorSite(usdPrim.GetStage(), + sourcePrim.GetPath()) }, TfStringPrintf("UsdUVTexture prim <%s> reads an 8 bit Normal " "Map, but has non-standard inputs:scale value " - "of (%.6g, %.6g, %.6g, %.6g). inputs:scale must be set to " - "(2, 2, 2, 1) so as fulfill the requirements " - "of the normals to be in tangent space of " - "[(-1,-1,-1), (1,1,1)] as documented in the " - "UsdPreviewSurface and UsdUVTexture docs.", + "of (%.6g, %.6g, %.6g, %.6g). inputs:scale must " + "be set to (2, 2, 2, 1) so as fulfill the " + "requirements of the normals to be in tangent " + "space of [(-1,-1,-1), (1,1,1)] as documented in " + "the UsdPreviewSurface and UsdUVTexture docs.", sourcePrim.GetPath().GetText(), scale[0], scale[1], scale[2], scale[3]) ); @@ -751,15 +761,16 @@ _NormalMapTextureValidator(const UsdPrim& usdPrim) { UsdShadeValidationErrorNameTokens->nonCompliantBias, UsdValidationErrorType::Error, UsdValidationErrorSites{ - UsdValidationErrorSite(usdPrim.GetStage(), sourcePrim.GetPath()) + UsdValidationErrorSite(usdPrim.GetStage(), + sourcePrim.GetPath()) }, TfStringPrintf("UsdUVTexture prim <%s> reads an 8 bit Normal " "Map, but has non-standard inputs:bias value of " - "(%.6g, %.6g, %.6g, %.6g). inputs:bias must be set to " - "[-1,-1,-1,0] so as to fulfill the requirements " - "of the normals to be in tangent space of " - "[(-1,-1,-1), (1,1,1)] as documented in the " - "UsdPreviewSurface and UsdUVTexture docs.", + "(%.6g, %.6g, %.6g, %.6g). inputs:bias must be " + "set to [-1,-1,-1,0] so as to fulfill the " + "requirements of the normals to be in tangent " + "space of [(-1,-1,-1), (1,1,1)] as documented in " + "the UsdPreviewSurface and UsdUVTexture docs.", sourcePrim.GetPath().GetText(), bias[0], bias[1], bias[2], bias[3]) ); From c67a5efb26101a3c1102665b2f54ba4b2e0efc27 Mon Sep 17 00:00:00 2001 From: Andy Date: Tue, 10 Dec 2024 16:46:48 -0500 Subject: [PATCH 3/5] fix: addressing PR comments - fixed many code styling bugs, including missing const's - applied calls to ValidateError in all usdShade tests - adding usdShadeShader schema to NormalMapTextureValidator - fixing a CMake typo to include usdValidation --- .../usdShadeValidators/plugInfo.json | 5 +- .../testenv/testUsdShadeValidators.cpp | 466 +++++++++--------- .../usdShadeValidators/validators.cpp | 56 ++- 3 files changed, 260 insertions(+), 267 deletions(-) diff --git a/pxr/usdValidation/usdShadeValidators/plugInfo.json b/pxr/usdValidation/usdShadeValidators/plugInfo.json index 2b4082956c..edcb909be6 100644 --- a/pxr/usdValidation/usdShadeValidators/plugInfo.json +++ b/pxr/usdValidation/usdShadeValidators/plugInfo.json @@ -19,7 +19,10 @@ "doc": "All properties named 'material:binding' or in that namespace should be relationships." }, "NormalMapTextureValidator" : { - "doc": "UsdUVTexture nodes that feed the _inputs:normals_ of a UsdPreviewSurface must ensure that the data is encoded and scaled properly. Specifically, since normals are expected to be in the range [(-1,-1,-1), (1,1,1)], the Texture node must transform 8-bit textures from their [0..1] range by setting its _inputs:scale_ to (2, 2, 2, 1) and _inputs:bias_ to (-1, -1, -1, 0). Normal map data is commonly expected to be linearly encoded. However, many image-writing tools automatically set the profile of three-channel, 8-bit images to SRGB. To prevent an unwanted transformation, the UsdUVTexture's _inputs:sourceColorSpace_ must be set to raw." + "doc": "UsdUVTexture nodes that feed the _inputs:normals_ of a UsdPreviewSurface must ensure that the data is encoded and scaled properly. Specifically, since normals are expected to be in the range [(-1,-1,-1), (1,1,1)], the Texture node must transform 8-bit textures from their [0..1] range by setting its _inputs:scale_ to (2, 2, 2, 1) and _inputs:bias_ to (-1, -1, -1, 0). Normal map data is commonly expected to be linearly encoded. However, many image-writing tools automatically set the profile of three-channel, 8-bit images to SRGB. To prevent an unwanted transformation, the UsdUVTexture's _inputs:sourceColorSpace_ must be set to raw.", + "schemaTypes": [ + "UsdShadeShader" + ] }, "ShaderSdrCompliance": { "doc": "Shader prim's input types must be conforming to their appropriate sdf types in the respective sdr shader.", diff --git a/pxr/usdValidation/usdShadeValidators/testenv/testUsdShadeValidators.cpp b/pxr/usdValidation/usdShadeValidators/testenv/testUsdShadeValidators.cpp index e8ace42895..8c54965f06 100644 --- a/pxr/usdValidation/usdShadeValidators/testenv/testUsdShadeValidators.cpp +++ b/pxr/usdValidation/usdShadeValidators/testenv/testUsdShadeValidators.cpp @@ -63,14 +63,48 @@ TestUsdShadeValidators() _tokens->usdShadeValidatorsPlugin); TF_AXIOM(metadata.size() == 8); for (const UsdValidationValidatorMetadata &metadata : metadata) { - - std::string name = metadata.name.GetString(); validatorMetadataNameSet.insert(metadata.name); } TF_AXIOM(validatorMetadataNameSet == expectedUsdShadeValidatorNames); } +void ValidatePrimError(const UsdValidationError &error, + const TfToken& expectedErrorIdentifier, + const SdfPath& expectedPrimPath, + const std::string& expectedErrorMsg, + UsdValidationErrorType expectedErrorType = + UsdValidationErrorType::Error) +{ + TF_AXIOM(error.GetIdentifier() == expectedErrorIdentifier); + TF_AXIOM(error.GetType() == expectedErrorType); + TF_AXIOM(error.GetSites().size() == 1u); + const UsdValidationErrorSite &errorSite = error.GetSites()[0]; + TF_AXIOM(errorSite.IsValid()); + TF_AXIOM(errorSite.IsPrim()); + TF_AXIOM(errorSite.GetPrim().GetPath() == + expectedPrimPath); + TF_AXIOM(error.GetMessage() == expectedErrorMsg); +} + +void ValidatePropertyError(const UsdValidationError &error, + const TfToken& expectedErrorIdentifier, + const SdfPath& expectedPropertyPath, + const std::string& expectedErrorMsg, + UsdValidationErrorType expectedErrorType = + UsdValidationErrorType::Error) +{ + TF_AXIOM(error.GetIdentifier() == expectedErrorIdentifier); + TF_AXIOM(error.GetType() == expectedErrorType); + TF_AXIOM(error.GetSites().size() == 1u); + const UsdValidationErrorSite &errorSite = error.GetSites()[0]; + TF_AXIOM(errorSite.IsValid()); + TF_AXIOM(errorSite.IsProperty()); + TF_AXIOM(errorSite.GetProperty().GetPath() == + expectedPropertyPath); + TF_AXIOM(error.GetMessage() == expectedErrorMsg); +} + void TestUsdShadeMaterialBindingCollections() { @@ -92,24 +126,15 @@ TestUsdShadeMaterialBindingCollections() const TfToken expectedErrorIdentifier( "usdShadeValidators:MaterialBindingCollectionValidator." "InvalidMaterialCollection"); - - const UsdValidationError &error = errors[0]; - const SdfPath expectedAttrPath = primPath.AppendProperty( UsdShadeTokens->materialBindingCollection); - - TF_AXIOM(error.GetIdentifier() == expectedErrorIdentifier); - TF_AXIOM(error.GetType() == UsdValidationErrorType::Error); - TF_AXIOM(error.GetSites().size() == 1u); - const UsdValidationErrorSite &errorSite = error.GetSites()[0]; - TF_AXIOM(errorSite.IsValid()); - TF_AXIOM(errorSite.IsProperty()); - TF_AXIOM(errorSite.GetProperty().GetPath() == expectedAttrPath); const std::string expectedErrorMsg = "Collection-based material binding on " " has 1 target , " "needs 2: a collection path and a UsdShadeMaterial path."; - TF_AXIOM(error.GetMessage() == expectedErrorMsg); + + ValidatePropertyError(errors[0], expectedErrorIdentifier, + expectedAttrPath, expectedErrorMsg); } // Test prim with relationship to a material binding collection @@ -122,23 +147,14 @@ TestUsdShadeMaterialBindingCollections() TF_AXIOM(errors.size() == 1u); const TfToken expectedErrorIdentifier( "usdShadeValidators:MaterialBindingCollectionValidator.InvalidResourcePath"); - - const UsdValidationError &error = errors[0]; const SdfPath expectedAttrPath = primPath.AppendProperty( UsdShadeTokens->materialBindingCollection); - - TF_AXIOM(error.GetIdentifier() == expectedErrorIdentifier); - TF_AXIOM(error.GetType() == UsdValidationErrorType::Error); - TF_AXIOM(error.GetSites().size() == 1u); - const UsdValidationErrorSite &errorSite = error.GetSites()[0]; - TF_AXIOM(errorSite.IsValid()); - TF_AXIOM(errorSite.IsProperty()); - TF_AXIOM(errorSite.GetProperty().GetPath() == expectedAttrPath); const std::string expectedErrorMsg = "Collection-based material binding targets an invalid collection" " ."; - TF_AXIOM(error.GetMessage() == expectedErrorMsg); + ValidatePropertyError(errors[0], expectedErrorIdentifier, + expectedAttrPath, expectedErrorMsg); } } @@ -172,41 +188,26 @@ TestUsdShadeMaterialBindingRelationships() const TfToken expectedErrorIdentifier( "usdShadeValidators:MaterialBindingRelationships.MaterialBindingPropNotARel"); { - const UsdValidationError &error = errors[0u]; - const SdfPath expectedAttrPath = primPath.AppendProperty(UsdShadeTokens->materialBinding); - TF_AXIOM(error.GetIdentifier() == expectedErrorIdentifier); - TF_AXIOM(error.GetType() == UsdValidationErrorType::Error); - TF_AXIOM(error.GetSites().size() == 1u); - const UsdValidationErrorSite &errorSite = error.GetSites()[0u]; - TF_AXIOM(errorSite.IsValid()); - TF_AXIOM(errorSite.IsProperty()); - TF_AXIOM(errorSite.GetProperty().GetPath() == expectedAttrPath); const std::string expectedErrorMsg = "Prim has material binding property " "'material:binding' that is not a relationship."; - TF_AXIOM(error.GetMessage() == expectedErrorMsg); + + ValidatePropertyError(errors[0], expectedErrorIdentifier, + expectedAttrPath, expectedErrorMsg); } { - const UsdValidationError &error = errors[1u]; - const SdfPath expectedAttrPath = primPath.AppendProperty(TfToken(SdfPath::JoinIdentifier( UsdShadeTokens->materialBinding, "someAttribute"))); - - TF_AXIOM(error.GetIdentifier() == expectedErrorIdentifier); - TF_AXIOM(error.GetType() == UsdValidationErrorType::Error); - TF_AXIOM(error.GetSites().size() == 1u); - const UsdValidationErrorSite &errorSite = error.GetSites()[0u]; - TF_AXIOM(errorSite.IsValid()); - TF_AXIOM(errorSite.IsProperty()); - TF_AXIOM(errorSite.GetProperty().GetPath() == expectedAttrPath); const std::string expectedErrorMsg = "Prim has material binding property " "'material:binding:someAttribute' that is not a relationship."; - TF_AXIOM(error.GetMessage() == expectedErrorMsg); + + ValidatePropertyError(errors[1], expectedErrorIdentifier, + expectedAttrPath, expectedErrorMsg); } } } @@ -251,42 +252,34 @@ TestUsdShadeShaderPropertyCompliance() const UsdPrim usdPrim = usdStage->GetPrimAtPath(SdfPath("/Test")); UsdValidationErrorVector errors = validator->Validate(usdPrim); + TF_AXIOM(errors.size() == 1u); + const TfToken expectedErrorIdentifier( "usdShadeValidators:ShaderSdrCompliance.MismatchedPropertyType"); - - TF_AXIOM(errors.size() == 1); - TF_AXIOM(errors[0].GetIdentifier() == expectedErrorIdentifier); - TF_AXIOM(errors[0].GetType() == UsdValidationErrorType::Error); - TF_AXIOM(errors[0].GetSites().size() == 1); - TF_AXIOM(errors[0].GetSites()[0].IsValid()); - TF_AXIOM(errors[0].GetSites()[0].IsProperty()); - TF_AXIOM(errors[0].GetSites()[0].GetProperty().GetPath() - == SdfPath("/Test.inputs:inputColor")); + const SdfPath expectedPropertyPath("/Test.inputs:inputColor"); const std::string expectedErrorMsg = "Incorrect type for " "/Test.inputs:inputColor. Expected 'color3f'; " "got 'float3'."; - TF_AXIOM(errors[0].GetMessage() == expectedErrorMsg); + + ValidatePropertyError(errors[0], expectedErrorIdentifier, + expectedPropertyPath, expectedErrorMsg); } { const UsdPrim usdPrim = usdStage->GetPrimAtPath(SdfPath("/Bogus")); + UsdValidationErrorVector errors = validator->Validate(usdPrim); + TF_AXIOM(errors.size() == 1u); + const TfToken expectedErrorIdentifier( "usdShadeValidators:ShaderSdrCompliance.MissingShaderIdInRegistry"); - - UsdValidationErrorVector errors = validator->Validate(usdPrim); - TF_AXIOM(errors[0].GetIdentifier() == expectedErrorIdentifier); - TF_AXIOM(errors.size() == 1); - TF_AXIOM(errors[0].GetType() == UsdValidationErrorType::Error); - TF_AXIOM(errors[0].GetSites().size() == 1); - TF_AXIOM(errors[0].GetSites()[0].IsValid()); - TF_AXIOM(errors[0].GetSites()[0].IsProperty()); - TF_AXIOM(errors[0].GetSites()[0].GetProperty().GetPath() - == SdfPath("/Bogus.info:id")); + const SdfPath expectedPropertyPath("/Bogus.info:id"); const std::string expectedErrorMsg = "shaderId 'Bogus' specified on " "shader prim not found in sdrRegistry."; - TF_AXIOM(errors[0].GetMessage() == expectedErrorMsg); + + ValidatePropertyError(errors[0], expectedErrorIdentifier, + expectedPropertyPath, expectedErrorMsg); } } @@ -376,25 +369,19 @@ TestUsdShadeSubsetMaterialBindFamilyName() { const UsdPrim usdPrim = usdStage->GetPrimAtPath( SdfPath("/SubsetsTest/Geom/Cube/materialBindMissingFamilyName")); - const TfToken expectedErrorIdentifier( - "usdShadeValidators:SubsetMaterialBindFamilyName.MissingFamilyNameOnGeomSubset"); - const UsdValidationErrorVector errors = validator->Validate(usdPrim); TF_AXIOM(errors.size() == 1u); - const UsdValidationError &error = errors[0u]; - TF_AXIOM(error.GetIdentifier() == expectedErrorIdentifier); - TF_AXIOM(error.GetType() == UsdValidationErrorType::Error); - TF_AXIOM(error.GetSites().size() == 1u); - const UsdValidationErrorSite &errorSite = error.GetSites()[0u]; - TF_AXIOM(errorSite.IsValid()); - TF_AXIOM(errorSite.IsPrim()); - TF_AXIOM(errorSite.GetPrim().GetPath() == usdPrim.GetPath()); + + const TfToken expectedErrorIdentifier( + "usdShadeValidators:SubsetMaterialBindFamilyName.MissingFamilyNameOnGeomSubset"); + const SdfPath expectedPrimPath = usdPrim.GetPath(); const std::string expectedErrorMsg = "GeomSubset prim " " " "with material bindings applied but no authored family name " "should set familyName to 'materialBind'."; - TF_AXIOM(error.GetMessage() == expectedErrorMsg); + ValidatePrimError(errors[0], expectedErrorIdentifier, + expectedPrimPath, expectedErrorMsg); } } @@ -418,23 +405,15 @@ TestUsdShadeSubsetsMaterialBindFamily() const UsdValidationErrorVector errors = validator->Validate(usdPrim); TF_AXIOM(errors.size() == 1u); - { - const TfToken expectedErrorIdentifier( + const TfToken expectedErrorIdentifier( "usdShadeValidators:SubsetsMaterialBindFamily.InvalidFamilyType"); - const UsdValidationError &error = errors[0u]; - TF_AXIOM(error.GetIdentifier() == expectedErrorIdentifier); - TF_AXIOM(error.GetType() == UsdValidationErrorType::Error); - TF_AXIOM(error.GetSites().size() == 1u); - const UsdValidationErrorSite &errorSite = error.GetSites()[0u]; - TF_AXIOM(errorSite.IsValid()); - TF_AXIOM(errorSite.IsPrim()); - TF_AXIOM(errorSite.GetPrim().GetPath() == usdPrim.GetPath()); - const std::string expectedErrorMsg + const SdfPath expectedPrimPath = usdPrim.GetPath(); + const std::string expectedErrorMsg = "Imageable prim has 'materialBind' " "subset family with invalid family type 'unrestricted'. Family " "type should be 'nonOverlapping' or 'partition' instead."; - TF_AXIOM(error.GetMessage() == expectedErrorMsg); - } + ValidatePrimError(errors[0], expectedErrorIdentifier, + expectedPrimPath, expectedErrorMsg); } } @@ -456,28 +435,27 @@ TestUsdShadeMaterialBindingAPIAppliedValidator() = usdPrim.CreateRelationship(TfToken("material:binding")); materialBinding.AddTarget(material.GetPath()); - const TfToken expectedErrorIdentifier( - "usdShadeValidators:MaterialBindingApiAppliedValidator.MissingMaterialBindingAPI"); - UsdValidationErrorVector errors = validator->Validate(usdPrim); - - TF_AXIOM(errors.size() == 1); - TF_AXIOM(errors[0].GetIdentifier() == expectedErrorIdentifier); - TF_AXIOM(errors[0].GetType() == UsdValidationErrorType::Error); - TF_AXIOM(errors[0].GetSites().size() == 1); - TF_AXIOM(errors[0].GetSites()[0].IsValid()); - TF_AXIOM(errors[0].GetSites()[0].IsPrim()); - TF_AXIOM(errors[0].GetSites()[0].GetPrim().GetPath() == SdfPath("/Test")); - const std::string expectedErrorMsg - = "Found material bindings but no MaterialBindingAPI applied on the prim " - "."; - TF_AXIOM(errors[0].GetMessage() == expectedErrorMsg); + { + const UsdValidationErrorVector errors = validator->Validate(usdPrim); + TF_AXIOM(errors.size() == 1u); + + const TfToken expectedErrorIdentifier( + "usdShadeValidators:MaterialBindingApiAppliedValidator.MissingMaterialBindingAPI"); + const SdfPath expectedPrimPath = usdPrim.GetPath(); + const std::string expectedErrorMsg + = "Found material bindings but no MaterialBindingAPI applied on the prim " + "."; + + ValidatePrimError(errors[0], expectedErrorIdentifier, + expectedPrimPath, expectedErrorMsg); + } // Apply the material binding API to the prim and bind the material UsdShadeMaterialBindingAPI bindingAPI = UsdShadeMaterialBindingAPI::Apply(usdPrim); bindingAPI.Bind(material); - errors = validator->Validate(usdPrim); + const UsdValidationErrorVector errors = validator->Validate(usdPrim); // Verify the errors are fixed TF_AXIOM(errors.empty()); @@ -505,21 +483,17 @@ TestUsdShadeEncapsulationRulesValidator() // non-container connectable const UsdValidationErrorVector errors = validator->Validate(insideShader.GetPrim()); + TF_AXIOM(errors.size() == 1); + const TfToken expectedErrorIdentifier( "usdShadeValidators:EncapsulationRulesValidator.ConnectableInNonContainer"); - - TF_AXIOM(errors.size() == 1); - TF_AXIOM(errors[0].GetIdentifier() == expectedErrorIdentifier); - TF_AXIOM(errors[0].GetType() == UsdValidationErrorType::Error); - TF_AXIOM(errors[0].GetSites().size() == 1); - TF_AXIOM(errors[0].GetSites()[0].IsValid()); - TF_AXIOM(errors[0].GetSites()[0].IsPrim()); - TF_AXIOM(errors[0].GetSites()[0].GetPrim().GetPath() - == SdfPath("/RootMaterial/Shader/InsideShader")); + const SdfPath expectedPrimPath("/RootMaterial/Shader/InsideShader"); const std::string expectedErrorMsg = "Connectable Shader cannot " "reside under a non-Container Connectable Shader"; - TF_AXIOM(errors[0].GetMessage() == expectedErrorMsg); + + ValidatePrimError(errors[0], expectedErrorIdentifier, + expectedPrimPath, expectedErrorMsg); } { @@ -540,42 +514,20 @@ TestUsdShadeEncapsulationRulesValidator() // non-connectable container ancestors const UsdValidationErrorVector errors = validator->Validate(insideScopeShader.GetPrim()); + TF_AXIOM(errors.size() == 1u); + const TfToken expectedErrorIdentifier( "usdShadeValidators:EncapsulationRulesValidator.InvalidConnectableHierarchy"); - TF_AXIOM(errors.size() == 1); - TF_AXIOM(errors[0].GetIdentifier() == expectedErrorIdentifier); - TF_AXIOM(errors[0].GetType() == UsdValidationErrorType::Error); - TF_AXIOM(errors[0].GetSites().size() == 1); - TF_AXIOM(errors[0].GetSites()[0].IsValid()); - TF_AXIOM(errors[0].GetSites()[0].IsPrim()); - TF_AXIOM(errors[0].GetSites()[0].GetPrim().GetPath() - == SdfPath("/RootMaterial/Scope/InsideShader")); + const SdfPath expectedPrimPath("/RootMaterial/Scope/InsideShader"); const std::string expectedErrorMsg = "Connectable Shader can only " "have Connectable Container ancestors up to Material ancestor " ", but its parent Scope is a Scope."; - TF_AXIOM(errors[0].GetMessage() == expectedErrorMsg); + ValidatePrimError(errors[0], expectedErrorIdentifier, + expectedPrimPath, expectedErrorMsg); } } -void ValidateError(const UsdValidationErrorVector& errors, - const TfToken& expectedErrorIdentifier, - const SdfPath& expectedPrimPath, - const std::string& expectedErrorMsg, - UsdValidationErrorType expectedErrorType = - UsdValidationErrorType::Error) -{ - TF_AXIOM(errors.size() == 1); - TF_AXIOM(errors[0].GetIdentifier() == expectedErrorIdentifier); - TF_AXIOM(errors[0].GetType() == expectedErrorType); - TF_AXIOM(errors[0].GetSites().size() == 1); - TF_AXIOM(errors[0].GetSites()[0].IsValid()); - TF_AXIOM(errors[0].GetSites()[0].IsPrim()); - TF_AXIOM(errors[0].GetSites()[0].GetPrim().GetPath() == - expectedPrimPath); - TF_AXIOM(errors[0].GetMessage() == expectedErrorMsg); -} - void TestUsdShadeNormalMapTextureValidator() { @@ -595,50 +547,60 @@ TestUsdShadeNormalMapTextureValidator() "/RootMaterial/UsdPreviewSurface"; UsdShadeShader usdPreviewSurfaceShader = UsdShadeShader::Define( usdStage, SdfPath(usdPreviewSurfaceShaderPath)); + const TfToken UsdPreviewSurface("UsdPreviewSurface"); usdPreviewSurfaceShader.CreateIdAttr( - VtValue(TfToken("UsdPreviewSurface"))); + VtValue(UsdPreviewSurface)); UsdPrim usdPreviewSurfaceShaderPrim = usdPreviewSurfaceShader.GetPrim(); UsdShadeShader usdUvTextureShader = UsdShadeShader::Define( usdStage, SdfPath("/RootMaterial/NormalTexture")); - usdUvTextureShader.CreateIdAttr(VtValue(TfToken("UsdUVTexture"))); + const TfToken UsdUVTexture("UsdUVTexture"); + usdUvTextureShader.CreateIdAttr(VtValue(UsdUVTexture)); // Add initial valid file and sourceColorSpace input values. std::string textureAssetPath = "./normalMap.jpg"; + const TfToken File("file"); UsdShadeInput fileInput = usdUvTextureShader.CreateInput( - TfToken("file"), SdfValueTypeNames->Asset); + File, SdfValueTypeNames->Asset); fileInput.Set(SdfAssetPath(textureAssetPath)); + const TfToken SourceColorSpace("sourceColorSpace"); UsdShadeInput sourceColorSpaceInput = usdUvTextureShader.CreateInput( - TfToken("sourceColorSpace"), SdfValueTypeNames->Token); - const TfToken rawToken("raw"); - sourceColorSpaceInput.Set(rawToken); + SourceColorSpace, SdfValueTypeNames->Token); + const TfToken Raw("raw"); + sourceColorSpaceInput.Set(Raw); // Connect the output of the UsdUVTexture Shader to the normal of the // UsdPreviewSurface Shader. - usdUvTextureShader.CreateOutput(TfToken("rgb"), SdfValueTypeNames->Float3); + const TfToken RGB("rgb"); + usdUvTextureShader.CreateOutput(RGB, SdfValueTypeNames->Float3); + const TfToken Normal("normal"); UsdShadeInput normalInput = usdPreviewSurfaceShader.CreateInput( - TfToken("normal"), SdfValueTypeNames->Normal3f); + Normal, SdfValueTypeNames->Normal3f); normalInput.ConnectToSource( SdfPath("/RootMaterial/NormalTexture.outputs:rgb")); - // Verify invalid bias & scale error, they should exists and do - // not exist at this point. - UsdValidationErrorVector errors = validator->Validate( - usdPreviewSurfaceShaderPrim); - TfToken expectedErrorIdentifier( - "usdShadeValidators:NormalMapTextureValidator.NonCompliantBiasAndScale"); - std::string expectedErrorMsg = - TfStringPrintf("UsdUVTexture prim <%s> reads 8 bit Normal Map " + // Verify invalid bias & scale error, both are required, but neither + // exist yet + { + const UsdValidationErrorVector errors = validator->Validate( + usdPreviewSurfaceShaderPrim); + TF_AXIOM(errors.size() == 1u); + + const TfToken expectedErrorIdentifier( + "usdShadeValidators:NormalMapTextureValidator.NonCompliantBiasAndScale"); + const std::string expectedErrorMsg = + TfStringPrintf("UsdUVTexture prim <%s> reads 8 bit Normal Map " "@./normalMap.jpg@, which requires that " "inputs:scale be set to (2, 2, 2, 1) and " "inputs:bias be set to (-1, -1, -1, 0) for proper " "interpretation as per the UsdPreviewSurface and " "UsdUVTexture docs.", usdUvTextureShader.GetPath().GetText()); - ValidateError(errors, - expectedErrorIdentifier, - usdUvTextureShader.GetPath(), - expectedErrorMsg); + ValidatePrimError(errors[0], + expectedErrorIdentifier, + usdUvTextureShader.GetPath(), + expectedErrorMsg); + } // Add bias and scale, but add a non-compliant bias value. UsdShadeInput biasInput = usdUvTextureShader.CreateInput( @@ -646,78 +608,93 @@ TestUsdShadeNormalMapTextureValidator() const GfVec4f compliantBias = GfVec4f(-1, -1, -1, 0); const GfVec4f nonCompliantVector = GfVec4f(-9, -9, -9, -9); biasInput.Set(nonCompliantVector); + const TfToken Scale("scale"); UsdShadeInput scaleInput = usdUvTextureShader.CreateInput( - TfToken("scale"), SdfValueTypeNames->Float4); + Scale, SdfValueTypeNames->Float4); const GfVec4f compliantScale = GfVec4f(2, 2, 2, 1); scaleInput.Set(compliantScale); // Verify the non-compliant bias value error occurs. - errors = validator->Validate(usdPreviewSurfaceShaderPrim); - expectedErrorIdentifier = TfToken( - "usdShadeValidators:NormalMapTextureValidator.NonCompliantBiasValues"); - expectedErrorMsg = - TfStringPrintf("UsdUVTexture prim <%s> reads an 8 bit Normal " - "Map, but has non-standard inputs:bias value of " - "(%.6g, %.6g, %.6g, %.6g). inputs:bias must be " - "set to [-1,-1,-1,0] so as to fulfill the " - "requirements of the normals to be in tangent " - "space of [(-1,-1,-1), (1,1,1)] as documented in " - "the UsdPreviewSurface and UsdUVTexture docs.", - usdUvTextureShader.GetPath().GetText(), - nonCompliantVector[0], nonCompliantVector[1], - nonCompliantVector[2], nonCompliantVector[3]); - ValidateError(errors, - expectedErrorIdentifier, - usdUvTextureShader.GetPath(), - expectedErrorMsg); + { + const UsdValidationErrorVector errors = + validator->Validate(usdPreviewSurfaceShaderPrim); + TF_AXIOM(errors.size() == 1u); + + const TfToken expectedErrorIdentifier( + "usdShadeValidators:NormalMapTextureValidator.NonCompliantBiasValues"); + const std::string expectedErrorMsg = + TfStringPrintf("UsdUVTexture prim <%s> reads an 8 bit Normal " + "Map, but has non-standard inputs:bias value of " + "(%.6g, %.6g, %.6g, %.6g). inputs:bias must be " + "set to [-1,-1,-1,0] so as to fulfill the " + "requirements of the normals to be in tangent " + "space of [(-1,-1,-1), (1,1,1)] as documented in " + "the UsdPreviewSurface and UsdUVTexture docs.", + usdUvTextureShader.GetPath().GetText(), + nonCompliantVector[0], nonCompliantVector[1], + nonCompliantVector[2], nonCompliantVector[3]); + ValidatePrimError(errors[0], + expectedErrorIdentifier, + usdUvTextureShader.GetPath(), + expectedErrorMsg); + } // Update to a compliant bias and a non-compliant scale value. biasInput.Set(compliantBias); scaleInput.Set(nonCompliantVector); // Verify the non-compliant scale value error occurs. - errors = validator->Validate(usdPreviewSurfaceShaderPrim); - expectedErrorIdentifier = TfToken( - "usdShadeValidators:NormalMapTextureValidator.NonCompliantScaleValues"); - expectedErrorMsg = - TfStringPrintf("UsdUVTexture prim <%s> reads an 8 bit Normal " - "Map, but has non-standard inputs:scale value " - "of (%.6g, %.6g, %.6g, %.6g). inputs:scale must " - "be set to (2, 2, 2, 1) so as fulfill the " - "requirements of the normals to be in tangent " - "space of [(-1,-1,-1), (1,1,1)] as documented in " - "the UsdPreviewSurface and UsdUVTexture docs.", - usdUvTextureShader.GetPath().GetText(), - nonCompliantVector[0], nonCompliantVector[1], - nonCompliantVector[2], nonCompliantVector[3]); - ValidateError(errors, - expectedErrorIdentifier, - usdUvTextureShader.GetPath(), - expectedErrorMsg, - UsdValidationErrorType::Warn); + { + const UsdValidationErrorVector errors = + validator->Validate(usdPreviewSurfaceShaderPrim); + TF_AXIOM(errors.size() == 1u); + + const TfToken expectedErrorIdentifier( + "usdShadeValidators:NormalMapTextureValidator.NonCompliantScaleValues"); + const std::string expectedErrorMsg = + TfStringPrintf("UsdUVTexture prim <%s> reads an 8 bit Normal " + "Map, but has non-standard inputs:scale value " + "of (%.6g, %.6g, %.6g, %.6g). inputs:scale must " + "be set to (2, 2, 2, 1) so as fulfill the " + "requirements of the normals to be in tangent " + "space of [(-1,-1,-1), (1,1,1)] as documented in " + "the UsdPreviewSurface and UsdUVTexture docs.", + usdUvTextureShader.GetPath().GetText(), + nonCompliantVector[0], nonCompliantVector[1], + nonCompliantVector[2], nonCompliantVector[3]); + ValidatePrimError(errors[0], + expectedErrorIdentifier, + usdUvTextureShader.GetPath(), + expectedErrorMsg, + UsdValidationErrorType::Warn); + } // Set a compliant scale value, and an invalid sourceColorSpace. scaleInput.Set(compliantScale); sourceColorSpaceInput.Set(TfToken("error")); // Verify the invalid sourceColorSpace error occurs. - errors = validator->Validate(usdPreviewSurfaceShaderPrim); - expectedErrorIdentifier = TfToken( - "usdShadeValidators:NormalMapTextureValidator.InvalidSourceColorSpace"); - expectedErrorMsg = - TfStringPrintf("UsdUVTexture prim <%s> that reads" - " Normal Map @%s@ should set " - "inputs:sourceColorSpace to 'raw'.", - usdUvTextureShader.GetPath().GetText(), - textureAssetPath.c_str()); - ValidateError(errors, - expectedErrorIdentifier, - usdUvTextureShader.GetPath(), - expectedErrorMsg); + { + const UsdValidationErrorVector errors = validator->Validate(usdPreviewSurfaceShaderPrim); + TF_AXIOM(errors.size() == 1u); + + const TfToken expectedErrorIdentifier( + "usdShadeValidators:NormalMapTextureValidator.InvalidSourceColorSpace"); + const std::string expectedErrorMsg = + TfStringPrintf("UsdUVTexture prim <%s> that reads" + " Normal Map @%s@ should set " + "inputs:sourceColorSpace to 'raw'.", + usdUvTextureShader.GetPath().GetText(), + textureAssetPath.c_str()); + ValidatePrimError(errors[0], + expectedErrorIdentifier, + usdUvTextureShader.GetPath(), + expectedErrorMsg); + } // Correct the sourceColorSpace, hook up the normal input of // UsdPreviewSurface to a non-shader output. - sourceColorSpaceInput.Set(rawToken); + sourceColorSpaceInput.Set(Raw); UsdGeomXform nonShaderPrim = UsdGeomXform::Define( usdStage, SdfPath("/RootMaterial/Xform")); UsdShadeConnectableAPI connectableNonShaderAPI(nonShaderPrim.GetPrim()); @@ -727,17 +704,21 @@ TestUsdShadeNormalMapTextureValidator() normalInput.ConnectToSource(nonShaderOutput); // Verify a non-shader connection error occurs. - errors = validator->Validate(usdPreviewSurfaceShaderPrim); - expectedErrorIdentifier = TfToken( - "usdShadeValidators:NormalMapTextureValidator.NonShaderConnection"); - expectedErrorMsg = - TfStringPrintf("UsdPreviewSurface.normal on prim <%s> is connected " - "to a non-Shader prim.", - usdPreviewSurfaceShaderPath.c_str()); - ValidateError(errors, - expectedErrorIdentifier, - usdPreviewSurfaceShader.GetPath(), - expectedErrorMsg); + { + const UsdValidationErrorVector errors = validator->Validate(usdPreviewSurfaceShaderPrim); + TF_AXIOM(errors.size() == 1u); + + const TfToken expectedErrorIdentifier( + "usdShadeValidators:NormalMapTextureValidator.NonShaderConnection"); + const std::string expectedErrorMsg = + TfStringPrintf("UsdPreviewSurface.normal on prim <%s> is connected " + "to a non-Shader prim.", + usdPreviewSurfaceShaderPath.c_str()); + ValidatePrimError(errors[0], + expectedErrorIdentifier, + usdPreviewSurfaceShader.GetPath(), + expectedErrorMsg); + } // Set the normal input back to a valid shader and update the file input // to an invalid file path. @@ -746,24 +727,27 @@ TestUsdShadeNormalMapTextureValidator() fileInput.Set(SdfAssetPath("./doesNotExist.jpg")); // Verify the invalid input file error occurs. - errors = validator->Validate(usdPreviewSurfaceShaderPrim); - expectedErrorIdentifier = - TfToken("usdShadeValidators:NormalMapTextureValidator.InvalidFile"); - expectedErrorMsg = - TfStringPrintf("UsdUVTexture prim <%s> has invalid or unresolvable " - "inputs:file of @%s@", - usdUvTextureShader.GetPath().GetText(), - "./doesNotExist.jpg"); - ValidateError(errors, - expectedErrorIdentifier, - usdUvTextureShader.GetPath(), - expectedErrorMsg); + { + const UsdValidationErrorVector errors = validator->Validate(usdPreviewSurfaceShaderPrim); + TF_AXIOM(errors.size() == 1u); + + const TfToken expectedErrorIdentifier("usdShadeValidators:NormalMapTextureValidator.InvalidFile"); + const std::string expectedErrorMsg = + TfStringPrintf("UsdUVTexture prim <%s> has invalid or unresolvable " + "inputs:file of @%s@", + usdUvTextureShader.GetPath().GetText(), + "./doesNotExist.jpg"); + ValidatePrimError(errors[0], + expectedErrorIdentifier, + usdUvTextureShader.GetPath(), + expectedErrorMsg); + } // Reset the file to a valid path. fileInput.Set(SdfAssetPath("./normalMap.jpg")); // Verify no errors exist. - errors = validator->Validate(usdPreviewSurfaceShaderPrim); + const UsdValidationErrorVector errors = validator->Validate(usdPreviewSurfaceShaderPrim); TF_AXIOM(errors.empty()); } diff --git a/pxr/usdValidation/usdShadeValidators/validators.cpp b/pxr/usdValidation/usdShadeValidators/validators.cpp index 97bfec1c1f..96b08ade28 100644 --- a/pxr/usdValidation/usdShadeValidators/validators.cpp +++ b/pxr/usdValidation/usdShadeValidators/validators.cpp @@ -551,7 +551,7 @@ _NormalMapTextureValidator(const UsdPrim& usdPrim) { const UsdShadeShader shader(usdPrim); TfToken shaderId; - TfToken UsdPreviewSurface("UsdPreviewSurface"); + const TfToken UsdPreviewSurface("UsdPreviewSurface"); // We may have failed to fetch an identifier for asset/source-based // nodes. OR, we could potentially be driven by a UsdPrimvarReader, @@ -560,7 +560,8 @@ _NormalMapTextureValidator(const UsdPrim& usdPrim) { return {}; } - const UsdShadeInput normalInput = shader.GetInput(TfToken("normal")); + const TfToken Normal("normal"); + const UsdShadeInput normalInput = shader.GetInput(Normal); if (!normalInput) { return {}; } @@ -600,7 +601,7 @@ _NormalMapTextureValidator(const UsdPrim& usdPrim) { } TfToken sourceShaderId; - TfToken UsdUVTexture("UsdUVTexture"); + const TfToken UsdUVTexture("UsdUVTexture"); bool gotShaderSourceId = sourceShader.GetShaderId(&sourceShaderId); @@ -611,8 +612,8 @@ _NormalMapTextureValidator(const UsdPrim& usdPrim) { return {}; } - const auto getInputValue = [](const UsdShadeShader &inputShader, - const TfToken &token, auto& outputValue) -> bool { + const auto _GetInputValue = [](const UsdShadeShader &inputShader, + const TfToken &token, auto *outputValue) -> bool { const UsdShadeInput input = inputShader.GetInput(token); if (!input) { return false; @@ -631,13 +632,14 @@ _NormalMapTextureValidator(const UsdPrim& usdPrim) { return false; } - return valueProducingAttributes[0].Get(&outputValue, + return valueProducingAttributes[0].Get(outputValue, UsdTimeCode::EarliestTime()); }; SdfAssetPath textureAssetPath; - bool valueForFileExists = getInputValue(sourceShader, TfToken("file"), - textureAssetPath); + const TfToken File("file"); + bool valueForFileExists = _GetInputValue(sourceShader, File, + &textureAssetPath); UsdValidationErrorVector errors; @@ -657,7 +659,7 @@ _NormalMapTextureValidator(const UsdPrim& usdPrim) { sourcePrim.GetPath().GetText(), assetPath.c_str())); } - auto textureIs8Bit = [](std::string resolvedPath) { + auto _TextureIs8Bit = [](std::string resolvedPath) { std::string extension = ArGetResolver().GetExtension(resolvedPath); extension = TfStringToLower(extension); @@ -667,17 +669,17 @@ _NormalMapTextureValidator(const UsdPrim& usdPrim) { return eightBitExtensions.find(extension) != eightBitExtensions.end(); }; - if (!textureIs8Bit(textureAssetPath.GetResolvedPath())) { + if (!_TextureIs8Bit(textureAssetPath.GetResolvedPath())) { // Nothing more is required for image depths > 8 bits, which // we assume FOR NOW, are floating point return errors; } TfToken colorSpace; - TfToken rawColorSpace("raw"); + const TfToken Raw("raw"); bool valueForColorSpaceExists = - getInputValue(sourceShader, TfToken("sourceColorSpace"), colorSpace); - if (!valueForColorSpaceExists || colorSpace != rawColorSpace) { + _GetInputValue(sourceShader, TfToken("sourceColorSpace"), &colorSpace); + if (!valueForColorSpaceExists || colorSpace != Raw) { errors.emplace_back( UsdShadeValidationErrorNameTokens->invalidSourceColorSpace, UsdValidationErrorType::Error, @@ -692,13 +694,15 @@ _NormalMapTextureValidator(const UsdPrim& usdPrim) { textureAssetPath.GetAssetPath().c_str())); } - GfVec4f bias; - bool valueForBiasExists = getInputValue(sourceShader, TfToken("bias"), - bias); + GfVec4f biasVector; + const TfToken Bias("bias"); + bool valueForBiasExists = _GetInputValue(sourceShader, Bias, + &biasVector); - GfVec4f scale; - bool valueForScaleExists = getInputValue(sourceShader, TfToken("scale"), - scale); + GfVec4f scaleVector; + const TfToken Scale("scale"); + bool valueForScaleExists = _GetInputValue(sourceShader, Scale, + &scaleVector); if (!(valueForBiasExists && valueForScaleExists)) { @@ -724,8 +728,8 @@ _NormalMapTextureValidator(const UsdPrim& usdPrim) { // guidelines, as some authoring tools may rely on this to scale an // effect of normal perturbations. // don't really care about fourth components... - bool nonCompliantScaleValues = scale[0] != 2 || - scale[1] != 2 || scale[2] != 2; + bool nonCompliantScaleValues = scaleVector[0] != 2 || + scaleVector[1] != 2 || scaleVector[2] != 2; if (nonCompliantScaleValues) { @@ -744,7 +748,8 @@ _NormalMapTextureValidator(const UsdPrim& usdPrim) { "space of [(-1,-1,-1), (1,1,1)] as documented in " "the UsdPreviewSurface and UsdUVTexture docs.", sourcePrim.GetPath().GetText(), - scale[0], scale[1], scale[2], scale[3]) + scaleVector[0], scaleVector[1], scaleVector[2], + scaleVector[3]) ); } @@ -754,8 +759,8 @@ _NormalMapTextureValidator(const UsdPrim& usdPrim) { // in the UsdPreviewSurface docs. Note this is true only when scale // values are respecting the requirements laid in the // UsdPreviewSurface / UsdUVTexture docs. We continue to warn! - if (!nonCompliantScaleValues && (bias[0] != -1 || bias[1] != -1 || - bias[2] != -1)) + if (!nonCompliantScaleValues && (biasVector[0] != -1 || + biasVector[1] != -1 || biasVector[2] != -1)) { errors.emplace_back( UsdShadeValidationErrorNameTokens->nonCompliantBias, @@ -772,7 +777,8 @@ _NormalMapTextureValidator(const UsdPrim& usdPrim) { "space of [(-1,-1,-1), (1,1,1)] as documented in " "the UsdPreviewSurface and UsdUVTexture docs.", sourcePrim.GetPath().GetText(), - bias[0], bias[1], bias[2], bias[3]) + biasVector[0], biasVector[1], biasVector[2], + biasVector[3]) ); } From 0a27df94e791966a640ac07f343935b78636446c Mon Sep 17 00:00:00 2001 From: Andy Date: Tue, 10 Dec 2024 16:51:52 -0500 Subject: [PATCH 4/5] fix: styling fixes - address tabs in error messages - address long lines --- .../testenv/testUsdShadeValidators.cpp | 67 ++++++++++--------- 1 file changed, 36 insertions(+), 31 deletions(-) diff --git a/pxr/usdValidation/usdShadeValidators/testenv/testUsdShadeValidators.cpp b/pxr/usdValidation/usdShadeValidators/testenv/testUsdShadeValidators.cpp index 8c54965f06..d98ac1b890 100644 --- a/pxr/usdValidation/usdShadeValidators/testenv/testUsdShadeValidators.cpp +++ b/pxr/usdValidation/usdShadeValidators/testenv/testUsdShadeValidators.cpp @@ -624,15 +624,15 @@ TestUsdShadeNormalMapTextureValidator() "usdShadeValidators:NormalMapTextureValidator.NonCompliantBiasValues"); const std::string expectedErrorMsg = TfStringPrintf("UsdUVTexture prim <%s> reads an 8 bit Normal " - "Map, but has non-standard inputs:bias value of " - "(%.6g, %.6g, %.6g, %.6g). inputs:bias must be " - "set to [-1,-1,-1,0] so as to fulfill the " - "requirements of the normals to be in tangent " - "space of [(-1,-1,-1), (1,1,1)] as documented in " - "the UsdPreviewSurface and UsdUVTexture docs.", - usdUvTextureShader.GetPath().GetText(), - nonCompliantVector[0], nonCompliantVector[1], - nonCompliantVector[2], nonCompliantVector[3]); + "Map, but has non-standard inputs:bias value of " + "(%.6g, %.6g, %.6g, %.6g). inputs:bias must be " + "set to [-1,-1,-1,0] so as to fulfill the " + "requirements of the normals to be in tangent " + "space of [(-1,-1,-1), (1,1,1)] as documented in " + "the UsdPreviewSurface and UsdUVTexture docs.", + usdUvTextureShader.GetPath().GetText(), + nonCompliantVector[0], nonCompliantVector[1], + nonCompliantVector[2], nonCompliantVector[3]); ValidatePrimError(errors[0], expectedErrorIdentifier, usdUvTextureShader.GetPath(), @@ -653,15 +653,15 @@ TestUsdShadeNormalMapTextureValidator() "usdShadeValidators:NormalMapTextureValidator.NonCompliantScaleValues"); const std::string expectedErrorMsg = TfStringPrintf("UsdUVTexture prim <%s> reads an 8 bit Normal " - "Map, but has non-standard inputs:scale value " - "of (%.6g, %.6g, %.6g, %.6g). inputs:scale must " - "be set to (2, 2, 2, 1) so as fulfill the " - "requirements of the normals to be in tangent " - "space of [(-1,-1,-1), (1,1,1)] as documented in " - "the UsdPreviewSurface and UsdUVTexture docs.", - usdUvTextureShader.GetPath().GetText(), - nonCompliantVector[0], nonCompliantVector[1], - nonCompliantVector[2], nonCompliantVector[3]); + "Map, but has non-standard inputs:scale value " + "of (%.6g, %.6g, %.6g, %.6g). inputs:scale must " + "be set to (2, 2, 2, 1) so as fulfill the " + "requirements of the normals to be in tangent " + "space of [(-1,-1,-1), (1,1,1)] as documented in " + "the UsdPreviewSurface and UsdUVTexture docs.", + usdUvTextureShader.GetPath().GetText(), + nonCompliantVector[0], nonCompliantVector[1], + nonCompliantVector[2], nonCompliantVector[3]); ValidatePrimError(errors[0], expectedErrorIdentifier, usdUvTextureShader.GetPath(), @@ -675,17 +675,18 @@ TestUsdShadeNormalMapTextureValidator() // Verify the invalid sourceColorSpace error occurs. { - const UsdValidationErrorVector errors = validator->Validate(usdPreviewSurfaceShaderPrim); + const UsdValidationErrorVector errors = + validator->Validate(usdPreviewSurfaceShaderPrim); TF_AXIOM(errors.size() == 1u); const TfToken expectedErrorIdentifier( "usdShadeValidators:NormalMapTextureValidator.InvalidSourceColorSpace"); const std::string expectedErrorMsg = TfStringPrintf("UsdUVTexture prim <%s> that reads" - " Normal Map @%s@ should set " - "inputs:sourceColorSpace to 'raw'.", - usdUvTextureShader.GetPath().GetText(), - textureAssetPath.c_str()); + " Normal Map @%s@ should set " + "inputs:sourceColorSpace to 'raw'.", + usdUvTextureShader.GetPath().GetText(), + textureAssetPath.c_str()); ValidatePrimError(errors[0], expectedErrorIdentifier, usdUvTextureShader.GetPath(), @@ -705,7 +706,8 @@ TestUsdShadeNormalMapTextureValidator() // Verify a non-shader connection error occurs. { - const UsdValidationErrorVector errors = validator->Validate(usdPreviewSurfaceShaderPrim); + const UsdValidationErrorVector errors = + validator->Validate(usdPreviewSurfaceShaderPrim); TF_AXIOM(errors.size() == 1u); const TfToken expectedErrorIdentifier( @@ -728,15 +730,17 @@ TestUsdShadeNormalMapTextureValidator() // Verify the invalid input file error occurs. { - const UsdValidationErrorVector errors = validator->Validate(usdPreviewSurfaceShaderPrim); + const UsdValidationErrorVector errors = + validator->Validate(usdPreviewSurfaceShaderPrim); TF_AXIOM(errors.size() == 1u); - const TfToken expectedErrorIdentifier("usdShadeValidators:NormalMapTextureValidator.InvalidFile"); + const TfToken expectedErrorIdentifier( + "usdShadeValidators:NormalMapTextureValidator.InvalidFile"); const std::string expectedErrorMsg = - TfStringPrintf("UsdUVTexture prim <%s> has invalid or unresolvable " - "inputs:file of @%s@", - usdUvTextureShader.GetPath().GetText(), - "./doesNotExist.jpg"); + TfStringPrintf("UsdUVTexture prim <%s> has invalid or " + "unresolvable inputs:file of @%s@", + usdUvTextureShader.GetPath().GetText(), + "./doesNotExist.jpg"); ValidatePrimError(errors[0], expectedErrorIdentifier, usdUvTextureShader.GetPath(), @@ -747,7 +751,8 @@ TestUsdShadeNormalMapTextureValidator() fileInput.Set(SdfAssetPath("./normalMap.jpg")); // Verify no errors exist. - const UsdValidationErrorVector errors = validator->Validate(usdPreviewSurfaceShaderPrim); + const UsdValidationErrorVector errors = + validator->Validate(usdPreviewSurfaceShaderPrim); TF_AXIOM(errors.empty()); } From 7b075acb8f086b2ba735d9738c4fbba091be3464 Mon Sep 17 00:00:00 2001 From: Andy Date: Tue, 10 Dec 2024 17:07:55 -0500 Subject: [PATCH 5/5] fix: update comment --- .../usdShadeValidators/testenv/testUsdShadeValidators.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pxr/usdValidation/usdShadeValidators/testenv/testUsdShadeValidators.cpp b/pxr/usdValidation/usdShadeValidators/testenv/testUsdShadeValidators.cpp index d98ac1b890..ebc4ec4297 100644 --- a/pxr/usdValidation/usdShadeValidators/testenv/testUsdShadeValidators.cpp +++ b/pxr/usdValidation/usdShadeValidators/testenv/testUsdShadeValidators.cpp @@ -579,8 +579,8 @@ TestUsdShadeNormalMapTextureValidator() normalInput.ConnectToSource( SdfPath("/RootMaterial/NormalTexture.outputs:rgb")); - // Verify invalid bias & scale error, both are required, but neither - // exist yet + // Verify invalid bias & scale error, they are expected to exist but + // do not. { const UsdValidationErrorVector errors = validator->Validate( usdPreviewSurfaceShaderPrim);