float texture painting enhancements

Fixes refresh bug with float texture painting.
Adds high resolution textures for float images to GUI.
Makes float texture painting faster by only uploading part of the texture on the GPU

Changes against trunk 36974

Pending for this patch is support for tiled textures.
Also, smear tool still uploads bytes to the GUI.

[psy-fi, 2011-05-30 19:49:39]

Cleaned-up version, omitting high-res stuff and merge problems

[LetterRip, 2011-05-30 21:41:35]

looks good asside from the repeating of the same 3 to 4 lines everywhere.

http://codereview.appspot.com/4560052/diff/16002/source/blender/imbuf/intern/...
File source/blender/imbuf/intern/divers.c (right):

http://codereview.appspot.com/4560052/diff/16002/source/blender/imbuf/intern/...
source/blender/imbuf/intern/divers.c:126: dstBytePxl[1]= dstBytePxl[2]=
dstBytePxl[3]= dstBytePxl[0] = FTOCHAR(srcFloatPxl[0]);
There are lots of places in the code where you do FTOCHAR on all four pixel
components same for linearrgb_to_srgb perhaps they should be refactored -
something like FTOCHAR_v3_v4 FTOCHAR_v4_v4 (don't know if those exist)
linearrgb_to_srgb_v3_v3 linearrgb_to_srgb_v3_v4

[ideasman42, 2011-05-30 22:51:31]

Looks fine, would prefer not to rename vars as a part of bugfixes, added
comments for minor changes.

http://codereview.appspot.com/4560052/diff/16002/source/blender/editors/sculp...
File source/blender/editors/sculpt_paint/paint_image.c (right):

http://codereview.appspot.com/4560052/diff/16002/source/blender/editors/sculp...
source/blender/editors/sculpt_paint/paint_image.c:3683: VECCOPY(rgba,
ps->brush->rgb);
// better just do...
srgb_to_linearrgb_v3_v3(rgba, ps->brush->rgb);

http://codereview.appspot.com/4560052/diff/16002/source/blender/editors/sculp...
source/blender/editors/sculpt_paint/paint_image.c:3707:
srgb_to_linearrgb_v3_v3(rgba, rgba);
// same again
srgb_to_linearrgb_v3_v3(rgba, ps->brush->rgb);

[psy-fi, 2011-05-31 00:44:56]

more cleanups, removed renames from other people's code, plus a function
intended for high-res

[psy-fi, 2011-05-31 00:47:29]

forgot the function declaration

[brechtvl, 2011-06-03 17:10:34]

Some comments below.

http://codereview.appspot.com/4560052/diff/13006/source/blender/blenlib/BLI_u...
File source/blender/blenlib/BLI_utildefines.h (right):

http://codereview.appspot.com/4560052/diff/13006/source/blender/blenlib/BLI_u...
source/blender/blenlib/BLI_utildefines.h:110: (v1)[3]=FTOCHAR((v2[3]));
(v1)[3]=FTOCHAR((v2[3]))
For multiple statement macros, always add {} brackets around them to avoid
surprises when doing e.g. if(..) F3TOCHAR3(..);

http://codereview.appspot.com/4560052/diff/13006/source/blender/editors/sculp...
File source/blender/editors/sculpt_paint/paint_image.c (right):

http://codereview.appspot.com/4560052/diff/13006/source/blender/editors/sculp...
source/blender/editors/sculpt_paint/paint_image.c:3700: mul_v3_v3(rgba,
rgba_br);
In IMB_rect_from_float, the ibuf->profile is taken into account to decide if
srgb to linear is necessary, probably a similar check should be done here too?

Also, I think a previous patch had these srgb_to_linearrgb_v3_v3 in brush.c,
this seems like it only fixes projection painting?

http://codereview.appspot.com/4560052/diff/13006/source/blender/gpu/intern/gp...
File source/blender/gpu/intern/gpu_draw.c (right):

http://codereview.appspot.com/4560052/diff/13006/source/blender/gpu/intern/gp...
source/blender/gpu/intern/gpu_draw.c:702: implementation for non-float images
will follow*/
Spaces are used for indentation here, convention is to use tabs only.

Also, more nitpicky, please follow the comment style, space after /* and before
*/, following lines indented with three spaces.

http://codereview.appspot.com/4560052/diff/13006/source/blender/gpu/intern/gp...
source/blender/gpu/intern/gpu_draw.c:704: IMB_partial_rect_from_float(ibuf,
buffer, x, y, w, h);
I don't understand what the purpose is here, of allocating a temporary float
buffer and using that for glTexSubImage2D instead of just using the code below
that also calls glTexSubImage2D, is this really faster?

[psy-fi, 2011-06-03 23:26:02]

OK, uploading another patch, this includes all the fixes proposed by brecht,
including proper support for color correction on float images based on color
profile. This fix only takes care of brush colors, texture sampling for painting
is not profile aware as far as I've seen.

http://codereview.appspot.com/4560052/diff/13006/source/blender/blenlib/BLI_u...
File source/blender/blenlib/BLI_utildefines.h (right):

http://codereview.appspot.com/4560052/diff/13006/source/blender/blenlib/BLI_u...
source/blender/blenlib/BLI_utildefines.h:110: (v1)[3]=FTOCHAR((v2[3]));
(v1)[3]=FTOCHAR((v2[3]))
On 2011/06/03 17:10:35, brechtvl wrote:
> For multiple statement macros, always add {} brackets around them to avoid
> surprises when doing e.g. if(..) F3TOCHAR3(..);

Done.

http://codereview.appspot.com/4560052/diff/13006/source/blender/editors/sculp...
File source/blender/editors/sculpt_paint/paint_image.c (right):

http://codereview.appspot.com/4560052/diff/13006/source/blender/editors/sculp...
source/blender/editors/sculpt_paint/paint_image.c:3700: mul_v3_v3(rgba,
rgba_br);
On 2011/06/03 17:10:35, brechtvl wrote:
> In IMB_rect_from_float, the ibuf->profile is taken into account to decide if
> srgb to linear is necessary, probably a similar check should be done here too?
> 
> Also, I think a previous patch had these srgb_to_linearrgb_v3_v3 in brush.c,
> this seems like it only fixes projection painting?

Done.

http://codereview.appspot.com/4560052/diff/13006/source/blender/gpu/intern/gp...
File source/blender/gpu/intern/gpu_draw.c (right):

http://codereview.appspot.com/4560052/diff/13006/source/blender/gpu/intern/gp...
source/blender/gpu/intern/gpu_draw.c:704: IMB_partial_rect_from_float(ibuf,
buffer, x, y, w, h);
Just made some profiling tests and strangely enough, you are right, using
glTexSubImage2D without the skip brpw stuff doesn't make too much of a
difference.
However this approach is needed if we are to support color correction with high
resolution float textures. The color corrected result written in buffer can then
be loaded into the texture.

[psy-fi, 2011-06-03 23:26:28]

patch

[psy-fi, 2011-06-03 23:29:14]

Please check out my blog in a few minutes to see side-by-side differences

http://psy-fidelity.blogspot.com/

[psy-fi, 2011-06-04 01:26:00]

A few more little corrections

[brechtvl, 2011-06-06 09:31:44]

LGTM. You can commit this to trunk then. And the float color blending does
indeed look much closer to what you'd expect.

[brechtvl, 2011-06-06 09:41:35]

A bit late, but added bf-codereview to reviewers lists for logging purposes.