[Bf-blender-cvs] [c5efd0939f7] master: Fix T80859: Thread safe pixeldata concersion from float to uchar and v.v.

Martijn Versteegh noreply at git.blender.org
Tue Sep 22 10:30:07 CEST 2020


Commit: c5efd0939f72eea1547812ccc0adb3c594734de2
Author: Martijn Versteegh
Date:   Tue Sep 22 10:27:58 2020 +0200
Branches: master
https://developer.blender.org/rBc5efd0939f72eea1547812ccc0adb3c594734de2

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

===================================================================

M	source/blender/editors/sculpt_paint/paint_image_proj.c

===================================================================

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



More information about the Bf-blender-cvs mailing list