Merge pull request #3034 from Autodesk/bailp/MAYA-128764/excessive-ae…

…-refreshes

MAYA-128764 fix AE refresh

lib/mayaUsd/ufe/Global.cpp

@@ -139,6 +139,7 @@ StagesSubject::Ptr g_StagesSubject;
 
 bool InPathChange::inGuard = false;
 bool InAddOrDeleteOperation::inGuard = false;
+int  InSetAttribute::inGuard = 0;
 
 //------------------------------------------------------------------------------
 // Functions

lib/mayaUsd/ufe/StagesSubject.cpp

@@ -172,8 +172,17 @@ void sendAttributeChanged(
         }
     } break;
     case AttributeChangeType::kAdded: {
-        notifyWithoutExceptions<Ufe::Attributes>(
-            Ufe::AttributeAdded(ufePath, changedToken.GetString()));
+        if (MayaUsd::ufe::InSetAttribute::inSetAttribute()) {
+            notifyWithoutExceptions<Ufe::Attributes>(
+                Ufe::AttributeValueChanged(ufePath, changedToken.GetString()));
+
+            if (MayaUsd::ufe::UsdCamera::isCameraToken(changedToken)) {
+                notifyWithoutExceptions<Ufe::Camera>(ufePath);
+            }
+        } else {
+            notifyWithoutExceptions<Ufe::Attributes>(
+                Ufe::AttributeAdded(ufePath, changedToken.GetString()));
+        }
     } break;
     case AttributeChangeType::kRemoved: {
         notifyWithoutExceptions<Ufe::Attributes>(

lib/mayaUsd/ufe/UsdAttribute.cpp

@@ -16,6 +16,7 @@
 #include "UsdAttribute.h"
 
 #include "Utils.h"
+#include "private/UfeNotifGuard.h"
 #include "private/Utils.h"
 
 #include <mayaUsd/ufe/StagesSubject.h>
@@ -84,6 +85,7 @@ template <typename T> bool setUsdAttr(MayaUsd::ufe::UsdAttribute& attr, const T&
     // our own in the StagesSubject, which we invoke here, so that only a
     // single UFE attribute changed notification is generated.
 
+    MayaUsd::ufe::InSetAttribute                    inSetAttr;
     MayaUsd::ufe::AttributeChangedNotificationGuard guard;
     const std::string                               errMsg = attr.isEditAllowedMsg();
     if (!errMsg.empty()) {
@@ -256,6 +258,18 @@ class SetUndoableCommand : public MayaUsd::ufe::UsdUndoableCommand<Ufe::Undoable
     {
     }
 
+    void undo() override
+    {
+        MayaUsd::ufe::InSetAttribute inSetAttr;
+        MayaUsd::ufe::UsdUndoableCommand<Ufe::UndoableCommand>::undo();
+    }
+
+    void redo() override
+    {
+        MayaUsd::ufe::InSetAttribute inSetAttr;
+        MayaUsd::ufe::UsdUndoableCommand<Ufe::UndoableCommand>::redo();
+    }
+
 protected:
     void executeImplementation() override { _attr->set(_newValue); }
 
@@ -278,6 +292,18 @@ class SetUndoableMetadataCommand : public MayaUsd::ufe::UsdUndoableCommand<Ufe::
     {
     }
 
+    void undo() override
+    {
+        MayaUsd::ufe::InSetAttribute inSetAttr;
+        MayaUsd::ufe::UsdUndoableCommand<Ufe::UndoableCommand>::undo();
+    }
+
+    void redo() override
+    {
+        MayaUsd::ufe::InSetAttribute inSetAttr;
+        MayaUsd::ufe::UsdUndoableCommand<Ufe::UndoableCommand>::redo();
+    }
+
 protected:
     void executeImplementation() override
     {

lib/mayaUsd/ufe/UsdAttributeHolder.cpp

@@ -16,6 +16,7 @@
 #include "UsdAttributeHolder.h"
 
 #include "Utils.h"
+#include "private/UfeNotifGuard.h"
 
 #include <mayaUsd/utils/editRouter.h>
 #include <mayaUsd/utils/editRouterContext.h>
@@ -41,6 +42,8 @@ bool setUsdAttrMetadata(
     const Ufe::Value&           value)
 {
     PXR_NAMESPACE_USING_DIRECTIVE
+    MayaUsd::ufe::InSetAttribute inSetAttr;
+
     // Special cases for known Ufe metadata keys.
 
     // Note: we allow the locking attribute to be changed even if attribute is locked
@@ -162,6 +165,7 @@ bool UsdAttributeHolder::set(const PXR_NS::VtValue& value, PXR_NS::UsdTimeCode t
 
     AttributeEditRouterContext ctx(_usdAttr.GetPrim(), _usdAttr.GetName());
 
+    InSetAttribute inSetAttr;
     return _usdAttr.Set(value, time);
 }
 
@@ -437,6 +441,8 @@ bool UsdAttributeHolder::setMetadata(const std::string& key, const Ufe::Value& v
 bool UsdAttributeHolder::clearMetadata(const std::string& key)
 {
     PXR_NAMESPACE_USING_DIRECTIVE
+    InSetAttribute inSetAttr;
+
     if (isValid()) {
         AttributeEditRouterContext ctx(_usdAttr.GetPrim(), _usdAttr.GetName());
 

lib/mayaUsd/ufe/private/UfeNotifGuard.h

@@ -47,6 +47,33 @@ class InPathChange
     static bool inGuard;
 };
 
+//! \brief Helper class to scope when we are in a set attribute operation.
+//
+//         It allows detecting that adding an attribute really was setting the
+//         attribute. When a USD prim did not have an opinion about an attribute
+//         value, it gets notified by USD as adding a property instead of setting
+//         a property, which is very unfortunate. This allows detecting this
+//         situation.
+//
+// This simple guard class can be used within a single scope.
+class InSetAttribute
+{
+public:
+    InSetAttribute() { inGuard += 1; }
+    ~InSetAttribute() { inGuard -= 1; }
+
+    // Delete the copy/move constructors assignment operators.
+    InSetAttribute(const InSetAttribute&) = delete;
+    InSetAttribute& operator=(const InSetAttribute&) = delete;
+    InSetAttribute(InSetAttribute&&) = delete;
+    InSetAttribute& operator=(InSetAttribute&&) = delete;
+
+    static bool inSetAttribute() { return inGuard > 0; }
+
+private:
+    static int inGuard;
+};
+
 //! \brief Helper class to scope when we are in an add or delete operation.
 //
 // This simple guard class can be used within a single scope, but does not have

test/lib/ufe/testAttribute.py

@@ -1248,68 +1248,68 @@ def testObservationWithFineGrainedNotifications(self):
         # "Props" and "Room_set". Ufe should be filtering out those notifications
         # so the global observer should still only see one notification.
         ufeCmd.execute(ball34XlateAttr.setCmd(ufe.Vector3d(4, 4, 15)))
-        ball34Obs.assertNotificationCount(self, numAdded = 1, numValue = 1)
+        ball34Obs.assertNotificationCount(self, numValue = 2)
         ball35Obs.assertNotificationCount(self)
-        globalObs.assertNotificationCount(self, numAdded = 1, numValue = 1)
+        globalObs.assertNotificationCount(self, numValue = 2)
 
         # The second modification only sends one USD notification for "xformOps:translate"
         # because all the spec's already exist. Ufe should also see one notification.
         ufeCmd.execute(ball34XlateAttr.setCmd(ufe.Vector3d(4, 4, 20)))
-        ball34Obs.assertNotificationCount(self, numAdded = 1, numValue = 2)
+        ball34Obs.assertNotificationCount(self, numValue = 3)
         ball35Obs.assertNotificationCount(self)
-        globalObs.assertNotificationCount(self, numAdded = 1, numValue = 2)
+        globalObs.assertNotificationCount(self, numValue = 3)
 
         # Undo, redo
         cmds.undo()
-        ball34Obs.assertNotificationCount(self, numAdded = 1, numValue = 3)
+        ball34Obs.assertNotificationCount(self, numValue = 4)
         ball35Obs.assertNotificationCount(self)
-        globalObs.assertNotificationCount(self, numAdded = 1, numValue = 3)
+        globalObs.assertNotificationCount(self, numValue = 4)
 
         cmds.redo()
-        ball34Obs.assertNotificationCount(self, numAdded = 1, numValue = 4)
+        ball34Obs.assertNotificationCount(self, numValue = 5)
         ball35Obs.assertNotificationCount(self)
-        globalObs.assertNotificationCount(self, numAdded = 1, numValue = 4)
+        globalObs.assertNotificationCount(self, numValue = 5)
 
         # get ready to undo the first modification
         cmds.undo()
-        ball34Obs.assertNotificationCount(self, numAdded = 1, numValue = 5)
+        ball34Obs.assertNotificationCount(self, numValue = 6)
         ball35Obs.assertNotificationCount(self)
-        globalObs.assertNotificationCount(self, numAdded = 1, numValue = 5)
+        globalObs.assertNotificationCount(self, numValue = 6)
 
         # Undo-ing the modification which created the USD specs is a little
         # different in USD, but from Ufe we should just still see one notification.
         cmds.undo()
-        ball34Obs.assertNotificationCount(self, numAdded = 1, numRemoved = 1, numValue = 5)
+        ball34Obs.assertNotificationCount(self, numRemoved = 1, numValue = 6)
         ball35Obs.assertNotificationCount(self)
-        globalObs.assertNotificationCount(self, numAdded = 1, numRemoved = 1, numValue = 5)
+        globalObs.assertNotificationCount(self, numRemoved = 1, numValue = 6)
 
         cmds.redo()
         # Note that UsdUndoHelper add an attribute with its value in one shot, which results in a
         # single AttributeAdded notification:
-        ball34Obs.assertNotificationCount(self, numAdded = 2, numRemoved = 1, numValue = 5)
+        ball34Obs.assertNotificationCount(self, numRemoved = 1, numValue = 7)
         ball35Obs.assertNotificationCount(self)
-        globalObs.assertNotificationCount(self, numAdded = 2, numRemoved = 1, numValue = 5)
+        globalObs.assertNotificationCount(self, numRemoved = 1, numValue = 7)
 
         # Make a change to ball35, global and ball35 observers change.
         ball35Attrs = ufe.Attributes.attributes(ball35)
         ball35XlateAttr = ball35Attrs.attribute('xformOp:translate')
 
         # "xformOp:translate"
         ufeCmd.execute(ball35XlateAttr.setCmd(ufe.Vector3d(4, 8, 15)))
-        ball34Obs.assertNotificationCount(self, numAdded = 2, numRemoved = 1, numValue = 5)
-        ball35Obs.assertNotificationCount(self, numAdded = 1, numValue = 1)
-        globalObs.assertNotificationCount(self, numAdded = 3, numRemoved = 1, numValue = 6)
+        ball34Obs.assertNotificationCount(self, numRemoved = 1, numValue = 7)
+        ball35Obs.assertNotificationCount(self, numValue = 2)
+        globalObs.assertNotificationCount(self, numRemoved = 1, numValue = 9)
 
         # Undo, redo
         cmds.undo()
-        ball34Obs.assertNotificationCount(self, numAdded = 2, numRemoved = 1, numValue = 5)
-        ball35Obs.assertNotificationCount(self, numAdded = 1, numRemoved = 1, numValue = 1)
-        globalObs.assertNotificationCount(self, numAdded = 3, numRemoved = 2, numValue = 6)
+        ball34Obs.assertNotificationCount(self, numRemoved = 1, numValue = 7)
+        ball35Obs.assertNotificationCount(self, numRemoved = 1, numValue = 2)
+        globalObs.assertNotificationCount(self, numRemoved = 2, numValue = 9)
 
         cmds.redo()
-        ball34Obs.assertNotificationCount(self, numAdded = 2, numRemoved = 1, numValue = 5)
-        ball35Obs.assertNotificationCount(self, numAdded = 2, numRemoved = 1, numValue = 1)
-        globalObs.assertNotificationCount(self, numAdded = 4, numRemoved = 2, numValue = 6)
+        ball34Obs.assertNotificationCount(self, numRemoved = 1, numValue = 7)
+        ball35Obs.assertNotificationCount(self, numRemoved = 1, numValue = 3)
+        globalObs.assertNotificationCount(self, numRemoved = 2, numValue = 10)
 
         # Test removeObserver.
         ufe.Attributes.removeObserver(ball34, ball34Obs)
@@ -1322,19 +1322,19 @@ def testObservationWithFineGrainedNotifications(self):
 
         ufeCmd.execute(ball34XlateAttr.setCmd(ufe.Vector3d(4, 4, 25)))
 
-        ball34Obs.assertNotificationCount(self, numAdded = 2, numRemoved = 1, numValue = 5)
-        ball35Obs.assertNotificationCount(self, numAdded = 2, numRemoved = 1, numValue = 1)
-        globalObs.assertNotificationCount(self, numAdded = 4, numRemoved = 2, numValue = 7)
+        ball34Obs.assertNotificationCount(self, numRemoved = 1, numValue = 7)
+        ball35Obs.assertNotificationCount(self, numRemoved = 1, numValue = 3)
+        globalObs.assertNotificationCount(self, numRemoved = 2, numValue = 11)
 
         ufe.Attributes.removeObserver(globalObs)
 
         self.assertEqual(ufe.Attributes.nbObservers(), kNbGlobalObs)
 
         ufeCmd.execute(ball34XlateAttr.setCmd(ufe.Vector3d(7, 8, 9)))
 
-        ball34Obs.assertNotificationCount(self, numAdded = 2, numRemoved = 1, numValue = 5)
-        ball35Obs.assertNotificationCount(self, numAdded = 2, numRemoved = 1, numValue = 1)
-        globalObs.assertNotificationCount(self, numAdded = 4, numRemoved = 2, numValue = 7)
+        ball34Obs.assertNotificationCount(self, numRemoved = 1, numValue = 7)
+        ball35Obs.assertNotificationCount(self, numRemoved = 1, numValue = 3)
+        globalObs.assertNotificationCount(self, numRemoved = 2, numValue = 11)
 
     def testAttrChangeRedoAfterPrimCreateRedo(self):
         '''Redo attribute change after redo of prim creation.'''