blender
/
blender
134
425
Fork 607
Code Issues 6.3k Pull Requests 774 Projects 19 Wiki Activity

WIP: IMB: Allow BW images in byte to float conversion #118622

Draft
Omar Emara wants to merge 8 commits from OmarEmaraDev/blender:refactor-imbuf-byte-to-float into main
pull from: OmarEmaraDev/blender:refactor-imbuf-byte-to-float
merge into: blender:main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
Conversation 7 Commits 8 Files Changed 3 +62 -34
5 changed files with 62 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit cd07fe75d1 - Show all commits

1
source/blender/blenlib/BLI_math_color.h
View File

@ -143,6 +143,7 @@ void rgb_float_set_hue_float_offset(float rgb[3], float hue_offset);
*/
void rgb_byte_set_hue_float_offset(unsigned char rgb[3], float hue_offset);
float uchar_to_float(const unsigned char value);
OmarEmaraDev marked this conversation as resolved Outdated
Sergey Sharybin commented 2024-03-05 15:37:56 +01:00
Outdated
Review
Copy Link

Not sure if we need such function, is just a simple division, which can not be SIMD-optimized (in possible contrast with rgb_uchar_to_float).

Not sure if we need such function, is just a simple division, which can not be SIMD-optimized (in possible contrast with `rgb_uchar_to_float`).
void rgb_uchar_to_float(float r_col[3], const unsigned char col_ub[3]);
void rgba_uchar_to_float(float r_col[4], const unsigned char col_ub[4]);
void rgb_float_to_uchar(unsigned char r_col[3], const float col_f[3]);

19
source/blender/blenlib/intern/math_color.c
View File

@ -374,19 +374,24 @@ void cpack_to_rgb(uint col, float *r_r, float *r_g, float *r_b)
*r_b = (float)((col >> 16) & 0xFF) * (1.0f / 255.0f);
}
float uchar_to_float(const uchar value)
{
return ((float)value) * (1.0f / 255.0f);
}
void rgb_uchar_to_float(float r_col[3], const uchar col_ub[3])
{
r_col[0] = ((float)col_ub[0]) * (1.0f / 255.0f);
r_col[1] = ((float)col_ub[1]) * (1.0f / 255.0f);
r_col[2] = ((float)col_ub[2]) * (1.0f / 255.0f);
r_col[0] = uchar_to_float(col_ub[0]);
r_col[1] = uchar_to_float(col_ub[1]);
r_col[2] = uchar_to_float(col_ub[2]);
}
void rgba_uchar_to_float(float r_col[4], const uchar col_ub[4])
{
r_col[0] = ((float)col_ub[0]) * (1.0f / 255.0f);
r_col[1] = ((float)col_ub[1]) * (1.0f / 255.0f);
r_col[2] = ((float)col_ub[2]) * (1.0f / 255.0f);
r_col[3] = ((float)col_ub[3]) * (1.0f / 255.0f);
r_col[0] = uchar_to_float(col_ub[0]);
r_col[1] = uchar_to_float(col_ub[1]);
r_col[2] = uchar_to_float(col_ub[2]);
r_col[3] = uchar_to_float(col_ub[3]);
}
void rgb_float_to_uchar(uchar r_col[3], const float col_f[3])

1
source/blender/imbuf/IMB_imbuf.hh
View File

@ -457,6 +457,7 @@ void IMB_buffer_float_from_byte(float *rect_to,
int profile_to,
int profile_from,
bool predivide,
int channels,
int width,
int height,
int stride_to,

4
source/blender/imbuf/intern/colormanagement.cc
View File

@ -1656,6 +1656,7 @@ static void *do_display_buffer_apply_thread(void *handle_v)
IB_PROFILE_SRGB,
OmarEmaraDev marked this conversation as resolved
Sergey Sharybin commented 2024-03-05 15:39:05 +01:00
Review
Copy Link

I think we should start removing this legacy profile. Will help simplifying this change as well, and I don't think we ever do color space conversion.

I think we should start removing this legacy profile. Will help simplifying this change as well, and I don't think we ever do color space conversion.
IB_PROFILE_SRGB,
false,
channels,
width,
height,
width,
@ -1908,6 +1909,7 @@ static void *do_processor_transform_thread(void *handle_v)
IB_PROFILE_SRGB,
IB_PROFILE_SRGB,
false,
channels,
width,
height,
width,
@ -2068,6 +2070,7 @@ void IMB_colormanagement_transform_from_byte(float *float_buffer,
IB_PROFILE_SRGB,
IB_PROFILE_SRGB,
true,
channels,
width,
height,
width,
@ -3623,6 +3626,7 @@ static void partial_buffer_update_rect(ImBuf *ibuf,
IB_PROFILE_SRGB,
IB_PROFILE_SRGB,
true,
channels,
width,
height,
width,

71
source/blender/imbuf/intern/divers.cc
View File

@ -352,54 +352,71 @@ void IMB_buffer_float_from_byte(float *rect_to,
int profile_to,
int profile_from,
bool predivide,
int channels,
int width,
int height,
int stride_to,
int stride_from)
{
float tmp[4];
int x, y;
/* we need valid profiles */
BLI_assert(profile_to != IB_PROFILE_NONE);
BLI_assert(profile_from != IB_PROFILE_NONE);
/* RGBA input */
for (y = 0; y < height; y++) {
const uchar *from = rect_from + size_t(stride_from) * y * 4;
float *to = rect_to + size_t(stride_to) * y * 4;
for (int y = 0; y < height; y++) {
const uchar *from = rect_from + size_t(stride_from) * y * channels;
float *to = rect_to + size_t(stride_to) * y * channels;
if (profile_to == profile_from) {
/* no color space conversion */
for (x = 0; x < width; x++, from += 4, to += 4) {
rgba_uchar_to_float(to, from);
for (int x = 0; x < width; x++) {
for (int i = 0; i < channels; i++, from++, to++) {
*to = uchar_to_float(*from);
}
}
}
else if (profile_to == IB_PROFILE_LINEAR_RGB) {
/* convert sRGB to linear */
if (predivide) {
for (x = 0; x < width; x++, from += 4, to += 4) {
if (predivide && channels == 4) {
for (int x = 0; x < width; x++, from += 4, to += 4) {
srgb_to_linearrgb_uchar4_predivide(to, from);
}
}
else {
for (x = 0; x < width; x++, from += 4, to += 4) {
else if (channels == 4) {
for (int x = 0; x < width; x++, from += 4, to += 4) {
srgb_to_linearrgb_uchar4(to, from);
}
}
else {
for (int x = 0; x < width; x++) {
for (int i = 0; i < channels; i++, from++, to++) {
*to = BLI_color_from_srgb_table[*from];
}
}
}
}
else if (profile_to == IB_PROFILE_SRGB) {
/* convert linear to sRGB */
if (predivide) {
for (x = 0; x < width; x++, from += 4, to += 4) {
rgba_uchar_to_float(tmp, from);
linearrgb_to_srgb_predivide_v4(to, tmp);
if (predivide && channels == 4) {
for (int x = 0; x < width; x++, from += 4, to += 4) {
float float_value[4];
rgba_uchar_to_float(float_value, from);
linearrgb_to_srgb_predivide_v4(to, float_value);
}
}
else if (channels == 4) {
for (int x = 0; x < width; x++, from += 4, to += 4) {
float float_value[4];
rgba_uchar_to_float(float_value, from);
linearrgb_to_srgb_v4(to, float_value);
}
}
else {
for (x = 0; x < width; x++, from += 4, to += 4) {
rgba_uchar_to_float(tmp, from);
linearrgb_to_srgb_v4(to, tmp);
for (int x = 0; x < width; x++) {
for (int i = 0; i < channels; i++, from++, to++) {
const float float_value = uchar_to_float(*from);
*to = linearrgb_to_srgb(float_value);
}
}
}
}
@ -755,7 +772,8 @@ void IMB_float_from_rect_ex(ImBuf *dst, const ImBuf *src, const rcti *region_to_
"Source buffer should have a byte buffer assigned.");
BLI_assert_msg(dst->x == src->x, "Source and destination buffer should have the same dimension");
BLI_assert_msg(dst->y == src->y, "Source and destination buffer should have the same dimension");
BLI_assert_msg(dst->channels = 4, "Destination buffer should have 4 channels.");
BLI_assert_msg(dst->channels = src->channels,
OmarEmaraDev marked this conversation as resolved Outdated
Sergey Sharybin commented 2024-03-05 15:40:05 +01:00
Outdated
Review
Copy Link

Wow, surely this is not an issue with this patch, but we should not be assigning the number of channels as part of an assert expression.

Wow, surely this is not an issue with this patch, but we should not be assigning the number of channels as part of an assert expression.
"Spurce and destination buffers should have the same number of channels.");
BLI_assert_msg(region_to_update->xmin >= 0,
"Region to update should be clipped to the given buffers.");
OmarEmaraDev marked this conversation as resolved Outdated
Sergey Sharybin commented 2024-03-14 15:17:50 +01:00
Outdated
Review
Copy Link

Shouldn't we initialize ibuf->channels based on the number of planes in the else branch here? Otherwise it could left uninitialized, or initialized to some default value which is not necessarily corresponding to the desired value?

Shouldn't we initialize `ibuf->channels` based on the number of planes in the `else` branch here? Otherwise it could left uninitialized, or initialized to some default value which is not necessarily corresponding to the desired value?
Omar Emara commented 2024-03-15 07:40:24 +01:00
Outdated
Review
Copy Link

Yah, sorry. Was remnant from the time where I though channels was shared between both byte and float buffers.

Yah, sorry. Was remnant from the time where I though channels was shared between both byte and float buffers.
BLI_assert_msg(region_to_update->ymin >= 0,
@ -766,9 +784,9 @@ void IMB_float_from_rect_ex(ImBuf *dst, const ImBuf *src, const rcti *region_to_
"Region to update should be clipped to the given buffers.");
float *rect_float = dst->float_buffer.data;
rect_float += (region_to_update->xmin + region_to_update->ymin * dst->x) * 4;
rect_float += (region_to_update->xmin + region_to_update->ymin * dst->x) * dst->channels;
uchar *rect = src->byte_buffer.data;
rect += (region_to_update->xmin + region_to_update->ymin * dst->x) * 4;
rect += (region_to_update->xmin + region_to_update->ymin * dst->x) * dst->channels;
const int region_width = BLI_rcti_size_x(region_to_update);
const int region_height = BLI_rcti_size_y(region_to_update);
@ -778,6 +796,7 @@ void IMB_float_from_rect_ex(ImBuf *dst, const ImBuf *src, const rcti *region_to_
IB_PROFILE_SRGB,
IB_PROFILE_SRGB,
false,
dst->channels,
region_width,
region_height,
src->x,
@ -788,7 +807,7 @@ void IMB_float_from_rect_ex(ImBuf *dst, const ImBuf *src, const rcti *region_to_
for (int i = 0; i < region_height; i++) {
IMB_colormanagement_colorspace_to_scene_linear(
float_ptr, region_width, 1, dst->channels, src->byte_buffer.colorspace, false);
float_ptr += 4 * dst->x;
float_ptr += dst->channels * dst->x;
}
/* Perform alpha conversion. */
@ -796,7 +815,7 @@ void IMB_float_from_rect_ex(ImBuf *dst, const ImBuf *src, const rcti *region_to_
float_ptr = rect_float;
for (int i = 0; i < region_height; i++) {
IMB_premultiply_rect_float(float_ptr, dst->channels, region_width, 1);
float_ptr += 4 * dst->x;
float_ptr += dst->channels * dst->x;
}
}
}
@ -814,15 +833,13 @@ void IMB_float_from_rect(ImBuf *ibuf)
*/
float *rect_float = ibuf->float_buffer.data;
if (rect_float == nullptr) {
const size_t size = IMB_get_rect_len(ibuf) * sizeof(float[4]);
const size_t size = IMB_get_rect_len(ibuf) * ibuf->channels * sizeof(float);
rect_float = static_cast<float *>(MEM_callocN(size, "IMB_float_from_rect"));
if (rect_float == nullptr) {
return;
}
ibuf->channels = 4;
IMB_assign_float_buffer(ibuf, rect_float, IB_TAKE_OWNERSHIP);
}