Fix T71759: Sculpt/Vertex/Weight Paint Brush Size Gets Undone After Undoing a Stroke.

Add code preserving scene's toolsettings accross undo.

IDPointers are dealt with special care, we try to keep existing ones for
some (like brushes) when possible.

Note that this covers ToolSettings, Brushes and Palettes currently.

I'm not especially happy about how this new code mixes with existing
'foreach_id' one, in particular in scene. But cannot think of a better,
more generic way to do it currently.

Maniphest Tasks: T71759

Differential Revision: https://developer.blender.org/D9311
This commit is contained in:
Bastien Montagne 2020-11-03 12:09:00 +01:00
parent 5610ccdc08
commit bc0a6b0400
Notes: blender-bot 2024-01-22 17:34:00 +01:00
Referenced by issue #82805, Undo in Sculpt Mode
Referenced by issue #71759, Sculpt/Vertex/Weight Paint Brush Size Gets Undone After Undoing a Stroke
Referenced by pull request #117417, Fix undo overreach in various paint modes for #69760 and related.
3 changed files with 238 additions and 29 deletions

View File

@ -355,6 +355,37 @@ static void brush_blend_read_expand(BlendExpander *expander, ID *id)
}
}
static int brush_undo_preserve_cb(LibraryIDLinkCallbackData *cb_data)
{
BlendLibReader *reader = cb_data->user_data;
ID *id_old = *cb_data->id_pointer;
/* Old data has not been remapped to new values of the pointers, if we want to keep the old
* pointer here we need its new address. */
ID *id_old_new = id_old != NULL ? BLO_read_get_new_id_address(reader, id_old->lib, id_old) :
NULL;
BLI_assert(id_old_new == NULL || ELEM(id_old, id_old_new, id_old_new->orig_id));
if (cb_data->cb_flag & IDWALK_CB_USER) {
id_us_plus_no_lib(id_old_new);
id_us_min(id_old);
}
*cb_data->id_pointer = id_old_new;
return IDWALK_RET_NOP;
}
static void brush_undo_preserve(BlendLibReader *reader, ID *id_new, ID *id_old)
{
/* Whole Brush is preserved accross undo's. */
BKE_lib_id_swap(NULL, id_new, id_old);
/* `id_new` now has content from `id_old`, we need to ensure those old ID pointers are valid.
* Note: Since we want to re-use all old pointers here, code is much simpler than for Scene. */
BKE_library_foreach_ID_link(NULL, id_new, brush_undo_preserve_cb, reader, IDWALK_NOP);
/* Note: We do not swap IDProperties, as dealing with potential ID pointers in those would be
* fairly delicate. */
SWAP(IDProperty *, id_new->properties, id_old->properties);
}
IDTypeInfo IDType_ID_BR = {
.id_code = ID_BR,
.id_filter = FILTER_ID_BR,
@ -377,7 +408,7 @@ IDTypeInfo IDType_ID_BR = {
.blend_read_lib = brush_blend_read_lib,
.blend_read_expand = brush_blend_read_expand,
.blend_read_undo_preserve = NULL,
.blend_read_undo_preserve = brush_undo_preserve,
};
static RNG *brush_rng;

View File

@ -125,6 +125,16 @@ static void palette_blend_read_data(BlendDataReader *reader, ID *id)
BLO_read_list(reader, &palette->colors);
}
static void palette_undo_preserve(BlendLibReader *UNUSED(reader), ID *id_new, ID *id_old)
{
/* Whole Palette is preserved accross undo's, and it has no extra pointer, simple. */
/* Note: We do not care about potential internal references to self here, Palette has none. */
/* Note: We do not swap IDProperties, as dealing with potential ID pointers in those would be
* fairly delicate. */
BKE_lib_id_swap(NULL, id_new, id_old);
SWAP(IDProperty *, id_new->properties, id_old->properties);
}
IDTypeInfo IDType_ID_PAL = {
.id_code = ID_PAL,
.id_filter = FILTER_ID_PAL,
@ -147,7 +157,7 @@ IDTypeInfo IDType_ID_PAL = {
.blend_read_lib = NULL,
.blend_read_expand = NULL,
.blend_read_undo_preserve = NULL,
.blend_read_undo_preserve = palette_undo_preserve,
};
static void paint_curve_copy_data(Main *UNUSED(bmain),

View File

@ -457,53 +457,214 @@ static void scene_foreach_rigidbodyworldSceneLooper(struct RigidBodyWorld *UNUSE
BKE_lib_query_foreachid_process(data, id_pointer, cb_flag);
}
static void scene_foreach_paint(LibraryForeachIDData *data, Paint *paint)
/* This code is shared by both the regular foreach_id looper, and the code trying to restore or
* preserve ID pointers like brushes accross undos. */
typedef enum eSceneForeachUndoPreserveProcess {
/* Undo when preserving toolsettings from old scene, we also want to try to preserve that ID
* pointer from its old scene's value. */
SCENE_FOREACH_UNDO_RESTORE,
/* Undo when preserving toolsettings from old scene, we want to keep the new value of that ID
* pointer. */
SCENE_FOREACH_UNDO_NO_RESTORE,
} eSceneForeachUndoPreserveProcess;
static void scene_foreach_toolsettings_id_pointer_process(
ID **id_p,
const eSceneForeachUndoPreserveProcess action,
BlendLibReader *reader,
ID **id_old_p,
const uint cb_flag)
{
BKE_LIB_FOREACHID_PROCESS(data, paint->brush, IDWALK_CB_USER);
for (int i = 0; i < paint->tool_slots_len; i++) {
BKE_LIB_FOREACHID_PROCESS(data, paint->tool_slots[i].brush, IDWALK_CB_USER);
switch (action) {
case SCENE_FOREACH_UNDO_RESTORE: {
ID *id_old = *id_old_p;
/* Old data has not been remapped to new values of the pointers, if we want to keep the old
* pointer here we need its new address. */
ID *id_old_new = id_old != NULL ? BLO_read_get_new_id_address(reader, id_old->lib, id_old) :
NULL;
if (id_old_new != NULL) {
BLI_assert(ELEM(id_old, id_old_new, id_old_new->orig_id));
*id_old_p = id_old_new;
if (cb_flag & IDWALK_CB_USER) {
id_us_plus_no_lib(id_old_new);
id_us_min(id_old);
}
break;
}
/* We failed to find a new valid pointer for the previous ID, just keep the current one as
* if we had been under SCENE_FOREACH_UNDO_NO_RESTORE case. */
SWAP(ID *, *id_p, *id_old_p);
break;
}
case SCENE_FOREACH_UNDO_NO_RESTORE:
/* Counteract the swap of the whole ToolSettings container struct. */
SWAP(ID *, *id_p, *id_old_p);
break;
}
BKE_LIB_FOREACHID_PROCESS(data, paint->palette, IDWALK_CB_USER);
}
static void scene_foreach_toolsettings(LibraryForeachIDData *data, ToolSettings *toolsett)
{
BKE_LIB_FOREACHID_PROCESS(data, toolsett->particle.scene, IDWALK_CB_NOP);
BKE_LIB_FOREACHID_PROCESS(data, toolsett->particle.object, IDWALK_CB_NOP);
BKE_LIB_FOREACHID_PROCESS(data, toolsett->particle.shape_object, IDWALK_CB_NOP);
#define BKE_LIB_FOREACHID_UNDO_PRESERVE_PROCESS( \
__data, __id, __do_undo_restore, __action, __reader, __id_old, __cb_flag) \
{ \
if (__do_undo_restore) { \
scene_foreach_toolsettings_id_pointer_process( \
(ID **)&(__id), __action, __reader, (ID **)&(__id_old), __cb_flag); \
} \
else { \
BKE_LIB_FOREACHID_PROCESS(__data, __id, __cb_flag); \
} \
} \
(void)0
scene_foreach_paint(data, &toolsett->imapaint.paint);
BKE_LIB_FOREACHID_PROCESS(data, toolsett->imapaint.stencil, IDWALK_CB_USER);
BKE_LIB_FOREACHID_PROCESS(data, toolsett->imapaint.clone, IDWALK_CB_USER);
BKE_LIB_FOREACHID_PROCESS(data, toolsett->imapaint.canvas, IDWALK_CB_USER);
static void scene_foreach_paint(LibraryForeachIDData *data,
Paint *paint,
const bool do_undo_restore,
BlendLibReader *reader,
Paint *paint_old)
{
BKE_LIB_FOREACHID_UNDO_PRESERVE_PROCESS(data,
paint->brush,
do_undo_restore,
SCENE_FOREACH_UNDO_RESTORE,
reader,
paint_old->brush,
IDWALK_CB_USER);
for (int i = 0; i < paint_old->tool_slots_len; i++) {
/* This is a bit tricky.
* - In case we do not do `undo_restore`, `paint` and `paint_old` pointers are the same, so
* this is equivalent to simply looping over slots from `paint`.
* - In case we do `undo_restore`, we only want to consider the slots from the old one, since
* those are the one we keep in the end.
* + In case the new data has less valid slots, we feed in a dummy NULL pointer.
* + In case the new data has more valid slots, the extra ones are ignored.
*/
Brush *brush_tmp = NULL;
Brush **brush_p = i < paint->tool_slots_len ? &paint->tool_slots[i].brush : &brush_tmp;
BKE_LIB_FOREACHID_UNDO_PRESERVE_PROCESS(data,
*brush_p,
do_undo_restore,
SCENE_FOREACH_UNDO_RESTORE,
reader,
paint_old->brush,
IDWALK_CB_USER);
}
BKE_LIB_FOREACHID_UNDO_PRESERVE_PROCESS(data,
paint->palette,
do_undo_restore,
SCENE_FOREACH_UNDO_RESTORE,
reader,
paint_old->palette,
IDWALK_CB_USER);
}
static void scene_foreach_toolsettings(LibraryForeachIDData *data,
ToolSettings *toolsett,
const bool do_undo_restore,
BlendLibReader *reader,
ToolSettings *toolsett_old)
{
BKE_LIB_FOREACHID_UNDO_PRESERVE_PROCESS(data,
toolsett->particle.scene,
do_undo_restore,
SCENE_FOREACH_UNDO_NO_RESTORE,
reader,
toolsett_old->particle.scene,
IDWALK_CB_NOP);
BKE_LIB_FOREACHID_UNDO_PRESERVE_PROCESS(data,
toolsett->particle.object,
do_undo_restore,
SCENE_FOREACH_UNDO_NO_RESTORE,
reader,
toolsett_old->particle.object,
IDWALK_CB_NOP);
BKE_LIB_FOREACHID_UNDO_PRESERVE_PROCESS(data,
toolsett->particle.shape_object,
do_undo_restore,
SCENE_FOREACH_UNDO_NO_RESTORE,
reader,
toolsett_old->particle.shape_object,
IDWALK_CB_NOP);
scene_foreach_paint(
data, &toolsett->imapaint.paint, do_undo_restore, reader, &toolsett_old->imapaint.paint);
BKE_LIB_FOREACHID_UNDO_PRESERVE_PROCESS(data,
toolsett->imapaint.stencil,
do_undo_restore,
SCENE_FOREACH_UNDO_RESTORE,
reader,
toolsett_old->imapaint.stencil,
IDWALK_CB_USER);
BKE_LIB_FOREACHID_UNDO_PRESERVE_PROCESS(data,
toolsett->imapaint.clone,
do_undo_restore,
SCENE_FOREACH_UNDO_RESTORE,
reader,
toolsett_old->imapaint.clone,
IDWALK_CB_USER);
BKE_LIB_FOREACHID_UNDO_PRESERVE_PROCESS(data,
toolsett->imapaint.canvas,
do_undo_restore,
SCENE_FOREACH_UNDO_RESTORE,
reader,
toolsett_old->imapaint.canvas,
IDWALK_CB_USER);
if (toolsett->vpaint) {
scene_foreach_paint(data, &toolsett->vpaint->paint);
scene_foreach_paint(
data, &toolsett->vpaint->paint, do_undo_restore, reader, &toolsett_old->vpaint->paint);
}
if (toolsett->wpaint) {
scene_foreach_paint(data, &toolsett->wpaint->paint);
scene_foreach_paint(
data, &toolsett->wpaint->paint, do_undo_restore, reader, &toolsett_old->wpaint->paint);
}
if (toolsett->sculpt) {
scene_foreach_paint(data, &toolsett->sculpt->paint);
BKE_LIB_FOREACHID_PROCESS(data, toolsett->sculpt->gravity_object, IDWALK_CB_NOP);
scene_foreach_paint(
data, &toolsett->sculpt->paint, do_undo_restore, reader, &toolsett_old->sculpt->paint);
BKE_LIB_FOREACHID_UNDO_PRESERVE_PROCESS(data,
toolsett->sculpt->gravity_object,
do_undo_restore,
SCENE_FOREACH_UNDO_NO_RESTORE,
reader,
toolsett_old->sculpt->gravity_object,
IDWALK_CB_NOP);
}
if (toolsett->uvsculpt) {
scene_foreach_paint(data, &toolsett->uvsculpt->paint);
scene_foreach_paint(
data, &toolsett->uvsculpt->paint, do_undo_restore, reader, &toolsett_old->uvsculpt->paint);
}
if (toolsett->gp_paint) {
scene_foreach_paint(data, &toolsett->gp_paint->paint);
scene_foreach_paint(
data, &toolsett->gp_paint->paint, do_undo_restore, reader, &toolsett_old->gp_paint->paint);
}
if (toolsett->gp_vertexpaint) {
scene_foreach_paint(data, &toolsett->gp_vertexpaint->paint);
scene_foreach_paint(data,
&toolsett->gp_vertexpaint->paint,
do_undo_restore,
reader,
&toolsett_old->gp_vertexpaint->paint);
}
if (toolsett->gp_sculptpaint) {
scene_foreach_paint(data, &toolsett->gp_sculptpaint->paint);
scene_foreach_paint(data,
&toolsett->gp_sculptpaint->paint,
do_undo_restore,
reader,
&toolsett_old->gp_sculptpaint->paint);
}
if (toolsett->gp_weightpaint) {
scene_foreach_paint(data, &toolsett->gp_weightpaint->paint);
scene_foreach_paint(data,
&toolsett->gp_weightpaint->paint,
do_undo_restore,
reader,
&toolsett_old->gp_weightpaint->paint);
}
BKE_LIB_FOREACHID_PROCESS(data, toolsett->gp_sculpt.guide.reference_object, IDWALK_CB_NOP);
BKE_LIB_FOREACHID_UNDO_PRESERVE_PROCESS(data,
toolsett->gp_sculpt.guide.reference_object,
do_undo_restore,
SCENE_FOREACH_UNDO_NO_RESTORE,
reader,
toolsett_old->gp_sculpt.guide.reference_object,
IDWALK_CB_NOP);
}
static void scene_foreach_layer_collection(LibraryForeachIDData *data, ListBase *lb)
@ -595,7 +756,7 @@ static void scene_foreach_id(ID *id, LibraryForeachIDData *data)
ToolSettings *toolsett = scene->toolsettings;
if (toolsett) {
scene_foreach_toolsettings(data, toolsett);
scene_foreach_toolsettings(data, toolsett, false, NULL, toolsett);
}
if (scene->rigidbody_world) {
@ -622,13 +783,20 @@ static void scene_foreach_cache(ID *id,
user_data);
}
static void scene_undo_preserve(BlendLibReader *UNUSED(reader), ID *id_new, ID *id_old)
static void scene_undo_preserve(BlendLibReader *reader, ID *id_new, ID *id_old)
{
Scene *scene_new = (Scene *)id_new;
Scene *scene_old = (Scene *)id_old;
SWAP(View3DCursor, scene_old->cursor, scene_new->cursor);
/* TODO: handle tool settings here too. */
if (scene_new->toolsettings != NULL && scene_old->toolsettings != NULL) {
/* First try to restore ID pointers that can be and should be preserved (like brushes or
* palettes), and counteract the swap of the whole ToolSettings structs below for the others
* (like object ones). */
scene_foreach_toolsettings(
NULL, scene_new->toolsettings, true, reader, scene_old->toolsettings);
SWAP(ToolSettings, *scene_old->toolsettings, *scene_new->toolsettings);
}
}
IDTypeInfo IDType_ID_SCE = {