From 10f2792f5abda405f5f79a2750a70dc53d0ba071 Mon Sep 17 00:00:00 2001 From: tallytalwar Date: Fri, 9 Sep 2022 13:33:01 -0700 Subject: [PATCH] Fix perf regression caused by change 2248004 - Basically when usdImaging is getting the material path to track by calling UsdShadeMaterialBindingAPI:ComputeBoundMaterial, 2248004 was always using the winning binding to get the target path, irrespective of whether the bound material exists or not. But we can do better and simply return the path of the bound material if it exists, instead of always getting the winning relationship targets (which is useful when the bound target doesn't exist on the loaded stage). (Internal change: 2248431) --- .../usdImaging/resolvedAttributeCache.h | 43 +++++++++++-------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/pxr/usdImaging/usdImaging/resolvedAttributeCache.h b/pxr/usdImaging/usdImaging/resolvedAttributeCache.h index 12cd2b2f66..3ed90a5805 100644 --- a/pxr/usdImaging/usdImaging/resolvedAttributeCache.h +++ b/pxr/usdImaging/usdImaging/resolvedAttributeCache.h @@ -704,8 +704,12 @@ struct UsdImaging_MaterialStrategy { // inherited path to bound target // depending on the load state, override, etc bound target path might not be // queried as a UsdShadeMaterial on the stage. - typedef SdfPath value_type; // inherited path to bound target - typedef UsdRelationship query_type; // Hold the winning relationship binding + + // inherited path to bound target + typedef SdfPath value_type; + // Hold the computed path of the bound material or target path of the + // winning material binding relationship + typedef SdfPath query_type; using ImplData = UsdImaging_MaterialBindingImplData; @@ -720,12 +724,21 @@ struct UsdImaging_MaterialStrategy { ImplData *implData) { UsdRelationship bindingRel; - UsdShadeMaterialBindingAPI(prim).ComputeBoundMaterial( - &implData->GetBindingsCache(), - &implData->GetCollectionQueryCache(), - implData->GetMaterialPurpose(), - &bindingRel); - return bindingRel; + UsdShadeMaterial materialPrim = + UsdShadeMaterialBindingAPI(prim).ComputeBoundMaterial( + &implData->GetBindingsCache(), + &implData->GetCollectionQueryCache(), + implData->GetMaterialPurpose(), + &bindingRel); + + if (materialPrim) { + return materialPrim.GetPath(); + } + + const SdfPath targetPath = + UsdShadeMaterialBindingAPI::GetResolvedTargetPathFromBindingRel( + bindingRel); + return targetPath; } static @@ -736,19 +749,15 @@ struct UsdImaging_MaterialStrategy { { TF_DEBUG(USDIMAGING_SHADERS).Msg("Looking for \"preview\" material " "binding for %s\n", prim.GetPath().GetText()); - if (*query) { - const SdfPath targetPath = - UsdShadeMaterialBindingAPI::GetResolvedTargetPathFromBindingRel( - *query); - if (!targetPath.IsEmpty()) { - return targetPath; - } - } + // query already contains the resolved material binding for the prim. // Hence, we don't need to inherit the binding from the parent here. // Futhermore, it may be wrong to inherit the binding from the parent, // because in the new scheme, a child of a bound prim can be unbound. - return value_type(); + // + // Note that query could be an empty SdfPath, which is the default + // value. + return *query; } static