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

Use data model undo framework for attribute undo. #1134

Merged
merged 1 commit into from
Feb 2, 2021

Conversation

ppt-adsk
Copy link
Collaborator

@ppt-adsk ppt-adsk commented Jan 29, 2021

UsdAttribute derived classes were not overriding the Ufe::Attribute::setCmd() virtual. This meant that we were using the implementation from UFE, rather than our new, data model correct undo system. This was bad in two ways:

  • The UFE implementation only does value undo, not data model undo
  • It uses the Ufe::Attribute to set the previous value, and in our implementation this Ufe::Attribute stores a UsdPrim that can become stale, as per the Autodesk MAYA-109522 internal bug.
    Now fixed, with an automated test to validate the fix.

@@ -140,6 +143,31 @@ void setUsdAttributeVectorFromUfe(
UFE_ASSERT_MSG(b, kErrorMsgFailedSet);
}

template <typename T, typename A = MayaUsd::ufe::TypedUsdAttribute<T>>
class SetUndoableCommand : public Ufe::UndoableCommand
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simple class that uses the new UsdUndoableItem support.

@@ -557,3 +557,50 @@ def testObservation(self):
self.assertEqual(ball34Obs.notifications, 3)
self.assertEqual(ball35Obs.notifications, 3)
self.assertEqual(globalObs.notifications, 7)

# Run last to avoid file new disturbing other tests.
def testZAttrChangeRedoAfterPrimCreateRedo(self):
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test-Driven Development: wrote test first, it failed (crashed), coded fix, test now passes.

@kxl-adsk kxl-adsk added the ufe-usd Related to UFE-USD plugin in Maya-Usd label Jan 30, 2021
Copy link
Collaborator

@seando-adsk seando-adsk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work with the test.

@ppt-adsk
Copy link
Collaborator Author

ppt-adsk commented Feb 1, 2021

All Windows Python 2 builds failed because of lack of disk space errors, but other builds succeeded. I therefore believe this pull request is good to go.

@ppt-adsk ppt-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Feb 1, 2021
@kxl-adsk kxl-adsk merged commit b733ccf into dev Feb 2, 2021
@kxl-adsk kxl-adsk deleted the tremblp/MAYA-109522/attribute_undo_redo branch February 2, 2021 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge Development process is finished, PR is ready for merge ufe-usd Related to UFE-USD plugin in Maya-Usd
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants