Skip to content

Commit

Permalink
Introduce new iop-order v3.1 where finalscale is before colorout.
Browse files Browse the repository at this point in the history
This fix color & luminosity shift when HQ mode is used.

Also use a new DT_DEFAULT_IOP_ORDER define and use it consistently
where needed. Less maintenance needed when a new order is introduced.

Fixes #17758, #13335, #13635 and #13682.
  • Loading branch information
TurboGit committed Nov 7, 2024
1 parent 029890f commit 6b18bae
Show file tree
Hide file tree
Showing 5 changed files with 162 additions and 12 deletions.
5 changes: 4 additions & 1 deletion src/common/exif.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3879,7 +3879,10 @@ gboolean dt_exif_xmp_read(dt_image_t *img,
// All iop-order version before 3 are legacy one. Starting
// with version 3 we have the first attempts to propose the
// final v3 iop-order.
iop_order_version = pos->toLong() < 3 ? DT_IOP_ORDER_LEGACY : DT_IOP_ORDER_V30;
iop_order_version = pos->toLong() < 3
? DT_IOP_ORDER_LEGACY
: DT_DEFAULT_IOP_ORDER_RAW;

iop_order_list = dt_ioppr_get_iop_order_list_version(iop_order_version);
}
else
Expand Down
139 changes: 134 additions & 5 deletions src/common/iop_order.c
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ const char *iop_order_string[] =
N_("custom"),
N_("legacy"),
N_("v3.0 RAW"),
N_("v3.0 JPEG")
N_("v3.0 JPEG"),

This comment has been minimized.

Copy link
@kofa73

kofa73 Nov 7, 2024

Contributor

Wouldn't the same kind of modification make sense for the non-raw/JPG pipeline order?

N_("v3.1 RAW")

This comment has been minimized.

Copy link
@kofa73

kofa73 Nov 7, 2024

Contributor

Wouldn't it make sense to call this module order 5.0, as it will be introduced with darktable 5.0? It'd make it more consistent and easier to remember. Weren't the previous ones given the number of the darktable release that introduced them?

This comment has been minimized.

Copy link
@TurboGit

TurboGit Nov 7, 2024

Author Member

Yes, good point let me propose a PR for this.

};

const char *dt_iop_order_string(const dt_iop_order_t order)
Expand Down Expand Up @@ -290,6 +291,122 @@ const dt_iop_order_entry_t v30_order[] = {
{ { 0.0f }, "", 0 }
};

const dt_iop_order_entry_t v31_order[] = {
{ { 1.0 }, "rawprepare", 0},
{ { 2.0 }, "invert", 0},
{ { 3.0f }, "temperature", 0},
{ { 4.0f }, "highlights", 0},
{ { 5.0f }, "cacorrect", 0},
{ { 6.0f }, "hotpixels", 0},
{ { 7.0f }, "rawdenoise", 0},
{ { 8.0f }, "demosaic", 0},
{ { 9.0f }, "denoiseprofile", 0},
{ {10.0f }, "bilateral", 0},
{ {11.0f }, "rotatepixels", 0},
{ {12.0f }, "scalepixels", 0},
{ {13.0f }, "lens", 0},
{ {13.5f }, "cacorrectrgb", 0}, // correct chromatic aberrations
// after lens correction so that
// lensfun does not reintroduce
// chromatic aberrations when trying
// to correct them
{ {14.0f }, "hazeremoval", 0},
{ {15.0f }, "ashift", 0},
{ {16.0f }, "flip", 0},
{ {16.5f }, "enlargecanvas", 0},
{ {16.7f }, "overlay", 0},
{ {17.0f }, "clipping", 0},
{ {18.0f }, "liquify", 0},
{ {19.0f }, "spots", 0},
{ {20.0f }, "retouch", 0},
{ {21.0f }, "exposure", 0},
{ {22.0f }, "mask_manager", 0},
{ {23.0f }, "tonemap", 0},
{ {24.0f }, "toneequal", 0}, // last module that need enlarged
// roi_in
{ {24.5f }, "crop", 0}, // should go after all modules
// that may need a wider roi_in
{ {25.0f }, "graduatednd", 0},
{ {26.0f }, "profile_gamma", 0},
{ {27.0f }, "equalizer", 0},
{ {28.0f }, "colorin", 0},
{ {28.5f }, "channelmixerrgb", 0},
{ {28.5f }, "diffuse", 0},
{ {28.5f }, "censorize", 0},
{ {28.5f }, "negadoctor", 0}, // Cineon film encoding comes
// after scanner input color
// profile
{ {28.5f }, "blurs", 0}, // physically-accurate blurs (motion and lens)
{ {28.5f }, "primaries", 0},
{ {29.0f }, "nlmeans", 0}, // signal processing (denoising)
// -> needs a signal as scene-referred as possible (even if it works in Lab)
{ {30.0f }, "colorchecker", 0}, // calibration to "neutral" exchange colour space
// -> improve colour calibration of colorin and reproductibility
// of further edits (styles etc.)
{ {31.0f }, "defringe", 0}, // desaturate fringes in Lab, so needs properly calibrated colours
// in order for chromaticity to be meaningful,
{ {32.0f }, "atrous", 0}, // frequential operation, needs a signal as scene-referred as possible to avoid halos
{ {33.0f }, "lowpass", 0}, // same
{ {34.0f }, "highpass", 0}, // same
{ {35.0f }, "sharpen", 0}, // same, worst than atrous in same use-case, less control overall

{ {37.0f }, "colortransfer", 0}, // probably better if source and destination colours are neutralized in the same
// colour exchange space, hence after colorin and colorcheckr,
// but apply after frequential ops in case it does non-linear witchcraft,
// just to be safe
{ {38.0f }, "colormapping", 0}, // same
{ {39.0f }, "channelmixer", 0}, // does exactly the same thing as colorin, aka RGB to RGB matrix conversion,
// but coefs are user-defined instead of calibrated and read from ICC profile.
// Really versatile yet under-used module, doing linear ops,
// very good in scene-referred workflow
{ {40.0f }, "basicadj", 0}, // module mixing view/model/control at once, usage should be discouraged
{ {41.0f }, "colorbalance", 0}, // scene-referred color manipulation
{ {41.2f }, "colorequal", 0},
{ {41.5f }, "colorbalancergb", 0}, // scene-referred color manipulation
{ {42.0f }, "rgbcurve", 0}, // really versatile way to edit colour in scene-referred and display-referred workflow
{ {43.0f }, "rgblevels", 0}, // same
{ {44.0f }, "basecurve", 0}, // conversion from scene-referred to display referred, reverse-engineered
// on camera JPEG default look
{ {45.0f }, "filmic", 0}, // same, but different (parametric) approach
{ {45.3f }, "sigmoid", 0},
{ {46.0f }, "filmicrgb", 0}, // same, upgraded
{ {36.0f }, "lut3d", 0}, // apply a creative style or film emulation, possibly non-linear
{ {47.0f }, "colisa", 0}, // edit contrast while damaging colour
{ {48.0f }, "tonecurve", 0}, // same
{ {49.0f }, "levels", 0}, // same
{ {50.0f }, "shadhi", 0}, // same
{ {51.0f }, "zonesystem", 0}, // same
{ {52.0f }, "globaltonemap", 0}, // same
{ {53.0f }, "relight", 0}, // flatten local contrast while pretending do add lightness
{ {54.0f }, "bilat", 0}, // improve clarity/local contrast after all the bad things we have done
// to it with tonemapping
{ {55.0f }, "colorcorrection", 0}, // now that the colours have been damaged by contrast manipulations,
// try to recover them - global adjustment of white balance for shadows and highlights
{ {56.0f }, "colorcontrast", 0}, // adjust chrominance globally
{ {57.0f }, "velvia", 0}, // same
{ {58.0f }, "vibrance", 0}, // same, but more subtle
{ {60.0f }, "colorzones", 0}, // same, but locally
{ {61.0f }, "bloom", 0}, // creative module
{ {62.0f }, "colorize", 0}, // creative module
{ {63.0f }, "lowlight", 0}, // creative module
{ {64.0f }, "monochrome", 0}, // creative module
{ {65.0f }, "grain", 0}, // creative module
{ {66.0f }, "soften", 0}, // creative module
{ {67.0f }, "splittoning", 0}, // creative module
{ {68.0f }, "vignette", 0}, // creative module
{ {69.0f }, "colorreconstruct", 0},// try to salvage blown areas before ICC intents in LittleCMS2 do things with them.
{ {69.4f }, "finalscale", 0},
{ {70.0f }, "colorout", 0},
{ {71.0f }, "clahe", 0},
{ {73.0f }, "overexposed", 0},
{ {74.0f }, "rawoverexposed", 0},
{ {75.0f }, "dither", 0},
{ {76.0f }, "borders", 0},
{ {77.0f }, "watermark", 0},
{ {78.0f }, "gamma", 0},
{ { 0.0f }, "", 0 }
};

// default order for JPEG/TIFF/PNG files, non-linear before colorin
const dt_iop_order_entry_t v30_jpg_order[] = {
// the following modules are not used anyway for non-RAW images :
Expand Down Expand Up @@ -462,7 +579,7 @@ dt_iop_order_t dt_ioppr_get_iop_order_version(const dt_imgid_t imgid)
{
const gboolean is_display_referred = dt_is_display_referred();
dt_iop_order_t iop_order_version =
is_display_referred ? DT_IOP_ORDER_LEGACY : DT_IOP_ORDER_V30;
is_display_referred ? DT_IOP_ORDER_LEGACY : DT_DEFAULT_IOP_ORDER_RAW;

// check current iop order version
sqlite3_stmt *stmt;
Expand Down Expand Up @@ -657,7 +774,11 @@ gboolean _check_iop_list_equal(GList *iop_order_list,
dt_iop_order_t dt_ioppr_get_iop_order_list_kind(GList *iop_order_list)
{
// first check if this is the v30 order RAW
if(_check_iop_list_equal(iop_order_list, v30_order))
if(_check_iop_list_equal(iop_order_list, v31_order))
{
return DT_IOP_ORDER_V31;
}
else if(_check_iop_list_equal(iop_order_list, v30_order))
{
return DT_IOP_ORDER_V30;
}
Expand Down Expand Up @@ -823,6 +944,10 @@ GList *dt_ioppr_get_iop_order_list_version(const dt_iop_order_t version)
{
iop_order_list = _table_to_list(v30_order);
}
else if(version == DT_IOP_ORDER_V31)
{
iop_order_list = _table_to_list(v31_order);
}
else if(version == DT_IOP_ORDER_V30_JPG)
{
iop_order_list = _table_to_list(v30_jpg_order);
Expand Down Expand Up @@ -928,6 +1053,10 @@ GList *dt_ioppr_get_iop_order_list(const dt_imgid_t imgid,
{
iop_order_list = _table_to_list(v30_order);
}
else if(version == DT_IOP_ORDER_V31)
{
iop_order_list = _table_to_list(v31_order);
}
else if(version == DT_IOP_ORDER_V30_JPG)
{
iop_order_list = _table_to_list(v30_jpg_order);
Expand Down Expand Up @@ -955,12 +1084,12 @@ GList *dt_ioppr_get_iop_order_list(const dt_imgid_t imgid,
dt_iop_order_t iop_order_version =
dt_is_display_referred()
? DT_IOP_ORDER_LEGACY
: DT_IOP_ORDER_V30;
: DT_DEFAULT_IOP_ORDER_RAW;

if(iop_order_version == DT_IOP_ORDER_LEGACY)
iop_order_list = _table_to_list(legacy_order);
else
iop_order_list = _table_to_list(v30_order);
iop_order_list = _table_to_list(v31_order);
}

if(sorted)
Expand Down
5 changes: 4 additions & 1 deletion src/common/iop_order.h
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,12 @@ typedef enum dt_iop_order_t
DT_IOP_ORDER_LEGACY = 1, // up to dt 2.6.3
DT_IOP_ORDER_V30 = 2, // starts with dt 3.0
DT_IOP_ORDER_V30_JPG = 3, // same as previous but tuned for non-linear input
DT_IOP_ORDER_LAST = 4
DT_IOP_ORDER_V31 = 4, // starts with dt 3.0
DT_IOP_ORDER_LAST = 5
} dt_iop_order_t;

#define DT_DEFAULT_IOP_ORDER_RAW DT_IOP_ORDER_V31

typedef struct dt_iop_order_entry_t
{
union {
Expand Down
2 changes: 1 addition & 1 deletion src/develop/develop.c
Original file line number Diff line number Diff line change
Expand Up @@ -1692,7 +1692,7 @@ static gboolean _dev_auto_apply_presets(dt_develop_t *dev)
{
// we have no auto-apply order, so apply iop order, depending of the workflow
if(is_scene_referred || is_workflow_none)
iop_list = dt_ioppr_get_iop_order_list_version(DT_IOP_ORDER_V30);
iop_list = dt_ioppr_get_iop_order_list_version(DT_DEFAULT_IOP_ORDER_RAW);
else
iop_list = dt_ioppr_get_iop_order_list_version(DT_IOP_ORDER_LEGACY);
}
Expand Down
23 changes: 19 additions & 4 deletions src/libs/ioporder.c
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,16 @@ void gui_reset(dt_lib_module_t *self)
{
dt_lib_ioporder_t *d = self->data;

// the module reset is use to select the v3.0 iop-order
// the module reset is use to select the proper default iop-order

GList *iop_order_list = dt_ioppr_get_iop_order_list_version(DT_IOP_ORDER_V30);
const gboolean is_ldr = dt_image_is_ldr(&darktable.develop->image_storage);

const dt_iop_order_t iop_order =
is_ldr
? DT_IOP_ORDER_V30_JPG
: DT_DEFAULT_IOP_ORDER_RAW;

GList *iop_order_list = dt_ioppr_get_iop_order_list_version(iop_order);

if(iop_order_list)
{
Expand All @@ -167,7 +174,7 @@ void gui_reset(dt_lib_module_t *self)

dt_dev_pixelpipe_rebuild(darktable.develop);

d->current_mode = DT_IOP_ORDER_V30;
d->current_mode = iop_order;
dt_lib_gui_set_label(self, _(dt_iop_order_string(d->current_mode)));
g_list_free_full(iop_order_list, free);
}
Expand All @@ -193,7 +200,15 @@ void init_presets(dt_lib_module_t *self)

list = dt_ioppr_get_iop_order_list_version(DT_IOP_ORDER_V30);
params = dt_ioppr_serialize_iop_order_list(list, &size);
dt_lib_presets_add(_("v3.0 for RAW input (default)"), self->plugin_name, self->version(),
dt_lib_presets_add(_("v3.0 for RAW input"), self->plugin_name, self->version(),
(const char *)params, (int32_t)size, TRUE, FALSE);
free(params);
dt_ioppr_iop_order_list_free(list);

// make it the default for new RAW
list = dt_ioppr_get_iop_order_list_version(DT_IOP_ORDER_V31);
params = dt_ioppr_serialize_iop_order_list(list, &size);
dt_lib_presets_add(_("v3.1 for RAW input (default)"), self->plugin_name, self->version(),
(const char *)params, (int32_t)size, TRUE,
is_display_referred ? 0 : FOR_RAW | FOR_MATRIX);
free(params);
Expand Down

0 comments on commit 6b18bae

Please sign in to comment.