Crash undo painting on linked image #79373

New Issue

Yevgeny Makarov · 2020-07-29T19:08:24+02:00

Yevgeny Makarov commented

2020-07-29 19:08:24 +02:00

System Information
Operating system: Darwin-19.5.0-x86_64-i386-64bit 64 Bits
Graphics card: AMD Radeon Pro 455 OpenGL Engine ATI Technologies Inc. 4.1 ATI-3.9.15

Blender Version
Broken: 2.91.0 Alpha
Broken: 2.81a
Worked: 2.79b

Short description of error
Crash when undoing painting on a linked image.

Exact steps for others to reproduce the error

open 'file.blend'
paint on the image (linked from 'lib.blend')
ctrl-z

lib.blend

file.blend

# backtrace
0   Blender                             0x000000010aef65a7 BLI_system_backtrace + 55
1   Blender                             0x000000010a0cbbf3 sig_handle_crash + 355
2   libsystem_platform.dylib            0x00007fff715a75fd _sigtramp + 29
3   libsystem_kernel.dylib              0x00007fff714f0170 mach_msg + 60
4   libsystem_c.dylib                   0x00007fff71440891 __vfprintf + 5379
5   libsystem_c.dylib                   0x00007fff71466ad3 __v2printf + 475
6   libsystem_c.dylib                   0x00007fff7144cee7 _vsnprintf + 417
7   libsystem_c.dylib                   0x00007fff7144cf90 vsnprintf + 68
8   Blender                             0x000000010aabbf5b CLG_logf + 2827
9   Blender                             0x000000010a9b77e4 uhandle_restore_list + 756
10  Blender                             0x000000010a9b6afc image_undosys_step_decode + 188
11  Blender                             0x000000010a3a4f1f undosys_step_decode + 271
12  Blender                             0x000000010a3a4d11 BKE_undosys_step_undo_with_data_ex + 257
13  Blender                             0x000000010a9718cf ed_undo_step_impl + 735
14  Blender                             0x000000010a9706c2 ed_undo_exec + 50
15  Blender                             0x000000010a0d71ab wm_operator_invoke + 411
16  Blender                             0x000000010a0d8809 wm_handler_operator_call + 633
17  Blender                             0x000000010a0d7ec6 wm_handlers_do_intern + 1814
18  Blender                             0x000000010a0d35ff wm_handlers_do + 31
19  Blender                             0x000000010a0d31ed wm_event_do_handlers + 3373
20  Blender                             0x000000010a0cc710 WM_main + 32
21  Blender                             0x000000010a0c85f7 main + 759
22  libdyld.dylib                       0x00007fff713aecc9 start + 1

**System Information** Operating system: Darwin-19.5.0-x86_64-i386-64bit 64 Bits Graphics card: AMD Radeon Pro 455 OpenGL Engine ATI Technologies Inc. 4.1 ATI-3.9.15 **Blender Version** Broken: 2.91.0 Alpha Broken: 2.81a Worked: 2.79b **Short description of error** Crash when undoing painting on a linked image. **Exact steps for others to reproduce the error** - open 'file.blend' - paint on the image (linked from 'lib.blend') - ctrl-z [lib.blend](https://archive.blender.org/developer/F8728998/lib.blend) [file.blend](https://archive.blender.org/developer/F8728999/file.blend) ``` # backtrace 0 Blender 0x000000010aef65a7 BLI_system_backtrace + 55 1 Blender 0x000000010a0cbbf3 sig_handle_crash + 355 2 libsystem_platform.dylib 0x00007fff715a75fd _sigtramp + 29 3 libsystem_kernel.dylib 0x00007fff714f0170 mach_msg + 60 4 libsystem_c.dylib 0x00007fff71440891 __vfprintf + 5379 5 libsystem_c.dylib 0x00007fff71466ad3 __v2printf + 475 6 libsystem_c.dylib 0x00007fff7144cee7 _vsnprintf + 417 7 libsystem_c.dylib 0x00007fff7144cf90 vsnprintf + 68 8 Blender 0x000000010aabbf5b CLG_logf + 2827 9 Blender 0x000000010a9b77e4 uhandle_restore_list + 756 10 Blender 0x000000010a9b6afc image_undosys_step_decode + 188 11 Blender 0x000000010a3a4f1f undosys_step_decode + 271 12 Blender 0x000000010a3a4d11 BKE_undosys_step_undo_with_data_ex + 257 13 Blender 0x000000010a9718cf ed_undo_step_impl + 735 14 Blender 0x000000010a9706c2 ed_undo_exec + 50 15 Blender 0x000000010a0d71ab wm_operator_invoke + 411 16 Blender 0x000000010a0d8809 wm_handler_operator_call + 633 17 Blender 0x000000010a0d7ec6 wm_handlers_do_intern + 1814 18 Blender 0x000000010a0d35ff wm_handlers_do + 31 19 Blender 0x000000010a0d31ed wm_event_do_handlers + 3373 20 Blender 0x000000010a0cc710 WM_main + 32 21 Blender 0x000000010a0c85f7 main + 759 22 libdyld.dylib 0x00007fff713aecc9 start + 1 ```

Yevgeny Makarov commented

2020-07-29 19:08:25 +02:00

Added subscriber: @jenkm

Philipp Oeser commented

2020-07-30 12:19:25 +02:00

Added subscribers: @mont29, @ideasman42, @lichtwerk

Philipp Oeser commented

2020-07-30 12:19:25 +02:00

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

Philipp Oeser commented

2020-07-30 12:19:25 +02:00

Can confirm the behavior.

on stroke:

uhandle_add seems to be doing its thing correctly (adding the image_ref.ptr)
undosys_id_ref_store seems to be doing its thing correctly (adding the image_ref.name - but NULLing the ptr again)
on undo:
undosys_id_ref_resolve checks for the id being linked though (here ) and prevents setting the ptr again.
uhandle_restore_list crashes because of that [missing image_ref.ptr]

Taking a step back, I am wondering how useful it is to paint on linked images?

In the file provided, the linked image is generated(packed), painting on this will never make it back (saving) into the lib file
if the image would not be packed, then I can see workflows that make sense doing this on linked images, saving would then work.

Thinking of possible solutions:

prevent painting on linked images all together [could probably be done in some poll]
allow undosys_id_ref_resolve to store the pointer (even if image is linked -- exception just for images) [see snippet below]
just prevent the crash [see snippet below]
workflow alternative: create an override for the linked image (undo then works -- we should still prevent the crash though)

P1555: T79373_snippet



diff --git a/source/blender/blenkernel/intern/undo_system.c b/source/blender/blenkernel/intern/undo_system.c
index 0809e8dda6d..4719ec6bae9 100644
--- a/source/blender/blenkernel/intern/undo_system.c
+++ b/source/blender/blenkernel/intern/undo_system.c
@@ -144,7 +144,9 @@ static void undosys_id_ref_resolve(void *user_data, UndoRefID *id_ref)
   Main *bmain = user_data;
   ListBase *lb = which_libbase(bmain, GS(id_ref->name));
   LISTBASE_FOREACH (ID *, id, lb) {
-    if (STREQ(id_ref->name, id->name) && (id->lib == NULL)) {
+    /* Allow for undoing painting on linked images. */
+    const bool is_image = (GS(id_ref->name) == ID_IM);
+    if (STREQ(id_ref->name, id->name) && (id->lib == NULL || is_image)) {
       id_ref->ptr = id;
       break;
     }
diff --git a/source/blender/editors/space_image/image_undo.c b/source/blender/editors/space_image/image_undo.c
index 27b84307f7d..5215b714dec 100644
--- a/source/blender/editors/space_image/image_undo.c
+++ b/source/blender/editors/space_image/image_undo.c
@@ -544,7 +544,11 @@ static void uhandle_restore_list(ListBase *undo_handles, bool use_init)
   LISTBASE_FOREACH (UndoImageHandle *, uh, undo_handles) {
     /* Tiles only added to second set of tiles. */
     Image *image = uh->image_ref.ptr;
-
+    /* Prevent crash undoing painting on linked images. */
+    if (image == NULL) {
+      CLOG_ERROR(&LOG, "Unable to get image for UndoImageHandle '%s' (might be linked data?)", uh->image_ref.name);
+      continue;
+    }
     ImBuf *ibuf = BKE_image_acquire_ibuf(image, &uh->iuser, NULL);
     if (UNLIKELY(ibuf == NULL)) {
       CLOG_ERROR(&LOG, "Unable to get buffer for image '%s'", image->id.name + 2);

Since this probably came with 651b8fb14e, I would kindly ask @ideasman42 for advice here.
Also @mont29 might have an opinion here?

Can confirm the behavior. on stroke: - `uhandle_add` seems to be doing its thing correctly (adding the `image_ref.ptr`) - `undosys_id_ref_store` seems to be doing its thing correctly (adding the `image_ref.name` - but NULLing the ptr again) on undo: - `undosys_id_ref_resolve` checks for the id being linked though ([here ](https://developer.blender.org/diffusion/B/browse/master/source/blender/blenkernel/intern/undo_system.c$147)) and prevents setting the ptr again. - `uhandle_restore_list` crashes because of that [missing image_ref.ptr] Taking a step back, I am wondering how useful it is to paint on linked images? - In the file provided, the linked image is generated(packed), painting on this will never make it back (saving) into the lib file - if the image would not be packed, then I can see workflows that make sense doing this on linked images, saving would then work. Thinking of possible solutions: - prevent painting on linked images all together [could probably be done in some poll] - allow `undosys_id_ref_resolve` to store the pointer (even if image is linked -- exception just for images) [see snippet below] - just prevent the crash [see snippet below] - workflow alternative: create an override for the linked image (undo then works -- we should still prevent the crash though) [P1555: T79373_snippet](https://archive.blender.org/developer/P1555.txt) ``` diff --git a/source/blender/blenkernel/intern/undo_system.c b/source/blender/blenkernel/intern/undo_system.c index 0809e8dda6d..4719ec6bae9 100644 --- a/source/blender/blenkernel/intern/undo_system.c +++ b/source/blender/blenkernel/intern/undo_system.c @@ -144,7 +144,9 @@ static void undosys_id_ref_resolve(void *user_data, UndoRefID *id_ref) Main *bmain = user_data; ListBase *lb = which_libbase(bmain, GS(id_ref->name)); LISTBASE_FOREACH (ID *, id, lb) { - if (STREQ(id_ref->name, id->name) && (id->lib == NULL)) { + /* Allow for undoing painting on linked images. */ + const bool is_image = (GS(id_ref->name) == ID_IM); + if (STREQ(id_ref->name, id->name) && (id->lib == NULL || is_image)) { id_ref->ptr = id; break; } diff --git a/source/blender/editors/space_image/image_undo.c b/source/blender/editors/space_image/image_undo.c index 27b84307f7d..5215b714dec 100644 --- a/source/blender/editors/space_image/image_undo.c +++ b/source/blender/editors/space_image/image_undo.c @@ -544,7 +544,11 @@ static void uhandle_restore_list(ListBase *undo_handles, bool use_init) LISTBASE_FOREACH (UndoImageHandle *, uh, undo_handles) { /* Tiles only added to second set of tiles. */ Image *image = uh->image_ref.ptr; - + /* Prevent crash undoing painting on linked images. */ + if (image == NULL) { + CLOG_ERROR(&LOG, "Unable to get image for UndoImageHandle '%s' (might be linked data?)", uh->image_ref.name); + continue; + } ImBuf *ibuf = BKE_image_acquire_ibuf(image, &uh->iuser, NULL); if (UNLIKELY(ibuf == NULL)) { CLOG_ERROR(&LOG, "Unable to get buffer for image '%s'", image->id.name + 2); ``` Since this probably came with 651b8fb14e, I would kindly ask @ideasman42 for advice here. Also @mont29 might have an opinion here?

Bastien Montagne commented

2020-07-30 14:33:05 +02:00

Painting on linked image should not be allowed... at all.