Page MenuHome

Thread-unsafe conversion of image data from float to char and v.v. (with patch)
Needs Triage, NormalPublic

Description

System Information
Operating system: n/a
Graphics card: n/a

Blender Version
Broken: at least 2.8, maybe earlier
Worked: n/a

Short description of error
When projection painting an image with float data onto an image with char data (or v.v.) the data gets converted in each thread without locking, leading to race conditions and wrong output.

Exact steps for others to reproduce the error
make sure you run on a multithreaded processor
-default startup
-set the base color of the material of the cube to image texture. create a new image texture using the default settings.
-set a camera background image from an .exr file containing float data.
-select cube, go to texture paint mode.
-from options -> external choose apply camera image. choose the .exr image

result=> sometimes nothing at all happens, sometimes the image gets applied but stippled, sometimes it works as expected.

You can get similar effects (though it (accidentally) works correcly more often) by projecting a jpg image (char data) onto a float texture in the material.

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_float_from_rect(ps->reproject_ibuf);

and

IMB_recr_from_float(ps->reproject_ibuf);

are called to convert the source image data to the correct format. However, ps->reproject_ibuf is shared between threads, so this is not thread-safe. The checks on line 5308 and 5319 are already a race condition.

I created a 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?

Is posting a patch here the best way to submit a patch? or should I create a pull request on github or something like that?

Event Timeline

Martijn Versteegh (Baardaap) renamed this task from Thread-unsafe conversion of image data from float to char and v.v. to Thread-unsafe conversion of image data from float to char and v.v. (with patch).Wed, Sep 16, 10:33 PM
Martijn Versteegh (Baardaap) created this task.

I'm a bit unsure if the condition:
''' if (ps->source != PROJ_SRC_VIEW && ps->source != PROJ_SRC_VIEW_FILL) '''
I use to decide to generate the missing data is the best way. I arrived at this by following the logic in do_projectpaint_thread() but it feels a bit too roundabout to me.

It is more convenient for you to submit the patch for review via https://developer.blender.org/differential/diff/create/

There are tools for developers to comment on the patch.

In the description you can refer to this task.

This makes the description of the problem more short and straightforward and helps the team that is responsible for testing and confirming bugs.

Ok, thanks for pointing me in the right direction.

I created diff D8936

But I'm not really sure what I'm supposed to put in all the field over there. It says I should add reviewers, but I have no idea (yet) who to add. And neither do I have any clu what the 'project' should be. BF_Blender? BF_Blender:unstaged ?

I'll learn in time, but right now I'm a bit overwhelmed :-)

Thanks for the diff, the entire 'Sculpt, Paint & Texture` team was automatically poked for review.

By the way, could you edit the description of this report, removing everything about the patch? (This information is better in the description for review).

Also, could you provide an .ext image with float data or a simple .blend file showing the problem?

(This report is a little complex and can be very time consuming just to reproduce the problem).

the entire 'Sculpt, Paint & Texture` team was automatically poked for review.

Whoops. That might be a rather overkill (understatement). I was trying to be humble and not throw random names of people I don't know in the reviewers field...

Please forgive my clumsyness.

Here is a blend file. A bit large I accidentally used a rather large .exr. Sorry!


The race condition occurs when you pres 'apply camera image' from the tool options. What happens and if the results are visibly wrong or right is rather random, though, because it's threading/timing related.

I think I need to claim it to couple the task to my diff?