Skip to content

Commit

Permalink
This changes addresses a behavorial difference that appeared in 23.11…
Browse files Browse the repository at this point in the history
… relating to UsdUtils.ExtractExternalReferences. In previous versions, udim paths would not be resolved during this function call. This change restores that behavior by disabling udim resolution for ExtractExternalReferences.

Fixes #3173

(Internal change: 2344689)
  • Loading branch information
matthewcpp authored and pixar-oss committed Oct 25, 2024
1 parent 329bda1 commit 7596547
Show file tree
Hide file tree
Showing 13 changed files with 110 additions and 13 deletions.
24 changes: 24 additions & 0 deletions pxr/usd/usdUtils/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,16 @@ pxr_install_test_dir(
DEST testUsdUtilsDependencyExtractor5
)

pxr_install_test_dir(
SRC testenv/testUsdUtilsDependencyExtractor
DEST testUsdUtilsDependencyExtractor6
)

pxr_install_test_dir(
SRC testenv/testUsdUtilsDependencyExtractor
DEST testUsdUtilsDependencyExtractor7
)

pxr_install_test_dir(
SRC testenv/testUsdUtilsFlattenLayerStack
DEST testUsdUtilsFlattenLayerStack
Expand Down Expand Up @@ -590,6 +600,20 @@ pxr_register_test(testUsdUtilsDependencyExtractor5
EXPECTED_RETURN_CODE 0
)

pxr_register_test(testUsdUtilsDependencyExtractor6
PYTHON
COMMAND "${CMAKE_INSTALL_PREFIX}/tests/testUsdUtilsDependencyExtractor udims/root.usda udims.txt"
DIFF_COMPARE udims.txt
EXPECTED_RETURN_CODE 0
)

pxr_register_test(testUsdUtilsDependencyExtractor7
PYTHON
COMMAND "${CMAKE_INSTALL_PREFIX}/tests/testUsdUtilsDependencyExtractor --resolve-udim-paths udims/root.usda udims-resolved.txt"
DIFF_COMPARE udims-resolved.txt
EXPECTED_RETURN_CODE 0
)

pxr_register_test(testUsdUtilsFlattenLayerStack
PYTHON
COMMAND "${CMAKE_INSTALL_PREFIX}/tests/testUsdUtilsFlattenLayerStack"
Expand Down
6 changes: 4 additions & 2 deletions pxr/usd/usdUtils/assetLocalization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ UsdUtils_LocalizationContext::_GetUdimTiles(
{
std::vector<std::string> additionalPaths;

if (!UsdShadeUdimUtils::IsUdimIdentifier(assetPath)) {
if (!_resolveUdimPaths || !UsdShadeUdimUtils::IsUdimIdentifier(assetPath)) {
return additionalPaths;
}

Expand Down Expand Up @@ -631,7 +631,8 @@ void UsdUtils_ExtractExternalReferences(
const UsdUtils_LocalizationContext::ReferenceType refTypesToInclude,
std::vector<std::string>* outSublayers,
std::vector<std::string>* outReferences,
std::vector<std::string>* outPayloads)
std::vector<std::string>* outPayloads,
const UsdUtilsExtractExternalReferencesParams& params)
{
TRACE_FUNCTION();

Expand All @@ -643,6 +644,7 @@ void UsdUtils_ExtractExternalReferences(
UsdUtils_LocalizationContext context(&delegate);
context.SetRefTypesToInclude(refTypesToInclude);
context.SetRecurseLayerDependencies(false);
context.SetResolveUdimPaths(params.GetResolveUdimPaths());

context.Process(SdfLayer::FindOrOpen(filePath));
client.SortAndRemoveDuplicates();
Expand Down
17 changes: 14 additions & 3 deletions pxr/usd/usdUtils/assetLocalization.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,13 @@ class UsdUtils_LocalizationContext {
dependenciesToSkip.end());
}

// Controls udim path resolution.
// If this value is set to false, asset paths containing udim patterns
// be re returned untouched.
inline void SetResolveUdimPaths(bool resolveUdimPaths) {
_resolveUdimPaths = resolveUdimPaths;
}

private:
void _ProcessLayer(const SdfLayerRefPtr& layer);
void _ProcessSublayers(const SdfLayerRefPtr& layer);
Expand All @@ -111,11 +118,11 @@ class UsdUtils_LocalizationContext {
bool processingMetadata = false);

// Searches for udim tiles associated with the given asset path.
static std::vector<std::string> _GetUdimTiles(const SdfLayerRefPtr& layer,
std::vector<std::string> _GetUdimTiles(const SdfLayerRefPtr& layer,
const std::string &assetPath);

// Discovers all dependencies for the supplied asset path
static std::vector<std::string> _GetDependencies(const SdfLayerRefPtr& layer,
std::vector<std::string> _GetDependencies(const SdfLayerRefPtr& layer,
const std::string &assetPath);

// Searches for the clips of a given templated string
Expand Down Expand Up @@ -158,6 +165,9 @@ class UsdUtils_LocalizationContext {
// Specifies if metadata filtering should be enabled
bool _metadataFilteringEnabled = false;

// Specifies if udim paths should be resolved during processing.
bool _resolveUdimPaths = true;

// user supplied list of dependencies that will be skipped when
// processing the asset
std::unordered_set<std::string> _dependenciesToSkip;
Expand All @@ -168,7 +178,8 @@ void UsdUtils_ExtractExternalReferences(
const UsdUtils_LocalizationContext::ReferenceType refTypesToInclude,
std::vector<std::string>* subLayers,
std::vector<std::string>* references,
std::vector<std::string>* payloads);
std::vector<std::string>* payloads,
const UsdUtilsExtractExternalReferencesParams& params = {});

PXR_NAMESPACE_CLOSE_SCOPE

Expand Down
5 changes: 3 additions & 2 deletions pxr/usd/usdUtils/dependencies.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,12 @@ UsdUtilsExtractExternalReferences(
const std::string& filePath,
std::vector<std::string>* subLayers,
std::vector<std::string>* references,
std::vector<std::string>* payloads)
std::vector<std::string>* payloads,
const UsdUtilsExtractExternalReferencesParams& params)
{
UsdUtils_ExtractExternalReferences(filePath,
UsdUtils_LocalizationContext::ReferenceType::All,
subLayers, references, payloads);
subLayers, references, payloads, params);
}

struct UsdUtils_ComputeAllDependenciesClient
Expand Down
26 changes: 25 additions & 1 deletion pxr/usd/usdUtils/dependencies.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,34 @@

PXR_NAMESPACE_OPEN_SCOPE

/// Structure which controls aspects of the
/// \ref UsdUtilsExtractExternalReferences function.
class UsdUtilsExtractExternalReferencesParams {
public:
/// Specifies whether UDIM template paths should be resolved when extracting
/// references. If true, the resolved paths for all discovered UDIM tiles
/// will be included in the references bucket and the template path will be
/// discarded. If false, no resolution will take place and the template path
/// will appear in the references bucket.
inline void SetResolveUdimPaths(bool resolveUdimPaths) {
_resolveUdimPaths = resolveUdimPaths;
}


inline bool GetResolveUdimPaths() const { return _resolveUdimPaths; }

private:
bool _resolveUdimPaths = false;
};

/// Parses the file at \p filePath, identifying external references, and
/// sorting them into separate type-based buckets. Sublayers are returned in
/// the \p sublayers vector, references, whether prim references, value clip
/// references or values from asset path attributes, are returned in the
/// \p references vector. Payload paths are returned in \p payloads.
/// The \p params parameter controls various settings that affect the
/// extraction process. See \ref UsdUtilsExtractExternalReferencesParams for
/// additional details.
///
/// \note No recursive chasing of dependencies is performed; that is the
/// client's responsibility, if desired.
Expand All @@ -45,7 +68,8 @@ void UsdUtilsExtractExternalReferences(
const std::string& filePath,
std::vector<std::string>* subLayers,
std::vector<std::string>* references,
std::vector<std::string>* payloads);
std::vector<std::string>* payloads,
const UsdUtilsExtractExternalReferencesParams& params = {});

/// Recursively computes all the dependencies of the given asset and populates
/// \p layers with all the dependencies that can be opened as an SdfLayer.
Expand Down
7 changes: 6 additions & 1 deletion pxr/usd/usdUtils/testenv/testUsdUtilsDependencyExtractor.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ def presult(ostr, fileName, refType, refs):
parser.add_argument('infile')
parser.add_argument('outfile', default='-')
parser.add_argument('--open-as-anon', dest='openAsAnon', action='store_true')
parser.add_argument('--resolve-udim-paths', dest="resolveUdimPaths", action='store_true')
args = parser.parse_args()

if not os.path.exists(args.infile):
Expand All @@ -46,8 +47,12 @@ def presult(ostr, fileName, refType, refs):
layer.SetPermissionToEdit(False)
identifier = args.infile

extractionParams = UsdUtils.ExtractExternalReferencesParams()
if (args.resolveUdimPaths):
extractionParams.SetResolveUdimPaths(True)

sublayers, references, payloads = \
UsdUtils.ExtractExternalReferences(identifier)
UsdUtils.ExtractExternalReferences(identifier, extractionParams)

with stream(args.outfile, 'w') as ofp:
presult(ofp, args.infile, 'sublayers', sublayers)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
udims/root.usda no sublayers
udims/root.usda references[001]: ./test.1001.txt
udims/root.usda references[002]: ./test.1002.txt
udims/root.usda no payloads
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
udims/root.usda no sublayers
udims/root.usda references[001]: ./test.<UDIM>.txt
udims/root.usda no payloads
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#usda 1.0

def "Test" {
asset udims = @./test.<UDIM>.txt@
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1001
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1002
10 changes: 9 additions & 1 deletion pxr/usd/usdUtils/usdzPackage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,17 @@ UsdUtilsCreateNewARKitUsdzPackage(
// the composition of the stage.
std::vector<std::string> sublayers, references, payloads;

// We are explicitly setting the UDIM path resolution option to false
// here because the following logic only cares if the root layer contains
// any external references and does reason about the contents of the
// results. UDIM path resolution has the potential to be expensive, for
// example in the case of network filesystem paths.
UsdUtilsExtractExternalReferencesParams params;
params.SetResolveUdimPaths(false);

UsdUtils_ExtractExternalReferences(resolvedPath,
UsdUtils_LocalizationContext::ReferenceType::CompositionOnly,
&sublayers, &references, &payloads);
&sublayers, &references, &payloads, params);

// Ensure that the root layer has the ".usdc" extension.
std::string targetBaseName = firstLayerName.empty() ?
Expand Down
14 changes: 11 additions & 3 deletions pxr/usd/usdUtils/wrapDependencies.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,12 @@ namespace {

static bp::tuple
_ExtractExternalReferences(
const std::string& filePath)
const std::string& filePath,
const UsdUtilsExtractExternalReferencesParams& params = {})
{
std::vector<std::string> subLayers, references, payloads;
UsdUtilsExtractExternalReferences(filePath,
&subLayers, &references, &payloads);
&subLayers, &references, &payloads, params);
return bp::make_tuple(subLayers, references, payloads);
}

Expand Down Expand Up @@ -65,8 +66,15 @@ _ComputeAllDependencies(

void wrapDependencies()
{
typedef UsdUtilsExtractExternalReferencesParams ExtractRefParams;
bp::class_<ExtractRefParams>("ExtractExternalReferencesParams")
.def("SetResolveUdimPaths", &ExtractRefParams::SetResolveUdimPaths)
.def("GetResolveUdimPaths", &ExtractRefParams::GetResolveUdimPaths)
;

bp::def("ExtractExternalReferences", _ExtractExternalReferences,
bp::arg("filePath"));
(bp::arg("filePath"),
bp::arg("parameters") = ExtractRefParams()));

bp::def("CreateNewUsdzPackage", UsdUtilsCreateNewUsdzPackage,
(bp::arg("assetPath"),
Expand Down

0 comments on commit 7596547

Please sign in to comment.