Skip to content

Commit

Permalink
Remove histogram color profile
Browse files Browse the repository at this point in the history
Aside from being coded with the ass (entirely copy-pasted), the histogram color profile is useless since the histogram is grabbed in display RGB. If we want to display the histogram in pipe RGB, then we can convert display to pipe RGB but if RGB got clipped in display, then we convert clipped signal.

The whole thing is misleading and was used in overexposed module, which completely voids its purpose. Say you display histogram in Rec 2020 (super large), but your display is sRGB (super small), then you clip in sRGB at 100%, but converting back to Rec 2020, your 100% becomes 90 % or something, and the scope shows no overexposure at all.

That's bullshit coded by idiots. Not sure why it took me 4 years to spot it.
  • Loading branch information
aurelienpierre committed Jan 6, 2023
1 parent 84762fd commit 8502f41
Show file tree
Hide file tree
Showing 10 changed files with 75 additions and 269 deletions.
23 changes: 11 additions & 12 deletions data/kernels/basic.cl
Original file line number Diff line number Diff line change
Expand Up @@ -2657,7 +2657,7 @@ typedef enum dt_clipping_preview_mode_t
} dt_clipping_preview_mode_t;

kernel void
overexposed (read_only image2d_t in, write_only image2d_t out, read_only image2d_t tmp, const int width, const int height,
overexposed (read_only image2d_t in, write_only image2d_t out, const int width, const int height,
const float lower, const float upper, const float4 lower_color, const float4 upper_color,
constant dt_colorspaces_iccprofile_info_cl_t *profile_info,
read_only image2d_t lut, const int use_work_profile, dt_clipping_preview_mode_t mode)
Expand All @@ -2668,14 +2668,13 @@ overexposed (read_only image2d_t in, write_only image2d_t out, read_only image2d
if(x >= width || y >= height) return;

float4 pixel = read_imagef(in, sampleri, (int2)(x, y));
float4 pixel_tmp = read_imagef(tmp, sampleri, (int2)(x, y));

if(mode == DT_CLIPPING_PREVIEW_ANYRGB)
{
if(pixel_tmp.x >= upper || pixel_tmp.y >= upper || pixel_tmp.z >= upper)
if(pixel.x >= upper || pixel.y >= upper || pixel.z >= upper)
pixel.xyz = upper_color.xyz;

else if(pixel_tmp.x <= lower && pixel_tmp.y <= lower && pixel_tmp.z <= lower)
else if(pixel.x <= lower && pixel.y <= lower && pixel.z <= lower)
pixel.xyz = lower_color.xyz;

}
Expand All @@ -2694,14 +2693,14 @@ overexposed (read_only image2d_t in, write_only image2d_t out, read_only image2d
else
{
float4 saturation = { 0.f, 0.f, 0.f, 0.f};
saturation = pixel_tmp - (float4)luminance;
saturation = native_sqrt(saturation * saturation / ((float4)(luminance * luminance) + pixel_tmp * pixel_tmp));
saturation = pixel - (float4)luminance;
saturation = native_sqrt(saturation * saturation / ((float4)(luminance * luminance) + pixel * pixel));

if(saturation.x > upper || saturation.y > upper || saturation.z > upper ||
pixel_tmp.x >= upper || pixel_tmp.y >= upper || pixel_tmp.z >= upper)
pixel.x >= upper || pixel.y >= upper || pixel.z >= upper)
pixel.xyz = upper_color.xyz;

else if(pixel_tmp.x <= lower && pixel_tmp.y <= lower && pixel_tmp.z <= lower)
else if(pixel.x <= lower && pixel.y <= lower && pixel.z <= lower)
pixel.xyz = lower_color.xyz;
}
}
Expand All @@ -2722,14 +2721,14 @@ overexposed (read_only image2d_t in, write_only image2d_t out, read_only image2d
if(luminance < upper && luminance > lower)
{
float4 saturation = { 0.f, 0.f, 0.f, 0.f};
saturation = pixel_tmp - (float4)luminance;
saturation = native_sqrt(saturation * saturation / ((float4)(luminance * luminance) + pixel_tmp * pixel_tmp));
saturation = pixel - (float4)luminance;
saturation = native_sqrt(saturation * saturation / ((float4)(luminance * luminance) + pixel * pixel));

if(saturation.x > upper || saturation.y > upper || saturation.z > upper ||
pixel_tmp.x >= upper || pixel_tmp.y >= upper || pixel_tmp.z >= upper)
pixel.x >= upper || pixel.y >= upper || pixel.z >= upper)
pixel.xyz = upper_color.xyz;

else if(pixel_tmp.x <= lower && pixel_tmp.y <= lower && pixel_tmp.z <= lower)
else if(pixel.x <= lower && pixel.y <= lower && pixel.z <= lower)
pixel.xyz = lower_color.xyz;
}
}
Expand Down
24 changes: 0 additions & 24 deletions src/common/colorspaces.c
Original file line number Diff line number Diff line change
Expand Up @@ -1490,13 +1490,10 @@ dt_colorspaces_t *dt_colorspaces_init()
// init display profile and softproof/gama checking from conf
res->display_type = dt_conf_get_int("ui_last/color/display_type");
res->softproof_type = dt_conf_get_int("ui_last/color/softproof_type");
res->histogram_type = dt_conf_get_int("ui_last/color/histogram_type");
const char *tmp = dt_conf_get_string_const("ui_last/color/display_filename");
g_strlcpy(res->display_filename, tmp, sizeof(res->display_filename));
tmp = dt_conf_get_string_const("ui_last/color/softproof_filename");
g_strlcpy(res->softproof_filename, tmp, sizeof(res->softproof_filename));
tmp = dt_conf_get_string_const("ui_last/color/histogram_filename");
g_strlcpy(res->histogram_filename, tmp, sizeof(res->histogram_filename));
res->display_intent = dt_conf_get_int("ui_last/color/display_intent");
res->softproof_intent = dt_conf_get_int("ui_last/color/softproof_intent");
res->mode = dt_conf_get_int("ui_last/color/mode");
Expand All @@ -1513,11 +1510,6 @@ dt_colorspaces_t *dt_colorspaces_init()
&& (!res->softproof_filename[0] || !g_file_test(res->softproof_filename, G_FILE_TEST_IS_REGULAR))))
res->softproof_type = DT_COLORSPACE_SRGB;

if((unsigned int)res->histogram_type >= DT_COLORSPACE_LAST
|| (res->histogram_type == DT_COLORSPACE_FILE
&& (!res->histogram_filename[0] || !g_file_test(res->histogram_filename, G_FILE_TEST_IS_REGULAR))))
res->histogram_type = DT_COLORSPACE_SRGB;

// temporary list of profiles to be added, we keep this separate to be able to sort it before adding
GList *temp_profiles;

Expand Down Expand Up @@ -1559,20 +1551,6 @@ dt_colorspaces_t *dt_colorspaces_init()
"output profile `%s' color space `%c%c%c%c' not supported for work or histogram profile\n",
prof->name, (char)(color_space >> 24), (char)(color_space >> 16), (char)(color_space >> 8),
(char)(color_space));

if(res->histogram_type == prof->type
&& (prof->type != DT_COLORSPACE_FILE
|| dt_colorspaces_is_profile_equal(prof->filename, res->histogram_filename)))
{
// bad histogram profile selected, we must reset it to sRGB
const char *name = dt_colorspaces_get_name(prof->type, prof->filename);
dt_control_log(_("profile `%s' not usable as histogram profile. it has been replaced by sRGB!"), name);
fprintf(stderr,
"[colorspaces] profile `%s' not usable as histogram profile. it has been replaced by sRGB!\n",
name);
res->histogram_type = DT_COLORSPACE_SRGB;
res->histogram_filename[0] = '\0';
}
}
}
res->profiles = g_list_concat(res->profiles, temp_profiles);
Expand All @@ -1590,10 +1568,8 @@ void dt_colorspaces_cleanup(dt_colorspaces_t *self)
// remember display profile and softproof/gama checking from conf
dt_conf_set_int("ui_last/color/display_type", self->display_type);
dt_conf_set_int("ui_last/color/softproof_type", self->softproof_type);
dt_conf_set_int("ui_last/color/histogram_type", self->histogram_type);
dt_conf_set_string("ui_last/color/display_filename", self->display_filename);
dt_conf_set_string("ui_last/color/softproof_filename", self->softproof_filename);
dt_conf_set_string("ui_last/color/histogram_filename", self->histogram_filename);
dt_conf_set_int("ui_last/color/display_intent", self->display_intent);
dt_conf_set_int("ui_last/color/softproof_intent", self->softproof_intent);
dt_conf_set_int("ui_last/color/mode", self->mode);
Expand Down
2 changes: 0 additions & 2 deletions src/common/colorspaces.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,8 @@ typedef struct dt_colorspaces_t
// the current set of selected profiles
dt_colorspaces_color_profile_type_t display_type;
dt_colorspaces_color_profile_type_t softproof_type;
dt_colorspaces_color_profile_type_t histogram_type;
char display_filename[512];
char softproof_filename[512];
char histogram_filename[512];
dt_iop_color_intent_t display_intent;
dt_iop_color_intent_t softproof_intent;

Expand Down
36 changes: 0 additions & 36 deletions src/common/iop_profile.c
Original file line number Diff line number Diff line change
Expand Up @@ -884,15 +884,6 @@ dt_ioppr_set_pipe_output_profile_info(struct dt_develop_t *dev,
return profile_info;
}

dt_iop_order_iccprofile_info_t *dt_ioppr_get_histogram_profile_info(struct dt_develop_t *dev)
{
dt_colorspaces_color_profile_type_t histogram_profile_type;
const char *histogram_profile_filename;
dt_ioppr_get_histogram_profile_type(&histogram_profile_type, &histogram_profile_filename);
return dt_ioppr_add_profile_info_to_list(dev, histogram_profile_type, histogram_profile_filename,
DT_INTENT_RELATIVE_COLORIMETRIC);
}

dt_iop_order_iccprofile_info_t *dt_ioppr_get_pipe_work_profile_info(struct dt_dev_pixelpipe_t *pipe)
{
return pipe->work_profile_info;
Expand Down Expand Up @@ -1022,33 +1013,6 @@ void dt_ioppr_get_export_profile_type(struct dt_develop_t *dev,
fprintf(stderr, "[dt_ioppr_get_export_profile_type] can't find colorout iop\n");
}

void dt_ioppr_get_histogram_profile_type(dt_colorspaces_color_profile_type_t *profile_type,
const char **profile_filename)
{
const dt_colorspaces_color_mode_t mode = darktable.color_profiles->mode;

// if in gamut check use soft proof
if(mode != DT_PROFILE_NORMAL || darktable.color_profiles->histogram_type == DT_COLORSPACE_SOFTPROOF)
{
*profile_type = darktable.color_profiles->softproof_type;
*profile_filename = darktable.color_profiles->softproof_filename;
}
else if(darktable.color_profiles->histogram_type == DT_COLORSPACE_WORK)
{
dt_ioppr_get_work_profile_type(darktable.develop, profile_type, profile_filename);
}
else if(darktable.color_profiles->histogram_type == DT_COLORSPACE_EXPORT)
{
dt_ioppr_get_export_profile_type(darktable.develop, profile_type, profile_filename);
}
else
{
*profile_type = darktable.color_profiles->histogram_type;
*profile_filename = darktable.color_profiles->histogram_filename;
}
}


__DT_CLONE_TARGETS__
void dt_ioppr_transform_image_colorspace(struct dt_iop_module_t *self, const float *const image_in,
float *const image_out, const int width, const int height,
Expand Down
15 changes: 4 additions & 11 deletions src/develop/pixelpipe_hb.c
Original file line number Diff line number Diff line change
Expand Up @@ -754,7 +754,6 @@ static void pixelpipe_picker_cl(int devid, dt_iop_module_t *module, dt_dev_pixel
static void _pixelpipe_pick_from_image(dt_iop_module_t *module,
const float *const pixel, const dt_iop_roi_t *roi_in,
const dt_iop_order_iccprofile_info_t *const display_profile,
const dt_iop_order_iccprofile_info_t *const histogram_profile,
dt_colorpicker_sample_t *const sample)
{
if(sample->size == DT_LIB_COLORPICKER_SIZE_BOX)
Expand Down Expand Up @@ -796,9 +795,6 @@ static void _pixelpipe_pick_from_image(dt_iop_module_t *module,
int converted_cst;
dt_ioppr_transform_image_colorspace(module, picked_rgb[0], sample->lab[0], 3, 1, IOP_CS_RGB, IOP_CS_LAB,
&converted_cst, display_profile);
if(display_profile && histogram_profile)
dt_ioppr_transform_image_colorspace_rgb(picked_rgb[0], sample->scope[0], 3, 1,
display_profile, histogram_profile, "primary picker");
}
else if(sample->size == DT_LIB_COLORPICKER_SIZE_POINT)
{
Expand All @@ -809,9 +805,7 @@ static void _pixelpipe_pick_from_image(dt_iop_module_t *module,
memcpy(sample->display[0], pixel + 4 * (roi_in->width * y + x), sizeof(dt_aligned_pixel_t));
dt_ioppr_transform_image_colorspace(module, sample->display[0], sample->lab[0], 1, 1, IOP_CS_RGB, IOP_CS_LAB,
&converted_cst, display_profile);
if(display_profile && histogram_profile)
dt_ioppr_transform_image_colorspace_rgb(sample->display[0], sample->scope[0], 1, 1,
display_profile, histogram_profile, "primary picker");

for(dt_lib_colorpicker_statistic_t stat = 1; stat < DT_LIB_COLORPICKER_STATISTIC_N; stat++)
{
memcpy(sample->display[stat], sample->display[0], sizeof(dt_aligned_pixel_t));
Expand All @@ -824,7 +818,6 @@ static void _pixelpipe_pick_from_image(dt_iop_module_t *module,
static void _pixelpipe_pick_samples(dt_develop_t *dev, dt_iop_module_t *module,
const float *const input, const dt_iop_roi_t *roi_in)
{
const dt_iop_order_iccprofile_info_t *const histogram_profile = dt_ioppr_get_histogram_profile_info(dev);
const dt_iop_order_iccprofile_info_t *const display_profile
= dt_ioppr_add_profile_info_to_list(dev, darktable.color_profiles->display_type,
darktable.color_profiles->display_filename, INTENT_RELATIVE_COLORIMETRIC);
Expand All @@ -834,12 +827,12 @@ static void _pixelpipe_pick_samples(dt_develop_t *dev, dt_iop_module_t *module,
{
dt_colorpicker_sample_t *sample = samples->data;
if(!sample->locked)
_pixelpipe_pick_from_image(module, input, roi_in, display_profile, histogram_profile, sample);
_pixelpipe_pick_from_image(module, input, roi_in, display_profile, sample);
samples = g_slist_next(samples);
}

if(darktable.lib->proxy.colorpicker.picker_proxy)
_pixelpipe_pick_from_image(module, input, roi_in, display_profile, histogram_profile,
_pixelpipe_pick_from_image(module, input, roi_in, display_profile,
darktable.lib->proxy.colorpicker.primary_sample);
}

Expand Down Expand Up @@ -2030,7 +2023,7 @@ static int dt_dev_pixelpipe_process_rec(dt_dev_pixelpipe_t *pipe, dt_develop_t *
// benefit via a histogram.
darktable.lib->proxy.histogram.process(darktable.lib->proxy.histogram.module, input,
roi_in.width, roi_in.height,
display_profile, dt_ioppr_get_histogram_profile_info(dev));
display_profile, display_profile);
}

if(dt_atomic_get_int(&pipe->shutdown))
Expand Down
12 changes: 6 additions & 6 deletions src/iop/colorzones.c
Original file line number Diff line number Diff line change
Expand Up @@ -822,14 +822,14 @@ static void _draw_color_picker(dt_iop_module_t *self, cairo_t *cr, dt_iop_colorz
GSList *samples = darktable.lib->proxy.colorpicker.live_samples;
if(samples)
{
const dt_iop_order_iccprofile_info_t *const histogram_profile
= dt_ioppr_get_histogram_profile_info(self->dev);
const dt_iop_order_iccprofile_info_t *const display_profile
= dt_ioppr_get_pipe_output_profile_info(self->dev->pipe);
const dt_iop_order_iccprofile_info_t *const work_profile
= dt_ioppr_get_iop_work_profile_info(self, self->dev->iop);
dt_aligned_pixel_t pick_mean, pick_min, pick_max;
int converted_cst;

if(work_profile && histogram_profile)
if(work_profile && display_profile)
{
dt_colorpicker_sample_t *sample = NULL;
for(; samples; samples = g_slist_next(samples))
Expand All @@ -849,11 +849,11 @@ static void _draw_color_picker(dt_iop_module_t *self, cairo_t *cr, dt_iop_colorz
}
pick_mean[3] = pick_min[3] = pick_max[3] = 1.f;

dt_ioppr_transform_image_colorspace_rgb(pick_mean, pick_mean, 1, 1, histogram_profile, work_profile,
dt_ioppr_transform_image_colorspace_rgb(pick_mean, pick_mean, 1, 1, display_profile, work_profile,
"color zones");
dt_ioppr_transform_image_colorspace_rgb(pick_min, pick_min, 1, 1, histogram_profile, work_profile,
dt_ioppr_transform_image_colorspace_rgb(pick_min, pick_min, 1, 1, display_profile, work_profile,
"color zones");
dt_ioppr_transform_image_colorspace_rgb(pick_max, pick_max, 1, 1, histogram_profile, work_profile,
dt_ioppr_transform_image_colorspace_rgb(pick_max, pick_max, 1, 1, display_profile, work_profile,
"color zones");

dt_ioppr_transform_image_colorspace(self, pick_mean, pick_mean, 1, 1, IOP_CS_RGB, IOP_CS_LAB,
Expand Down
Loading

0 comments on commit 8502f41

Please sign in to comment.