From c9ffd4241727dadf9ceef2c2837dfd29ad54fa76 Mon Sep 17 00:00:00 2001 From: Sean Kim Date: Fri, 3 Jan 2025 15:06:24 +0100 Subject: [PATCH] Fix #132530: Undo after Apply Base doesn't correctly restore data With the recent sculpt refactor project prior to 4.3, the Sculpt undo system was changed such that an individual `SculptUndoStep` would store a single type on the `StepData` struct instead of storing individual nodes that each had their own type. This works for most operators supported by Sculpt Mode, however, the `Apply Base` multires operator was dependent on needing to store both Geometry and Position data. To restore old functionality, this commit removes the need for storing this Position data by instead forcing the object space Sculpt Mode multires data to be flushed to the tangent space MDisp data prior to saving the current geometry state so that it can be restored correctly. This has the following benefits: * We can continue to assume that Geometry steps represent full-mesh changes and should require no further specialized processing. * This better aligns with the future state of trying to flush this multires data on a per-stroke basis instead of needing to wait unti lthe user either quits or exits sculpt mode. Pull Request: https://projects.blender.org/blender/blender/pulls/132569 --- .../editors/sculpt_paint/sculpt_undo.cc | 35 +++---------------- 1 file changed, 4 insertions(+), 31 deletions(-) diff --git a/source/blender/editors/sculpt_paint/sculpt_undo.cc b/source/blender/editors/sculpt_paint/sculpt_undo.cc index c9be1f739e4..c106550e96b 100644 --- a/source/blender/editors/sculpt_paint/sculpt_undo.cc +++ b/source/blender/editors/sculpt_paint/sculpt_undo.cc @@ -234,7 +234,6 @@ struct StepData { * the object when undoing the operation * * Modified geometry is stored after the modification and is used to redo the modification. */ - bool geometry_clear_pbvh; NodeGeometry geometry_original; NodeGeometry geometry_modified; @@ -722,10 +721,8 @@ static void geometry_free_data(NodeGeometry *geometry) static void restore_geometry(StepData &step_data, Object &object) { - if (step_data.geometry_clear_pbvh) { - BKE_sculptsession_free_pbvh(object); - DEG_id_tag_update(&object.id, ID_RECALC_GEOMETRY); - } + BKE_sculptsession_free_pbvh(object); + DEG_id_tag_update(&object.id, ID_RECALC_GEOMETRY); Mesh *mesh = static_cast(object.data); @@ -1290,7 +1287,6 @@ static void geometry_push(const Object &object) step_data->type = Type::Geometry; step_data->applied = false; - step_data->geometry_clear_pbvh = true; NodeGeometry *geometry = geometry_get(*step_data); store_geometry_data(geometry, object); @@ -2134,26 +2130,6 @@ static bool use_multires_mesh(bContext *C) return sculpt_session->multires.active; } -static void push_all_grids(const Depsgraph &depsgraph, Object *object) -{ - /* It is possible that undo push is done from an object state where there is no tree. This - * happens, for example, when an operation which tagged for geometry update was performed prior - * to the current operation without making any stroke in between. - * - * Skip pushing nodes based on the following logic: on redo Type::Position will - * ensure pbvh::Tree for the new base geometry, which will have same coordinates as if we create - * pbvh::Tree here. - */ - const bke::pbvh::Tree *pbvh = bke::object::pbvh_get(*object); - if (pbvh == nullptr) { - return; - } - - IndexMaskMemory memory; - const IndexMask node_mask = bke::pbvh::all_leaf_nodes(*pbvh, memory); - push_nodes(depsgraph, *object, node_mask, Type::Position); -} - void push_multires_mesh_begin(bContext *C, const char *str) { if (!use_multires_mesh(C)) { @@ -2161,15 +2137,13 @@ void push_multires_mesh_begin(bContext *C, const char *str) } const Scene &scene = *CTX_data_scene(C); - const Depsgraph &depsgraph = *CTX_data_depsgraph_pointer(C); Object *object = CTX_data_active_object(C); + multires_flush_sculpt_updates(object); + push_begin_ex(scene, *object, str); geometry_push(*object); - get_step_data()->geometry_clear_pbvh = false; - - push_all_grids(depsgraph, object); } void push_multires_mesh_end(bContext *C, const char *str) @@ -2182,7 +2156,6 @@ void push_multires_mesh_end(bContext *C, const char *str) Object *object = CTX_data_active_object(C); geometry_push(*object); - get_step_data()->geometry_clear_pbvh = false; push_end(*object); }