From 2702f270fb693e983ac09c534ceec70b77650e09 Mon Sep 17 00:00:00 2001 From: Pierre Baillargeon Date: Fri, 30 Jun 2023 14:43:20 -0400 Subject: [PATCH 1/3] EMSUSD 243 load reference and payload without default prim - Refactor add reference and add payload commands into a common base class. - (The code were almost similar and now have even more in common.) - Add validation of the ref or payload targeted prim to catch common errors. - For example, the targeted prim must have the same type as the referencing prim. - For example: an xform inside an xform, a mesh inside a mesh. - Target the first valid root prim when no explicit prim were given and there is no default prim. - Add warnings for common errors to help the user know what went wrong. - Add a unit test for files without default prim. --- lib/usdUfe/ufe/CMakeLists.txt | 2 + lib/usdUfe/ufe/UsdUndoAddPayloadCommand.cpp | 27 +--- lib/usdUfe/ufe/UsdUndoAddPayloadCommand.h | 25 +--- .../ufe/UsdUndoAddRefOrPayloadCommand.cpp | 141 ++++++++++++++++++ .../ufe/UsdUndoAddRefOrPayloadCommand.h | 57 +++++++ lib/usdUfe/ufe/UsdUndoAddReferenceCommand.cpp | 27 +--- lib/usdUfe/ufe/UsdUndoAddReferenceCommand.h | 25 +--- test/lib/ufe/testReferenceCommands.py | 28 ++++ .../twoSpheres/sphere_no_default.usda | 19 +++ 9 files changed, 275 insertions(+), 76 deletions(-) create mode 100644 lib/usdUfe/ufe/UsdUndoAddRefOrPayloadCommand.cpp create mode 100644 lib/usdUfe/ufe/UsdUndoAddRefOrPayloadCommand.h create mode 100644 test/testSamples/twoSpheres/sphere_no_default.usda diff --git a/lib/usdUfe/ufe/CMakeLists.txt b/lib/usdUfe/ufe/CMakeLists.txt index 4633c6cf1d..c72363a481 100644 --- a/lib/usdUfe/ufe/CMakeLists.txt +++ b/lib/usdUfe/ufe/CMakeLists.txt @@ -23,6 +23,7 @@ if(CMAKE_UFE_V2_FEATURES_AVAILABLE) UsdUndoAddNewPrimCommand.cpp UsdUndoAddPayloadCommand.cpp UsdUndoAddReferenceCommand.cpp + UsdUndoAddRefOrPayloadCommand.cpp UsdUndoClearPayloadsCommand.cpp UsdUndoClearReferencesCommand.cpp UsdUndoCreateGroupCommand.cpp @@ -65,6 +66,7 @@ if(CMAKE_UFE_V2_FEATURES_AVAILABLE) UsdUndoAddNewPrimCommand.h UsdUndoAddPayloadCommand.h UsdUndoAddReferenceCommand.h + UsdUndoAddRefOrPayloadCommand.h UsdUndoClearPayloadsCommand.h UsdUndoClearReferencesCommand.h UsdUndoCreateGroupCommand.h diff --git a/lib/usdUfe/ufe/UsdUndoAddPayloadCommand.cpp b/lib/usdUfe/ufe/UsdUndoAddPayloadCommand.cpp index 22e002c266..c2abca6dba 100644 --- a/lib/usdUfe/ufe/UsdUndoAddPayloadCommand.cpp +++ b/lib/usdUfe/ufe/UsdUndoAddPayloadCommand.cpp @@ -16,36 +16,23 @@ #include "UsdUndoAddPayloadCommand.h" -#include -#include - namespace USDUFE_NS_DEF { UsdUndoAddPayloadCommand::UsdUndoAddPayloadCommand( const PXR_NS::UsdPrim& prim, const std::string& filePath, bool prepend) - : _prim(prim) - , _sdfPayload() - , _filePath(filePath) - , _listPos( - prepend ? PXR_NS::UsdListPositionBackOfPrependList - : PXR_NS::UsdListPositionBackOfAppendList) + : UsdUndoAddPayloadCommand(prim, filePath, {}, prepend) { } -void UsdUndoAddPayloadCommand::executeImplementation() +UsdUndoAddPayloadCommand::UsdUndoAddPayloadCommand( + const PXR_NS::UsdPrim& prim, + const std::string& filePath, + const std::string& primPath, + bool prepend) + : UsdUndoAddRefOrPayloadCommand(prim, filePath, primPath, getListPosition(prepend), true) { - if (!_prim.IsValid()) - return; - - if (PXR_NS::TfStringEndsWith(_filePath, ".mtlx")) { - _sdfPayload = PXR_NS::SdfPayload(_filePath, PXR_NS::SdfPath("/MaterialX")); - } else { - _sdfPayload = PXR_NS::SdfPayload(_filePath); - } - PXR_NS::UsdPayloads primPayloads = _prim.GetPayloads(); - primPayloads.AddPayload(_sdfPayload, _listPos); } } // namespace USDUFE_NS_DEF diff --git a/lib/usdUfe/ufe/UsdUndoAddPayloadCommand.h b/lib/usdUfe/ufe/UsdUndoAddPayloadCommand.h index ea132914b6..8eb2b0ce82 100644 --- a/lib/usdUfe/ufe/UsdUndoAddPayloadCommand.h +++ b/lib/usdUfe/ufe/UsdUndoAddPayloadCommand.h @@ -16,20 +16,12 @@ #ifndef USD_UFE_ADD_PAYLOAD_COMMAND #define USD_UFE_ADD_PAYLOAD_COMMAND -#include -#include - -#include -#include - -#include - -#include +#include namespace USDUFE_NS_DEF { //! \brief Command to add a payload to a prim. -class USDUFE_PUBLIC UsdUndoAddPayloadCommand : public UsdUndoableCommand +class USDUFE_PUBLIC UsdUndoAddPayloadCommand : public UsdUndoAddRefOrPayloadCommand { public: UsdUndoAddPayloadCommand( @@ -37,14 +29,11 @@ class USDUFE_PUBLIC UsdUndoAddPayloadCommand : public UsdUndoableCommand +#include +#include +#include +#include + +namespace USDUFE_NS_DEF { + +using namespace PXR_NS; + +static std::string validatePrimSpec(const UsdPrim& prim, const SdfPrimSpecHandle& primSpec) +{ + if (!primSpec) + return "is not valid."; + + // A common error is to reference a prim that is not the same type as the prim + // that contains the reference. Since only the type of the prim that contains + // the reference is used, the referenced prim might not show up. + // + // This happens a lot when trying to reference geometry (mesh) instead of the + // prim containing the geometry. Of vis-versa, referencing a prim inside a mesh. + const std::string& primType = prim.GetTypeName(); + const std::string& targetType = primSpec->GetTypeName(); + if (primType != targetType) + return TfStringPrintf( + "does not have the same type as the targeted prim: [%s] vs [%s].", + primType.c_str(), + targetType.c_str()); + + return ""; +} + +static PXR_NS::SdfPath +getPrimPath(const UsdPrim& prim, const std::string& filePath, const std::string& primPath) +{ + // When an explicit prim path was given, use that. + if (!primPath.empty()) + return SdfPath(primPath); + + // If no prim path were specified and we are referencing a MaterialX file + // then use the MaterialX prim as the target for the reference. + // + // TODO: should we force this even when the referenced file has a default prim? + if (TfStringEndsWith(filePath, ".mtlx")) + return SdfPath("/MaterialX"); + + // Retrieve the layer for analysis. + // + // Note: we don't print any warning if the layer cannot be found as we assume + // the load itself will also fail and print an error. + SdfLayerRefPtr layerRef = SdfLayer::FindOrOpen(filePath); + if (!layerRef) + return SdfPath(); + + // If the referenced file has a default prim, leave the prim path empty. + if (layerRef->HasDefaultPrim()) { + TfToken primName = layerRef->GetDefaultPrim(); + SdfPrimSpecHandle primSpec = layerRef->GetPrimAtPath(SdfPath(primName.GetText())); + const std::string errorMessage = validatePrimSpec(prim, primSpec); + if (!errorMessage.empty()) + TF_WARN( + (std::string("The default prim in file [%s] ") + errorMessage).c_str(), + filePath.c_str()); + return SdfPath(); + } + + // If the referenced file has no default prim, return the path to the first + // valid root prim we find. + std::string errorMessage = " is absent."; + for (const SdfPrimSpecHandle primSpec : layerRef->GetRootPrims()) { + if (!primSpec) + continue; + + errorMessage = validatePrimSpec(prim, primSpec); + if (errorMessage.empty()) + return primSpec->GetPath(); + } + + TF_WARN( + (std::string("The file [%s] does not contain a default prim and the root prim ") + + errorMessage) + .c_str(), + filePath.c_str()); + + return SdfPath(); +} + +/* static */ UsdListPosition UsdUndoAddRefOrPayloadCommand::getListPosition(bool prepend) +{ + return prepend ? UsdListPositionBackOfPrependList : UsdListPositionBackOfAppendList; +} + +UsdUndoAddRefOrPayloadCommand::UsdUndoAddRefOrPayloadCommand( + const UsdPrim& prim, + const std::string& filePath, + const std::string& primPath, + UsdListPosition listPos, + bool isPayload) + : _prim(prim) + , _filePath(filePath) + , _primPath(primPath) + , _listPos(listPos) + , _isPayload(isPayload) +{ +} + +void UsdUndoAddRefOrPayloadCommand::executeImplementation() +{ + if (!_prim.IsValid()) + return; + + SdfPath primPath = getPrimPath(_prim, _filePath, _primPath); + if (_isPayload) { + SdfPayload payload(_filePath, primPath); + UsdPayloads primPayloads = _prim.GetPayloads(); + primPayloads.AddPayload(payload, _listPos); + } else { + SdfReference ref(_filePath, primPath); + UsdReferences primRefs = _prim.GetReferences(); + primRefs.AddReference(ref, _listPos); + } +} + +} // namespace USDUFE_NS_DEF diff --git a/lib/usdUfe/ufe/UsdUndoAddRefOrPayloadCommand.h b/lib/usdUfe/ufe/UsdUndoAddRefOrPayloadCommand.h new file mode 100644 index 0000000000..56de801151 --- /dev/null +++ b/lib/usdUfe/ufe/UsdUndoAddRefOrPayloadCommand.h @@ -0,0 +1,57 @@ +// +// Copyright 2023 Autodesk +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +#ifndef USD_UFE_ADD_REF_OR_PAYLOAD_COMMAND +#define USD_UFE_ADD_REF_OR_PAYLOAD_COMMAND + +#include +#include + +#include +#include + +#include + +#include + +namespace USDUFE_NS_DEF { + +//! \brief Command to add a reference or payload to a prim. +class USDUFE_PUBLIC UsdUndoAddRefOrPayloadCommand : public UsdUndoableCommand +{ +public: + UsdUndoAddRefOrPayloadCommand( + const PXR_NS::UsdPrim& prim, + const std::string& filePath, + const std::string& primPath, + PXR_NS::UsdListPosition listPos, + bool isPayload); + +protected: + static PXR_NS::UsdListPosition getListPosition(bool prepend); + + void executeImplementation() override; + +private: + PXR_NS::UsdPrim _prim; + std::string _filePath; + std::string _primPath; + PXR_NS::UsdListPosition _listPos; + bool _isPayload; +}; + +} // namespace USDUFE_NS_DEF + +#endif /* USD_UFE_ADD_REF_OR_PAYLOAD_COMMAND */ diff --git a/lib/usdUfe/ufe/UsdUndoAddReferenceCommand.cpp b/lib/usdUfe/ufe/UsdUndoAddReferenceCommand.cpp index e968cab91b..d06bd6ce70 100644 --- a/lib/usdUfe/ufe/UsdUndoAddReferenceCommand.cpp +++ b/lib/usdUfe/ufe/UsdUndoAddReferenceCommand.cpp @@ -15,36 +15,23 @@ // #include "UsdUndoAddReferenceCommand.h" -#include -#include - namespace USDUFE_NS_DEF { -//! \brief Command to add a reference to a prim. UsdUndoAddReferenceCommand::UsdUndoAddReferenceCommand( const PXR_NS::UsdPrim& prim, const std::string& filePath, bool prepend) - : _prim(prim) - , _sdfRef() - , _filePath(filePath) - , _listPos( - prepend ? PXR_NS::UsdListPositionBackOfPrependList - : PXR_NS::UsdListPositionBackOfAppendList) + : UsdUndoAddReferenceCommand(prim, filePath, {}, prepend) { } -void UsdUndoAddReferenceCommand::executeImplementation() +UsdUndoAddReferenceCommand::UsdUndoAddReferenceCommand( + const PXR_NS::UsdPrim& prim, + const std::string& filePath, + const std::string& primPath, + bool prepend) + : UsdUndoAddRefOrPayloadCommand(prim, filePath, primPath, getListPosition(prepend), false) { - if (!_prim.IsValid()) - return; - - _sdfRef = PXR_NS::TfStringEndsWith(_filePath, ".mtlx") - ? PXR_NS::SdfReference(_filePath, PXR_NS::SdfPath("/MaterialX")) - : PXR_NS::SdfReference(_filePath); - - PXR_NS::UsdReferences primRefs = _prim.GetReferences(); - primRefs.AddReference(_sdfRef, _listPos); } } // namespace USDUFE_NS_DEF diff --git a/lib/usdUfe/ufe/UsdUndoAddReferenceCommand.h b/lib/usdUfe/ufe/UsdUndoAddReferenceCommand.h index 09b67273f0..bb81a77285 100644 --- a/lib/usdUfe/ufe/UsdUndoAddReferenceCommand.h +++ b/lib/usdUfe/ufe/UsdUndoAddReferenceCommand.h @@ -16,20 +16,12 @@ #ifndef USD_UFE_ADD_REFERENCE_COMMAND #define USD_UFE_ADD_REFERENCE_COMMAND -#include -#include - -#include -#include - -#include - -#include +#include namespace USDUFE_NS_DEF { //! \brief Command to add a reference to a prim. -class USDUFE_PUBLIC UsdUndoAddReferenceCommand : public UsdUndoableCommand +class USDUFE_PUBLIC UsdUndoAddReferenceCommand : public UsdUndoAddRefOrPayloadCommand { public: UsdUndoAddReferenceCommand( @@ -37,14 +29,11 @@ class USDUFE_PUBLIC UsdUndoAddReferenceCommand : public UsdUndoableCommand Date: Tue, 4 Jul 2023 15:57:48 -0400 Subject: [PATCH 2/3] EMSUSD-243 split warning when there is no default prim. - First we print a status message that there is no default prim and that we wwill use the first valid prim. - Afterwards, if no valid root prim is found, we print a warning about why we could not find a valid root prim. --- .../ufe/UsdUndoAddRefOrPayloadCommand.cpp | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/lib/usdUfe/ufe/UsdUndoAddRefOrPayloadCommand.cpp b/lib/usdUfe/ufe/UsdUndoAddRefOrPayloadCommand.cpp index e6c626ba3a..8cd67afd14 100644 --- a/lib/usdUfe/ufe/UsdUndoAddRefOrPayloadCommand.cpp +++ b/lib/usdUfe/ufe/UsdUndoAddRefOrPayloadCommand.cpp @@ -28,7 +28,7 @@ using namespace PXR_NS; static std::string validatePrimSpec(const UsdPrim& prim, const SdfPrimSpecHandle& primSpec) { if (!primSpec) - return "is not valid."; + return "is not valid"; // A common error is to reference a prim that is not the same type as the prim // that contains the reference. Since only the type of the prim that contains @@ -40,7 +40,7 @@ static std::string validatePrimSpec(const UsdPrim& prim, const SdfPrimSpecHandle const std::string& targetType = primSpec->GetTypeName(); if (primType != targetType) return TfStringPrintf( - "does not have the same type as the targeted prim: [%s] vs [%s].", + "does not have the same type as the targeted prim: [%s] vs [%s]", primType.c_str(), targetType.c_str()); @@ -75,15 +75,19 @@ getPrimPath(const UsdPrim& prim, const std::string& filePath, const std::string& SdfPrimSpecHandle primSpec = layerRef->GetPrimAtPath(SdfPath(primName.GetText())); const std::string errorMessage = validatePrimSpec(prim, primSpec); if (!errorMessage.empty()) - TF_WARN( - (std::string("The default prim in file [%s] ") + errorMessage).c_str(), - filePath.c_str()); + TF_WARN("The default prim in file [%s] %s.", errorMessage.c_str(), filePath.c_str()); return SdfPath(); } // If the referenced file has no default prim, return the path to the first // valid root prim we find. - std::string errorMessage = " is absent."; + + TF_STATUS( + "The file [%s] does not contain a default prim, the first valid root prim " + "will be used.", + filePath.c_str()); + + std::string errorMessage; for (const SdfPrimSpecHandle primSpec : layerRef->GetRootPrims()) { if (!primSpec) continue; @@ -93,11 +97,11 @@ getPrimPath(const UsdPrim& prim, const std::string& filePath, const std::string& return primSpec->GetPath(); } - TF_WARN( - (std::string("The file [%s] does not contain a default prim and the root prim ") - + errorMessage) - .c_str(), - filePath.c_str()); + if (errorMessage.empty()) { + TF_WARN("Could not find any valid root prim."); + } else { + TF_WARN("Could not find a valid root prim, the root prim %s.", errorMessage.c_str()); + } return SdfPath(); } From e3b0935ca87bec746c84092883418d8edb806503 Mon Sep 17 00:00:00 2001 From: Pierre Baillargeon Date: Tue, 4 Jul 2023 16:20:49 -0400 Subject: [PATCH 3/3] EMSUSD-243 fixing warning arguments order. --- lib/usdUfe/ufe/UsdUndoAddRefOrPayloadCommand.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/usdUfe/ufe/UsdUndoAddRefOrPayloadCommand.cpp b/lib/usdUfe/ufe/UsdUndoAddRefOrPayloadCommand.cpp index 8cd67afd14..d5c85dd0ef 100644 --- a/lib/usdUfe/ufe/UsdUndoAddRefOrPayloadCommand.cpp +++ b/lib/usdUfe/ufe/UsdUndoAddRefOrPayloadCommand.cpp @@ -75,7 +75,7 @@ getPrimPath(const UsdPrim& prim, const std::string& filePath, const std::string& SdfPrimSpecHandle primSpec = layerRef->GetPrimAtPath(SdfPath(primName.GetText())); const std::string errorMessage = validatePrimSpec(prim, primSpec); if (!errorMessage.empty()) - TF_WARN("The default prim in file [%s] %s.", errorMessage.c_str(), filePath.c_str()); + TF_WARN("The default prim in file [%s] %s.", filePath.c_str(), errorMessage.c_str()); return SdfPath(); }