LOOKDEVX-1592 - Clean up references, targets, and connections on delete

lib/mayaUsd/ufe/UsdUndoDeleteCommand.cpp

@@ -19,6 +19,7 @@
 
 #include <usdUfe/ufe/Utils.h>
 #include <usdUfe/utils/layers.h>
+#include <usdUfe/utils/usdUtils.h>
 
 #include <pxr/usd/sdf/layer.h>
 #include <pxr/usd/usd/editContext.h>
@@ -67,6 +68,14 @@ void UsdUndoDeleteCommand::execute()
 #ifdef UFE_V4_FEATURES_AVAILABLE
         UsdAttributes::removeAttributesConnections(_prim);
 #endif
+        // Let removeAttributesConnections be run first as it will also cleanup
+        // attributes that were authored only to be the destination of a connection.
+        if (!UsdUfe::cleanReferencedPath(_prim)) {
+            const std::string error = TfStringPrintf(
+                "Failed to cleanup references to prim \"%s\".", _prim.GetPath().GetText());
+            TF_WARN("%s", error.c_str());
+            throw std::runtime_error(error);
+        }
         PrimSpecFunc deleteFunc
             = [stage](const UsdPrim& prim, const SdfPrimSpecHandle& primSpec) -> void {
             PXR_NS::UsdEditContext ctx(stage, primSpec->GetLayer());

lib/usdUfe/utils/usdUtils.cpp

@@ -107,6 +107,36 @@ void replaceInternalReferencePath(
     }
 }
 
+void removeInternalReferencePath(
+    const UsdPrim&            deletedPrim,
+    const SdfReferencesProxy& referencesList,
+    SdfListOpType             op)
+{
+    // set the listProxy based on the SdfListOpType
+    SdfReferencesProxy::ListProxy listProxy = referencesList.GetAppendedItems();
+    if (op == SdfListOpTypePrepended) {
+        listProxy = referencesList.GetPrependedItems();
+    } else if (op == SdfListOpTypeOrdered) {
+        listProxy = referencesList.GetOrderedItems();
+    } else if (op == SdfListOpTypeAdded) {
+        listProxy = referencesList.GetAddedItems();
+    } else if (op == SdfListOpTypeDeleted) {
+        listProxy = referencesList.GetDeletedItems();
+    }
+
+    for (size_t idx = 0; idx < listProxy.size();) {
+        const SdfReference ref = listProxy[idx];
+        if (UsdUfe::isInternalReference(ref)) {
+            if (deletedPrim.GetPath() == ref.GetPrimPath()
+                || ref.GetPrimPath().HasPrefix(deletedPrim.GetPath())) {
+                listProxy.Erase(idx);
+                continue; // Not increasing idx
+            }
+        }
+        ++idx;
+    }
+}
+
 // This template method updates the SdfPath for inherited or specialized arcs
 // when the path to the concrete prim they refer to has changed.
 // HS January 13, 2021: Find a better generic way to consolidate this method with
@@ -143,6 +173,34 @@ void replacePath(const UsdPrim& oldPrim, const SdfPath& newPath, const T& proxy,
     }
 }
 
+// This template method cleans the SdfPath for inherited or specialized arcs
+// when the path to the concrete prim they refer to has becomed invalid.
+// HS January 13, 2021: Find a better generic way to consolidate this method with
+// removeReferenceItems
+template <typename T> void removePath(const UsdPrim& deletedPrim, const T& proxy, SdfListOpType op)
+{
+    // set the listProxy based on the SdfListOpType
+    typename T::ListProxy listProxy { proxy.GetAppendedItems() };
+    if (op == SdfListOpTypePrepended) {
+        listProxy = proxy.GetPrependedItems();
+    } else if (op == SdfListOpTypeOrdered) {
+        listProxy = proxy.GetOrderedItems();
+    } else if (op == SdfListOpTypeAdded) {
+        listProxy = proxy.GetAddedItems();
+    } else if (op == SdfListOpTypeDeleted) {
+        listProxy = proxy.GetDeletedItems();
+    }
+
+    for (size_t idx = 0; idx < listProxy.size();) {
+        const SdfPath path = listProxy[idx];
+        if (deletedPrim.GetPath() == path.GetPrimPath() || path.HasPrefix(deletedPrim.GetPath())) {
+            listProxy.Erase(idx);
+            continue; // Not increasing idx
+        }
+        ++idx;
+    }
+}
+
 void replacePropertyPath(const UsdPrim& oldPrim, const SdfPath& newPath, UsdProperty& prop)
 {
     if (prop.Is<UsdAttribute>()) {
@@ -180,6 +238,58 @@ void replacePropertyPath(const UsdPrim& oldPrim, const SdfPath& newPath, UsdProp
     }
 }
 
+void removePropertyPath(const UsdPrim& deletedPrim, UsdProperty& prop)
+{
+    if (prop.Is<UsdAttribute>()) {
+        UsdAttribute  attr = prop.As<UsdAttribute>();
+        SdfPathVector sources;
+        attr.GetConnections(&sources);
+        bool hasChanged = false;
+        for (size_t i = 0; i < sources.size();) {
+            const SdfPath& path = sources[i];
+            if (deletedPrim.GetPath() == path.GetPrimPath()
+                || path.HasPrefix(deletedPrim.GetPath())) {
+                hasChanged = true;
+                sources.erase(sources.cbegin() + i);
+                continue;
+            }
+            ++i;
+        }
+        if (hasChanged) {
+            if (sources.empty()) {
+                attr.ClearConnections();
+                if (!attr.HasValue()) {
+                    prop.GetPrim().RemoveProperty(prop.GetName());
+                }
+            } else {
+                attr.SetConnections(sources);
+            }
+        }
+    } else if (prop.Is<UsdRelationship>()) {
+        UsdRelationship rel = prop.As<UsdRelationship>();
+        SdfPathVector   targets;
+        rel.GetTargets(&targets);
+        bool hasChanged = false;
+        for (size_t i = 0; i < targets.size();) {
+            const SdfPath& path = targets[i];
+            if (deletedPrim.GetPath() == path.GetPrimPath()
+                || path.HasPrefix(deletedPrim.GetPath())) {
+                hasChanged = true;
+                targets.erase(targets.cbegin() + i);
+                continue;
+            }
+            ++i;
+        }
+        if (hasChanged) {
+            if (targets.empty()) {
+                rel.ClearTargets(true);
+            } else {
+                rel.SetTargets(targets);
+            }
+        }
+    }
+}
+
 } // namespace
 
 namespace USDUFE_NS_DEF {
@@ -261,6 +371,53 @@ bool updateReferencedPath(const UsdPrim& oldPrim, const SdfPath& newPath)
     return true;
 }
 
+bool cleanReferencedPath(const UsdPrim& deletedPrim)
+{
+    SdfChangeBlock changeBlock;
+
+    for (const auto& p : deletedPrim.GetStage()->Traverse()) {
+
+        auto primSpec = getPrimSpecAtEditTarget(p);
+        // check different composition arcs
+        if (p.HasAuthoredReferences()) {
+            if (primSpec) {
+
+                SdfReferencesProxy referencesList = primSpec->GetReferenceList();
+
+                // update append/prepend lists individually
+                removeInternalReferencePath(deletedPrim, referencesList, SdfListOpTypeAppended);
+                removeInternalReferencePath(deletedPrim, referencesList, SdfListOpTypePrepended);
+            }
+        } else if (p.HasAuthoredInherits()) {
+            if (primSpec) {
+
+                SdfInheritsProxy inheritsList = primSpec->GetInheritPathList();
+
+                // update append/prepend lists individually
+                removePath<SdfInheritsProxy>(deletedPrim, inheritsList, SdfListOpTypeAppended);
+                removePath<SdfInheritsProxy>(deletedPrim, inheritsList, SdfListOpTypePrepended);
+            }
+        } else if (p.HasAuthoredSpecializes()) {
+            if (primSpec) {
+                SdfSpecializesProxy specializesList = primSpec->GetSpecializesList();
+
+                // update append/prepend lists individually
+                removePath<SdfSpecializesProxy>(
+                    deletedPrim, specializesList, SdfListOpTypeAppended);
+                removePath<SdfSpecializesProxy>(
+                    deletedPrim, specializesList, SdfListOpTypePrepended);
+            }
+        }
+
+        // Need to repath connections and relationships:
+        for (auto& prop : p.GetProperties()) {
+            removePropertyPath(deletedPrim, prop);
+        }
+    }
+
+    return true;
+}
+
 bool isInternalReference(const SdfReference& ref) { return ref.IsInternal(); }
 
 } // namespace USDUFE_NS_DEF

lib/usdUfe/utils/usdUtils.h

@@ -41,6 +41,12 @@ void printCompositionQuery(const UsdPrim& prim, std::ostream& os);
 USDUFE_PUBLIC
 bool updateReferencedPath(const UsdPrim& oldPrim, const SdfPath& newPath);
 
+//! This function automatically cleans the SdfPath for different
+//  composition arcs (internal references, inherits, specializes) when
+//  the path to the concrete prim they refer to becomes invalid.
+USDUFE_PUBLIC
+bool cleanReferencedPath(const UsdPrim& deletedPrim);
+
 //! Returns true if reference is internal.
 USDUFE_PUBLIC
 bool isInternalReference(const SdfReference&);

test/lib/ufe/testDeleteCmd.py

@@ -29,7 +29,7 @@
 
 import ufe
 
-from pxr import Sdf
+from pxr import Sdf, UsdShade
 
 import os
 import unittest
@@ -526,6 +526,82 @@ def testDeleteAndRemoveConnections(self):
         self.assertFalse(surface2Prim.HasProperty('outputs:out'))
         self.assertFalse(surface3Prim.HasProperty('inputs:bsdf'))
 
+    def testDeleteAndRemoveReference(self):
+        '''Test deleting a prim and its references'''
+
+        # open treeRef.ma scene in testSamples
+        mayaUtils.openTreeRefScene()
+
+        # validate the default edit target to be the Rootlayer.
+        mayaPathSegment = mayaUtils.createUfePathSegment('|TreeRef_usd|TreeRef_usdShape')
+        stage = mayaUsd.ufe.getStage(str(mayaPathSegment))
+        self.assertTrue(stage)
+        self.assertEqual(stage.GetEditTarget().GetLayer(), stage.GetRootLayer())
+        self.assertTrue(stage.GetPrimAtPath('/TreeBase/leaf_ref_1').HasAuthoredReferences())
+
+        # delete the greenLeaf and check the internal references were cleaned:
+        cmds.delete('|TreeRef_usd|TreeRef_usdShape,/TreeBase/leavesXform/greenLeaf')
+        self.assertFalse(stage.GetPrimAtPath('/TreeBase/leaf_ref_1').HasAuthoredReferences())
+        # Ref2 has an "external" reference, but to this file. It will not be
+        # cleaned up because external, baut maybe it should have since the
+        # referenced file happens to be the root layer?
+        self.assertTrue(stage.GetPrimAtPath('/TreeBase/leaf_ref_2').HasAuthoredReferences())
+
+        # validate undo
+        cmds.undo()
+        ref1 = stage.GetPrimAtPath('/TreeBase/leaf_ref_1')
+        self.assertTrue(stage.GetPrimAtPath('/TreeBase/leaf_ref_1').HasAuthoredReferences())
+
+        # validate redo
+        cmds.redo()
+        self.assertFalse(stage.GetPrimAtPath('/TreeBase/leaf_ref_1').HasAuthoredReferences())
+
+        # Try deleting leavesXform hierarchy, which also deletes greenLeaf and
+        # should clean-up the reference.
+        cmds.undo()
+        cmds.delete('|TreeRef_usd|TreeRef_usdShape,/TreeBase/leavesXform')
+        self.assertFalse(stage.GetPrimAtPath('/TreeBase/leaf_ref_1').HasAuthoredReferences())
+
+        # validate undo
+        cmds.undo()
+        self.assertTrue(stage.GetPrimAtPath('/TreeBase/leaf_ref_1').HasAuthoredReferences())
+
+        # validate redo
+        cmds.redo()
+        self.assertFalse(stage.GetPrimAtPath('/TreeBase/leaf_ref_1').HasAuthoredReferences())
+
+    def testDeleteAndInheritMaterial(self):
+        '''Test deleting a material cleans up all places where it was assigned'''
+
+        cmds.file(new=True, force=True)
+        testFile = testUtils.getTestScene('tree', 'treeMat.usda')
+        pathString,stage = mayaUtils.createProxyFromFile(testFile)
+        self.assertTrue(stage)
+        self.assertTrue(pathString)
+        matAPI = UsdShade.MaterialBindingAPI(stage.GetPrimAtPath('/TreeBase/leavesXform/leaves'))
+
+        self.assertEqual(matAPI.ComputeBoundMaterial()[0].GetPrim().GetPath(), '/mtlFoliage/GreenMat')
+
+        cmds.delete(pathString + ',/mtlFoliage/GreenMat')
+        self.assertEqual(matAPI.ComputeBoundMaterial()[0].GetPrim().GetPath(), '/mtl/BrownMat')
+
+        cmds.undo()
+        self.assertEqual(matAPI.ComputeBoundMaterial()[0].GetPrim().GetPath(), '/mtlFoliage/GreenMat')
+
+        cmds.redo()
+        self.assertEqual(matAPI.ComputeBoundMaterial()[0].GetPrim().GetPath(), '/mtl/BrownMat')
+
+        cmds.undo()
+        cmds.delete(pathString + ',/mtlFoliage')
+        self.assertEqual(matAPI.ComputeBoundMaterial()[0].GetPrim().GetPath(), '/mtl/BrownMat')
+
+        cmds.undo()
+        self.assertEqual(matAPI.ComputeBoundMaterial()[0].GetPrim().GetPath(), '/mtlFoliage/GreenMat')
+
+        cmds.redo()
+        self.assertEqual(matAPI.ComputeBoundMaterial()[0].GetPrim().GetPath(), '/mtl/BrownMat')
+
+
     def testDeleteRestrictionMutedLayer(self):
         '''
         Test delete restriction - we don't allow removal of a prim

test/testSamples/tree/treeMat.usda

@@ -0,0 +1,71 @@
+#usda 1.0
+(
+    defaultPrim = "TreeBase"
+    upAxis = "Y"
+)
+
+def Xform "TreeBase" (
+    prepend apiSchemas = ["MaterialBindingAPI"]
+)
+{
+    rel material:binding = </mtl/BrownMat>
+
+    def Xform "leavesXform" (
+        prepend apiSchemas = ["MaterialBindingAPI"]
+    )
+    {
+        rel material:binding = </mtlFoliage/GreenMat>
+        double3 xformOp:translate = (0, 2.5, 0)
+        uniform token[] xformOpOrder = ["xformOp:translate"]
+
+        def Sphere "leaves"
+        {
+            float3[] extent = [(-2, -2, -2), (2, 2, 2)]
+            color3f[] primvars:displayColor = [(0, 1, 0)]
+            double radius = 2
+        }
+    }
+
+    def Cylinder "trunk" (
+        active = true
+        kind = "component"
+    )
+    {
+        uniform token axis = "Y"
+        float3[] extent = [(-1, -1, -1), (1, 1, 1)]
+        double height = 2
+        color3f[] primvars:displayColor = [(0.66, 0.33, 0)]
+        double radius = 0.5
+    }
+}
+
+def Scope "mtl"
+{
+    def Material "BrownMat"
+    {
+        token outputs:surface.connect = </mtl/BrownMat/BrownSrf.outputs:surface>
+
+        def Shader "BrownSrf"
+        {
+            uniform token info:id = "UsdPreviewSurface"
+            color3f inputs:diffuseColor = (0.1293, 0.0625, 0.0050000004)
+            token outputs:surface
+        }
+    }
+}
+
+def Scope "mtlFoliage"
+{
+    def Material "GreenMat"
+    {
+        token outputs:surface.connect = </mtlFoliage/GreenMat/GreenSrf.outputs:surface>
+
+        def Shader "GreenSrf"
+        {
+            uniform token info:id = "UsdPreviewSurface"
+            color3f inputs:diffuseColor = (0, 1, 0)
+            token outputs:surface
+        }
+    }
+}
+