From 8ca49d2e3a304a0134f3c36c81fa4cb2a95894f9 Mon Sep 17 00:00:00 2001 From: Alexander Gavrilov Date: Fri, 14 Jul 2023 17:04:52 +0300 Subject: [PATCH] Fix undo overreach in various paint modes for #69760 and related. This tries to fix undo overreach on selection of vertex groups, shape keys, uv maps, colors and attributes in various modes. - The Vertex Paint mode is now a subtype of Sculpt, so it should use a sculpt undo step on entering the mode, and property changes shouldn't write undo steps. - Sculpt undo modes were deliberately over-undoing color attribute selection changes for some reason. - The Weight Paint mode uses memfile, so it should save steps for property changes, except for those excluded from undo, e.g. by bc0a6b0400. - If a memfile undo step happens anyway for some reason in sculpt and vertex paint modes, immediately following property changes to object and mesh datablocks should also push to avoid over-undo if the immediately following scuplt step is reverted (unrelated ones like materials or scene seem safe). --- source/blender/editors/include/ED_undo.hh | 5 ++- .../editors/interface/interface_handlers.cc | 22 ++++++++--- .../editors/sculpt_paint/paint_vertex.cc | 12 ++++++ .../editors/sculpt_paint/sculpt_undo.cc | 3 -- source/blender/editors/undo/CMakeLists.txt | 3 ++ source/blender/editors/undo/ed_undo.cc | 39 ++++++++++++++++--- .../windowmanager/intern/wm_operators.cc | 2 +- 7 files changed, 70 insertions(+), 16 deletions(-) diff --git a/source/blender/editors/include/ED_undo.hh b/source/blender/editors/include/ED_undo.hh index 26ac9dbed01..76cb8b8580f 100644 --- a/source/blender/editors/include/ED_undo.hh +++ b/source/blender/editors/include/ED_undo.hh @@ -16,6 +16,7 @@ struct CLG_LogRef; struct Object; struct Scene; struct MemFile; +struct PropertyRNA; struct UndoStack; struct ViewLayer; struct bContext; @@ -71,7 +72,9 @@ bool ED_undo_is_memfile_compatible(const bContext *C); * For example, changing a brush property isn't stored by sculpt-mode undo steps. * This workaround is needed until the limitation is removed, see: #61948. */ -bool ED_undo_is_legacy_compatible_for_property(bContext *C, ID *id); +bool ED_undo_is_legacy_compatible_for_property(const bContext *C, + const ID *id, + const PropertyRNA *prop); /** * This function addresses the problem of restoring undo steps when multiple windows are used. diff --git a/source/blender/editors/interface/interface_handlers.cc b/source/blender/editors/interface/interface_handlers.cc index 4cf9ec14a02..8f4032267b9 100644 --- a/source/blender/editors/interface/interface_handlers.cc +++ b/source/blender/editors/interface/interface_handlers.cc @@ -958,8 +958,8 @@ static void ui_apply_but_undo(uiBut *but) } else { ID *id = but->rnapoin.owner_id; - if (!ED_undo_is_legacy_compatible_for_property(static_cast(but->block->evil_C), - id)) + if (!ED_undo_is_legacy_compatible_for_property( + static_cast(but->block->evil_C), id, but->rnaprop)) { skip_undo = true; } @@ -969,10 +969,20 @@ static void ui_apply_but_undo(uiBut *but) if (skip_undo == false) { /* XXX: disable all undo pushes from UI changes from sculpt mode as they cause memfile undo * steps to be written which cause lag: #71434. */ - if (BKE_paintmode_get_active_from_context(static_cast(but->block->evil_C)) == - PaintMode::Sculpt) - { - skip_undo = true; + bContext *C = static_cast(but->block->evil_C); + PaintMode paint_mode = BKE_paintmode_get_active_from_context(C); + + /* Paint modes that are based on the sculpt engine. */ + if (ELEM(paint_mode, PaintMode::Sculpt, PaintMode::Vertex)) { + /* If the last step already was a memfile save, still push for changes + * to sculptable datablock properties to avoid over-undo. */ + ID *id = but->rnapoin.owner_id; + UndoStep *undo_step = CTX_wm_manager(C)->undo_stack->step_active; + if (!id || !undo_step || undo_step->type != BKE_UNDOSYS_TYPE_MEMFILE || + !ELEM(GS(id->name), ID_ME, ID_OB)) + { + skip_undo = true; + } } } diff --git a/source/blender/editors/sculpt_paint/paint_vertex.cc b/source/blender/editors/sculpt_paint/paint_vertex.cc index ebccb4b6f21..e1c648faf45 100644 --- a/source/blender/editors/sculpt_paint/paint_vertex.cc +++ b/source/blender/editors/sculpt_paint/paint_vertex.cc @@ -836,6 +836,18 @@ static int vpaint_mode_toggle_exec(bContext *C, wmOperator *op) } ED_object_vpaintmode_enter_ex(bmain, depsgraph, scene, ob); BKE_paint_toolslots_brush_validate(bmain, &ts->vpaint->paint); + + /* Recheck the mode after entering it, and create a sculpt style undo step, + * same as in sculpt_mode_toggle_exec for Sculpt Mode. */ + if (ob->mode & mode_flag) { + /* Without this the memfile undo step is used, + * while it works it causes lag when undoing the first undo step, see #71564. */ + wmWindowManager *wm = CTX_wm_manager(C); + if (wm->op_undo_depth <= 1) { + undo::push_begin(ob, op); + undo::push_end(ob); + } + } } BKE_mesh_batch_cache_dirty_tag((Mesh *)ob->data, BKE_MESH_BATCH_DIRTY_ALL); diff --git a/source/blender/editors/sculpt_paint/sculpt_undo.cc b/source/blender/editors/sculpt_paint/sculpt_undo.cc index 031495509e0..04eb7072e73 100644 --- a/source/blender/editors/sculpt_paint/sculpt_undo.cc +++ b/source/blender/editors/sculpt_paint/sculpt_undo.cc @@ -1826,9 +1826,6 @@ static void sculpt_undosys_step_decode_undo(bContext *C, sculpt_undosys_step_decode_undo_impl(C, depsgraph, us_iter); if (us_iter == us) { - if (us_iter->step.prev && us_iter->step.prev->type == BKE_UNDOSYS_TYPE_SCULPT) { - set_active_layer(C, &((SculptUndoStep *)us_iter->step.prev)->active_color_end); - } break; } diff --git a/source/blender/editors/undo/CMakeLists.txt b/source/blender/editors/undo/CMakeLists.txt index 5c7a2d036ac..23846059f3e 100644 --- a/source/blender/editors/undo/CMakeLists.txt +++ b/source/blender/editors/undo/CMakeLists.txt @@ -11,6 +11,9 @@ set(INC ../../bmesh ../../makesrna ../../windowmanager + + # RNA_prototypes.h + ${CMAKE_BINARY_DIR}/source/blender/makesrna ) set(INC_SYS diff --git a/source/blender/editors/undo/ed_undo.cc b/source/blender/editors/undo/ed_undo.cc index a515081c2ad..49371bc3368 100644 --- a/source/blender/editors/undo/ed_undo.cc +++ b/source/blender/editors/undo/ed_undo.cc @@ -50,6 +50,7 @@ #include "RNA_access.hh" #include "RNA_define.hh" #include "RNA_enum_types.hh" +#include "RNA_prototypes.h" #include "UI_interface.hh" #include "UI_resources.hh" @@ -447,7 +448,30 @@ bool ED_undo_is_memfile_compatible(const bContext *C) return true; } -bool ED_undo_is_legacy_compatible_for_property(bContext *C, ID *id) +static bool ed_undo_skip_in_paint_mode(const ID *id, const PropertyRNA *prop) +{ + if (id == nullptr) { + return true; + } + /* Don't create undo steps for brush and tool settings. */ + switch (GS(id->name)) { + case ID_BR: + return true; + case ID_SCE: + if (prop == nullptr) { + return true; + } + /* Retain undo steps for outliner hide toggles, which also belong to Scene. */ + return BLI_findindex(RNA_struct_type_properties(&RNA_ObjectBase), prop) < 0 && + BLI_findindex(RNA_struct_type_properties(&RNA_LayerCollection), prop) < 0; + default: + return false; + } +} + +bool ED_undo_is_legacy_compatible_for_property(const bContext *C, + const ID *id, + const PropertyRNA *prop) { const Scene *scene = CTX_data_scene(C); ViewLayer *view_layer = CTX_data_view_layer(C); @@ -456,10 +480,15 @@ bool ED_undo_is_legacy_compatible_for_property(bContext *C, ID *id) Object *obact = BKE_view_layer_active_object_get(view_layer); if (obact != nullptr) { if (obact->mode & OB_MODE_ALL_PAINT) { - /* Don't store property changes when painting - * (only do undo pushes on brush strokes which each paint operator handles on its own). */ - CLOG_INFO(&LOG, 1, "skipping undo for paint-mode"); - return false; + /* Don't store tool property changes when painting. + * + * In Sculpt and Vertex Paint, most other property change undo pushes will be also + * suppressed by another check later in ui_apply_but_undo, unless preceeded by + * a memfile push; however these properties should always be blocked. */ + if (ed_undo_skip_in_paint_mode(id, prop)) { + CLOG_INFO(&LOG, 1, "skipping undo for paint-mode"); + return false; + } } if (obact->mode & OB_MODE_EDIT) { if ((id == nullptr) || (obact->data == nullptr) || diff --git a/source/blender/windowmanager/intern/wm_operators.cc b/source/blender/windowmanager/intern/wm_operators.cc index fbafb80c990..15a266cde40 100644 --- a/source/blender/windowmanager/intern/wm_operators.cc +++ b/source/blender/windowmanager/intern/wm_operators.cc @@ -3358,7 +3358,7 @@ static int radial_control_modal(bContext *C, wmOperator *op, const wmEvent *even wmWindowManager *wm = CTX_wm_manager(C); if (wm->op_undo_depth == 0) { ID *id = rc->ptr.owner_id; - if (ED_undo_is_legacy_compatible_for_property(C, id)) { + if (ED_undo_is_legacy_compatible_for_property(C, id, rc->prop)) { ED_undo_push(C, op->type->name); } } -- 2.30.2