diff options
author | Martijn Versteegh <Baardaap> | 2020-09-22 11:27:58 +0300 |
---|---|---|
committer | Bastien Montagne <bastien@blender.org> | 2020-09-22 11:30:05 +0300 |
commit | c5efd0939f72eea1547812ccc0adb3c594734de2 (patch) | |
tree | ef71321675ae5f77fc3ee4f664ebec18f98a25ed /source/blender/editors/sculpt_paint | |
parent | af7378bf2aed56494e86280f716612b02de6b7e8 (diff) |
Fix T80859: Thread safe pixeldata concersion from float to uchar and v.v.
I created a bugreport T80859 earlier.
The problem is in the file source/blender/editors/sculpt_paint/paint_image_proj.c (line numbers against commit d376aea61)
at the lines 5309 and 5320 the functions `IMB_rect_from_float(...)` and `IMB_float_from_rect(...)` are called from threaded code on a shared ibuf.
However, ps->reproject_ibuf is shared between threads, so this is not thread-safe.
This is a race condition and leads to wrong output when projecting float data onto a uchar texture or vice versa on a multithreaded system.
The checks on line 5308 and 5319 are already a race condition.
I created this patch which fixes the problem , but I'm not sure if I'm overlooking something.
This makes sure reproject_ibuf contains the correct formats *before* the threadpool is started.
I replaced the original location of the conversions with BLI_asserts(). That may be overkill?
Reviewed By: #sculpt_paint_texture, mont29
Maniphest Tasks: T80859
Differential Revision: https://developer.blender.org/D8936
Diffstat (limited to 'source/blender/editors/sculpt_paint')
-rw-r--r-- | source/blender/editors/sculpt_paint/paint_image_proj.c | 41 |
1 files changed, 33 insertions, 8 deletions
diff --git a/source/blender/editors/sculpt_paint/paint_image_proj.c b/source/blender/editors/sculpt_paint/paint_image_proj.c index bc34efc44a2..fbc99e5aa57 100644 --- a/source/blender/editors/sculpt_paint/paint_image_proj.c +++ b/source/blender/editors/sculpt_paint/paint_image_proj.c @@ -5305,10 +5305,7 @@ static void do_projectpaint_thread(TaskPool *__restrict UNUSED(pool), void *ph_v } else { if (is_floatbuf) { - if (UNLIKELY(ps->reproject_ibuf->rect_float == NULL)) { - IMB_float_from_rect(ps->reproject_ibuf); - ps->reproject_ibuf_free_float = true; - } + BLI_assert(ps->reproject_ibuf->rect_float != NULL); bicubic_interpolation_color(ps->reproject_ibuf, NULL, @@ -5325,10 +5322,7 @@ static void do_projectpaint_thread(TaskPool *__restrict UNUSED(pool), void *ph_v } } else { - if (UNLIKELY(ps->reproject_ibuf->rect == NULL)) { - IMB_rect_from_float(ps->reproject_ibuf); - ps->reproject_ibuf_free_uchar = true; - } + BLI_assert(ps->reproject_ibuf->rect != NULL); bicubic_interpolation_color(ps->reproject_ibuf, projPixel->newColor.ch, @@ -5577,6 +5571,37 @@ static bool project_paint_op(void *state, const float lastpos[2], const float po image_pool = BKE_image_pool_new(); + if (!ELEM(ps->source, PROJ_SRC_VIEW, PROJ_SRC_VIEW_FILL)) { + /*This means we are reprojecting an image, + make sure the image has the needed data available.*/ + bool float_dest = false; + bool uchar_dest = false; + /* check if the destination images are float or uchar */ + for (i = 0; i < ps->image_tot; i++) { + if (ps->projImages[i].ibuf->rect != NULL) { + uchar_dest = true; + } + if (ps->projImages[i].ibuf->rect_float) { + float_dest = true; + } + } + + /* generate missing data if needed */ + if (float_dest && ps->reproject_ibuf->rect_float == NULL) + { + IMB_float_from_rect(ps->reproject_ibuf); + ps->reproject_ibuf_free_float = true; + } + if (uchar_dest && ps->reproject_ibuf->rect == NULL) + { + IMB_rect_from_float(ps->reproject_ibuf); + ps->reproject_ibuf_free_uchar = true; + } + + } + + + /* get the threads running */ for (a = 0; a < ps->thread_tot; a++) { |