Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

usdValidation: usdUtils: MissingReferenceValidator #3450

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
+161 −17
Open

usdValidation: usdUtils: MissingReferenceValidator #3450

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dedd413
usdValidation: usdUtils: MissingReferenceValidator
beersandrew Dec 3, 2024
a079178
feat: adding an asset reference test
beersandrew Dec 12, 2024
0fa86de
feat: added unused variant test
beersandrew Dec 12, 2024
d71c0a1
fix: address PR comments
beersandrew Dec 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions pxr/usdValidation/usdUtilsValidators/plugInfo.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@
"Info": {
"Validators": {
"FileExtensionValidator": {
"doc": "Only valid core layer extensions (.usd, .usda, .usdc, .usdz), valid core texture extensions (.exr, .jpg, .jpeg, .png) and embedded audio files (.M4A, .MP3, .WAV) are allowed in a package.",
"keywords": [
"UsdzValidators"
]
"doc": "Only valid core layer extensions (.usd, .usda, .usdc, .usdz), valid core texture extensions (.exr, .jpg, .jpeg, .png) and embedded audio files (.M4A, .MP3, .WAV) are allowed in a package.",
"keywords": [
"UsdzValidators"
]
},
"MissingReferenceValidator": {
"doc": "The composed USD stage should not contain any unresolvable asset dependencies (in every possible variation of the asset), when using the default asset resolver."
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets remove the "every possible variation of the asset" thats part of the usdchecker to iterate through various variants. Validator should just be checking a stage/variant.

Copy link
Contributor

Choose a reason for hiding this comment

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

Lets leave it as is, since UsdUtilsComputeAllDependencies operates at Sdf level and will give unresolved references across variants.

},
"PackageEncapsulationValidator": {
"doc": "If the root layer is a package, then its recommended for the composed stage to not contain references to files outside the package. The package should be self-contained, warn if not.",
Expand Down
111 changes: 107 additions & 4 deletions pxr/usdValidation/usdUtilsValidators/testenv/testUsdUtilsValidators.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@

#include "pxr/base/arch/systemInfo.h"
#include "pxr/base/tf/pathUtils.h"
#include "pxr/usd/usd/editContext.h"
#include "pxr/usd/usd/variantSets.h"
#include "pxr/usd/usdShade/shader.h"
#include "pxr/usdValidation/usdUtilsValidators/validatorTokens.h"
#include "pxr/usdValidation/usdValidation/error.h"
#include "pxr/usdValidation/usdValidation/registry.h"
Expand All @@ -30,7 +33,7 @@ TestUsdUsdzValidators()
UsdValidationValidatorMetadataVector metadata
= registry.GetValidatorMetadataForPlugin(
_tokens->usdUtilsValidatorsPlugin);
TF_AXIOM(metadata.size() == 2);
TF_AXIOM(metadata.size() == 3);
// Since other validators can be registered with a UsdUtilsValidators
// keyword, our validators registered in usd are a subset of the entire
// set.
Expand All @@ -41,24 +44,24 @@ TestUsdUsdzValidators()

const std::set<TfToken> expectedValidatorNames
= { UsdUtilsValidatorNameTokens->packageEncapsulationValidator,
UsdUtilsValidatorNameTokens->fileExtensionValidator };
UsdUtilsValidatorNameTokens->fileExtensionValidator,
UsdUtilsValidatorNameTokens->missingReferenceValidator };

TF_AXIOM(validatorMetadataNameSet == expectedValidatorNames);
}


void ValidateError(const UsdValidationError &error,
const std::string& expectedErrorMsg,
const TfToken& expectedErrorIdentifier,
UsdValidationErrorType expectedErrorType =
UsdValidationErrorType::Error)
{
TF_AXIOM(error.GetMessage() == expectedErrorMsg);
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.GetLayer().IsInvalid());
TF_AXIOM(error.GetMessage() == expectedErrorMsg);
}

static void
Expand Down Expand Up @@ -160,12 +163,112 @@ TestFileExtensionValidator()
TF_AXIOM(errors.empty());
}

static void
TestMissingReferenceValidator()
{
UsdValidationRegistry& registry = UsdValidationRegistry::GetInstance();

// Verify the validator exists
const UsdValidationValidator *validator = registry.GetOrLoadValidatorByName(
UsdUtilsValidatorNameTokens->missingReferenceValidator);

TF_AXIOM(validator);

const UsdStageRefPtr& stage = UsdStage::CreateInMemory();

const TfToken xformToken("Xform");
const UsdPrim xform = stage->DefinePrim(SdfPath("/Prim"), xformToken);
const SdfReference badLayerReference("doesNotExist.usd");
xform.GetReferences().AddReference(badLayerReference);

// Validate an error occurs for a missing layer reference
{
const UsdValidationErrorVector errors = validator->Validate(stage);
TF_AXIOM(errors.size() == 1);
const std::string expectedErrorMessage = "Found unresolvable external "
"dependency 'doesNotExist.usd'.";
const TfToken expectedIdentifier =
TfToken(
"usdUtilsValidators:MissingReferenceValidator.UnresolvableDependency");

ValidateError(errors[0], expectedErrorMessage,
expectedIdentifier);
}

// Remove the bad layer reference and add a shader prim for the next test
xform.GetReferences().RemoveReference(badLayerReference);
UsdShadeShader shader = UsdShadeShader::Define(stage,
SdfPath("/Prim/Shader"));
const TfToken notFoundAsset("notFoundAsset");
UsdShadeInput notFoundAssetInput = shader.CreateInput(
notFoundAsset, SdfValueTypeNames->Asset);
notFoundAssetInput.Set(SdfAssetPath("doesNotExist.jpg"));

// Validate an error occurs for a missing asset reference
{
const UsdValidationErrorVector errors = validator->Validate(stage);
TF_AXIOM(errors.size() == 1);
const std::string expectedErrorMessage = "Found unresolvable external "
"dependency 'doesNotExist.jpg'.";
const TfToken expectedIdentifier =
TfToken(
"usdUtilsValidators:MissingReferenceValidator.UnresolvableDependency");

ValidateError(errors[0], expectedErrorMessage,
expectedIdentifier);
}

// Remove shader prim and add a variant set for the next test
stage->RemovePrim(shader.GetPath());
UsdVariantSets variantSets = xform.GetVariantSets();
UsdVariantSet testVariantSet = variantSets.AddVariantSet("testVariantSet");

testVariantSet.AddVariant("invalid");
testVariantSet.AddVariant("valid");

testVariantSet.SetVariantSelection("invalid");

{
UsdEditContext context(testVariantSet.GetVariantEditContext());
UsdShadeShader shader = UsdShadeShader::Define(stage, xform.GetPath().AppendChild(TfToken("Shader")));
UsdShadeInput invalidAssetInput = shader.CreateInput(
TfToken("invalidAsset"), SdfValueTypeNames->Asset);
invalidAssetInput.Set(SdfAssetPath("doesNotExistOnVariant.jpg"));
}
testVariantSet.SetVariantSelection("valid");

// Validate an error occurs on a nonexistent asset on an inactive variant
{
const UsdValidationErrorVector errors = validator->Validate(stage);
TF_AXIOM(errors.size() == 1);
const std::string expectedErrorMessage = "Found unresolvable external "
"dependency 'doesNotExistOnVariant.jpg'.";
const TfToken expectedIdentifier =
TfToken(
"usdUtilsValidators:MissingReferenceValidator.UnresolvableDependency");

ValidateError(errors[0], expectedErrorMessage,
expectedIdentifier);
}

// Remove the variant set and add a valid reference
SdfPrimSpecHandle primSpec = stage->GetRootLayer()->GetPrimAtPath(xform.GetPath());
primSpec->RemoveVariantSet("testVariantSet");
xform.GetPrim().GetReferences().AddReference("pass.usdz");

const UsdValidationErrorVector errors = validator->Validate(stage);

// Verify the errors are gone
TF_AXIOM(errors.empty());
}

int
main()
{
TestUsdUsdzValidators();
TestPackageEncapsulationValidator();
TestFileExtensionValidator();
TestMissingReferenceValidator();

return EXIT_SUCCESS;
}
22 changes: 13 additions & 9 deletions pxr/usdValidation/usdUtilsValidators/validatorTokens.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,26 @@

PXR_NAMESPACE_OPEN_SCOPE

#define USD_UTILS_VALIDATOR_NAME_TOKENS \
((packageEncapsulationValidator, \
"usdUtilsValidators:PackageEncapsulationValidator")) \
((fileExtensionValidator, \
"usdUtilsValidators:FileExtensionValidator"))

#define USD_UTILS_VALIDATOR_KEYWORD_TOKENS \
(UsdUtilsValidators) \
#define USD_UTILS_VALIDATOR_NAME_TOKENS \
((packageEncapsulationValidator, \
"usdUtilsValidators:PackageEncapsulationValidator")) \
((fileExtensionValidator, \
"usdUtilsValidators:FileExtensionValidator")) \
((missingReferenceValidator, \
"usdUtilsValidators:MissingReferenceValidator")) \

#define USD_UTILS_VALIDATOR_KEYWORD_TOKENS \
(UsdUtilsValidators) \
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if UsdUtilsValidators should be the name of it! (I also want to move this pxr/usdValidation/usdUtilsValidators to be called something else). :/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From my perspective, the naming started from "which folder in the usd code base does this validator depend on?". Now that the validation code is moved out of those folders, this feels like a broader problem of defining validation domains, which may be fine for the others already (usdGeom, usdShade... etc).

Maybe this validator could go into pxr/usd/usdValidation? I'm not sure if it's important for that project to not have external dependencies on usdUtils or not.

Copy link
Contributor

Choose a reason for hiding this comment

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

Slightly correction the suffix here is the name of the plugin (the directory name happens to be matching the plugin name :D).

I do plan to move the usdUtilsValidators plugin sometime later so lets leave it as-is for now.

(UsdzValidators)

#define USD_UTILS_VALIDATION_ERROR_NAME_TOKENS \
((layerNotInPackage, "LayerNotInPackage")) \
((assetNotInPackage, "AssetNotInPackage")) \
((invalidLayerInPackage, "InvalidLayerInPackage")) \
((unsupportedFileExtensionInPackage, \
"UnsupportedFileExtensionInPackage"))
"UnsupportedFileExtensionInPackage")) \
((unresolvableDependency, "UnresolvableDependency"))


///\def
/// Tokens representing validator names. Note that for plugin provided
Expand Down
34 changes: 34 additions & 0 deletions pxr/usdValidation/usdUtilsValidators/validators.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,44 @@ _FileExtensionValidator(const UsdStagePtr& usdStage) {
return errors;
}

static
UsdValidationErrorVector
_MissingReferenceValidator(const UsdStagePtr& usdStage) {
const SdfLayerRefPtr& rootLayer = usdStage->GetRootLayer();

SdfLayerRefPtrVector layers;
std::vector<std::basic_string<char>> assets, unresolvedPaths;
const SdfAssetPath& path = SdfAssetPath(rootLayer->GetIdentifier());

UsdUtilsComputeAllDependencies(path, &layers, &assets, &unresolvedPaths,
nullptr);

UsdValidationErrorVector errors;
for(const std::basic_string<char>& unresolvedPath : unresolvedPaths)
{
errors.emplace_back(
UsdUtilsValidationErrorNameTokens->unresolvableDependency,
UsdValidationErrorType::Error,
UsdValidationErrorSites {
UsdValidationErrorSite(
rootLayer, SdfPath(unresolvedPath))
},
TfStringPrintf(
("Found unresolvable external dependency "
"'%s'."), unresolvedPath.c_str())
);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tallytalwar One note here, it looks like complianceChecker.py is doing another check here which gets some diagnostic information, but it seems like this process of collecting diagnostic information should be started before opening the stage (here). I tried doing this inside the validator with stage.Reload() but was unable to populate the diagnostics this way.

In my specific test, the errors were similar so I wanted your opinion on if it's possible to do something similar here, or if it's necessary to move the diagnostic part over in this case.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we might not need an explicit validator for this now! https://github.com/PixarAnimationStudios/OpenUSD/blob/dev/pxr/usdValidation/usdValidation/coreValidators.cpp#L16-L29 should provide coverage for this? Do you want to try running it through the CoreValidator and see if it gives you the appropriate composition error wrapped by a CoreValidation error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, it does! awesome. I'll close this one out then. Thanks


return errors;
}

TF_REGISTRY_FUNCTION(UsdValidationRegistry)
{
UsdValidationRegistry &registry = UsdValidationRegistry::GetInstance();

registry.RegisterPluginValidator(
UsdUtilsValidatorNameTokens->missingReferenceValidator, _MissingReferenceValidator);

registry.RegisterPluginValidator(
UsdUtilsValidatorNameTokens->packageEncapsulationValidator,
_PackageEncapsulationValidator);
Expand Down