Animation Keyframe Undo/Redo Bug #66325

Closed
opened 2019-07-02 06:22:55 +02:00 by Timothy Cook · 20 comments

Developer note: this report is hard to follow:

With this file:

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.

Developer note: this report is hard to follow: With this file: - [Animation Keyframe Bug.blend](https://archive.blender.org/developer/F7562101/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 - 8b2b79c2108b) released 06June (USA). Latest version I've tried, checking to see if it has been resolved is (2.8 - dcf520cdad79) 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](https://archive.blender.org/developer/F7560590/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.
Author

Added subscriber: @CookItOff

Added subscriber: @CookItOff

Added subscriber: @dr.sybren

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.

@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.
Author

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

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](https://archive.blender.org/developer/F7562101/Animation_Keyframe_Bug.blend) [2019-07-02 08-57-27.mov](https://archive.blender.org/developer/F7562102/2019-07-02_08-57-27.mov)
Campbell Barton was assigned by Sybren A. Stüvel 2019-07-09 11:25:26 +02:00

Added subscriber: @ideasman42

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.

I can confirm this on Blender 516afd0162cd2e217ec50ce522f87317b73f0088. 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.
Author

@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

@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.

The mem-file undo state stored is written before the depsgraph has been re-evaluated. - This patch [P1044](https://archive.blender.org/developer/P1044.txt) 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](https://archive.blender.org/developer/P1043.txt) for reference). ---- Will check with @Sergey if there are better alternatives to [P1044](https://archive.blender.org/developer/P1044.txt).

Added subscriber: @Sergey

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?

@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.

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?

@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.

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...

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](https://archive.blender.org/developer/P1046.txt) 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.

@ideasman42, I updated patch at [P1048](https://archive.blender.org/developer/P1048.txt). Seems to work in the simple case. Deeper review and testing is needed though.

This issue was referenced by 523de7ae9b

This issue was referenced by 523de7ae9ba737faba76f46ee08d59a5507dc421
Author

@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:

  1. 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.

  2. 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

@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: 1. 2.80 was working without this issue all the way through to 2.80.74 (8b2b79c2108b) . The version released on June 6th after the one I just mentioned is where the "undo" became out of sync. 2. 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.

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.
Author

@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

@Sergey Sorry for the misunderstanding. I read the message you posted here, *“I updated patch at [P1048](https://archive.blender.org/developer/P1048.txt). 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'

Changed status from 'Open' to: 'Resolved'
Sign in to join this conversation.
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
5 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#66325
No description provided.