Modernize (or Remove) GPencil Undo System #84703

New Issue

Bastien Montagne · 2021-01-14T11:25:09+01:00

Bastien Montagne commented

2021-01-14 11:25:09 +01:00

Current usage of ED_undo_gpencil_step from ed_undo_step_impl is logically broken in both undo_name and undo_index cases. It would only work (do something) in case undo_direction is valid, i.e. only when undoing/redoing one step (the ctrl-Z shortcut), not when undoing e.g. a step from the history.

However, this seems to not be an actual problem in practice, because from quick investigation, it looks like GPencil undo mode/code is never actually used (its global static undo_nodes list seems to always be empty when undoing or redoing, so ED_undo_gpencil_step never gets called).

What seems to happen is that undo_nodes is filled with one or two steps during interactive edit (stroke drawing etc.), but once the modal edit is done the whole undo_nodes is cleared and a regular memfile (aka global) undo step is stored instead.

This needs further investigation to fully assess current status of this code, and decide then whether it is worth integrating GPencil undo into modern undo system/stack (as hinted in current TODO comments in ed_undo_step_impl), or if we can get fully rid of it.

Current usage of `ED_undo_gpencil_step` from `ed_undo_step_impl` is logically broken in both `undo_name` and `undo_index` cases. It would only work (do something) in case `undo_direction` is valid, i.e. only when undoing/redoing one step (the `ctrl-Z` shortcut), not when undoing e.g. a step from the history. However, this seems to not be an actual problem in practice, because from quick investigation, it looks like GPencil undo mode/code is never actually used (its global static `undo_nodes` list seems to always be empty when undoing or redoing, so `ED_undo_gpencil_step` never gets called). What seems to happen is that `undo_nodes` is filled with one or two steps during interactive edit (stroke drawing etc.), but once the modal edit is done the whole `undo_nodes` is cleared and a regular memfile (aka global) undo step is stored instead. This needs further investigation to fully assess current status of this code, and decide then whether it is worth integrating GPencil undo into modern undo system/stack (as hinted in current TODO comments in `ed_undo_step_impl`), or if we can get fully rid of it.

Bastien Montagne commented

2021-01-14 11:25:09 +01:00

Changed status from 'Needs Triage' to: 'Confirmed'