Skip to content

Commit

Permalink
Fix for Github #2074
Browse files Browse the repository at this point in the history
This change fixes an issue with UsdUtils::ComputeAllDependencies in which the resoved Asset Path was being used to open layers as opposed to an AR identifier.  This resulted in layer::GetIdentifier() returning an incorrect value (in this case the resolved path)

Fixes #2074

(Internal change: 2276164)
  • Loading branch information
matthewcpp authored and pixar-oss committed May 16, 2023
1 parent 6cb5c59 commit e670db4
Show file tree
Hide file tree
Showing 8 changed files with 265 additions and 12 deletions.
24 changes: 24 additions & 0 deletions pxr/usd/usdUtils/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ pxr_test_scripts(
testenv/testUsdUtilsConstantsGroup.py
testenv/testUsdUtilsCreateNewUsdzPackage.py
testenv/testUsdUtilsDependencies.py
testenv/testUsdUtilsDependenciesCustomResolver.py
testenv/testUsdUtilsDependencyExtractor.py
testenv/testUsdUtilsFlattenLayerStack.py
testenv/testUsdUtilsIntrospection.py
Expand All @@ -88,6 +89,18 @@ pxr_test_scripts(
testenv/testUsdUtilsVarSelsSessionLayer.py
)

pxr_build_test_shared_lib(TestUsdUtilsDependenciesCustomResolver
CREATE_FRAMEWORK
INSTALL_PREFIX UsdUtilsPlugins
LIBRARIES
ar
sdf
tf

CPPFILES
testenv/TestUsdUtilsDependenciesCustomResolver_plugin.cpp
)

pxr_build_test(testUsdUtilsCoalescingDiagnosticDelegateCpp
LIBRARIES
tf
Expand Down Expand Up @@ -153,6 +166,11 @@ pxr_install_test_dir(
DEST testUsdUtilsDependencies
)

pxr_install_test_dir(
SRC testenv/testUsdUtilsDependenciesCustomResolver
DEST testUsdUtilsDependenciesCustomResolver
)

pxr_install_test_dir(
SRC testenv/testUsdUtilsDependencyExtractor
DEST testUsdUtilsDependencyExtractor1
Expand Down Expand Up @@ -429,6 +447,12 @@ pxr_register_test(testUsdUtilsDependencies
EXPECTED_RETURN_CODE 0
)

pxr_register_test(testUsdUtilsDependenciesCustomResolver
PYTHON
COMMAND "${CMAKE_INSTALL_PREFIX}/tests/testUsdUtilsDependenciesCustomResolver"
EXPECTED_RETURN_CODE 0
)

pxr_register_test(testUsdUtilsDependencyExtractor1
PYTHON
COMMAND "${CMAKE_INSTALL_PREFIX}/tests/testUsdUtilsDependencyExtractor ascii.usda ascii-usda.txt"
Expand Down
30 changes: 19 additions & 11 deletions pxr/usd/usdUtils/dependencies.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,21 +98,24 @@ class _FileAnalyzer {
const SdfLayerRefPtr &layer,
const _DepType &depType)>;

// Opens the file at \p resolvedFilePath and analyzes its external
// dependencies.
// Attempts to open the file at \p referencePath and analyzes its external
// dependencies. Opening the layer using this non-resolved path ensures
// that layer metadata is correctly set. If the file cannot be opened then
// the analyzer simply retains the \p resolvedPath for later use.
//
// For each dependency that is detected, the provided (optional) callback
// functions are invoked.
//
// \p processPathFunc is invoked first with the raw (un-remapped) path.
// Then \p remapPathFunc is invoked.
_FileAnalyzer(const std::string &resolvedFilePath,
_FileAnalyzer(const std::string &referencePath,
const std::string &resolvedPath,
_ReferenceTypesToInclude refTypesToInclude=
_ReferenceTypesToInclude::All,
bool enableMetadataFiltering = false,
const RemapAssetPathFunc &remapPathFunc={},
const ProcessAssetPathFunc &processPathFunc={}) :
_filePath(resolvedFilePath),
_filePath(resolvedPath),
_refTypesToInclude(refTypesToInclude),
_metadataFilteringEnabled(enableMetadataFiltering),
_remapPathFunc(remapPathFunc),
Expand All @@ -121,15 +124,16 @@ class _FileAnalyzer {
// If this file can be opened on a USD stage or referenced into a USD
// stage via composition, then analyze the file, collect & update all
// references. If not, return early.
if (!UsdStage::IsSupportedFile(_filePath)) {
if (!UsdStage::IsSupportedFile(referencePath)) {
return;
}

TRACE_FUNCTION();

_layer = SdfLayer::FindOrOpen(_filePath);
_layer = SdfLayer::FindOrOpen(referencePath);
if (!_layer) {
TF_WARN("Unable to open layer at path @%s@.", _filePath.c_str());
TF_WARN("Unable to open layer at path @%s@.",
referencePath.c_str());
return;
}

Expand Down Expand Up @@ -709,7 +713,8 @@ class _AssetLocalizer {

auto &resolver = ArGetResolver();

std::string rootFilePath = resolver.Resolve(assetPath.GetAssetPath());
const std::string assetPathStr = assetPath.GetAssetPath();
std::string rootFilePath = resolver.Resolve(assetPathStr);

// Ensure that the resolved path is not empty.
if (rootFilePath.empty()) {
Expand Down Expand Up @@ -748,7 +753,8 @@ class _AssetLocalizer {
seenFiles.insert(rootFilePath);
std::string destFilePath = TfStringCatPaths(destDir,
TfGetBaseName(rootFilePath));
filesToLocalize.emplace(destFilePath, _FileAnalyzer(rootFilePath,
filesToLocalize.emplace(destFilePath, _FileAnalyzer(
assetPathStr, rootFilePath,
/*refTypesToInclude*/ _ReferenceTypesToInclude::All,
/*enableMetadataFiltering*/ enableMetadataFiltering,
remapAssetPathFunc, processPathFunc));
Expand Down Expand Up @@ -855,7 +861,7 @@ class _AssetLocalizer {
destDirForRef, remappedRef);

filesToLocalize.emplace(destFilePathForRef, _FileAnalyzer(
resolvedRefFilePath,
refAssetPath, resolvedRefFilePath,
/* refTypesToInclude */ _ReferenceTypesToInclude::All,
/*enableMetadataFiltering*/ enableMetadataFiltering,
remapAssetPathFunc, processPathFunc));
Expand Down Expand Up @@ -1121,9 +1127,11 @@ _ExtractExternalReferences(
std::vector<std::string>* references,
std::vector<std::string>* payloads)
{
auto &resolver = ArGetResolver();

// We only care about knowing what the dependencies are. Hence, set
// remapPathFunc to empty.
_FileAnalyzer(filePath, refTypesToInclude,
_FileAnalyzer(filePath, resolver.Resolve(filePath), refTypesToInclude,
/*enableMetadataFiltering*/ false,
/*remapPathFunc*/ {},
[&subLayers, &references, &payloads](
Expand Down
2 changes: 1 addition & 1 deletion pxr/usd/usdUtils/dependencies.h
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ UsdUtilsCreateNewARKitUsdzPackage(
/// Any unresolved (layer and non-layer) asset paths are populated in
/// \p unresolvedPaths.
///
/// The input vectors to be populated with the results are are *cleared* before
/// The input vectors to be populated with the results are *cleared* before
/// any results are added to them.
///
/// Returns true if the given asset was resolved correctly.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"Plugins": [
{
"Type": "library",
"Name": "TestUsdUtilsDependenciesCustomResolver",
"Root": "@TEST_PLUG_INFO_ROOT@",
"LibraryPath": "@TEST_PLUG_INFO_LIBRARY_PATH@",
"ResourcePath": "@TEST_PLUG_INFO_RESOURCE_PATH@",
"Info": {
"Types": {
"CustomResolver": {
"bases": ["ArResolver"],
"uriSchemes": ["test", "testresolved"]
}
}
}
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
//
// Copyright 2020 Pixar
//
// Licensed under the Apache License, Version 2.0 (the "Apache License")
// with the following modification; you may not use this file except in
// compliance with the Apache License and the following modification to it:
// Section 6. Trademarks. is deleted and replaced with:
//
// 6. Trademarks. This License does not grant permission to use the trade
// names, trademarks, service marks, or product names of the Licensor
// and its affiliates, except as required to comply with Section 4(c) of
// the License and to reproduce the content of the NOTICE file.
//
// You may obtain a copy of the Apache License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the Apache License with the above modification is
// distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the Apache License for the specific
// language governing permissions and limitations under the Apache License.
//
#include "pxr/pxr.h"

#include "pxr/usd/ar/defaultResolver.h"
#include "pxr/usd/ar/defineResolver.h"
#include "pxr/usd/ar/filesystemAsset.h"

#include <string>

PXR_NAMESPACE_USING_DIRECTIVE

/// This bare bones resolver is specifically setup to use different URI schemes
/// for identifier creation and asset resolution.
/// Identifiers are in form of test:path
/// Resolved paths are in form testresolved:path
class CustomResolver
: public ArResolver
{
public:
CustomResolver()
{
}

protected:
std::string _CreateIdentifier(
const std::string& assetPath,
const ArResolvedPath& anchorAssetPath) const final
{
return assetPath;
}

std::string _CreateIdentifierForNewAsset(
const std::string& assetPath,
const ArResolvedPath& anchorAssetPath) const final
{
return assetPath;
}

ArResolvedPath _Resolve(
const std::string& assetPath) const final
{
const std::string resolved = "testresolved:" +
assetPath.substr(assetPath.find_first_of(":") + 1);

return ArResolvedPath(resolved);
}

ArResolvedPath _ResolveForNewAsset(
const std::string& assetPath) const final
{
return _Resolve(assetPath);
}

std::shared_ptr<ArAsset> _OpenAsset(
const ArResolvedPath& resolvedPath) const final
{
const std::string pathStr = resolvedPath.GetPathString();
const std::string filesystemPath =
pathStr.substr(pathStr.find_first_of(":") + 1);

return ArFilesystemAsset::Open(ArResolvedPath(filesystemPath));
}

std::shared_ptr<ArWritableAsset>
_OpenAssetForWrite(
const ArResolvedPath& resolvedPath,
WriteMode writeMode) const final
{
return nullptr;
}

};

AR_DEFINE_RESOLVER(CustomResolver, ArResolver);
81 changes: 81 additions & 0 deletions pxr/usd/usdUtils/testenv/testUsdUtilsDependenciesCustomResolver.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
#!/pxrpythonsubst
#
# Copyright 2023 Pixar
#
# Licensed under the Apache License, Version 2.0 (the "Apache License")
# with the following modification; you may not use this file except in
# compliance with the Apache License and the following modification to it:
# Section 6. Trademarks. is deleted and replaced with:
#
# 6. Trademarks. This License does not grant permission to use the trade
# names, trademarks, service marks, or product names of the Licensor
# and its affiliates, except as required to comply with Section 4(c) of
# the License and to reproduce the content of the NOTICE file.
#
# You may obtain a copy of the Apache License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the Apache License with the above modification is
# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the Apache License for the specific
# language governing permissions and limitations under the Apache License.
from pxr import UsdUtils, Plug, Sdf

import os, unittest


class TestUsdUtilsDependenciesCustomResolver(unittest.TestCase):
@classmethod
def setUpClass(cls):
# Register test resolver plugins
# Test plugins are installed relative to this script
testRoot = os.path.join(
os.path.dirname(os.path.abspath(__file__)), 'UsdUtilsPlugins')

pr = Plug.Registry()

testURIResolverPath = os.path.join(
testRoot, 'lib/TestUsdUtilsDependenciesCustomResolver*/Resources/')
pr.RegisterPlugins(testURIResolverPath)

def test_ComputeAllDependencies(self):
"""Tests that ComputeAllDependencies correctly sets identifier and """
"""resolved paths when using a resolver with multiple uri schemes """

mainIdentifier = "test:main.usda"
expectedMainResolvedPath = "testresolved:main.usda"
dependencyIdentifier = "test:dependency.usda"
expectedDependencyResolvedPath = "testresolved:dependency.usda"

layers, _, _ = UsdUtils.ComputeAllDependencies(mainIdentifier)

self.assertEqual(len(layers), 2)
layer0 = layers[0]
self.assertEqual(layer0.identifier, mainIdentifier)
self.assertEqual(layer0.resolvedPath, expectedMainResolvedPath)

layer1 = layers[1]
self.assertEqual(layer1.identifier, dependencyIdentifier)
self.assertEqual(layer1.resolvedPath, expectedDependencyResolvedPath)

def test_findOrOpenLayer(self):
mainIdentifier = "test:main.usda"
expectedMainResolvedPath = "testresolved:main.usda"
dependencyIdentifier = "test:dependency.usda"
expectedDependencyResolvedPath = "testresolved:dependency.usda"

UsdUtils.ComputeAllDependencies(mainIdentifier)

layer = Sdf.Layer.FindOrOpen(mainIdentifier)
self.assertEqual(layer.identifier, mainIdentifier)
self.assertEqual(layer.resolvedPath, expectedMainResolvedPath)

layer = Sdf.Layer.FindOrOpen(dependencyIdentifier)
self.assertEqual(layer.identifier, dependencyIdentifier)
self.assertEqual(layer.resolvedPath, expectedDependencyResolvedPath)


if __name__=="__main__":
unittest.main()
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#usda 1.0
(
upAxis = "Y"
metersPerUnit = 0.01
defaultPrim = "dependency"
)

def XForm "dependency"
{

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#usda 1.0
(
upAxis = "Y"
metersPerUnit = 0.01
defaultPrim = "World"
subLayers = [
@test:dependency.usda@
]
)

def XForm "main"
{

}

0 comments on commit e670db4

Please sign in to comment.