griefith/test2
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix T80859: Thread safe pixeldata concersion from float to uchar and v.v.

Browse Source 
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
This commit is contained in:
Martijn Versteegh
2020-09-22 10:27:58 +02:00
committed by Bastien Montagne
parent af7378bf2a
commit c5efd0939f
1 changed files with 33 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

41
source/blender/editors/sculpt_paint/paint_image_proj.c
View File

@@ -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++) {