Animation Keyframe Undo/Redo Bug #66325

Developer note: this report is hard to follow:

With this file:

Animation Keyframe Bug.blend
Alt-LMB to select the column on frame 1.
X to delete the frames.
Undo
Redo

Notice the rotation of the cube was not updated on redo.

System Information
Operating system: Windows 10
Graphics card: AMD RX 580

Blender Version
Broken: Every version that came after: (2.8 - 8b2b79c210) released 06June (USA). Latest version I've tried, checking to see if it has been resolved is (2.8 - dcf520cdad) 01July

Short description of error
When undoing keyframes repeatedly (at least two or greater) the undo system will get out of sync and you can get lost, pretty quickly, on what has been undone.

I've included a video to help explain, by showing, what is exactly going on. First part of the video shows the version of 2.8 that works properly, and the second part shows the latest version of 2.8 repeating the same steps but with the Undo getting out of sync.
Sorry for the pop in the audio, I didn't notice until I saved the video. :)

Exact steps for others to reproduce the error
Included a video with this bug report.
Keyframe Undo Bug.mov

Just a FYI. I filed a similar bug report about this issue earlier last month but I cannot find any trace of it... maybe I forgot to hit "Create New Task". Or maybe I didn't explain the issue well and it was viewed as not being a bug; then it was filed away.

Hope this helps
Tim.

Added subscriber: @CookItOff

Added subscriber: @dr.sybren

@CookItOff Please describe the steps the developers need to do to reproduce the problem, and include an as-small-and-simple-as-possible blend file that demonstrates the issue. We can't debug it based on a complex example shown in a 6+ minute video only.

No prob, here are the steps and a blend file. And a very short video showing the steps.

Go to the Animation tab.
Select the Cube
Delete first row of keyframes (Frame 1)

No need to move the play head leave it at frame one.

Delete second row of keyframes (Frame 5)
Delete third row of keyframes (Frame 10)
Delete fourth row of keyframes (Frame 15)
Delete fifth row of keyframes (Frame 20)

Now HERE is where the problem happens---

Undo delete from frame 20-- This will work as normal
Undo delete from frame 15 -- this will move the cube in the viewport (which corresponds to the frame 15 delete) BUT at the same time deselects the keyframe row for frame 20 and the keyframe row for frame 15 doesn't appear on the dope sheet yet.
Hitting undo again will make no changes to the viewport but the Frame 15 keyframe will only then return to the dope sheet.
You can keep repeating the undo steps returning to the cube's original position, and every UNDO step will be out of sync.

Second issue caused by the bug:
The bug also seems to prevent redoing of keyframes

Pick a frame
Delete it
Undo it
(Shift-Control-Delete) Redo ---- Keyframe will disappear again but the Viewport will have no action.

Hope this helps
Tim

Below is the blend file and really short video.
Animation Keyframe Bug.blend

2019-07-02 08-57-27.mov

Added subscriber: @ideasman42

I can confirm this on Blender 516afd0162.

Not a high-prio issue, as Blender can be easily brought back into sync by performing any other action (like going to the prev/next frame).

Assigning to @ideasman42 as he worked on the undo system.

@dr.sybren @ideasman42 I just wanted to add that Sybren's work around only works for the Redo portion of this bug. Changing frames does nothing for the Undo Sync issues. The Redo issues doesn't really effect anything, and I probably shouldn't have even mentioned it.
The Undo issues are making editing complex animations nearly impossible for me, with any 2.8 version after the June 6th version I mentioned originally. It's just way too hard to keep track of steps I've undone if I'm wanting to use the RC.

Thanks, Tim

The mem-file undo state stored is written before the depsgraph has been re-evaluated.

This patch P1044 writes the undo information after depsgraph evaluation has been handled by the event loop (complicates writing undo steps).
A simpler fix would be to call a function evaluate the depsgraph before writing the mem-file undo step, however this will have a different code-path to the main event loop - not handling windows/notifiers, etc in the same order, so would rather not do this, although it could probably be made to work reasonably well, it would be possible to store data in the undo step based on incorrectly evaluated data, as well as interfering with evaluation in the main event loop afterwards.

Also looked into evaluating on load however this isn't an option since we want to be able to undo un-keyframed changes to an object or pose for eg (P1043 for reference).

Will check with @Sergey if there are better alternatives to P1044.

Added subscriber: @Sergey

@ideasman42, you'd need explain root of the issue. I.m not sure how undo is different from save+reload the file (apart from file load re-evaluating animation, so non-keyed changes are lost). If undo is flackey, then save+reload also is? And the issue is somewhere deeper?

Save happens after the depsgraph is evaluated via wm_event_do_refresh_wm_and_depsgraph, where as undo is written beforehand.

@ideasman42, think i see now.

ED_undo_push() is called when operator is finished in wm_operator_finished(). This is a bit counter intuitive to me, but well.

However, I am still not quite sure why dependency graph can be evaluated correctly when operator changes data, but not when undo does it? Did you investigate why is it so? It it because depsgraph tags different things on redo in comparison to what operator tags?

And last but not least, how did it work in 2.79?

Operators that need evaluated data can ask for it, or use

In #66325#730143, @Sergey wrote:
@ideasman42, think i see now.

ED_undo_push() is called when operator is finished in wm_operator_finished(). This is a bit counter intuitive to me, but well.

However, I am still not quite sure why dependency graph can be evaluated correctly when operator changes data, but not when undo does it? Did you investigate why is it so? It it because depsgraph tags different things on redo in comparison to what operator tags?

From what I can tell, the system is working correctly,

Animation data is edited from operator.
Operator causes undo push (writing memfile)
Depsgraph handles update from tagged items and writes to DNA location/rotation ... etc.

Then when accessing undo, the transform values are used before animation wrote into these values.

And last but not least, how did it work in 2.79?

2.79b works, old & new depsgraph.

This could use some more investigation exactly why it works when 2.8x don't, although it's possible we come to the same conclusion.

I've talked to Brecht, and think now it is clear what is going on and how it worked before.

In Blender 2.79 recalc flags were stored in the original IDs (such as object->recalc for example). Those recalc flags were put to the undo steps, so then after undo dependency graph new what exactly to do: since animation was tagged for update, it will update animation (even though "regular" undo would have skipped animation to avoid loss of unkeyed changed).

This was also how new dependency graph with copy-on-write operated prior to ba4e6e59b2. We've stopped tagging original IDs because it was causing issues with temporary dependency graphs.

Proposed fix is: store recalc flags from DEG_id_tag_update() and DEG_id_tag_update_ex() in the original ID. This way update tags can be restored when new dependency graph is created after undo.

Here is a quickly-put-together patch which implements this: P1046

There are probably some missing aspects, but hope it is enough to do initial tests here, and that it indicates the root issue and solution.

EDIT: Managed to redo original issue, and there is still something missing in the proposed patch, looking into...

@ideasman42, I updated patch at P1048. Seems to work in the simple case. Deeper review and testing is needed though.

This issue was referenced by 523de7ae9b

@ideasman42 @Sergey
I just checked to see if this was still an issue, using the latest version of Blender 2.80.75, and the "Undo Sync" issue is still there.

A couple of things I want to mention after reading through the notes each of you posted:

2.80 was working without this issue all the way through to 2.80.74 (8b2b79c210) . The version released on June 6th after the one I just mentioned is where the "undo" became out of sync.
I'm not sure if the patch that that is mentioned last is incorporated into the latest build (2.80.75), but if it's not and you are still testing I would like to help; I just don't know how to build with a new patch.

I also see that my direction for steps to take may be unclear. If I need to send another test blend and a more descriptive outline to reproduce the problem just let me know.

Thanks,
Tim

The report is still marked as open, which means we are still working on it. For until the work is finished and report is marked as solved it is not really expected that issue solves itself.

@Sergey Sorry for the misunderstanding. I read the message you posted here, “I updated patch at P1048. Seems to work in the simple case. Deeper review and testing is needed though.”
and I wasn’t sure if that means it goes into a beta build for testing, and if you were unsure on how to replicate the issue properly.

I know y’all are super busy and working hard to resolve all these bugs... I’m just trying to help on my end.

Thanks,
Tim

Changed status from 'Open' to: 'Resolved'

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Animation Keyframe Undo/Redo Bug #66325