Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MAYA-128764 fix AE refresh #3034

Merged
merged 4 commits into from
Apr 26, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/mayaUsd/ufe/Global.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ StagesSubject::Ptr g_StagesSubject;

bool InPathChange::inGuard = false;
bool InAddOrDeleteOperation::inGuard = false;
int InSetAttribute::inGuard = 0;

//------------------------------------------------------------------------------
// Functions
Expand Down
13 changes: 11 additions & 2 deletions lib/mayaUsd/ufe/StagesSubject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,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>(
Expand Down
26 changes: 26 additions & 0 deletions lib/mayaUsd/ufe/UsdAttribute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "UsdAttribute.h"

#include "Utils.h"
#include "private/UfeNotifGuard.h"
#include "private/Utils.h"

#include <mayaUsd/ufe/StagesSubject.h>
Expand Down Expand Up @@ -86,6 +87,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()) {
Expand Down Expand Up @@ -263,6 +265,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); }

Expand All @@ -285,6 +299,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
{
Expand Down
6 changes: 6 additions & 0 deletions lib/mayaUsd/ufe/UsdAttributeHolder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "UsdAttributeHolder.h"

#include "Utils.h"
#include "private/UfeNotifGuard.h"

#include <mayaUsd/utils/editRouter.h>
#include <mayaUsd/utils/editRouterContext.h>
Expand All @@ -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
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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());

Expand Down
27 changes: 27 additions & 0 deletions lib/mayaUsd/ufe/private/UfeNotifGuard.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
58 changes: 29 additions & 29 deletions test/lib/ufe/testAttribute.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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.'''
Expand Down
7 changes: 7 additions & 0 deletions test/lib/ufe/testUINodeGraphNode.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,19 @@ def testPosition(self):
self.doPosAndSizeTests(uiNodeGraphNode.hasPosition, uiNodeGraphNode.setPosition,
uiNodeGraphNode.getPosition, uiNodeGraphNode.setPositionCmd)

# None of these changes should force a render refresh:
self.assertEqual(initialUpdateCount, cmds.getAttr('|transform1|proxyShape1.updateId'))
self.assertEqual(initialResyncCount, cmds.getAttr('|transform1|proxyShape1.resyncId'))

@unittest.skipIf(os.getenv('UFE_PREVIEW_VERSION_NUM', '0000') < '4100',
'Size interface only available in Ufe preview version greater equal to 4.0.100, or 0.5.0.')
def testSize(self):
ball3Path = ufe.PathString.path('|transform1|proxyShape1,/Ball_set/Props/Ball_3')
ball3SceneItem = ufe.Hierarchy.createItem(ball3Path)

initialUpdateCount = cmds.getAttr('|transform1|proxyShape1.updateId')
initialResyncCount = cmds.getAttr('|transform1|proxyShape1.resyncId')

if(hasattr(ufe, "UINodeGraphNode_v4_1")):
uiNodeGraphNode = ufe.UINodeGraphNode_v4_1.uiNodeGraphNode(ball3SceneItem)
else:
Expand Down