diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index d0c112d9e07..21b1045b7d5 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -808,6 +808,21 @@ changes (where available). the chromaticity, hue, color or HSV blend modes in display RGB, where OpenCL gave the right color. +- Fixed stale color temperature and illuminant color in color calibration + after white balance changes when using "as shot in camera". + +- Fixed input color profile and highlight reconstruction (opposed and + segmentation modes) still applying part of the white balance + correction when the white balance module, set to "as shot to + reference", was switched off on a raw image. Edits made that way + render with different colors. + +- Fixed a color cast when white balance in "as shot to reference" mode + was pasted from another image, or applied through a style or preset: + white balance applied the pasted multipliers, but input color profile + completed the correction with the target image's own as-shot + multipliers. + ## Lua ### API Version diff --git a/dev-doc/GUI_Threading.md b/dev-doc/GUI_Threading.md index 135a0421e94..24c109b0a83 100644 --- a/dev-doc/GUI_Threading.md +++ b/dev-doc/GUI_Threading.md @@ -415,7 +415,9 @@ instance. It is torn down again straight away, and `exposure`'s `gui_cleanup()` instance behind it. In tree the callers carry that check: `dt_dev_exposure_get_effective_exposure()` resolves a live, enabled instance itself and passes that one, and the other two getters go through a helper that requires -`proxy.exposure.module` to be set (`src/develop/develop.c`). +`proxy.exposure.module` to be set (`src/develop/develop.c`). `dt_dev_cat_is_active()`, the +only caller of color calibration's `proxy.cat_is_active`, passes only the `channelmixerrgb` +instances it finds in `dev->iop` (`src/develop/develop.c`). The producing side — usually `commit_params()` or `process()` — must take the same lock. Publishing a raw pointer into `gui_data` through a proxy is worse still: the reader then diff --git a/dev-doc/IOP_Module_API.md b/dev-doc/IOP_Module_API.md index 62a89ad6dc9..01c3ead0899 100644 --- a/dev-doc/IOP_Module_API.md +++ b/dev-doc/IOP_Module_API.md @@ -488,7 +488,7 @@ Called when an image is loaded, including when switching images. Its purpose is When invoked through the wrapper `dt_iop_reload_defaults()`, the framework calls `dt_iop_load_default_params()` after the module's own `reload_defaults()` returns, copying `default_params` into `self->params`. So on the wrapper path, writing to `default_params` inside `reload_defaults()` immediately affects `self->params` as well. Most callers use the wrapper (for example `_dt_dev_load_pipeline_defaults()`, the per-module reset button, and instance duplication). -**Caveat: direct calls skip only the framework copy, not the module body.** A few sites invoke `module->reload_defaults(module)` directly instead of through the wrapper (for example, `dt_dev_read_history_ext()` calls `temperature->reload_defaults(temperature)` to refresh the white balance state). Those callers do **not** get the automatic `dt_iop_load_default_params()`, so the wholesale `default_params` -> `self->params` copy does not happen. This does **not** mean `self->params` is left untouched: the module's own `reload_defaults()` body may still write specific `self->params` fields directly. `temperature.reload_defaults()` does exactly this: it writes `p->preset` (via `p = self->params`) together with `default_params`, while the white balance coefficients are written to `default_params` only. On a direct call, the result is therefore a *partial* update: the fields the body touches change in `self->params`, the rest do not, and no history entry is recorded. If you add such a call, prefer `dt_iop_reload_defaults()`, or check exactly which `self->params` fields the body changes and sync the rest yourself. +**Caveat: direct calls skip only the framework copy, not the module body.** A few sites invoke `module->reload_defaults(module)` directly instead of through the wrapper (for example, `dt_dev_read_history_ext()` calls `temperature->reload_defaults(temperature)` to refresh the white balance state). Those callers do **not** get the automatic `dt_iop_load_default_params()`, so the wholesale `default_params` -> `self->params` copy does not happen. This does **not** mean `self->params` is left untouched: the module's own `reload_defaults()` body may still write specific `self->params` fields directly. `temperature.reload_defaults()` does exactly this: it writes `p->preset` and `p->late_correction` (via `p = self->params`) together with `default_params`, while the white balance coefficients are written to `default_params` only. On a direct call, the result is therefore a *partial* update: the fields the body touches change in `self->params`, the rest do not, and no history entry is recorded. If you add such a call, prefer `dt_iop_reload_defaults()`, or check exactly which `self->params` fields the body changes and sync the rest yourself. #### Job 1 (all modules that implement this): compute image-specific default params @@ -513,7 +513,9 @@ Widgets created with the `_from_params` helpers take their default value (the on Some modules use `reload_defaults()` to write shared state, such as `dev->chroma`, that other modules depend on. This happens whether or not the module is enabled or has a history entry. -Example: `temperature.c` always computes the camera's as-shot white balance coefficients and its D65 reference coefficients, and writes them into `dev->chroma.as_shot[]` and `dev->chroma.D65coeffs[]`. `channelmixerrgb.c` reads `D65coeffs` in its own `reload_defaults()`, `commit_params()` and `process()` for chromatic adaptation. White balance allows only one instance, which is always present when defaults are loaded, so its `reload_defaults()` runs on every image load. Color calibration therefore has this reference even when the white balance module is disabled or absent from the history. +Example: `temperature.c` always computes the camera's as-shot white balance coefficients and its D65 reference coefficients, and writes them into `dev->chroma.as_shot[]` and `dev->chroma.wb.D65coeffs[]`. `channelmixerrgb.c` reads `D65coeffs` for chromatic adaptation, from `dev->chroma` in its own `reload_defaults()` and from the per-pipe copy `pipe->wb` in `commit_params()` and `process()`. White balance allows only one instance, which is always present when defaults are loaded, so its `reload_defaults()` runs on every image load. Color calibration therefore has this reference even when the white balance module is disabled or absent from the history. + +[How white balance and color calibration share state](iop/wb_and_colorcalibration/README.md) lists which module writes and reads each field of `dev->chroma` and `pipe->wb`, and explains why the reverse-order default load is safe. #### Where `default_params` is consumed diff --git a/dev-doc/Module_Lifecycle.md b/dev-doc/Module_Lifecycle.md index 51248f46026..359adeb0625 100644 --- a/dev-doc/Module_Lifecycle.md +++ b/dev-doc/Module_Lifecycle.md @@ -365,17 +365,25 @@ history up to the given position, updates every module's GUI, then calls ## 4. Cross-module interactions -### 4a. White balance -> color calibration (through `dev->chroma`) +### 4a. White balance -> color calibration (through `dev->chroma` and `pipe->wb`) White balance (`temperature.c`) and color calibration (`channelmixerrgb.c`) share state in -`dev->chroma`; there is no signal. White balance writes the camera's reference coefficients in -`reload_defaults()` and the coefficients it applies in `commit_params()`; color calibration -reads them in `reload_defaults()`, `commit_params()` and `process()`. +`dev->chroma` and in each pipe's `pipe->wb`. There is no signal. White balance writes the +camera's reference coefficients into `dev->chroma` in `reload_defaults()`. In +`commit_params()`, it writes the coefficients it applies into `dev->chroma` and into its pipe's +`pipe->wb`. Color calibration reads `dev->chroma` in `reload_defaults()`, and `pipe->wb` in +`commit_params()` and `process()`. Its GUI calls `dt_dev_get_gui_wb()`, which asks white +balance, through a proxy, for the values that white balance's current params give. Commit order does not guarantee that white balance commits first: `synch_all` replays the history in history order, and `synch_top` re-commits only the top item. Color calibration -therefore recomputes its camera illuminant (`DT_ILLUMINANT_CAMERA`) in `process()`, which runs -after all the pipe's commits are done. +therefore recomputes the two illuminants that depend on white balance, `DT_ILLUMINANT_CAMERA` +and `DT_ILLUMINANT_FROM_WB`, in `process()`, which runs after all the pipe's commits are done. + +[How white balance and color calibration +share state](iop/wb_and_colorcalibration/README.md) lists which module writes and reads each +field, explains the per-pipe copy `pipe->wb` and the proxies between the two modules' GUIs, and +shows why the reverse-order default load stays correct. ### 4b. Color-profile change (colorin / colorout) @@ -424,7 +432,8 @@ Shared state that a `reload_defaults()` depends on must therefore be **reset to value before the reverse default-load**. The framework does this for white balance: it calls `dt_dev_reset_chroma()` just before `_dt_dev_load_pipeline_defaults()`. The neutral white balance coefficients make color calibration compute its default illuminant from the image's own -as-shot coefficients, instead of from values left by the previous image. +as-shot coefficients, instead of from values left by the previous image. [How white balance +and color calibration share state](iop/wb_and_colorcalibration/README.md) has the details. --- @@ -508,9 +517,17 @@ section above. | `enter` | views/darkroom.c | Darkroom entry: load image, create GUIs, reload defaults, replay history | | `_dev_load_requested_image` | views/darkroom.c | Darkroom image switch: keep base instances, read history, replay | +The white balance and color calibration symbols (`temperature.*`, `channelmixerrgb.*`, and the +`dev->chroma` helpers) are listed in [How white balance and color calibration share +state](iop/wb_and_colorcalibration/README.md#key-symbols). + ## See also - [IOP_Module_API.md](IOP_Module_API.md): the callback API reference (signatures, the two [`reload_defaults`](IOP_Module_API.md#reload_defaults---per-image-defaults) jobs). - [pixelpipe_architecture.md](pixelpipe_architecture.md): pipeline architecture, including the [ordering-asymmetry note](pixelpipe_architecture.md#pipeline-ordering-asymmetry). +- [How white balance and color calibration share + state](iop/wb_and_colorcalibration/README.md): the data that the two modules exchange through + `dev->chroma` and `pipe->wb`, as a worked example of the ordering rule in + [section 5](#5-pipeline-ordering-asymmetry). diff --git a/dev-doc/New_Module_Guide.md b/dev-doc/New_Module_Guide.md index 3ea2c9c2c16..37699a3968f 100644 --- a/dev-doc/New_Module_Guide.md +++ b/dev-doc/New_Module_Guide.md @@ -83,7 +83,7 @@ Ensure your module is in the correct place in `iop_order.c`. - **Scene-referred** modules should generally be earlier. - **Display-referred** modules should be later. -**Note:** Pipeline ordering affects `commit_params()` execution order. Modules that share state (e.g., via `dev->chroma`) depend on earlier modules having committed first. See [pixelpipe_architecture.md](pixelpipe_architecture.md#pipeline-ordering-asymmetry) for details. +**Note:** `commit_params()` does not always run in pipe order. During history replay, modules commit in history order. A module that reads state written by an earlier module, as color calibration reads white balance's `pipe->wb`, cannot rely on that module having committed first. See [pixelpipe_architecture.md](pixelpipe_architecture.md#pipeline-ordering-asymmetry) for details. ### Shortcuts By using `dt_bauhaus` widgets, shortcuts are automatically supported. diff --git a/dev-doc/iop/wb_and_colorcalibration/README.md b/dev-doc/iop/wb_and_colorcalibration/README.md new file mode 100644 index 00000000000..5c3f81b7d06 --- /dev/null +++ b/dev-doc/iop/wb_and_colorcalibration/README.md @@ -0,0 +1,439 @@ +# How white balance and color calibration share state + +White balance (`temperature.c`) hands its data to color calibration (`channelmixerrgb.c`) +through shared state. No signal passes between them. White balance writes its state into the +dev's `dev->chroma` and into each pipe's `pipe->wb`. Color calibration and several other +modules read it from there. Whether a reader sees current values depends on the order in which +darktable runs the two modules' callbacks. + +The two modules' GUIs ask each other through proxies, function pointers in `dev->proxy`. White +balance's GUI asks color calibration whether any instance applies chromatic adaptation. Color +calibration's GUI asks white balance for the values that white balance's current params give, +instead of reading `dev->chroma`. [White balance asks color calibration through a +proxy](#white-balance-asks-color-calibration-through-a-proxy) and [Color calibration asks white +balance through a proxy](#color-calibration-asks-white-balance-through-a-proxy) describe these +calls. + +This page describes that state and the ordering it relies on. For the general mechanics (pipe +change flags, `synch_all` and `synch_top`, the reverse-order default load), see +[Module_Lifecycle.md](../../Module_Lifecycle.md). + +--- + +## Where the state lives + +`dev->chroma` is a `dt_dev_chroma_t` (`develop.h`) on the dev +([`dt_develop_t`](../../pixelpipe_architecture.md#dt_develop_t)). It holds: + +- `temperature`, a pointer to the white balance instance, and `adaptation`, a pointer to the + color calibration instance that does chromatic adaptation. +- `as_shot[]`, the camera's as-shot coefficients. +- `wb`, a `dt_dev_wb_t` (`format.h`). Its `coeffs[]` are the coefficients that white balance + applies, `D65coeffs[]` is the camera's D65 reference, and `late_correction` says whether a + later module completes the correction to D65. + +Each pipe has its own `dt_dev_wb_t`, `pipe->wb`. White balance's `commit_params()` fills it +from the params that it commits, through `_wb_from_params()`. + +### Four places hold white balance values + +White balance values are also in two other places: the white balance piece's own data, and the +descriptor of each buffer that passes down the pipe. The table compares all four. For +`dev->chroma` and `pipe->wb`, it names only the main writers and readers. [Who writes and reads +each field](#who-writes-and-reads-each-field) lists all of them. + +| Place | Scope | Written by | Holds | Read by | +|---|---|---|---|---| +| `dev->chroma.wb` | all pipes of a dev | `temperature.commit_params()`, which copies `pipe->wb`, and `temperature.reload_defaults()` for `D65coeffs[]` | the values that white balance last committed, in any pipe | color calibration's `reload_defaults()`, and white balance's GUI for `D65coeffs[]`. `_wb_from_params()` gets `D65coeffs[]` from it | +| white balance's `piece->data` (`dt_iop_temperature_data_t`) | the white balance piece of one pipe | `temperature.commit_params()` | the coefficients that white balance multiplies into the image | white balance's `process()` and `process_cl()` | +| `pipe->wb` | one pipe | `temperature.commit_params()`, through `_wb_from_params()` | what white balance applied, the same for every module in the pipe | colorin, highlight reconstruction, and color calibration | +| `pipe->dsc.temperature` | one buffer | white balance's `process()` and `process_cl()`, through `_update_pipe_dsc()`, and colorin, which rewrites `coeffs[]` to `D65coeffs[]` when it completes the late correction | the white balance multiplied into this buffer | highlight reconstruction, demosaic (dual demosaic and capture sharpening), denoise (profiled), and the details mask | + +`pipe->dsc` is a `dt_iop_buffer_dsc_t`, the descriptor of the buffer that the pipe passes to the +next module. The pipe cache stores it with the buffer. If white balance is disabled, its +`process()` does not run, and `temperature` keeps the values of the image's input descriptor: +`enabled` is `FALSE` and `coeffs[]` are `0` (`dt_image_init()`). + +### Why each pipe has its own copy + +All pipes of a dev (main view, preview, and second window) share `dev->chroma`, and they run in +parallel on separate threads. White balance's `commit_params()` rewrites `dev->chroma` once per +pipe, while other pipes may still be processing. + +Late correction is split across the pipe. White balance multiplies the data by `coeffs[]` near +the start, and colorin later multiplies it by `D65coeffs[] / coeffs[]`. If colorin read +`dev->chroma`, it could get coefficients from a different commit than the one that white +balance applied to its input. The pixelpipe cache keys that mixed result on the pipe's own +parameters, so the wrong image stays in the cache. + +So code that runs in a pipe thread reads `pipe->wb`, never `dev->chroma.wb`. The one exception +is white balance's `commit_params()`, which copies `dev->chroma.wb.D65coeffs[]` into +`pipe->wb`. Color calibration's GUI does not read `dev->chroma.wb` either. It calls +`dt_dev_get_gui_wb()`, as described in [Color calibration asks white balance through a +proxy](#color-calibration-asks-white-balance-through-a-proxy). The remaining readers are white +balance's GUI and color calibration's `reload_defaults()`. + +--- + +## Who writes and reads each field + +### `dev->chroma` + +| Field | Written by | Read by | +|---|---|---| +| `as_shot[]` | `temperature.reload_defaults()` | `temperature` only (its "as shot" preset, GUI, and debug output) | +| `wb.D65coeffs[]` | `temperature.reload_defaults()` | `temperature` (GUI, `commit_params()`, and `_gui_wb_proxy()`, which pass it to `_wb_from_params()`), `channelmixerrgb` (`reload_defaults()`) | +| `wb.coeffs[]` | `temperature.commit_params()`, copied from `pipe->wb` | `channelmixerrgb` (`reload_defaults()`) | +| `wb.late_correction` | `temperature.commit_params()`, copied from `pipe->wb` | `channelmixerrgb` (`reload_defaults()`, through `_dev_is_D65_chroma()`) | +| `temperature` | `temperature.commit_params()` | `channelmixerrgb` (`_set_trouble_messages()`) | +| `adaptation` | `channelmixerrgb` (`_declare_cat_on_pipe()`, only when the module has a GUI; `gui_cleanup()` clears it) | `channelmixerrgb` (`reload_defaults()`, `_set_trouble_messages()`) | + +`dt_dev_get_gui_wb()` also copies all of `dev->chroma.wb` when no white balance instance +answers its proxy. + +In `channelmixerrgb`, only `reload_defaults()` reads `dev->chroma.wb`, through +`_get_d65_correction_ratios()` and `_dev_is_D65_chroma()`. While the defaults load, white +balance's params do not hold the edit's white balance yet, so `reload_defaults()` cannot ask +the proxy. The GUI functions `gui_changed()`, `_set_trouble_messages()`, +`_preview_pipe_finished_callback()`, `_illuminant_color_draw()`, `_update_approx_cct()`, and +`_auto_set_illuminant()` get their `dt_dev_wb_t` from `dt_dev_get_gui_wb()`. + +Two functions also reset fields. `dt_dev_reset_chroma()` sets `temperature` and `adaptation` to +`NULL` and `wb.coeffs[]` to `1.0`. It runs before the defaults of each image are loaded, and +from `channelmixerrgb.cleanup_pipe()`. `dt_dev_init_chroma()` also sets `wb` and `as_shot[]` to +neutral, when the dev is created and when it is cleaned up. + +Notes: + +- `temperature.reload_defaults()` writes `as_shot[]` and `wb.D65coeffs[]` on every image load, + even when white balance is disabled or absent from the history. So color calibration always + has the camera's reference. The exception is a call without a valid image, from the presets + code, which returns before it writes them. For images without a camera matrix, `as_shot[]` + stays `1.0`. +- In the `none` workflow, when both modules are enabled in the history, + `dt_dev_read_history_ext()` calls `temperature->reload_defaults(temperature)` a second time, + directly. This call skips the framework wrapper, so it updates only some fields of + `self->params`. + [IOP_Module_API.md](../../IOP_Module_API.md#reload_defaults---per-image-defaults) and + [Module_Lifecycle.md](../../Module_Lifecycle.md) describe the consequences. +- `channelmixerrgb` does not read `as_shot[]`. Color calibration takes its default illuminant + from `dev->image_storage.wb_coeffs`, as described in [Why the reverse-order default load is + safe](#why-the-reverse-order-default-load-is-safe). + +### `pipe->wb` + +`temperature.commit_params()` writes all three fields through `_wb_from_params()`. White balance +applies nothing when the piece is disabled or the image is true monochrome: + +- `coeffs[]` from the params, or `1.0` when white balance applies nothing. +- `late_correction` from the params. It is `FALSE` in camera reference mode + (`DT_IOP_TEMP_D65`) and when white balance applies nothing. +- `D65coeffs[]`, copied from `dev->chroma.wb`. + +`commit_params()` then copies `coeffs[]` and `late_correction` from `pipe->wb` into +`dev->chroma.wb`. + +`dt_dev_wb_set_neutral()` sets `pipe->wb` to neutral when the pipe is created. + +These functions read `pipe->wb`, all in the pipe thread: + +| Module | Functions | Use | +|---|---|---| +| color calibration | `commit_params()`, `process()`, `process_cl()` | the `DT_ILLUMINANT_CAMERA` and `DT_ILLUMINANT_FROM_WB` illuminants | +| colorin | `process()`, `process_cl()` | the late correction to D65 | +| highlights, clip mode | `process_clip()`, `process_cl()` | clip thresholds under late correction | +| opposed highlight reconstruction | `_process_opposed()`, `process_opposed_cl()` | the late correction | +| segmentation-based highlight reconstruction | `_process_segmentation()` | the late correction | + +--- + +## White balance asks color calibration through a proxy + +White balance asks color calibration two questions: + +- When the user switches from camera reference to "as shot" or a camera preset, `_update_preset()` + (`temperature.c`) sets `p->late_correction` to the result of `dt_dev_cat_is_active()` + (`develop.c`). The answer becomes part of white balance's params, and so of the history. + Switching to "user modified" or "from image area" keeps the stored flag, as camera reference + ignores it (`_wb_from_params()`) and the D65-screen workaround needs it off. +- `_update_controls_lock()` (`temperature.c`) calls `dt_dev_cat_overrides_wb()` to decide whether + to disable the controls that choose multipliers. This changes no params. + +Both functions (`develop.c`) use `_cat_source()`. It calls `dev->proxy.cat_source` for each +`channelmixerrgb` instance in `dev->iop` that is in the pipe (`iop_order != INT_MAX`), and +returns the maximum of the answers. The values of `dt_dev_cat_source_t` are ordered for this: + +| Value | Instance | Maximum over all instances means | +|---|---|---| +| `DT_DEV_CAT_NONE` | does no CAT | no instance does CAT | +| `DT_DEV_CAT_OWN_ILLUMINANT` | does CAT with an illuminant set in color calibration | CAT, and white balance's multipliers do not set the colors | +| `DT_DEV_CAT_FROM_WB` | does CAT with `DT_ILLUMINANT_FROM_WB` | at least one instance follows white balance | + +`dt_dev_cat_is_active()` tests for "not `NONE`", and `dt_dev_cat_overrides_wb()` for +`OWN_ILLUMINANT`. Masks play no part, and neither does `dev->chroma.adaptation`, which holds one +instance and skips masked ones. So swapping the illuminants of two masked instances changes +neither answer. + +Color calibration's `gui_init()` sets the pointer to `_cat_proxy_source()`. That function reads +the instance's current params. An instance does CAT when `_applies_cat()` is true: it is enabled, +its illuminant is not `DT_ILLUMINANT_PIPE`, the image is not monochrome, and its adaptation is +not `DT_ADAPTATION_RGB`. The illuminants derived from multipliers count whatever adaptation the +params hold, because `commit_params()` forces CAT16 for them. + +Color calibration does not notify white balance. White balance calls `_update_controls_lock()` +from `_update_preset()`, which its own GUI paths all reach, and on +`DT_SIGNAL_DEVELOP_PREVIEW_PIPE_FINISHED`. Every change in color calibration, including adding, +removing or switching off an instance, runs the preview pipe. The handler does not call +`gui_update()`, which would reset the color picker. It cancels an active picker while the +controls are locked. The signal does not fire if the pipe aborts, for example on an image +change, but `gui_update()` then runs anyway. + +The proxy has three limits: + +- Call it from the GTK main thread only (`develop.h`). It reads params that the GUI changes. +- Only `darktable.develop` gets the pointer. For any other dev, such as the one that + `_pin_image()` creates, and in `darktable-cli`, `dt_dev_cat_is_active()` returns `FALSE`. +- Nothing clears the pointer. The instance that `_init_module_so()` (`imageop.c`) creates and + destroys at startup also sets it, so a set pointer does not mean that a live instance exists. + This is why `dt_dev_cat_is_active()` asks only the instances in `dev->iop`. + +--- + +## Color calibration asks white balance through a proxy + +Color calibration's GUI uses white balance's values in several places. It sets `p->x` and +`p->y` from them in `gui_changed()` when the user selects `DT_ILLUMINANT_CAMERA` or +`DT_ILLUMINANT_FROM_WB`, or switches away from one of them, so the values go into the history. +It also uses them for the illuminant swatch, the approximate CCT, the spot measurement, and the +trouble messages. + +`dev->chroma.wb` does not suit these readers. It changes only when a pipe commits, so after a +white balance edit it holds the previous values until the next commit. A pipe thread can also +rewrite it while the GTK thread reads it. + +So these functions call `dt_dev_get_gui_wb()` (`develop.c`). It looks up the white balance +instance in `dev->iop` and calls `dev->proxy.gui_wb` for it. White balance's `gui_init()` sets +that pointer to `_gui_wb_proxy()`, which passes the instance's current params, +`self->enabled`, `self->hide_enable_button`, and `dev->chroma.wb.D65coeffs[]` to +`_wb_from_params()`. `commit_params()` uses the same function, so the GUI gets the values that +the next commit writes into `pipe->wb`. The GTK thread owns the params, so the GUI sees an edit +before any pipe commits it. + +This proxy has the same three limits as `cat_source`. Call it from the GTK main thread +only. Only `darktable.develop` gets the pointer. Nothing clears the pointer, so +`dt_dev_get_gui_wb()` asks only an instance that it finds in `dev->iop`. If no instance +answers, the helper copies `dev->chroma.wb`. For a `NULL` dev, it returns neutral values. + +--- + +## Commit order changes the stored illuminant, not the rendered one + +`synch_all` commits in two passes: + +1. The defaults of every piece, in **pipe order**. +2. The active history items, in **history order** (`_dev_pixelpipe_synch()`). + +During the history pass, `temperature.commit_params()` writes `pipe->wb` and the `dev->chroma` +fields listed above. `channelmixerrgb.commit_params()` reads `pipe->wb` to build its chromatic +adaptation matrix. History order is not pipe order. If the history ends in `[..., color +calibration, white balance]`, color calibration commits before white balance's last commit. It +reads the `pipe->wb` left by white balance's previous commit on the same pipe. + +`synch_top` re-commits only the top history item. If the user edits white balance while it is +the top item, `_dev_add_history_item_ext()` (`develop.c`) sets `DT_DEV_PIPE_TOP_CHANGED`, and +only white balance re-commits. Color calibration's `piece->data` keeps the illuminant that its +last commit or its last processing run computed. + +In both cases, processing replaces that illuminant. Only two illuminants depend on white +balance, `DT_ILLUMINANT_CAMERA` and `DT_ILLUMINANT_FROM_WB` (`_get_corrected_illuminant_xy()` in +`channelmixerrgb.c`). `channelmixerrgb.process()` and `process_cl()` look up both again from +`pipe->wb` into a local variable (`_get_pipe_illuminant()`), and leave `piece->data` as it is. +Processing starts only after all of the pipe's commits are done, so the lookup sees white +balance's final `pipe->wb`. The one exception is a failed `DT_ILLUMINANT_CAMERA` lookup, which +uses the illuminant from `piece->data`. [A failed as-shot lookup keeps +the saved illuminant](#a-failed-as-shot-lookup-keeps-the-saved-illuminant) explains why that is +safe. + +### A failed lookup from white balance gives D50 + +Every `DT_ILLUMINANT_FROM_WB` lookup goes through `_illuminant_xy_from_wb_or_D50()` +(`channelmixerrgb.c`). `commit_params()`, `process()`, `process_cl()`, and the GUI call it. If +the lookup fails, the helper returns D50. D50 is the pipe's own illuminant, so chromatic +adaptation then leaves the image unchanged. + +The helper calls `find_illuminant_xy_from_wb_coeffs()` when `late_correction` or +`camera_reference` is set. Otherwise it calls `_illuminant_xy_from_unprepared_wb()`, which uses the +as-shot lookup with the ratios `D65coeffs[] / coeffs[]`. White balance's multipliers then stay in the data and take +the place of the reference multipliers, as in the D65-screen workaround. They say nothing about +the scene, and using them as the illuminant would adapt from the calibration screen. A new +scene-referred edit reaches this case when an auto-applied workaround preset meets the default +illuminant. This is the calculation of `DT_ILLUMINANT_CAMERA`, with two differences that keep the +illuminant continuous and the failed lookup as described here. `_get_d65_correction_ratios()` +returns ratios of 1 when all multipliers are at most 1 (`changed_chroma`), which would make the +illuminant depend on their scale, and it accepts a zero multiplier, which fails here. The test +uses the mode, not `_dev_is_D65_chroma()`: multipliers that only happen to equal the reference +must not jump in color away from their neighbors. + +The lookup fails when the image has no usable camera matrix, or when one of the first three +values in `pipe->wb.coeffs[]` is zero or is not a normal float (`_wb_coeffs_invalid()` in +`illuminants.h`). White balance allows a zero coefficient. Its params start at `0.0`, and its +sliders take their range from the params. + +The fallback does not use the saved `p->x` and `p->y`. In this mode nothing keeps them up to +date. They hold the values from when the user selected the illuminant, or whatever a preset or +a pasted history stored. + +Commit order still changes the illuminant that `commit_params()` stores. Take a history whose +last two entries are color calibration in `DT_ILLUMINANT_FROM_WB` mode and white balance with a +zero red coefficient: + +- If color calibration commits first, its lookup uses the valid coefficients of white + balance's previous commit, and it stores that illuminant. +- If white balance commits first, color calibration's lookup fails, and it stores D50. + +Processing fails the same lookup in both cases and replaces the stored illuminant with D50, so +the two renders are the same. After `synch_top`, processing replaces the illuminant from before +the user's white balance edit in the same way. The stored illuminant has no other effect. +`commit_params()` also derives `d->p` from it, but only Bradford adaptation uses `d->p`, and +`commit_params()` always selects CAT16 for this illuminant. + +### A failed as-shot lookup keeps the saved illuminant + +A failed `DT_ILLUMINANT_CAMERA` lookup does not depend on commit order. The lookup scales the +image's own as-shot coefficients by ratios that are positive in the three channels the lookup +checks (`_get_d65_correction_ratios()`). So it fails only because of the image, for example +when the image has no camera matrix. It then fails at commit time too, and the illuminant +comes from `p->x` and `p->y` in every order. A D50 fallback for this illuminant would change +how existing edits render. + +--- + +## Why the reverse-order default load is safe + +`_dt_dev_load_pipeline_defaults()` walks the modules in **reverse** pipe order, so +`channelmixerrgb.reload_defaults()` runs **before** `temperature.reload_defaults()`. Color +calibration reads `dev->chroma` before white balance has refreshed it for the new image. A +reset before the load keeps the result correct. + +`dt_dev_read_history_ext()` calls `dt_dev_reset_chroma()` before the reverse default load. That +function clears the modules' trouble messages, sets `adaptation` and `temperature` to `NULL`, +and sets `wb.coeffs[]` to `1.0`: + +```c +chr->adaptation = NULL; +chr->temperature = NULL; +for_four_channels(c) + chr->wb.coeffs[c] = 1.0f; +``` + +It keeps the previous image's `wb.D65coeffs[]`, `as_shot[]`, and `wb.late_correction`. Only +`dt_dev_init_chroma()` sets those to neutral. + +`channelmixerrgb.reload_defaults()` then does two things: + +1. It calls `_get_d65_correction_ratios()`, which returns identity ratios (`1.0`) unless all of + these are true: the image has a camera matrix, `_dev_is_D65_chroma()` is false, + `D65coeffs[]` are valid, and `changed_chroma` is true: + + ```c + const gboolean changed_chroma = + wb->coeffs[0] > 1.0f || wb->coeffs[1] > 1.0f || wb->coeffs[2] > 1.0f; + ``` + + Right after the reset, `wb.coeffs[]` is exactly `1.0`, so `changed_chroma` is false and the + ratios stay at identity. The previous image's `D65coeffs[]` and `late_correction` cannot + change the result. If `late_correction` is set, `_dev_is_D65_chroma()` returns true, which + also gives identity ratios. +2. With identity ratios, `find_illuminant_xy_from_as_shot_coeffs()` computes the default + illuminant from `img->wb_coeffs` (`dev->image_storage.wb_coeffs`). These are the current + image's as-shot coefficients, read from the raw file. + +In the scene-referred workflow, `reload_defaults()` then selects `DT_ILLUMINANT_FROM_WB` if a +lookup with identity ratios succeeds. That lookup does not use `dev->chroma.wb`, which an +interactive reset of the module, without `dt_dev_reset_chroma()`, finds holding the edit's +values. `p->x` and `p->y` keep the values computed above. The other workflows keep +`DT_ILLUMINANT_CAMERA` and the snap to daylight or black body in +`_check_if_close_to_daylight()`. + +So the reset of `wb.coeffs[]` is what ties color calibration's default illuminant to the +current image. The default depends only on the image's own raw coefficients, never on what +`dev->chroma` held for the previous image. + +--- + +## Keep the first commit of each image a `synch_all` + +`synch_top` re-commits only the top history item. It needs an earlier full commit to have +written `pipe->wb`. Two facts make this hold today: + +- The first commit on every pipe is a `synch_all`. In the darkroom, `dt_dev_read_history_ext()` + calls `dt_dev_pipe_synch_all()` at load, and `dt_dev_pixelpipe_change()` skips `synch_top` + when the `SYNCH` flag is set (`... && !sync_all`). Export calls + `dt_dev_pixelpipe_synch_all()` directly. +- The first pass of `synch_all` commits every piece in pipe order. White balance therefore + writes `pipe->wb` before any later module commits or processes. + +`pipe->wb` is neutral only when the pipe is created (`dt_dev_wb_set_neutral()` in +`_dev_pixelpipe_init_cached()`). The darkroom keeps its pipes when it switches images. Until +white balance's first commit, `pipe->wb` still holds the previous image's values, and the first +`synch_all` overwrites them before anything reads them. + +Do not add a path that runs `synch_top` before the first `synch_all` of an image. Such a path +would let later modules read the previous image's `pipe->wb`. + +--- + +## Developer rules + +- Code that runs in a pipe thread reads `pipe->wb`, not `dev->chroma.wb`. +- GTK code that needs white balance's current values calls `dt_dev_get_gui_wb()`, not + `dev->chroma.wb`. +- A `reload_defaults()` must not depend on another module's `reload_defaults()` having run + earlier in the same walk over the modules. The walks do not all go in one direction. + `_dt_dev_load_pipeline_defaults()` walks the last module first, so modules earlier in the + pipe have not written shared state for the current image yet. `_pin_image()` and + `dt_dev_reload_history_items()` walk the first module first. +- If a `reload_defaults()` reads shared state, reset that state to neutral values before the + walk. `dt_dev_reset_chroma()` does this for `dev->chroma` before the defaults of each image + load. Color calibration stays correct only because that reset sets `wb.coeffs[]` to `1.0`, + which keeps the previous image's `D65coeffs[]` and `late_correction` out of the result. + +--- + +## Key symbols + +| Symbol | File | Role | +|---|---|---| +| `dt_dev_wb_t` | format.h | Holds `coeffs[]`, `D65coeffs[]`, `late_correction`, and `camera_reference`; the type of `dev->chroma.wb` and `pipe->wb` | +| `dt_dev_reset_chroma` | develop.c | Clears trouble messages, `adaptation`, and `temperature`; sets `wb.coeffs[]` to `1.0` | +| `dt_dev_init_chroma` | develop.c | Resets, then sets `wb` and `as_shot[]` to neutral; runs when the dev is created and cleaned up | +| `dt_dev_wb_set_neutral` | develop.c | Sets a `dt_dev_wb_t` to neutral; also used when a pipe is created (`pixelpipe_hb.c`) | +| `temperature.reload_defaults` | temperature.c | Writes `dev->chroma.as_shot[]` and `dev->chroma.wb.D65coeffs[]` | +| `temperature.commit_params` | temperature.c | Fills `pipe->wb` through `_wb_from_params()`; copies its `coeffs[]` and `late_correction` into `dev->chroma.wb`; writes `dev->chroma.temperature` | +| `_wb_from_params` | temperature.c | Computes the `dt_dev_wb_t` that white balance applies with the given params; used by `commit_params()` and `_gui_wb_proxy()` | +| `_gui_wb_proxy` | temperature.c | The proxy that white balance's `gui_init()` registers; returns `_wb_from_params()` for the instance's current params | +| `dt_dev_get_gui_wb` | develop.c | GUI: asks the white balance instance in `dev->iop`, through `dev->proxy.gui_wb`; copies `dev->chroma.wb` if no instance answers | +| `_update_preset` | temperature.c | GUI: when the user leaves camera reference for "as shot" or a camera preset, sets `p->late_correction` from `dt_dev_cat_is_active()`; calls `_update_controls_lock()` | +| `_update_controls_lock` | temperature.c | GUI: disables the controls that choose multipliers while `dt_dev_cat_overrides_wb()` is true, and shows the message | +| `dt_dev_cat_source_t` | develop.h | Where a color calibration instance gets its illuminant; ordered, so that the maximum describes the pipe | +| `dt_dev_cat_is_active` | develop.c | GUI: true if a color calibration instance in `dev->iop` applies chromatic adaptation | +| `dt_dev_cat_overrides_wb` | develop.c | GUI: true if an instance applies chromatic adaptation and none uses `DT_ILLUMINANT_FROM_WB` | +| `_cat_proxy_source` | channelmixerrgb.c | The proxy that color calibration's `gui_init()` registers; returns the instance's `dt_dev_cat_source_t` | +| `_applies_cat` | channelmixerrgb.c | True if the instance applies chromatic adaptation, matching what `commit_params()` does | +| `channelmixerrgb.reload_defaults` | channelmixerrgb.c | Computes the default illuminant from `img->wb_coeffs`; selects `DT_ILLUMINANT_FROM_WB` in the scene-referred workflow | +| `channelmixerrgb.commit_params` | channelmixerrgb.c | Reads `pipe->wb` to build the chromatic adaptation matrix | +| `_get_corrected_illuminant_xy` | channelmixerrgb.c | Illuminant xy for the params' illuminant; uses the given `dt_dev_wb_t` for `DT_ILLUMINANT_CAMERA` and `DT_ILLUMINANT_FROM_WB` | +| `_declare_cat_on_pipe` | channelmixerrgb.c | Registers the instance that does chromatic adaptation in `dev->chroma.adaptation` (GUI only) | +| `_get_d65_correction_ratios` | channelmixerrgb.c | `D65coeffs[] / coeffs[]` of the given `dt_dev_wb_t`, gated by `changed_chroma` | +| `_dev_is_D65_chroma` | channelmixerrgb.c | True when `late_correction` is set or `coeffs[]` equals `D65coeffs[]` | +| `find_illuminant_xy_from_as_shot_coeffs` | illuminants.h | Default illuminant xy from `img->wb_coeffs` | +| `find_illuminant_xy_from_wb_coeffs` | illuminants.h | Illuminant xy from white balance coefficients (the `DT_ILLUMINANT_FROM_WB` illuminant) | +| `_illuminant_xy_from_wb_or_D50` | channelmixerrgb.c | Illuminant xy from `find_illuminant_xy_from_wb_coeffs()` with late correction or in camera reference mode, otherwise from `_illuminant_xy_from_unprepared_wb()`, or D50 if that fails; used by every `DT_ILLUMINANT_FROM_WB` lookup | +| `_illuminant_xy_from_unprepared_wb` | channelmixerrgb.c | Illuminant xy from the as-shot coefficients, rescaled for multipliers that take the place of the camera reference | + +## See also + +- [Module_Lifecycle.md](../../Module_Lifecycle.md): the general lifecycle (load, edits, history + replay, pipe flags) and the rule for forward and reverse module order. +- [pixelpipe_architecture.md](../../pixelpipe_architecture.md): the pipeline architecture and + an overview of the ordering asymmetry. diff --git a/dev-doc/pixelpipe_architecture.md b/dev-doc/pixelpipe_architecture.md index 1c602592377..223177b858c 100644 --- a/dev-doc/pixelpipe_architecture.md +++ b/dev-doc/pixelpipe_architecture.md @@ -135,9 +135,9 @@ Two key pipeline operations iterate modules in **different** orders: - **`commit_params()`** (through `synch_all`) first commits every piece's defaults in forward pipe order, then replays the history items in **history order**, which is the order of the user's edits. So a downstream module can commit before an upstream one. Only processing itself is guaranteed to run in pipe order. - **`_dt_dev_load_pipeline_defaults()`** runs in **reverse** pipe order (e.g., channelmixerrgb before temperature). It is called when an image is loaded (`dt_dev_read_history_ext()`). -This matters for modules that communicate via shared state. For example, `temperature.c` writes white balance coefficients into `dev->chroma.wb_coeffs`, and `channelmixerrgb.c` reads them in `reload_defaults()`, `commit_params()` and `process()`. During reverse-order default loading, channelmixerrgb runs first, before temperature has refreshed its values, so any shared state a `reload_defaults()` depends on must be reset to a neutral value beforehand. During history replay, channelmixerrgb may commit before temperature, so it recomputes its white-balance-dependent illuminant in `process()`. +This matters for modules that communicate via shared state. For example, `temperature.c` writes white balance coefficients into `dev->chroma.wb.coeffs` and into each pipe's `pipe->wb`. `channelmixerrgb.c` reads `dev->chroma` in `reload_defaults()`, and `pipe->wb` in `commit_params()` and `process()`. During reverse-order default loading, channelmixerrgb runs first, before temperature has refreshed its values, so any shared state a `reload_defaults()` depends on must be reset to a neutral value beforehand. During history replay, channelmixerrgb may commit before temperature, so it recomputes its white-balance-dependent illuminant in `process()`. -**Consequence:** Shared state (like `dev->chroma`) must be reset before reverse-order iteration. The framework does this via `dt_dev_reset_chroma()` immediately before `_dt_dev_load_pipeline_defaults()`; the neutral `wb_coeffs` it writes is what keeps the dependent defaults image-local. See [Module_Lifecycle.md](Module_Lifecycle.md#5-pipeline-ordering-asymmetry). +**Consequence:** Shared state (like `dev->chroma`) must be reset before reverse-order iteration. The framework does this via `dt_dev_reset_chroma()` immediately before `_dt_dev_load_pipeline_defaults()`; the neutral `wb.coeffs` it writes is what keeps the dependent defaults image-local. See [Module_Lifecycle.md](Module_Lifecycle.md#5-pipeline-ordering-asymmetry). For the white balance and color calibration case in full, see [How white balance and color calibration share state](iop/wb_and_colorcalibration/README.md). ## Introspection Connection diff --git a/src/bauhaus/bauhaus.c b/src/bauhaus/bauhaus.c index a34eb4033e9..cc176add705 100644 --- a/src/bauhaus/bauhaus.c +++ b/src/bauhaus/bauhaus.c @@ -2808,8 +2808,13 @@ static gboolean _popup_draw(GtkWidget *widget, float label_width = 0.0f; if(!entry->sensitive) { - set_color(cr, text_color_insensitive); - label = g_markup_printf_escaped("%s", entry->label); + // dt_bauhaus_combobox_add_section() marks its titles with this data. + // Other entries are disabled: they must not stand out like a title + const gboolean is_section = entry->data == GINT_TO_POINTER(-1); + set_color(cr, is_section ? text_color_insensitive : text_color); + label = is_section + ? g_markup_printf_escaped("%s", entry->label) + : g_markup_printf_escaped("%s", entry->label); } else { diff --git a/src/common/exif.cc b/src/common/exif.cc index 8c82a5efd18..eacf0fd625f 100644 --- a/src/common/exif.cc +++ b/src/common/exif.cc @@ -2543,7 +2543,7 @@ static bool _exif_decode_exif_data(dt_image_t *img, Exiv2::ExifData &exifData) dt_dng_illuminant_t illu[3] = { DT_LS_Unknown, DT_LS_Unknown, DT_LS_Unknown }; // make sure for later testing fallback later via - // `find_temperature_from_raw_coeffs` if there is no valid illuminant + // `find_illuminant_xy_from_as_shot_coeffs` if there is no valid illuminant dt_mark_colormatrix_invalid(&img->d65_color_matrix[0]); #if EXIV2_TEST_VERSION(0,27,4) diff --git a/src/common/illuminants.h b/src/common/illuminants.h index 5f9530677c8..7f40a335cd4 100644 --- a/src/common/illuminants.h +++ b/src/common/illuminants.h @@ -34,6 +34,7 @@ typedef enum dt_illuminant_t DT_ILLUMINANT_BB = 6, // $DESCRIPTION: "Planckian (black body)" general black body radiator - not CIE standard DT_ILLUMINANT_CUSTOM = 7, // $DESCRIPTION: "custom" input x and y directly - bypass search DT_ILLUMINANT_CAMERA = 10,// $DESCRIPTION: "as shot in camera" read RAW EXIF for WB + DT_ILLUMINANT_FROM_WB = 11,// $DESCRIPTION: "as set in white balance module" read coefficients from the white balance module DT_ILLUMINANT_LAST, DT_ILLUMINANT_DETECT_SURFACES = 8, DT_ILLUMINANT_DETECT_EDGES = 9, @@ -215,19 +216,22 @@ static inline void illuminant_CCT_to_RGB(const float t, dt_aligned_pixel_t RGB) illuminant_xy_to_RGB(x, y, RGB); } +// Derive illuminant chromaticity from WB module coefficients +static inline gboolean find_illuminant_xy_from_wb_coeffs(const dt_image_t *img, const dt_aligned_pixel_t wb_coeffs, + float *chroma_x, float *chroma_y); // Fetch image from pipeline and read EXIF for camera RAW WB coeffs -static inline gboolean find_temperature_from_raw_coeffs(const dt_image_t *img, const dt_aligned_pixel_t custom_wb, +static inline gboolean find_illuminant_xy_from_as_shot_coeffs(const dt_image_t *img, const dt_aligned_pixel_t correction_ratios, float *chroma_x, float *chroma_y); -static inline int illuminant_to_xy(const dt_illuminant_t illuminant, // primary type of illuminant - const dt_image_t *img, // image container - const dt_aligned_pixel_t custom_wb, // optional user-set WB coeffs - float *x_out, float *y_out, // chromaticity output - const float t, // temperature in K, if needed - const dt_illuminant_fluo_t fluo, // sub-type of fluorescent illuminant, if needed - const dt_illuminant_led_t iled) // sub-type of led illuminant, if needed +static inline int illuminant_to_xy(const dt_illuminant_t illuminant, // primary type of illuminant + const dt_image_t *img, // image container + const dt_aligned_pixel_t correction_ratios, // optional D65 correction ratios derived from user-set coefficients + float *x_out, float *y_out, // chromaticity output + const float t, // temperature in K, if needed + const dt_illuminant_fluo_t fluo, // sub-type of fluorescent illuminant, if needed + const dt_illuminant_led_t iled) // sub-type of led illuminant, if needed { /** * Compute the x and y chromaticity coordinates in Yxy spaces for standard illuminants @@ -294,14 +298,19 @@ static inline int illuminant_to_xy(const dt_illuminant_t illuminant, // primary // Model valid for T in [1667 ; 25000] K CCT_to_xy_blackbody(t, &x, &y); if(y != 0.f && x != 0.f) break; - // else t is out of bounds -> use custom/original values (next case) + // else t is out of bounds -> use the camera illuminant (next case): + // existing edits can hold such a t, and they render with it } case DT_ILLUMINANT_CAMERA: { - // Detect WB from RAW EXIF - if(img) - if(find_temperature_from_raw_coeffs(img, custom_wb, &x, &y)) break; + // Detect WB from RAW EXIF, correcting with D65/wb_coeff ratios + if(img && find_illuminant_xy_from_as_shot_coeffs(img, correction_ratios, &x, &y)) + break; + + // xy calculation failed + return FALSE; } + case DT_ILLUMINANT_FROM_WB: // needs white balance's coefficients: the caller derives x and y case DT_ILLUMINANT_CUSTOM: // leave x and y as-is case DT_ILLUMINANT_DETECT_EDGES: case DT_ILLUMINANT_DETECT_SURFACES: @@ -325,15 +334,15 @@ static inline int illuminant_to_xy(const dt_illuminant_t illuminant, // primary static inline void WB_coeffs_to_illuminant_xy(const float CAM_to_XYZ[4][3], const dt_aligned_pixel_t WB, float *x, float *y) { - // Find the illuminant chromaticity x y from RAW WB coeffs and camera input matrice + // Find the illuminant chromaticity x y from RAW WB coeffs and camera input matrix dt_aligned_pixel_t XYZ, LMS; // Simulate white point, aka convert (1, 1, 1) in camera space to XYZ - // warning : we multiply the transpose of CAM_to_XYZ since the pseudoinverse transposes it + // warning: we multiply the transpose of CAM_to_XYZ since the pseudoinverse transposes it XYZ[0] = CAM_to_XYZ[0][0] / WB[0] + CAM_to_XYZ[1][0] / WB[1] + CAM_to_XYZ[2][0] / WB[2]; XYZ[1] = CAM_to_XYZ[0][1] / WB[0] + CAM_to_XYZ[1][1] / WB[1] + CAM_to_XYZ[2][1] / WB[2]; XYZ[2] = CAM_to_XYZ[0][2] / WB[0] + CAM_to_XYZ[1][2] / WB[1] + CAM_to_XYZ[2][2] / WB[2]; - // Matrices white point is D65. We need to convert it for darktable's pipe (D50) + // Matrix's white point is D65. We need to convert it for darktable's pipe (D50) static const dt_aligned_pixel_t D65 = { 0.941238f, 1.040633f, 1.088932f, 0.f }; const float p = powf(1.088932f / 0.818155f, 0.0834f); @@ -385,31 +394,11 @@ static inline void matrice_pseudoinverse(float (*in)[3], float (*out)[3], int si } } - -static gboolean find_temperature_from_raw_coeffs(const dt_image_t *img, const dt_aligned_pixel_t custom_wb, - float *chroma_x, float *chroma_y) +// returns TRUE if OK, FALSE if failed +static gboolean get_CAM_to_XYZ(const dt_image_t * img, float(* CAM_to_XYZ)[3]) { - if(img == NULL) return FALSE; - if(!dt_image_is_matrix_correction_supported(img)) return FALSE; - - gboolean has_valid_coeffs = TRUE; - const int num_coeffs = (img->flags & DT_IMAGE_4BAYER) ? 4 : 3; - - // Check coeffs - for(int k = 0; has_valid_coeffs && k < num_coeffs; k++) - if(!dt_isnormal(img->wb_coeffs[k]) || img->wb_coeffs[k] == 0.0f) has_valid_coeffs = FALSE; - - if(!has_valid_coeffs) return FALSE; - - // Get white balance camera factors - dt_aligned_pixel_t WB = { img->wb_coeffs[0], img->wb_coeffs[1], img->wb_coeffs[2], img->wb_coeffs[3] }; - - // Adapt the camera coeffs with custom white balance if provided - // this can deal with WB coeffs that don't use the input matrix reference - if(custom_wb) - for(size_t k = 0; k < 4; k++) WB[k] *= custom_wb[k]; - - // Get the camera input profile (matrice of primaries) + if(img == NULL || CAM_to_XYZ == NULL) return FALSE; + // Get the camera input profile (matrice of primaries) - embedded or raw library DB float XYZ_to_CAM[4][3]; dt_mark_colormatrix_invalid(&XYZ_to_CAM[0][0]); @@ -438,21 +427,86 @@ static gboolean find_temperature_from_raw_coeffs(const dt_image_t *img, const dt if(!dt_is_valid_colormatrix(XYZ_to_CAM[0][0])) return FALSE; - // Bloody input matrices define XYZ -> CAM transform, as if we often needed camera profiles to output - // So we need to invert them. Here go your CPU cycles again. - float CAM_to_XYZ[4][3]; dt_mark_colormatrix_invalid(&CAM_to_XYZ[0][0]); matrice_pseudoinverse(XYZ_to_CAM, CAM_to_XYZ, 3); - if(!dt_is_valid_colormatrix(CAM_to_XYZ[0][0])) return FALSE; + return dt_is_valid_colormatrix(CAM_to_XYZ[0][0]); +} + +static inline gboolean _wb_coeffs_invalid(const dt_aligned_pixel_t wb_coeffs, const int num_coeffs) +{ + for(int k = 0; k < num_coeffs; k++) + if(!dt_isnormal(wb_coeffs[k])) return TRUE; + + return FALSE; +} + +// a small multiplier puts the illuminant near a camera primary, which can lie +// outside the spectral locus: x, y or z, or a CAT16 cone response, becomes +// zero or negative, and CAT16 would then divide by zero or flip the sign of a +// channel. CAT16 is the adaptation color calibration forces for the illuminants +// derived from multipliers (_check_if_close_to_daylight()). The D and BB fallback +// in illuminant_to_xy() gets here too, with whatever adaptation the edit stored +static inline gboolean _is_usable_illuminant_xy(const float x, const float y) +{ + const float z = 1.f - x - y; + // written as a positive test, so that NaN fails too + if(!(x > 0.f && y > 0.f && z > 0.f)) return FALSE; + + dt_aligned_pixel_t XYZ = { 0.f }, LMS = { 0.f }; + illuminant_xy_to_XYZ(x, y, XYZ); + convert_XYZ_to_CAT16_LMS(XYZ, LMS); + return LMS[0] > 0.f && LMS[1] > 0.f && LMS[2] > 0.f; +} + +// returns FALSE if failed; TRUE if successful. chroma_x and chroma_y are +// written only on success: some callers ignore the result and keep using them +static gboolean find_illuminant_xy_from_wb_coeffs(const dt_image_t *img, const dt_aligned_pixel_t wb_coeffs, + float *chroma_x, float *chroma_y) +{ + if(img == NULL || wb_coeffs == NULL) return FALSE; + if(!dt_image_is_matrix_correction_supported(img)) return FALSE; + // it's enough to check the first 3 here, as WB_coeffs_to_illuminant_xy only uses indices 0 to 2 + if(_wb_coeffs_invalid(wb_coeffs, 3)) return FALSE; + + float CAM_to_XYZ[4][3]; + if(!get_CAM_to_XYZ(img, CAM_to_XYZ)) { + return FALSE; + } float x, y; - WB_coeffs_to_illuminant_xy(CAM_to_XYZ, WB, &x, &y); + WB_coeffs_to_illuminant_xy(CAM_to_XYZ, wb_coeffs, &x, &y); + if(!_is_usable_illuminant_xy(x, y)) return FALSE; + *chroma_x = x; *chroma_y = y; - return TRUE; } +// returns FALSE if failed; TRUE if successful +static gboolean find_illuminant_xy_from_as_shot_coeffs(const dt_image_t *img, const dt_aligned_pixel_t correction_ratios, + float *chroma_x, float *chroma_y) +{ + if(img == NULL) return FALSE; + const int num_coeffs = (img->flags & DT_IMAGE_4BAYER) ? 4 : 3; + + if(_wb_coeffs_invalid(img->wb_coeffs, num_coeffs)) return FALSE; + + // Get as-shot white balance camera factors (from raw) + // component wise raw-RGB * wb_coeffs should provide R=G=B for a neutral patch under + // the scene illuminant + dt_aligned_pixel_t WB = { img->wb_coeffs[0], img->wb_coeffs[1], img->wb_coeffs[2], img->wb_coeffs[3] }; + + // Adapt the camera coeffs with custom D65 coefficients if provided ('caveats' workaround) + // this can deal with WB coeffs that don't use the input matrix reference. + // correction_ratios come from channelmixerrgb.c _get_d65_correction_ratios(): + // D65coeffs[k] / coeffs[k] of a dt_dev_wb_t, or 1.0 + if(correction_ratios) + for(size_t k = 0; k < 4; k++) WB[k] *= correction_ratios[k]; + // for a neutral surface, raw RGB * img->wb_coeffs would produce neutral R=G=B + + return find_illuminant_xy_from_wb_coeffs(img, WB, chroma_x, chroma_y); +} + DT_OMP_DECLARE_SIMD() static inline float planckian_normal(const float x, const float t) @@ -471,31 +525,6 @@ static inline float planckian_normal(const float x, const float t) } -DT_OMP_DECLARE_SIMD() -static inline void blackbody_xy_to_tinted_xy(const float x, const float y, const float t, const float tint, - float *x_out, float *y_out) -{ - // Move further away from planckian locus in the orthogonal direction, by an amount of "tint" - const float n = planckian_normal(x, t); - const float norm = sqrtf(1.f + n * n); - *x_out = x + tint * n / norm; - *y_out = y - tint / norm; -} - - -DT_OMP_DECLARE_SIMD() -static inline float get_tint_from_tinted_xy(const float x, const float y, const float t) -{ - // Find the distance between planckian locus and arbitrary x y chromaticity in the orthogonal direction - const float n = planckian_normal(x, t); - const float norm = sqrtf(1.f + n * n); - float x_bb, y_bb; - CCT_to_xy_blackbody(t, &x_bb, &y_bb); - const float tint = -(y - y_bb) * norm; - return tint; -} - - DT_OMP_DECLARE_SIMD() static inline void xy_to_uv(const float xy[2], float uv[2]) { diff --git a/src/develop/develop.c b/src/develop/develop.c index 0540198e3f6..b4be637cdaf 100644 --- a/src/develop/develop.c +++ b/src/develop/develop.c @@ -3867,6 +3867,75 @@ void dt_dev_exposure_handle_event(int n_press, gdouble delta, darktable.develop->proxy.exposure.handle_event(n_press, delta, state, is_blackpoint); } +static dt_dev_cat_source_t _cat_source(dt_develop_t *dev) +{ + // only color calibration's gui_init() sets this, only on darktable.develop, + // and nothing clears it. The throw-away instance from _init_module_so() sets + // it at startup, so a set pointer does not mean a live instance: ask only the + // instances in dev->iop. Other devs, and darktable-cli, keep it NULL + if(!dev || !dev->proxy.cat_source) + return DT_DEV_CAT_NONE; + + dt_dev_cat_source_t source = DT_DEV_CAT_NONE; + + for(const GList *modules = dev->iop; modules; modules = g_list_next(modules)) + { + dt_iop_module_t *mod = modules->data; + + if(dt_iop_module_is(mod, "channelmixerrgb") && mod->iop_order != INT_MAX) + source = MAX(source, dev->proxy.cat_source(mod)); + } + + return source; +} + +gboolean dt_dev_cat_is_active(dt_develop_t *dev) +{ + return _cat_source(dev) != DT_DEV_CAT_NONE; +} + +gboolean dt_dev_cat_overrides_wb(dt_develop_t *dev) +{ + return _cat_source(dev) == DT_DEV_CAT_OWN_ILLUMINANT; +} + +static const dt_iop_module_t *_find_temperature(dt_develop_t *dev) +{ + for(const GList *modules = dev->iop; modules; modules = g_list_next(modules)) + { + const dt_iop_module_t *mod = modules->data; + + if(dt_iop_module_is(mod, "temperature")) + return mod; + } + + return NULL; +} + +void dt_dev_get_gui_wb(dt_develop_t *dev, dt_dev_wb_t *wb) +{ + if(!dev) + { + dt_dev_wb_set_neutral(wb); + return; + } + + // only temperature's gui_init() sets the proxy, only on darktable.develop, + // and nothing clears it: other devs, and darktable-cli, keep it NULL. The + // throw-away instance from _init_module_so() sets it at startup, so a set + // proxy does not mean a live instance: look one up in dev->iop + const dt_iop_module_t *temperature = dev->proxy.gui_wb ? _find_temperature(dev) : NULL; + + if(temperature) + { + dev->proxy.gui_wb(temperature, wb); + } + else + { + memcpy(wb, &dev->chroma.wb, sizeof(dt_dev_wb_t)); + } +} + void dt_dev_modulegroups_set(dt_develop_t *dev, const uint32_t group) { @@ -4472,19 +4541,27 @@ void dt_dev_reset_chroma(dt_develop_t *dev) chr->adaptation = NULL; chr->temperature = NULL; for_four_channels(c) - chr->wb_coeffs[c] = 1.0f; + chr->wb.coeffs[c] = 1.0f; +} + +void dt_dev_wb_set_neutral(dt_dev_wb_t *wb) +{ + wb->late_correction = FALSE; + wb->camera_reference = FALSE; + for_four_channels(c) + { + wb->coeffs[c] = 1.0f; + wb->D65coeffs[c] = 1.0; + } } void dt_dev_init_chroma(dt_develop_t *dev) { dt_dev_reset_chroma(dev); dt_dev_chroma_t *chr = &dev->chroma; - chr->late_correction = FALSE; + dt_dev_wb_set_neutral(&chr->wb); for_four_channels(c) - { - chr->D65coeffs[c] = 1.0; chr->as_shot[c] = 1.0; - } } // clang-format off diff --git a/src/develop/develop.h b/src/develop/develop.h index 753fbc4de41..982fe114d79 100644 --- a/src/develop/develop.h +++ b/src/develop/develop.h @@ -167,33 +167,32 @@ typedef struct dt_dev_viewport_t struct dt_develop_t *dev; } dt_dev_viewport_t; -/* keep track on what and where we do chromatic adaptation, used - a) to display warnings in GUI of modules that should probably not be doing - white balance - b) missing required white balance - c) allow late correction of as-shot or modified coeffs to D65 in color-input - but keep processing until then with more reliable (at least for highlights, - raw chromatic aberrations and more. - d) avoids keeping of fixed data in temperature gui data - e) we have 3 coefficients kept here - - the currently used wb_coeffs in temperature module - - D65coeffs and as_shot are read from exif data - f) - late_correction set by temperature if we want to process data as following - If we use the new DT_IOP_TEMP_D65_LATE mode in temperature.c and don#t have - any temp parameters changes later we can calc correction coeffs to modify - as_shot rgb data to D65 +/* white balance and chromatic adaptation state, shared by all pipes of a dev + - temperature, adaptation: pipe threads set them + - as_shot: only temperature's GUI reads it + - wb: what temperature's commit_params() last wrote, in any pipe. processing code reads + pipe->wb instead. GUI code calls dt_dev_get_gui_wb(). the exceptions are + temperature.c commit_params() and channelmixerrgb.c reload_defaults(), see their + comments. */ typedef struct dt_dev_chroma_t { struct dt_iop_module_t *temperature; // always available for GUI reports struct dt_iop_module_t *adaptation; // set if one module is processing this without blending - dt_aligned_pixel_t wb_coeffs; // coeffs actually set by temperature - double D65coeffs[4]; // both read from exif data or "best guess" double as_shot[4]; - gboolean late_correction; + dt_dev_wb_t wb; } dt_dev_chroma_t; +// where a channelmixerrgb instance gets the illuminant for chromatic adaptation. +// ordered: the maximum over all instances describes the whole pipe +typedef enum dt_dev_cat_source_t +{ + DT_DEV_CAT_NONE = 0, // this instance does no CAT + DT_DEV_CAT_OWN_ILLUMINANT, // an illuminant set in channelmixerrgb + DT_DEV_CAT_FROM_WB // DT_ILLUMINANT_FROM_WB +} dt_dev_cat_source_t; + typedef struct dt_develop_t { gboolean gui_attached; // != 0 if the gui should be notified of changes in hist stack and modules should be @@ -285,11 +284,19 @@ typedef struct dt_develop_t /* proxy for communication between plugins and develop/darkroom */ struct { - // list of exposure iop instances, with plugin hooks, used by - // histogram dragging functions each element is - // dt_dev_proxy_exposure_t + // exposure iop hooks, used by histogram dragging functions dt_dev_proxy_exposure_t exposure; + // channelmixerrgb reports where an instance gets its illuminant from; + // temperature asks through dt_dev_cat_is_active() and dt_dev_cat_overrides_wb() + // thread contract: must only be called from the GTK main thread + dt_dev_cat_source_t (*cat_source)(struct dt_iop_module_t *cat); + + // temperature reports the WB that its commit_params() would publish for its + // current params; color calibration's GUI asks through dt_dev_get_gui_wb() + // thread contract: must only be called from the GTK main thread + void (*gui_wb)(const struct dt_iop_module_t *temperature, dt_dev_wb_t *wb); + // this module receives right-drag events if not already claimed struct dt_iop_module_t *rotate; @@ -582,6 +589,20 @@ void dt_dev_exposure_handle_event(int n_press, gdouble delta, GdkModifierType state, const gboolean is_blackpoint); +/** TRUE if a live color calibration instance is actually applying CAT. + GTK main thread only: the accessor reads params. */ +gboolean dt_dev_cat_is_active(dt_develop_t *dev); + +/** TRUE if a live color calibration instance is applying CAT and none of them + uses the illuminant from white balance, so white balance's multipliers don't + decide the colors. GTK main thread only: the accessor reads params. */ +gboolean dt_dev_cat_overrides_wb(dt_develop_t *dev); + +/** the WB that temperature's current params give, as its commit_params() + would publish it; dev->chroma.wb if no temperature instance answers. + GTK main thread only: the accessor reads params. */ +void dt_dev_get_gui_wb(dt_develop_t *dev, dt_dev_wb_t *wb); + /* * modulegroups plugin hooks */ @@ -730,6 +751,8 @@ static inline struct dt_iop_module_t *dt_dev_gui_module(void) return darktable.develop ? darktable.develop->gui_module : NULL; } +void dt_dev_wb_set_neutral(dt_dev_wb_t *wb); + G_END_DECLS // clang-format off diff --git a/src/develop/format.h b/src/develop/format.h index 6536f66d8fc..a1d6ba65be5 100644 --- a/src/develop/format.h +++ b/src/develop/format.h @@ -26,6 +26,17 @@ struct dt_dev_pixelpipe_iop_t; struct dt_dev_pixelpipe_t; struct dt_iop_module_t; +// white balance state: what temperature applied, plus the fixed per-image +// references it is measured against +typedef struct dt_dev_wb_t +{ + dt_aligned_pixel_t coeffs; // coeffs actually set by temperature + // an image constant, but kept here so processing code never reads dev->chroma + double D65coeffs[4]; // read from exif data or "best guess" + gboolean late_correction; + gboolean camera_reference; // temperature is in its 'camera reference' mode +} dt_dev_wb_t; + typedef enum dt_iop_buffer_type_t { TYPE_UNKNOWN, TYPE_FLOAT, @@ -52,6 +63,10 @@ typedef struct dt_iop_buffer_dsc_t struct { gboolean enabled; + + /** white balance multiplied into this buffer. pipe->wb.coeffs keeps what + temperature applied; this copy is rewritten to D65coeffs by colorin when + it completes the late correction */ dt_aligned_pixel_t coeffs; } temperature; diff --git a/src/develop/pixelpipe_hb.c b/src/develop/pixelpipe_hb.c index 5ed66470855..a801c774dc3 100644 --- a/src/develop/pixelpipe_hb.c +++ b/src/develop/pixelpipe_hb.c @@ -326,6 +326,7 @@ static gboolean _dev_pixelpipe_init_cached(dt_dev_pixelpipe_t *pipe, memset(pipe->mask_distort_buf, 0, sizeof(pipe->mask_distort_buf)); memset(pipe->mask_distort_buf_size, 0, sizeof(pipe->mask_distort_buf_size)); pipe->mask_cache_size = 0; + dt_dev_wb_set_neutral(&pipe->wb); return dt_dev_pixelpipe_cache_init(pipe, entries, size, fraction); } @@ -840,6 +841,7 @@ void dt_dev_pixelpipe_synch_all(dt_dev_pixelpipe_t *pipe, dt_develop_t *dev) double start = dt_get_debug_wtime(); dev->cropping.exposer = NULL; + dt_print_pipe(DT_DEBUG_PARAMS, "synch all module defaults", pipe, NULL, DT_DEVICE_NONE, NULL, NULL); diff --git a/src/develop/pixelpipe_hb.h b/src/develop/pixelpipe_hb.h index b1e5c2d1480..169781ca160 100644 --- a/src/develop/pixelpipe_hb.h +++ b/src/develop/pixelpipe_hb.h @@ -229,6 +229,11 @@ typedef struct dt_dev_pixelpipe_t // and should be modified by process*(), if necessary. dt_iop_buffer_dsc_t dsc; + // white balance of the generation this pipe is rendering, written by + // temperature's commit_params. dev->chroma is shared by all pipes and can + // change while this one runs, so modules must read it from here, not from dev + dt_dev_wb_t wb; + /** work profile info of the image */ struct dt_iop_order_iccprofile_info_t *work_profile_info; /** input profile info **/ diff --git a/src/iop/channelmixerrgb.c b/src/iop/channelmixerrgb.c index 4a84c1a9a88..d44f9bdbdde 100644 --- a/src/iop/channelmixerrgb.c +++ b/src/iop/channelmixerrgb.c @@ -149,8 +149,8 @@ typedef struct dt_iop_channelmixer_rgb_gui_data_t GtkWidget *scale_grey_R, *scale_grey_G, *scale_grey_B; GtkWidget *normalize_R, *normalize_G, *normalize_B, *normalize_sat, *normalize_light, *normalize_grey; GtkWidget *color_picker; - float xy[2]; - float XYZ[4]; + float colorchecker_xy[2]; + float ai_wb_XYZ[4]; point_t box[4]; // the current coordinates, possibly non rectangle, of the bounding box for the color checker point_t ideal_box[4]; // the desired coordinates of the perfect rectangle bounding box for the color checker @@ -591,12 +591,9 @@ void init_presets(dt_iop_module_so_t *self) self->version(), &p, sizeof(p), TRUE, DEVELOP_BLEND_CS_RGB_SCENE); } -static gboolean _dev_is_D65_chroma(const dt_develop_t *dev) +static gboolean _dev_is_D65_chroma(const dt_dev_wb_t *wb) { - const dt_dev_chroma_t *chr = &dev->chroma; - return chr->late_correction - ? dt_dev_equal_chroma(chr->wb_coeffs, chr->as_shot) - : dt_dev_equal_chroma(chr->wb_coeffs, chr->D65coeffs); + return wb->late_correction || dt_dev_equal_chroma(wb->coeffs, wb->D65coeffs); } static gboolean _area_mapping_active(const dt_iop_channelmixer_rgb_gui_data_t *g) @@ -612,38 +609,102 @@ static const char *_area_mapping_section_text(const dt_iop_channelmixer_rgb_gui_ return _area_mapping_active(g) ? _("area color mapping (active)") : _("area color mapping"); } -static gboolean _get_white_balance_coeff(const dt_iop_module_t *self, - dt_aligned_pixel_t custom_wb) +static gboolean _get_d65_correction_ratios(const dt_iop_module_t *self, + const dt_dev_wb_t *wb, + dt_aligned_pixel_t out_correction_ratios) { - const dt_dev_chroma_t *chr = &self->dev->chroma; - // Init output with a no-op for_four_channels(k) - custom_wb[k] = 1.0f; + out_correction_ratios[k] = 1.0f; if(!dt_image_is_matrix_correction_supported(&self->dev->image_storage)) return TRUE; // If we use D65 there are unchanged corrections - if(_dev_is_D65_chroma(self->dev)) + if(_dev_is_D65_chroma(wb)) return FALSE; const gboolean valid_chroma = - chr->D65coeffs[0] > 0.0 && chr->D65coeffs[1] > 0.0 && chr->D65coeffs[2] > 0.0; + wb->D65coeffs[0] > 0.0 && wb->D65coeffs[1] > 0.0 && wb->D65coeffs[2] > 0.0; const gboolean changed_chroma = - chr->wb_coeffs[0] > 1.0f || chr->wb_coeffs[1] > 1.0f || chr->wb_coeffs[2] > 1.0f; + wb->coeffs[0] > 1.0f || wb->coeffs[1] > 1.0f || wb->coeffs[2] > 1.0f; // Otherwise - for example because the user made a correct preset, find the // WB adaptation ratio if(valid_chroma && changed_chroma) { for_four_channels(k) - custom_wb[k] = (float)chr->D65coeffs[k] / chr->wb_coeffs[k]; + { + if(wb->coeffs[k] > 1e-6f) + out_correction_ratios[k] = wb->D65coeffs[k] / wb->coeffs[k]; + else + out_correction_ratios[k] = 1.0f; + } } return FALSE; } +// without late correction, temperature's multipliers stay in the data and take +// the place of the camera reference, as coefficients measured on a D65 screen do +// (the 'caveats' workaround in find_illuminant_xy_from_as_shot_coeffs()). They say +// nothing about the scene, but the as-shot coefficients do, once rescaled for them. +// _get_d65_correction_ratios() doesn't suit: its changed_chroma gate would make the +// illuminant depend on the scale of the multipliers +static gboolean _illuminant_xy_from_unprepared_wb(const dt_image_t *img, + const dt_dev_wb_t *wb, + float *x, float *y) +{ + if(_wb_coeffs_invalid(wb->coeffs, 3)) + return FALSE; + + dt_aligned_pixel_t correction_ratios = { 1.f, 1.f, 1.f, 1.f }; + for(int k = 0; k < 3; k++) + correction_ratios[k] = wb->D65coeffs[k] / wb->coeffs[k]; + + return find_illuminant_xy_from_as_shot_coeffs(img, correction_ratios, x, y); +} + +// returns FALSE if the lookup failed; x and y are then D50, so callers never +// keep an illuminant from an earlier white balance setting +static gboolean _illuminant_xy_from_wb_or_D50(const dt_iop_module_t *self, + const dt_dev_wb_t *wb, + float *x, float *y) +{ + const dt_image_t *img = &(self->dev->image_storage); + + // tests the mode: _dev_is_D65_chroma() would also accept unprepared multipliers + // that happen to equal the reference, a jump in color from their neighbors + const gboolean found = wb->late_correction || wb->camera_reference + ? find_illuminant_xy_from_wb_coeffs(img, wb->coeffs, x, y) + : _illuminant_xy_from_unprepared_wb(img, wb, x, y); + + if(found) + return TRUE; + + *x = D50xyY.x; + *y = D50xyY.y; + return FALSE; +} + +static void _get_corrected_illuminant_xy(const dt_iop_module_t *self, + const dt_dev_wb_t *wb, + const dt_iop_channelmixer_rgb_params_t *p, + float *x, float *y) +{ + if(p->illuminant == DT_ILLUMINANT_FROM_WB) + { + _illuminant_xy_from_wb_or_D50(self, wb, x, y); + return; + } + + dt_aligned_pixel_t correction_ratios; + _get_d65_correction_ratios(self, wb, correction_ratios); + illuminant_to_xy(p->illuminant, &(self->dev->image_storage), correction_ratios, + x, y, p->temperature, p->illum_fluo, + p->illum_led); +} + DT_OMP_DECLARE_SIMD(aligned(input, output:16) uniform(compression, clip)) static inline void _gamut_mapping(const dt_aligned_pixel_t input, @@ -1709,8 +1770,8 @@ static void _extract_color_checker(const float *const restrict in, dt_D50_XYZ_to_xyY(illuminant_XYZ, illuminant_xyY); // save the illuminant in GUI struct for commit - g->xy[0] = illuminant_xyY[0]; - g->xy[1] = illuminant_xyY[1]; + g->colorchecker_xy[0] = illuminant_xyY[0]; + g->colorchecker_xy[1] = illuminant_xyY[1]; // and recompute back the LMS to be sure we use the parameters that // will be computed later @@ -1983,62 +2044,129 @@ static void _validate_color_checker(const float *const restrict in, dt_free_align(patches); } -static void _set_trouble_messages(dt_iop_module_t *self) +static gboolean _applies_cat(const dt_iop_module_t *self) { + if(!self->enabled) + return FALSE; + const dt_iop_channelmixer_rgb_params_t *p = self->params; + + // commit_params() forces CAT16 for these, whatever adaptation the params hold + const gboolean forced_cat = p->illuminant == DT_ILLUMINANT_CAMERA + || p->illuminant == DT_ILLUMINANT_FROM_WB; + + return !(p->illuminant == DT_ILLUMINANT_PIPE + || (p->adaptation == DT_ADAPTATION_RGB && !forced_cat)) + && !dt_image_is_monochrome(&self->dev->image_storage); +} + +static void _set_trouble_messages(dt_iop_module_t *self) +{ const dt_iop_channelmixer_rgb_gui_data_t *g = self->gui_data; + const dt_iop_channelmixer_rgb_params_t *p = self->params; const dt_develop_t *dev = self->dev; const dt_dev_chroma_t *chr = &dev->chroma; + // module handle snapshots: dt_dev_reset_chroma() may NULL them + // from the pipe worker thread. single aligned pointer loads can't tear; + // the pointed-to modules are only freed on this (main) thread, + // so the snapshots stay valid for this call + dt_iop_module_t *const temperature = chr->temperature; + dt_iop_module_t *const adaptation = chr->adaptation; + dt_print(DT_DEBUG_PIPE | DT_DEBUG_VERBOSE, "trouble message for %s%s : temp=%p adapt=%p", - self->op, dt_iop_get_instance_id(self), chr->temperature, chr->adaptation); + self->op, dt_iop_get_instance_id(self), temperature, adaptation); + + dt_dev_wb_t wb; + dt_dev_get_gui_wb(self->dev, &wb); + + const gboolean wb_unprepared = temperature + && temperature->enabled + && !wb.camera_reference + && !_dev_is_D65_chroma(&wb); + + // comes before the returns below, which depend on the one instance in + // chr->adaptation: a masked instance never gets there (_declare_cat_on_pipe()) + if(wb_unprepared + && _applies_cat(self) + && p->illuminant == DT_ILLUMINANT_FROM_WB) + { + // temperature has one message, which the registered instance sets, as below. + // Only if every instance is masked does no other instance warn there + if(adaptation == self || !adaptation) + dt_iop_set_module_trouble_message(temperature, + _("not prepared for color calibration's illuminant (details)"), + _("color calibration's 'as set in white balance module' illuminant\n" + "only follows this module if 'prepare data for color calibration'\n" + "is ticked. otherwise, it gives the same result as its\n" + "'as shot in camera' illuminant.\n" + "if these coefficients were measured on a D65 screen to\n" + "replace the camera reference, select another illuminant\n" + "in color calibration"), + NULL); + + dt_iop_set_module_trouble_message(self, + _("white balance is not prepared for this illuminant (details)"), + _("this illuminant only follows white balance if 'prepare data\n" + "for color calibration' is ticked there. otherwise, it gives\n" + "the same result as the 'as shot in camera' illuminant.\n" + "if white balance's coefficients were measured on a D65\n" + "screen to replace the camera reference, select another\n" + "illuminant here"), + NULL); + return; + } // in temperature module we make sure this is only presented if temperature is enabled - if(!chr->temperature) + if(!temperature) { - if(chr->adaptation) - dt_iop_clear_module_trouble_message(chr->adaptation); + if(adaptation) + dt_iop_clear_module_trouble_message(adaptation); + dt_iop_clear_module_trouble_message(self); return; } - if(!chr->adaptation) + if(!adaptation) { - dt_iop_clear_module_trouble_message(chr->temperature); + // a masked instance may have set temperature's message above, and this + // instance's call can come after it + const gboolean any_instance_follows_wb = dt_dev_cat_is_active(self->dev) + && !dt_dev_cat_overrides_wb(self->dev); + + if(!wb_unprepared || !any_instance_follows_wb) + dt_iop_clear_module_trouble_message(temperature); dt_iop_clear_module_trouble_message(self); return; } - const gboolean valid = - self->enabled - && !(p->illuminant == DT_ILLUMINANT_PIPE || p->adaptation == DT_ADAPTATION_RGB) - && !dt_image_is_monochrome(&dev->image_storage); + const gboolean valid = _applies_cat(self); - const gboolean temperature_enabled = chr->temperature && chr->temperature->enabled; - const gboolean adaptation_enabled = chr->adaptation && chr->adaptation->enabled; + const gboolean temperature_enabled = temperature->enabled; + const gboolean adaptation_enabled = adaptation->enabled; - // our first and biggest problem : white balance module is being - // clever with WB coeffs - const gboolean problem1 = valid - && chr->adaptation == self + // this instance does CAT while WB uses an incompatible setup (neither camera reference + // D65, nor another mode with late_correction) + const gboolean wb_applied_twice = valid + && adaptation == self && temperature_enabled - && !_dev_is_D65_chroma(dev); + && !_dev_is_D65_chroma(&wb); - // our second biggest problem : another channelmixerrgb instance is doing CAT - // earlier in the pipe and we don't use masking here. - const gboolean problem2 = valid - && adaptation_enabled + // another channelmixerrgb instance is doing CAT + // earlier in the pipe, and we don't use masking here + const gboolean double_cat = valid + && _applies_cat(adaptation) && temperature_enabled - && chr->adaptation != self + && adaptation != self && !g->is_blending; - // our third and minor problem: white balance module is not active but default_enabled - // and we do chromatic adaptation in color calibration - const gboolean problem3 = valid + // white balance is off on an image that needs it (default_enabled), but we rely on + // it to pre-process the data (neutralizing coeffs + late_correction, or D65 coeffs) + const gboolean wb_missing = valid && adaptation_enabled && !temperature_enabled - && chr->temperature->default_enabled; + && temperature->default_enabled; - if(problem2) + if(double_cat) { dt_iop_set_module_trouble_message(self, _("double CAT applied"), @@ -2050,29 +2178,35 @@ static void _set_trouble_messages(dt_iop_module_t *self) return; } - if(problem1) + if(wb_applied_twice) { - dt_iop_set_module_trouble_message(chr->temperature, + dt_iop_set_module_trouble_message(temperature, _("white balance applied twice (details)"), _("the color calibration module is enabled and already provides\n" "chromatic adaptation.\n" - "set the white balance here to camera reference (D65)\n" - "or disable chromatic adaptation in color calibration."), + "tick 'prepare data for color calibration' here,\n" + "set the white balance to camera reference (D65)\n" + "or disable chromatic adaptation in color calibration.\n" + "ignore this if the coefficients were measured on a D65 screen\n" + "to replace the camera reference"), NULL); dt_iop_set_module_trouble_message(self, _("white balance module error (details)"), _("the white balance module is not using the camera\n" "reference illuminant, which will cause issues here\n" - "with chromatic adaptation. either set it to reference\n" - "or disable chromatic adaptation here."), + "with chromatic adaptation. tick 'prepare data for\n" + "color calibration' there, set it to reference\n" + "or disable chromatic adaptation here.\n" + "ignore this if its coefficients were measured on a D65\n" + "screen to replace the camera reference"), NULL); return; } - if(problem3) + if(wb_missing) { - dt_iop_set_module_trouble_message(chr->temperature, + dt_iop_set_module_trouble_message(temperature, _("white balance missing (details)"), _("this module is not providing a valid reference illuminant\n" "causing chromatic adaptation issues in color calibration.\n" @@ -2090,10 +2224,59 @@ static void _set_trouble_messages(dt_iop_module_t *self) return; } - if(chr->adaptation && chr->adaptation == self) + if(adaptation == self) + dt_iop_clear_module_trouble_message(temperature); + dt_iop_clear_module_trouble_message(self); +} + +// The illuminants derived from pipe->wb are computed per run, into the +// caller's variable. pipe->wb can change without a new commit_params() +// (only temperature is recommitted when its history item changes), so a +// result kept in the piece data would make later runs depend on earlier ones. +static void _get_pipe_illuminant(const dt_iop_module_t *self, + const dt_dev_pixelpipe_t *pipe, + const dt_iop_channelmixer_rbg_data_t *d, + dt_aligned_pixel_t illuminant) +{ + float x = 0.f; + float y = 0.f; + gboolean from_pipe_wb = FALSE; + + if(d->illuminant_type == DT_ILLUMINANT_CAMERA) { - dt_iop_clear_module_trouble_message(chr->temperature); - dt_iop_clear_module_trouble_message(self); + // The camera illuminant is a behaviour rather than a preset of + // values: it uses whatever is in the RAW EXIF. But it depends on + // what temperature.c is doing and needs to be updated + // accordingly, to give a consistent result. We initialise the + // CAT defaults using the temperature coeffs at startup, but if + // temperature is changed later, we get no notification of the + // change here, so we can't update the defaults. So we need to + // re-run the detection at runtime… + dt_aligned_pixel_t correction_ratios; + _get_d65_correction_ratios(self, &pipe->wb, correction_ratios); + from_pipe_wb = find_illuminant_xy_from_as_shot_coeffs(&(self->dev->image_storage), + correction_ratios, &x, &y); + } + else if(d->illuminant_type == DT_ILLUMINANT_FROM_WB) + { + _illuminant_xy_from_wb_or_D50(self, &pipe->wb, &x, &y); + from_pipe_wb = TRUE; + } + + if(from_pipe_wb) + { + // Convert illuminant from xyY to XYZ + dt_aligned_pixel_t XYZ; + illuminant_xy_to_XYZ(x, y, XYZ); + + // Convert illuminant from XYZ to Bradford modified LMS + convert_any_XYZ_to_LMS(XYZ, illuminant, d->adaptation); + illuminant[3] = 0.f; + } + else + { + // for a failed camera lookup, commit_params() set it from the saved p->x/p->y + copy_pixel(illuminant, d->illuminant); } } @@ -2105,10 +2288,11 @@ void process(dt_iop_module_t *self, const dt_iop_roi_t *const roi_out) { dt_iop_channelmixer_rbg_data_t *data = piece->data; + dt_dev_pixelpipe_t *pipe = piece->pipe; const dt_iop_order_iccprofile_info_t *const work_profile = - dt_ioppr_get_pipe_current_profile_info(self, piece->pipe); + dt_ioppr_get_pipe_current_profile_info(self, pipe); const dt_iop_order_iccprofile_info_t *const input_profile = - dt_ioppr_get_pipe_input_profile_info(piece->pipe); + dt_ioppr_get_pipe_input_profile_info(pipe); dt_iop_channelmixer_rgb_gui_data_t *g = self->gui_data; if(!dt_iop_have_required_input_format(4 /*we need full-color pixels*/, @@ -2117,7 +2301,7 @@ void process(dt_iop_module_t *self, return; // image has been copied through to output and module's // trouble flag has been updated - if(dt_pipe_is_preview(piece->pipe)) + if(dt_pipe_is_preview(pipe)) _declare_cat_on_pipe(self, FALSE); dt_colormatrix_t RGB_to_XYZ; @@ -2145,7 +2329,7 @@ void process(dt_iop_module_t *self, #ifdef AI_ACTIVATED gboolean exit = FALSE; #endif - if(g->run_profile && dt_pipe_is_preview(piece->pipe)) + if(g->run_profile && dt_pipe_is_preview(pipe)) { dt_iop_gui_enter_critical_section(self); _extract_color_checker(in, out, roi_in, g, RGB_to_XYZ, @@ -2158,7 +2342,7 @@ void process(dt_iop_module_t *self, if(data->illuminant_type == DT_ILLUMINANT_DETECT_EDGES || data->illuminant_type == DT_ILLUMINANT_DETECT_SURFACES) { - if(dt_pipe_is_full(piece->pipe)) + if(dt_pipe_is_full(pipe)) { // detection on full image only dt_iop_gui_enter_critical_section(self); @@ -2166,8 +2350,8 @@ void process(dt_iop_module_t *self, // as scratch space since we will be overwriting it afterwards // anyway _auto_detect_WB(in, out, data->illuminant_type, roi_in->width, roi_in->height, - ch, RGB_to_XYZ, g->XYZ); - dt_dev_pixelpipe_cache_invalidate_later(piece->pipe, self->iop_order, "AI RGB: "); + ch, RGB_to_XYZ, g->ai_wb_XYZ); + dt_dev_pixelpipe_cache_invalidate_later(pipe, self->iop_order, "AI RGB: "); dt_iop_gui_leave_critical_section(self); } @@ -2183,35 +2367,8 @@ void process(dt_iop_module_t *self, #endif } - if(data->illuminant_type == DT_ILLUMINANT_CAMERA) - { - // The camera illuminant is a behaviour rather than a preset of - // values: it uses whatever is in the RAW EXIF. But it depends on - // what temperature.c is doing and needs to be updated - // accordingly, to give a consistent result. We initialise the - // CAT defaults using the temperature coeffs at startup, but if - // temperature is changed later, we get no notification of the - // change here, so we can't update the defaults. So we need to - // re-run the detection at runtime… - float x, y; - dt_aligned_pixel_t custom_wb; - _get_white_balance_coeff(self, custom_wb); - - if(find_temperature_from_raw_coeffs(&(self->dev->image_storage), custom_wb, &(x), &(y))) - { - // Convert illuminant from xyY to XYZ - dt_aligned_pixel_t XYZ; - illuminant_xy_to_XYZ(x, y, XYZ); - - // Convert illuminant from XYZ to Bradford modified LMS - convert_any_XYZ_to_LMS(XYZ, data->illuminant, data->adaptation); - data->illuminant[3] = 0.f; - } - else - { - // just use whatever was defined in commit_params hoping the defaults work… - } - } + dt_aligned_pixel_t illuminant; + _get_pipe_illuminant(self, pipe, data, illuminant); // force loop unswitching in a controlled way switch(data->adaptation) @@ -2220,7 +2377,7 @@ void process(dt_iop_module_t *self, { _loop_switch(in, out, roi_out->width, roi_out->height, ch, XYZ_to_RGB, RGB_to_XYZ, data->MIX, - data->illuminant, data->saturation, data->lightness, data->grey, + illuminant, data->saturation, data->lightness, data->grey, data->p, data->gamut, data->clip, data->apply_grey, DT_ADAPTATION_FULL_BRADFORD, data->version); break; @@ -2229,7 +2386,7 @@ void process(dt_iop_module_t *self, { _loop_switch(in, out, roi_out->width, roi_out->height, ch, XYZ_to_RGB, RGB_to_XYZ, data->MIX, - data->illuminant, data->saturation, data->lightness, data->grey, + illuminant, data->saturation, data->lightness, data->grey, data->p, data->gamut, data->clip, data->apply_grey, DT_ADAPTATION_LINEAR_BRADFORD, data->version); break; @@ -2238,7 +2395,7 @@ void process(dt_iop_module_t *self, { _loop_switch(in, out, roi_out->width, roi_out->height, ch, XYZ_to_RGB, RGB_to_XYZ, data->MIX, - data->illuminant, data->saturation, data->lightness, data->grey, + illuminant, data->saturation, data->lightness, data->grey, data->p, data->gamut, data->clip, data->apply_grey, DT_ADAPTATION_CAT16, data->version); break; @@ -2247,7 +2404,7 @@ void process(dt_iop_module_t *self, { _loop_switch(in, out, roi_out->width, roi_out->height, ch, XYZ_to_RGB, RGB_to_XYZ, data->MIX, - data->illuminant, data->saturation, data->lightness, data->grey, + illuminant, data->saturation, data->lightness, data->grey, data->p, data->gamut, data->clip, data->apply_grey, DT_ADAPTATION_XYZ, data->version); break; @@ -2256,7 +2413,7 @@ void process(dt_iop_module_t *self, { _loop_switch(in, out, roi_out->width, roi_out->height, ch, XYZ_to_RGB, RGB_to_XYZ, data->MIX, - data->illuminant, data->saturation, data->lightness, data->grey, + illuminant, data->saturation, data->lightness, data->grey, data->p, data->gamut, data->clip, data->apply_grey, DT_ADAPTATION_RGB, data->version); break; @@ -2270,7 +2427,7 @@ void process(dt_iop_module_t *self, // run dE validation at output if(self->dev->gui_attached && g) - if(g->run_validation && dt_pipe_is_preview(piece->pipe)) + if(g->run_validation && dt_pipe_is_preview(pipe)) { _validate_color_checker(out, roi_out, g, RGB_to_XYZ, XYZ_to_RGB, XYZ_to_CAM); g->run_validation = FALSE; @@ -2288,37 +2445,15 @@ int process_cl(dt_iop_module_t *self, dt_iop_channelmixer_rbg_data_t *const d = piece->data; const dt_iop_channelmixer_rgb_global_data_t *const gd = self->global_data; + const dt_dev_pixelpipe_t *pipe = piece->pipe; const dt_iop_order_iccprofile_info_t *const work_profile = - dt_ioppr_get_pipe_current_profile_info(self, piece->pipe); + dt_ioppr_get_pipe_current_profile_info(self, pipe); - if(dt_pipe_is_preview(piece->pipe)) + if(dt_pipe_is_preview(pipe)) _declare_cat_on_pipe(self, FALSE); - if(d->illuminant_type == DT_ILLUMINANT_CAMERA) - { - // The camera illuminant is a behaviour rather than a preset of - // values: it uses whatever is in the RAW EXIF. But it depends on - // what temperature.c is doing and needs to be updated - // accordingly, to give a consistent result. We initialise the - // CAT defaults using the temperature coeffs at startup, but if - // temperature is changed later, we get no notification of the - // change here, so we can't update the defaults. So we need to - // re-run the detection at runtime… - float x, y; - dt_aligned_pixel_t custom_wb; - _get_white_balance_coeff(self, custom_wb); - - if(find_temperature_from_raw_coeffs(&(self->dev->image_storage), custom_wb, &(x), &(y))) - { - // Convert illuminant from xyY to XYZ - dt_aligned_pixel_t XYZ; - illuminant_xy_to_XYZ(x, y, XYZ); - - // Convert illuminant from XYZ to Bradford modified LMS - convert_any_XYZ_to_LMS(XYZ, d->illuminant, d->adaptation); - d->illuminant[3] = 0.f; - } - } + dt_aligned_pixel_t illuminant; + _get_pipe_illuminant(self, pipe, d, illuminant); cl_int err = CL_MEM_OBJECT_ALLOCATION_FAILURE; @@ -2328,7 +2463,7 @@ int process_cl(dt_iop_module_t *self, return err; } - const int devid = piece->pipe->devid; + const int devid = pipe->devid; const int width = roi_in->width; const int height = roi_in->height; @@ -2379,7 +2514,7 @@ int process_cl(dt_iop_module_t *self, CLARG(dev_in), CLARG(dev_out), CLARG(width), CLARG(height), CLARG(input_matrix_cl), CLARG(output_matrix_cl), - CLARG(MIX_cl), CLARG(d->illuminant), CLARG(d->saturation), + CLARG(MIX_cl), CLARG(illuminant), CLARG(d->saturation), CLARG(d->lightness), CLARG(d->grey), CLARG(d->p), CLARG(d->gamut), CLARG(d->clip), CLARG(d->apply_grey), CLARG(d->version)); @@ -2923,8 +3058,8 @@ static void _commit_profile_callback(GtkWidget *widget, dt_iop_gui_enter_critical_section(self); - p->x = g->xy[0]; - p->y = g->xy[1]; + p->x = g->colorchecker_xy[0]; + p->y = g->colorchecker_xy[1]; p->illuminant = DT_ILLUMINANT_CUSTOM; _check_if_close_to_daylight(p->x, p->y, &p->temperature, NULL, NULL); @@ -2984,8 +3119,8 @@ static void _develop_ui_pipe_finished_callback(gpointer instance, dt_iop_module_ return; dt_iop_gui_enter_critical_section(self); - p->x = g->XYZ[0]; - p->y = g->XYZ[1]; + p->x = g->ai_wb_XYZ[0]; + p->y = g->ai_wb_XYZ[1]; dt_iop_gui_leave_critical_section(self); _check_if_close_to_daylight(p->x, p->y, @@ -3022,6 +3157,40 @@ static void _preview_pipe_finished_callback(gpointer instance, dt_iop_module_t * dt_iop_gui_enter_critical_section(self); gtk_label_set_markup(GTK_LABEL(g->label_delta_E), g->delta_E_label_text); + + dt_iop_channelmixer_rgb_params_t *p = self->params; + if(p->illuminant == DT_ILLUMINANT_FROM_WB) + { + // WB coefficients might have changed temperature.c; recalculate xy and update the GUI + float x, y; + + dt_dev_wb_t wb; + dt_dev_get_gui_wb(self->dev, &wb); + + _illuminant_xy_from_wb_or_D50(self, &wb, &x, &y); + + DT_ENTER_GUI_UPDATE(); + _update_approx_cct(self); + _update_illuminant_color(self); + + dt_aligned_pixel_t xyY = { x, y, 1.f }; + dt_aligned_pixel_t Lch; + dt_xyY_to_Lch(xyY, Lch); + + if(Lch[1] > 0) + dt_bauhaus_slider_set(g->illum_x, rad2degf(Lch[2])); + dt_bauhaus_slider_set(g->illum_y, Lch[1]); + + DT_LEAVE_GUI_UPDATE(); + } + else if(p->illuminant == DT_ILLUMINANT_CAMERA) + { + DT_ENTER_GUI_UPDATE(); + _update_approx_cct(self); + _update_illuminant_color(self); + DT_LEAVE_GUI_UPDATE(); + } + dt_iop_gui_leave_critical_section(self); _set_trouble_messages(self); } @@ -3092,16 +3261,15 @@ void commit_params(dt_iop_module_t *self, // find x y coordinates of illuminant for CIE 1931 2° observer float x = p->x; float y = p->y; - dt_aligned_pixel_t custom_wb; - _get_white_balance_coeff(self, custom_wb); - illuminant_to_xy(p->illuminant, &(self->dev->image_storage), - custom_wb, &x, &y, p->temperature, p->illum_fluo, p->illum_led); - - // if illuminant is set as camera, x and y are set on-the-fly at - // commit time, so we need to set adaptation too - if(p->illuminant == DT_ILLUMINANT_CAMERA) + // the camera illuminant keeps the saved p->x/p->y here: it is the fallback + // for when _get_pipe_illuminant() fails to look it up from pipe->wb + if(p->illuminant != DT_ILLUMINANT_CAMERA) + _get_corrected_illuminant_xy(self, &pipe->wb, p, &x, &y); + + // if illuminant is from camera or user WB coefficients, x and y are set on-the-fly, + // so we need to set adaptation too + if(p->illuminant == DT_ILLUMINANT_CAMERA || p->illuminant == DT_ILLUMINANT_FROM_WB) _check_if_close_to_daylight(x, y, NULL, NULL, &(d->adaptation)); - d->illuminant_type = p->illuminant; // Convert illuminant from xyY to XYZ @@ -3244,6 +3412,7 @@ static void _update_illuminants(const dt_iop_module_t *self) break; } case DT_ILLUMINANT_CAMERA: + case DT_ILLUMINANT_FROM_WB: { gtk_widget_set_visible(g->adaptation, TRUE); gtk_widget_set_visible(g->temperature, FALSE); @@ -3536,11 +3705,12 @@ static gboolean _illuminant_color_draw(GtkWidget *widget, // camera RAW is chosen float x = p->x; float y = p->y; - dt_aligned_pixel_t RGB = { 0 }; - dt_aligned_pixel_t custom_wb; - _get_white_balance_coeff(self, custom_wb); - illuminant_to_xy(p->illuminant, &(self->dev->image_storage), custom_wb, - &x, &y, p->temperature, p->illum_fluo, p->illum_led); + dt_aligned_pixel_t RGB = { 0.f }; + + dt_dev_wb_t wb; + dt_dev_get_gui_wb(self->dev, &wb); + + _get_corrected_illuminant_xy(self, &wb, p, &x, &y); illuminant_xy_to_RGB(x, y, RGB); cairo_set_source_rgb(cr, RGB[0], RGB[1], RGB[2]); cairo_rectangle(cr, INNER_PADDING, margin, width, height); @@ -3640,10 +3810,11 @@ static void _update_approx_cct(const dt_iop_module_t *self) float x = p->x; float y = p->y; - dt_aligned_pixel_t custom_wb; - _get_white_balance_coeff(self, custom_wb); - illuminant_to_xy(p->illuminant, &(self->dev->image_storage), - custom_wb, &x, &y, p->temperature, p->illum_fluo, p->illum_led); + + dt_dev_wb_t wb; + dt_dev_get_gui_wb(self->dev, &wb); + + _get_corrected_illuminant_xy(self, &wb, p, &x, &y); dt_illuminant_t test_illuminant; float t = 5000.f; @@ -3845,6 +4016,24 @@ void init(dt_iop_module_t *self) d->red[0] = d->green[1] = d->blue[2] = 1.0; } +static void _add_or_remove_illuminant(const dt_iop_module_t *self, + const int illuminant, + const gboolean use_illuminant) +{ + // we only call this if g is not NULL + dt_iop_channelmixer_rgb_gui_data_t *g = self->gui_data; + const int pos = dt_bauhaus_combobox_get_from_value(g->illuminant, illuminant); + if(use_illuminant) + { + if(pos == -1) + dt_bauhaus_combobox_add_introspection(g->illuminant, NULL, + self->so->get_f("illuminant")->Enum.values, + illuminant, illuminant); + } + else if (pos != -1) + dt_bauhaus_combobox_remove_at(g->illuminant, pos); +} + void reload_defaults(dt_iop_module_t *self) { dt_iop_channelmixer_rgb_params_t *d = self->default_params; @@ -3881,13 +4070,28 @@ void reload_defaults(dt_iop_module_t *self) { d->adaptation = DT_ADAPTATION_CAT16; - dt_aligned_pixel_t custom_wb; - if(!_get_white_balance_coeff(self, custom_wb)) + // can't get WB via dt_dev_get_gui_wb() here: while defaults load, temperature's + // params still belong to the previous image, or are temperature's defaults + dt_aligned_pixel_t correction_ratios; + if(!_get_d65_correction_ratios(self, &self->dev->chroma.wb, correction_ratios)) { - if(find_temperature_from_raw_coeffs(img, custom_wb, &(d->x), &(d->y))) + if(find_illuminant_xy_from_as_shot_coeffs(img, correction_ratios, &(d->x), &(d->y))) d->illuminant = DT_ILLUMINANT_CAMERA; - _check_if_close_to_daylight(d->x, d->y, - &(d->temperature), &(d->illuminant), &(d->adaptation)); + + // the white balance read above may be another edit's, so it must not decide + // this: ask only about the image. x and y stay as computed: they are the + // saved values that gui_changed() replaces when the user changes the illuminant + const dt_aligned_pixel_t no_correction = { 1.f, 1.f, 1.f, 1.f }; + float as_shot_x, as_shot_y; + const gboolean follow_wb = + dt_is_scene_referred() + && find_illuminant_xy_from_as_shot_coeffs(img, no_correction, &as_shot_x, &as_shot_y); + + if(follow_wb) + d->illuminant = DT_ILLUMINANT_FROM_WB; + + _check_if_close_to_daylight(d->x, d->y, &(d->temperature), + follow_wb ? NULL : &(d->illuminant), &(d->adaptation)); } } @@ -3912,16 +4116,9 @@ void reload_defaults(dt_iop_module_t *self) g->delta_E_label_text = NULL; } - const int pos = dt_bauhaus_combobox_get_from_value(g->illuminant, DT_ILLUMINANT_CAMERA); - if(dt_image_is_matrix_correction_supported(img) && !dt_image_is_monochrome(img)) - { - if(pos == -1) - dt_bauhaus_combobox_add_introspection(g->illuminant, NULL, - self->so->get_f("illuminant")->Enum.values, - DT_ILLUMINANT_CAMERA, DT_ILLUMINANT_CAMERA); - } - else - dt_bauhaus_combobox_remove_at(g->illuminant, pos); + const gboolean has_color_matrix = dt_image_is_matrix_correction_supported(img) && !dt_image_is_monochrome(img); + _add_or_remove_illuminant(self, DT_ILLUMINANT_CAMERA, has_color_matrix); + _add_or_remove_illuminant(self, DT_ILLUMINANT_FROM_WB, has_color_matrix); gui_changed(self, NULL, NULL); } @@ -3970,6 +4167,9 @@ void gui_changed(dt_iop_module_t *self, dt_iop_channelmixer_rgb_params_t *p = self->params; dt_iop_channelmixer_rgb_gui_data_t *g = self->gui_data; + dt_dev_wb_t wb; + dt_dev_get_gui_wb(self->dev, &wb); + if(w == g->illuminant) { if(previous) @@ -3984,12 +4184,21 @@ void gui_changed(dt_iop_module_t *self, // illuminant = "as set in camera", temperature and // chromaticity are inited with the preset content when // illuminant is changed. - dt_aligned_pixel_t custom_wb; - _get_white_balance_coeff(self, custom_wb); - find_temperature_from_raw_coeffs(&(self->dev->image_storage), custom_wb, + dt_aligned_pixel_t correction_ratios; + _get_d65_correction_ratios(self, &wb, correction_ratios); + find_illuminant_xy_from_as_shot_coeffs(&(self->dev->image_storage), correction_ratios, &(p->x), &(p->y)); _check_if_close_to_daylight(p->x, p->y, &(p->temperature), NULL, &(p->adaptation)); } + if(*prev_illuminant == DT_ILLUMINANT_FROM_WB) + { + // If illuminant was previously "as set in white balance module", + // (x, y) were computed at runtime from the WB module's coefficients + // and p->x, p->y may hold stale values. Recompute them now so + // the new illuminant mode starts from the correct chromaticity. + _illuminant_xy_from_wb_or_D50(self, &wb, &(p->x), &(p->y)); + _check_if_close_to_daylight(p->x, p->y, &(p->temperature), NULL, &(p->adaptation)); + } } if(p->illuminant == DT_ILLUMINANT_D) @@ -4001,15 +4210,23 @@ void gui_changed(dt_iop_module_t *self, if(p->illuminant == DT_ILLUMINANT_CAMERA) { // Get camera WB and update illuminant - dt_aligned_pixel_t custom_wb; - _get_white_balance_coeff(self, custom_wb); - const gboolean found = find_temperature_from_raw_coeffs(&(self->dev->image_storage), - custom_wb, &(p->x), &(p->y)); + dt_aligned_pixel_t correction_ratios; + _get_d65_correction_ratios(self, &wb, correction_ratios); + const gboolean found = find_illuminant_xy_from_as_shot_coeffs(&(self->dev->image_storage), + correction_ratios, &(p->x), &(p->y)); _check_if_close_to_daylight(p->x, p->y, &(p->temperature), NULL, &(p->adaptation)); if(found) dt_control_log(_("white balance successfully extracted from raw image")); } + else if(p->illuminant == DT_ILLUMINANT_FROM_WB) + { + const gboolean found = _illuminant_xy_from_wb_or_D50(self, &wb, &(p->x), &(p->y)); + _check_if_close_to_daylight(p->x, p->y, &(p->temperature), NULL, &(p->adaptation)); + + if(found) + dt_control_log(_("white balance successfully extracted from white balance module")); + } #ifdef AI_ACTIVATED else if(p->illuminant == DT_ILLUMINANT_DETECT_EDGES || p->illuminant == DT_ILLUMINANT_DETECT_SURFACES) @@ -4037,7 +4254,8 @@ void gui_changed(dt_iop_module_t *self, // illuminant to allow swapping modes if(p->illuminant != DT_ILLUMINANT_CUSTOM - && p->illuminant != DT_ILLUMINANT_CAMERA) + && p->illuminant != DT_ILLUMINANT_CAMERA + && p->illuminant != DT_ILLUMINANT_FROM_WB) { // We are in any mode defining (x, y) indirectly from an // interface, so commit (x, y) explicitly @@ -4047,7 +4265,8 @@ void gui_changed(dt_iop_module_t *self, if(p->illuminant != DT_ILLUMINANT_D && p->illuminant != DT_ILLUMINANT_BB - && p->illuminant != DT_ILLUMINANT_CAMERA) + && p->illuminant != DT_ILLUMINANT_CAMERA + && p->illuminant != DT_ILLUMINANT_FROM_WB) { // We are in any mode not defining explicitly a temperature, so // find the the closest CCT and commit it @@ -4132,7 +4351,7 @@ void gui_changed(dt_iop_module_t *self, // If "as shot in camera" illuminant is used, CAT space is forced automatically // therefore, make the control insensitive dt_bauhaus_combobox_set_from_value(g->adaptation, p->adaptation); - gtk_widget_set_sensitive(g->adaptation, p->illuminant != DT_ILLUMINANT_CAMERA); + gtk_widget_set_sensitive(g->adaptation, p->illuminant != DT_ILLUMINANT_CAMERA && p->illuminant != DT_ILLUMINANT_FROM_WB); _declare_cat_on_pipe(self, FALSE); @@ -4208,15 +4427,17 @@ static void _auto_set_illuminant(dt_iop_module_t *self, // find x y coordinates of illuminant for CIE 1931 2° observer float x = p->x; float y = p->y; + + dt_dev_wb_t wb; + dt_dev_get_gui_wb(self->dev, &wb); + + _get_corrected_illuminant_xy(self, &wb, p, &x, &y); + dt_adaptation_t adaptation = p->adaptation; - dt_aligned_pixel_t custom_wb; - _get_white_balance_coeff(self, custom_wb); - illuminant_to_xy(p->illuminant, &(self->dev->image_storage), - custom_wb, &x, &y, p->temperature, p->illum_fluo, p->illum_led); // if illuminant is set as camera, x and y are set on-the-fly at // commit time, so we need to set adaptation too - if(p->illuminant == DT_ILLUMINANT_CAMERA) + if(p->illuminant == DT_ILLUMINANT_CAMERA || p->illuminant == DT_ILLUMINANT_FROM_WB) _check_if_close_to_daylight(x, y, NULL, NULL, &adaptation); // Convert illuminant from xyY to XYZ @@ -4410,6 +4631,18 @@ static void _dt_active_images_callback(gpointer instance, g->checker_ready = FALSE; } +// reads params, so GTK main thread only +static dt_dev_cat_source_t _cat_proxy_source(dt_iop_module_t *self) +{ + if(!self || !self->dev || !_applies_cat(self)) + return DT_DEV_CAT_NONE; + + const dt_iop_channelmixer_rgb_params_t *p = self->params; + + return p->illuminant == DT_ILLUMINANT_FROM_WB ? DT_DEV_CAT_FROM_WB + : DT_DEV_CAT_OWN_ILLUMINANT; +} + void gui_init(dt_iop_module_t *self) { dt_iop_channelmixer_rgb_gui_data_t *g = IOP_GUI_ALLOC(channelmixer_rgb); @@ -4430,7 +4663,7 @@ void gui_init(dt_iop_module_t *self) g->delta_E_in = NULL; g->delta_E_label_text = NULL; - g->XYZ[0] = NAN; + g->ai_wb_XYZ[0] = NAN; #ifdef AI_ACTIVATED DT_CONTROL_SIGNAL_HANDLE(DT_SIGNAL_DEVELOP_UI_PIPE_FINISHED, _develop_ui_pipe_finished_callback); @@ -4744,6 +4977,8 @@ void gui_init(dt_iop_module_t *self) DT_CONTROL_SIGNAL_CONNECT(DT_SIGNAL_ACTIVE_IMAGES_CHANGE, _dt_active_images_callback, g); + + darktable.develop->proxy.cat_source = _cat_proxy_source; } void gui_cleanup(dt_iop_module_t *self) diff --git a/src/iop/colorin.c b/src/iop/colorin.c index 63b6cb84e4f..d29b60f0334 100644 --- a/src/iop/colorin.c +++ b/src/iop/colorin.c @@ -817,17 +817,22 @@ int process_cl(dt_iop_module_t *self, return dt_opencl_enqueue_copy_image(devid, dev_in, dev_out, CLIMG_ORIGIN, CLIMG_ORIGIN, region); } - const dt_dev_chroma_t *chr = &self->dev->chroma; - const gboolean corrected = chr->late_correction; - dt_aligned_pixel_t coeffs = { corrected ? chr->D65coeffs[0] / chr->as_shot[0] : 1.0f, - corrected ? chr->D65coeffs[1] / chr->as_shot[1] : 1.0f, - corrected ? chr->D65coeffs[2] / chr->as_shot[2] : 1.0f, - corrected ? chr->D65coeffs[3] / chr->as_shot[3] : 1.0f }; + const dt_dev_wb_t *wb = &pipe->wb; + const gboolean corrected = wb->late_correction; + dt_aligned_pixel_t coeffs; + for_four_channels(k) + { + if(corrected && wb->coeffs[k] > 1e-6f) + coeffs[k] = wb->D65coeffs[k] / wb->coeffs[k]; + else + coeffs[k] = 1.0f; + } + if(corrected) { for_four_channels(k) { - pipe->dsc.temperature.coeffs[k] = chr->D65coeffs[k]; + pipe->dsc.temperature.coeffs[k] = wb->D65coeffs[k]; // note: tiling takes care about processed_maximum pipe->dsc.processed_maximum[k] *= coeffs[k]; } @@ -1367,19 +1372,24 @@ void process(dt_iop_module_t *self, ivoid, ovoid, roi_in, roi_out)) return; - const dt_dev_chroma_t *chr = &self->dev->chroma; - const dt_iop_colorin_data_t *const d = piece->data; - const gboolean corrected = chr->late_correction && d->type != DT_COLORSPACE_LAB; - const dt_aligned_pixel_t coeffs = { corrected ? chr->D65coeffs[0] / chr->as_shot[0] : 1.0f, - corrected ? chr->D65coeffs[1] / chr->as_shot[1] : 1.0f, - corrected ? chr->D65coeffs[2] / chr->as_shot[2] : 1.0f, - corrected ? chr->D65coeffs[3] / chr->as_shot[3] : 1.0f }; dt_dev_pixelpipe_t *pipe = piece->pipe; + const dt_dev_wb_t *wb = &pipe->wb; + const dt_iop_colorin_data_t *const d = piece->data; + const gboolean corrected = wb->late_correction && d->type != DT_COLORSPACE_LAB; + dt_aligned_pixel_t coeffs; + for_four_channels(k) + { + if(corrected && wb->coeffs[k] > 1e-6f) + coeffs[k] = wb->D65coeffs[k] / wb->coeffs[k]; + else + coeffs[k] = 1.0f; + } + if(corrected) { for_four_channels(k) { - pipe->dsc.temperature.coeffs[k] = chr->D65coeffs[k]; + pipe->dsc.temperature.coeffs[k] = wb->D65coeffs[k]; // note: tiling takes care about processed_maximum pipe->dsc.processed_maximum[k] *= coeffs[k]; } diff --git a/src/iop/highlights.c b/src/iop/highlights.c index 5d2e4a2e72a..da9f30e5fd6 100644 --- a/src/iop/highlights.c +++ b/src/iop/highlights.c @@ -679,13 +679,13 @@ int process_cl(dt_iop_module_t *self, } else // (dmode == DT_IOP_HIGHLIGHTS_CLIP) { - const dt_dev_chroma_t *chr = &self->dev->chroma; dt_aligned_pixel_t clips = { clipper, clipper, clipper, clipper}; - if(chr->late_correction) + const dt_dev_wb_t *wb = &pipe->wb; + if(pipe->dsc.temperature.enabled && wb->late_correction) { - clips[0] *= chr->as_shot[0] / chr->D65coeffs[0]; - clips[1] *= chr->as_shot[1] / chr->D65coeffs[1]; - clips[2] *= chr->as_shot[2] / chr->D65coeffs[2]; + clips[0] *= wb->coeffs[0] / wb->D65coeffs[0]; + clips[1] *= wb->coeffs[1] / wb->D65coeffs[1]; + clips[2] *= wb->coeffs[2] / wb->D65coeffs[2]; } dev_xtrans = dt_opencl_copy_host_to_device_constant(devid, sizeof(piece->xtrans), piece->xtrans); @@ -758,13 +758,14 @@ static void process_clip(dt_iop_module_t *self, else { const uint8_t(*const xtrans)[6] = piece->xtrans; - const dt_dev_chroma_t *chr = &self->dev->chroma; dt_aligned_pixel_t clips = { clip, clip, clip, clip}; - if(chr->late_correction) + dt_dev_pixelpipe_t *pipe = piece->pipe; + const dt_dev_wb_t *wb = &pipe->wb; + if(pipe->dsc.temperature.enabled && wb->late_correction) { - clips[0] *= chr->as_shot[0] / chr->D65coeffs[0]; - clips[1] *= chr->as_shot[1] / chr->D65coeffs[1]; - clips[2] *= chr->as_shot[2] / chr->D65coeffs[2]; + clips[0] *= wb->coeffs[0] / wb->D65coeffs[0]; + clips[1] *= wb->coeffs[1] / wb->D65coeffs[1]; + clips[2] *= wb->coeffs[2] / wb->D65coeffs[2]; } for(int row = 0; row < roi_out->height; row++) { diff --git a/src/iop/hlreconstruct/opposed.c b/src/iop/hlreconstruct/opposed.c index 6b6585890a7..2ae1a5c5ea2 100644 --- a/src/iop/hlreconstruct/opposed.c +++ b/src/iop/hlreconstruct/opposed.c @@ -42,8 +42,7 @@ static dt_hash_t _opposed_hash(dt_dev_pixelpipe_iop_t *piece) { const dt_iop_highlights_data_t *d = piece->data; dt_hash_t hash = dt_dev_pixelpipe_piece_hash(piece, NULL, FALSE); - hash = dt_hash(hash, &d->clip, sizeof(d->clip)); - return dt_hash(hash, &piece->module->dev->chroma.late_correction, sizeof(int)); + return dt_hash(hash, &d->clip, sizeof(d->clip)); } static inline float _calc_linear_refavg(const float *in, const int color) @@ -249,7 +248,8 @@ static float *_process_opposed(dt_iop_module_t *self, const uint8_t(*const xtrans)[6] = (const uint8_t(*const)[6])piece->xtrans; const uint32_t filters = piece->filters; - const dt_iop_buffer_dsc_t *dsc = &piece->pipe->dsc; + dt_dev_pixelpipe_t *pipe = piece->pipe; + const dt_iop_buffer_dsc_t *dsc = &pipe->dsc; const gboolean wbon = dsc->temperature.enabled; const dt_aligned_pixel_t icoeffs = { wbon ? dsc->temperature.coeffs[0] : 1.0f, wbon ? dsc->temperature.coeffs[1] : 1.0f, @@ -257,12 +257,16 @@ static float *_process_opposed(dt_iop_module_t *self, 1.0f }; const dt_aligned_pixel_t clips = { clipval * icoeffs[0], clipval * icoeffs[1], clipval * icoeffs[2], 1.0f }; - const dt_dev_chroma_t *chr = &self->dev->chroma; - const gboolean late = chr->late_correction; - const dt_aligned_pixel_t correction = { late ? (float)(chr->D65coeffs[0] / chr->as_shot[0]) : 1.0f, - late ? (float)(chr->D65coeffs[1] / chr->as_shot[1]) : 1.0f, - late ? (float)(chr->D65coeffs[2] / chr->as_shot[2]) : 1.0f, - 1.0f }; + const dt_dev_wb_t *wb = &pipe->wb; + const gboolean late = wb->late_correction; + dt_aligned_pixel_t correction; + for_four_channels(k) + { + if(late && wb->coeffs[k] > 1e-6f) + correction[k] = wb->D65coeffs[k] / wb->coeffs[k]; + else + correction[k] = 1.0f; + } const size_t iwidth = roi_in->width; const size_t iheight = roi_in->height; @@ -473,11 +477,12 @@ static cl_int process_opposed_cl(dt_iop_module_t *self, dt_iop_highlights_data_t *d = piece->data; const dt_iop_highlights_global_data_t *gd = self->global_data; dt_iop_highlights_gui_data_t *g = self->gui_data; - const int devid = piece->pipe->devid; + dt_dev_pixelpipe_t *pipe = piece->pipe; + const int devid = pipe->devid; const uint32_t filters = piece->filters; const float clipval = highlights_clip_magics[DT_IOP_HIGHLIGHTS_OPPOSED] * d->clip; - const dt_iop_buffer_dsc_t *dsc = &piece->pipe->dsc; + const dt_iop_buffer_dsc_t *dsc = &pipe->dsc; const gboolean wbon = dsc->temperature.enabled; const dt_aligned_pixel_t icoeffs = { wbon ? dsc->temperature.coeffs[0] : 1.0f, wbon ? dsc->temperature.coeffs[1] : 1.0f, @@ -486,12 +491,16 @@ static cl_int process_opposed_cl(dt_iop_module_t *self, dt_aligned_pixel_t clips = { clipval * icoeffs[0], clipval * icoeffs[1], clipval * icoeffs[2], 1.0f}; - const dt_dev_chroma_t *chr = &self->dev->chroma; - const gboolean late = chr->late_correction; - dt_aligned_pixel_t correction = { late ? (float)(chr->D65coeffs[0] / chr->as_shot[0]) : 1.0f, - late ? (float)(chr->D65coeffs[1] / chr->as_shot[1]) : 1.0f, - late ? (float)(chr->D65coeffs[2] / chr->as_shot[2]) : 1.0f, - 1.0f }; + const dt_dev_wb_t *wb = &pipe->wb; + const gboolean late = wb->late_correction; + dt_aligned_pixel_t correction; + for_four_channels(k) + { + if(late && wb->coeffs[k] > 1e-6f) + correction[k] = wb->D65coeffs[k] / wb->coeffs[k]; + else + correction[k] = 1.0f; + } const dt_aligned_pixel_t lo_clips = { 0.2f * clips[0], 0.2f * clips[1], 0.2f * clips[2], 1.0f }; cl_int err = CL_MEM_OBJECT_ALLOCATION_FAILURE; diff --git a/src/iop/hlreconstruct/segbased.c b/src/iop/hlreconstruct/segbased.c index 1a685084ee3..c013f09ccec 100644 --- a/src/iop/hlreconstruct/segbased.c +++ b/src/iop/hlreconstruct/segbased.c @@ -467,18 +467,23 @@ static void _process_segmentation(dt_dev_pixelpipe_iop_t *piece, { const uint8_t(*const xtrans)[6] = (const uint8_t(*const)[6])piece->xtrans; const uint32_t filters = piece->filters; - const gboolean fullpipe = dt_pipe_is_full(piece->pipe); + const dt_dev_pixelpipe_t *pipe = piece->pipe; + const gboolean fullpipe = dt_pipe_is_full(pipe); const float clipval = MAX(0.1f, highlights_clip_magics[DT_IOP_HIGHLIGHTS_SEGMENTS] * d->clip); - const dt_aligned_pixel_t icoeffs = { piece->pipe->dsc.temperature.coeffs[0], piece->pipe->dsc.temperature.coeffs[1], piece->pipe->dsc.temperature.coeffs[2]}; + const dt_aligned_pixel_t icoeffs = { pipe->dsc.temperature.coeffs[0], pipe->dsc.temperature.coeffs[1], pipe->dsc.temperature.coeffs[2]}; const dt_aligned_pixel_t clips = { clipval * icoeffs[0], clipval * icoeffs[1], clipval * icoeffs[2]}; - const dt_dev_chroma_t *chr = &piece->module->dev->chroma; - const gboolean late = chr->late_correction; - const dt_aligned_pixel_t correction = { late ? (float)(chr->D65coeffs[0] / chr->as_shot[0]) : 1.0f, - late ? (float)(chr->D65coeffs[1] / chr->as_shot[1]) : 1.0f, - late ? (float)(chr->D65coeffs[2] / chr->as_shot[2]) : 1.0f, - 1.0f }; + const dt_dev_wb_t *wb = &pipe->wb; + const gboolean late = wb->late_correction; + dt_aligned_pixel_t correction; + for_four_channels(k) + { + if(late && wb->coeffs[k] > 1e-6f) + correction[k] = wb->D65coeffs[k] / wb->coeffs[k]; + else + correction[k] = 1.0f; + } const dt_aligned_pixel_t cube_coeffs = { cbrtf(clips[0] * correction[0]), cbrtf(clips[1] * correction[1]), cbrtf(clips[2] * correction[2]), @@ -488,7 +493,7 @@ static void _process_segmentation(dt_dev_pixelpipe_iop_t *piece, const int recovery_closing[NUM_RECOVERY_MODES] = { 0, 0, 0, 2, 2, 0, 2}; const int recovery_close = recovery_closing[recovery_mode]; - const int segmentation_limit = (piece->pipe->iwidth * piece->pipe->iheight) * sqrf(piece->pipe->iscale) / 4000; // 250 segments per mpix + const int segmentation_limit = (pipe->iwidth * pipe->iheight) * sqrf(pipe->iscale) / 4000; // 250 segments per mpix const size_t pwidth = dt_round_size(roi_in->width / 3, 2) + 2 * HL_BORDER; const size_t pheight = dt_round_size(roi_in->height / 3, 2) + 2 * HL_BORDER; diff --git a/src/iop/temperature.c b/src/iop/temperature.c index e73954e5672..cbd543c49bc 100644 --- a/src/iop/temperature.c +++ b/src/iop/temperature.c @@ -42,7 +42,7 @@ #include "common/colorspaces.h" #include "external/cie_colorimetric_tables.c" -DT_MODULE_INTROSPECTION(4, dt_iop_temperature_params_t) +DT_MODULE_INTROSPECTION(5, dt_iop_temperature_params_t) #define INITIALBLACKBODYTEMPERATURE 4000 @@ -52,7 +52,7 @@ DT_MODULE_INTROSPECTION(4, dt_iop_temperature_params_t) #define DT_IOP_LOWEST_TINT 0.135 #define DT_IOP_HIGHEST_TINT 2.326 -#define DT_IOP_NUM_OF_STD_TEMP_PRESETS 5 +#define DT_IOP_NUM_OF_STD_TEMP_PRESETS 4 // If you reorder presets combo, change this consts #define DT_IOP_TEMP_UNKNOWN -1 @@ -60,7 +60,6 @@ DT_MODULE_INTROSPECTION(4, dt_iop_temperature_params_t) #define DT_IOP_TEMP_SPOT 1 #define DT_IOP_TEMP_USER 2 #define DT_IOP_TEMP_D65 3 -#define DT_IOP_TEMP_D65_LATE 4 static void _gui_sliders_update(dt_iop_module_t *self); @@ -71,6 +70,7 @@ typedef struct dt_iop_temperature_params_t float blue; // $MIN: 0.0 $MAX: 8.0 float various; // $MIN: 0.0 $MAX: 8.0 int preset; + gboolean late_correction; // $DEFAULT: FALSE $DESCRIPTION: "prepare data for color calibration" } dt_iop_temperature_params_t; typedef struct dt_iop_temperature_gui_data_t @@ -83,9 +83,9 @@ typedef struct dt_iop_temperature_gui_data_t GtkWidget *btn_asshot; //As Shot GtkWidget *btn_user; GtkWidget *btn_d65; - GtkWidget *btn_d65_late; GtkWidget *temp_label; - GtkWidget *balance_label; + GtkWidget *check_late_correction; + GtkWidget *label_locked; int preset_cnt; int preset_num[54]; double mod_coeff[4]; @@ -141,6 +141,16 @@ int legacy_params(dt_iop_module_t *self, int preset; } dt_iop_temperature_params_v4_t; + typedef struct dt_iop_temperature_params_v5_t + { + float red; + float green; + float blue; + float various; + int preset; + gboolean late_correction; + } dt_iop_temperature_params_v5_t; + if(old_version == 2) { typedef struct dt_iop_temperature_params_v2_t @@ -178,6 +188,28 @@ int legacy_params(dt_iop_module_t *self, *new_version = 4; return 0; } + if(old_version == 4) + { + const int legacy_d65_late = 4; + const dt_iop_temperature_params_v4_t *o = (dt_iop_temperature_params_v4_t *)old_params; + dt_iop_temperature_params_v5_t *n = malloc(sizeof(dt_iop_temperature_params_v5_t)); + + memcpy(n, o, sizeof(dt_iop_temperature_params_v4_t)); + if(o->preset == legacy_d65_late) + { + n->preset = DT_IOP_TEMP_AS_SHOT; + n->late_correction = TRUE; + } + else + { + n->preset = o->preset > legacy_d65_late ? o->preset - 1 : o->preset; + n->late_correction = FALSE; + } + *new_params = n; + *new_params_size = sizeof(dt_iop_temperature_params_v5_t); + *new_version = 5; + return 0; + } return 1; } @@ -192,7 +224,7 @@ static inline void _temp_params_from_array(dt_iop_temperature_params_t *p, static inline void _temp_array_from_params(double a[4], const dt_iop_temperature_params_t *p) { - float *coeffs = (float *)p; + const float *coeffs = (const float *)p; for_four_channels(c) a[c] = coeffs[c]; } @@ -511,11 +543,9 @@ static inline void scaled_copy_4wide(float *const outp, outp[c] = inp[c] * coeffs[c]; } -static inline void _publish_chroma(dt_dev_pixelpipe_iop_t *piece) +static inline void _update_pipe_dsc(dt_dev_pixelpipe_iop_t *piece) { const dt_iop_temperature_data_t *const d = piece->data; - dt_iop_module_t *self = piece->module; - dt_dev_chroma_t *chr = &self->dev->chroma; piece->pipe->dsc.temperature.enabled = piece->enabled; for_four_channels(k) @@ -523,9 +553,32 @@ static inline void _publish_chroma(dt_dev_pixelpipe_iop_t *piece) piece->pipe->dsc.temperature.coeffs[k] = d->coeffs[k]; piece->pipe->dsc.processed_maximum[k] = d->coeffs[k] * piece->pipe->dsc.processed_maximum[k]; - chr->wb_coeffs[k] = d->coeffs[k]; } - chr->late_correction = d->preset == DT_IOP_TEMP_D65_LATE && piece->enabled; +} + +// the WB that temperature applies with params p, for the modules that +// complete the correction: pipe->wb at commit time, and color calibration's +// GUI before any pipe has committed +static void _wb_from_params(const dt_iop_temperature_params_t *p, + const gboolean enabled, + const gboolean true_monochrome, + const double *const D65coeffs, + dt_dev_wb_t *wb) +{ + // a style or pasted history can enable the module on a true monochrome + // image; nothing is applied there, whatever enabled says + const gboolean applied = enabled && !true_monochrome; + const float *const pcoeffs = (const float *)p; + + for_four_channels(k) + { + wb->coeffs[k] = applied ? pcoeffs[k] : 1.0f; + wb->D65coeffs[k] = D65coeffs[k]; + } + wb->camera_reference = applied && p->preset == DT_IOP_TEMP_D65; + // no late correction in 'camera reference' mode; if not applied, + // nothing for a later module to finish + wb->late_correction = applied && !wb->camera_reference && p->late_correction; } void process(dt_iop_module_t *self, @@ -634,7 +687,7 @@ void process(dt_iop_module_t *self, } } - _publish_chroma(piece); + _update_pipe_dsc(piece); } #ifdef HAVE_OPENCL @@ -674,7 +727,7 @@ int process_cl(dt_iop_module_t *self, CLARG(dev_coeffs), CLARG(filters), CLARG(dev_xtrans)); if(err != CL_SUCCESS) goto error; - _publish_chroma(piece); + _update_pipe_dsc(piece); error: dt_opencl_release_mem_object(dev_coeffs); @@ -688,42 +741,43 @@ void commit_params(dt_iop_module_t *self, dt_dev_pixelpipe_t *pipe, dt_dev_pixelpipe_iop_t *piece) { - dt_iop_temperature_params_t *p = (dt_iop_temperature_params_t *)p1; - dt_iop_temperature_data_t *d = piece->data; - float *tcoeffs = (float *)p; + const dt_iop_temperature_params_t *const p = p1; + dt_iop_temperature_data_t *const d = piece->data; + // first items in the struct are the coefficients + const float *const pcoeffs = (const float *)p; - if(self->hide_enable_button) - piece->enabled = FALSE; + d->preset = p->preset; - dt_dev_chroma_t *chr = &self->dev->chroma; + dt_dev_chroma_t *const chr = &self->dev->chroma; + + const gboolean true_monochrome = self->hide_enable_button; - if(self->hide_enable_button) + if(true_monochrome) { - for_four_channels(k) - chr->wb_coeffs[k] = 1.0f; - return; + // nothing to white balance + piece->enabled = FALSE; } for_four_channels(k) - { - d->coeffs[k] = tcoeffs[k]; - chr->wb_coeffs[k] = piece->enabled ? d->coeffs[k] : 1.0f; - } + d->coeffs[k] = true_monochrome ? 1.0f : pcoeffs[k]; + + // D65coeffs is the one value read from dev->chroma here: only + // reload_defaults() writes it, on the GTK thread at image load + _wb_from_params(p, piece->enabled, true_monochrome, chr->wb.D65coeffs, &pipe->wb); + + // color calibration's reload_defaults() still reads the shared copy + for_four_channels(k) + chr->wb.coeffs[k] = pipe->wb.coeffs[k]; + chr->wb.late_correction = pipe->wb.late_correction; + + // Always publish the module handle for channelmixerrgb _set_trouble_messages, + // regardless of the enabled state. This lets color calibration warn about a disabled + // white balance module while it is still doing chromatic adaptation. + chr->temperature = self; // 4Bayer images not implemented in OpenCL yet if(self->dev->image_storage.flags & DT_IMAGE_4BAYER) piece->process_cl_ready = FALSE; - - d->preset = p->preset; - - /* Make sure the chroma information stuff is valid - If piece is disabled we always clear the trouble message and - make sure chroma does know there is no temperature module. - */ - chr->late_correction = p->preset == DT_IOP_TEMP_D65_LATE && piece->enabled; - chr->temperature = piece->enabled ? self : NULL; - if(dt_pipe_is_preview(pipe) && !piece->enabled) - dt_iop_clear_module_trouble_message(self); } void init_pipe(dt_iop_module_t *self, @@ -941,9 +995,9 @@ static void _color_rgb_sliders(dt_iop_module_t *self) //real (ish) //we consider daylight wb to be "reference white" const double white[3] = { - 1.0 / chr->D65coeffs[0], - 1.0 / chr->D65coeffs[1], - 1.0 / chr->D65coeffs[2], + 1.0 / chr->wb.D65coeffs[0], + 1.0 / chr->wb.D65coeffs[1], + 1.0 / chr->wb.D65coeffs[2], }; const float rchanmul = dt_bauhaus_slider_get(g->scale_r); @@ -956,8 +1010,8 @@ static void _color_rgb_sliders(dt_iop_module_t *self) dt_bauhaus_slider_set_stop (g->scale_r, 0.0, white[0]*0.0, white[1]*gchanmul, white[2]*bchanmul); dt_bauhaus_slider_set_stop - (g->scale_r, chr->D65coeffs[0]/rchanmulmax, - white[0]*chr->D65coeffs[0], white[1]*gchanmul, white[2]*bchanmul); + (g->scale_r, chr->wb.D65coeffs[0]/rchanmulmax, + white[0]*chr->wb.D65coeffs[0], white[1]*gchanmul, white[2]*bchanmul); dt_bauhaus_slider_set_stop (g->scale_r, 1.0, white[0]*1.0, white[1]*(gchanmul/gchanmulmax), white[2]*(bchanmul/bchanmulmax)); @@ -965,8 +1019,8 @@ static void _color_rgb_sliders(dt_iop_module_t *self) dt_bauhaus_slider_set_stop (g->scale_g, 0.0, white[0]*rchanmul, white[1]*0.0, white[2]*bchanmul); dt_bauhaus_slider_set_stop - (g->scale_g, chr->D65coeffs[1]/bchanmulmax, - white[0]*rchanmul, white[1]*chr->D65coeffs[1], white[2]*bchanmul); + (g->scale_g, chr->wb.D65coeffs[1]/bchanmulmax, + white[0]*rchanmul, white[1]*chr->wb.D65coeffs[1], white[2]*bchanmul); dt_bauhaus_slider_set_stop (g->scale_g, 1.0, white[0]*(rchanmul/rchanmulmax), white[1]*1.0, white[2]*(bchanmul/bchanmulmax)); @@ -974,8 +1028,8 @@ static void _color_rgb_sliders(dt_iop_module_t *self) dt_bauhaus_slider_set_stop (g->scale_b, 0.0, white[0]*rchanmul, white[1]*gchanmul, white[2]*0.0); dt_bauhaus_slider_set_stop - (g->scale_b, chr->D65coeffs[2]/bchanmulmax, - white[0]*rchanmul, white[1]*gchanmul, white[2]*chr->D65coeffs[2]); + (g->scale_b, chr->wb.D65coeffs[2]/bchanmulmax, + white[0]*rchanmul, white[1]*gchanmul, white[2]*chr->wb.D65coeffs[2]); dt_bauhaus_slider_set_stop (g->scale_b, 1.0, white[0]*(rchanmul/rchanmulmax), white[1]*(gchanmul/gchanmulmax), white[2]*1.0); @@ -1011,9 +1065,9 @@ static void _color_temptint_sliders(dt_iop_module_t *self) const dt_dev_chroma_t *chr = &self->dev->chroma; //we consider daylight wb to be "reference white" const double dayligh_white[3] = { - 1.0 / chr->D65coeffs[0], - 1.0 / chr->D65coeffs[1], - 1.0 / chr->D65coeffs[2], + 1.0 / chr->wb.D65coeffs[0], + 1.0 / chr->wb.D65coeffs[1], + 1.0 / chr->wb.D65coeffs[2], }; double cur_coeffs[4] = {0.0}; @@ -1138,27 +1192,71 @@ static inline const char *_preset_to_str(const int preset) case DT_IOP_TEMP_SPOT: return "by spot"; case DT_IOP_TEMP_USER: return "user defined"; case DT_IOP_TEMP_D65: return "camera reference"; - case DT_IOP_TEMP_D65_LATE: return "as shot to reference"; default: return "other"; } } +// while channelmixerrgb adapts from its own illuminant, the multipliers set here +// don't decide the colors, so only the modes that need no adjusting stay available. +// "user modified" is one of them: a loaded preset can use it for the coefficients +// of the 'caveats' workaround +static void _update_controls_lock(dt_iop_module_t *self) +{ + dt_iop_temperature_gui_data_t *g = self->gui_data; + + const gboolean locked = dt_dev_cat_overrides_wb(self->dev); + + GtkWidget *const controls[] = { g->scale_k, g->scale_tint, + g->scale_r, g->scale_g, g->scale_b, g->scale_y, + g->finetune, g->colorpicker }; + for(size_t i = 0; i < G_N_ELEMENTS(controls); i++) + gtk_widget_set_sensitive(controls[i], !locked); + + dt_bauhaus_combobox_entry_set_sensitive(g->presets, DT_IOP_TEMP_SPOT, !locked); + // the first entry after the standard ones is the camera name, which + // _generate_preset_combo() adds as a section: it must stay insensitive + for(int pos = DT_IOP_NUM_OF_STD_TEMP_PRESETS + 1; pos < g->preset_cnt; pos++) + dt_bauhaus_combobox_entry_set_sensitive(g->presets, pos, !locked); + gtk_widget_queue_draw(g->presets); + + gtk_widget_set_visible(g->label_locked, locked); +} + static void _update_preset(dt_iop_module_t *self, int mode) { dt_iop_temperature_params_t *p = self->params; - dt_dev_chroma_t *chr = &self->dev->chroma; + dt_iop_temperature_gui_data_t *g = self->gui_data; + + const gboolean is_current_reference = p->preset == DT_IOP_TEMP_D65; + const gboolean is_new_mode_manual = mode != DT_IOP_TEMP_D65; + const gboolean is_new_mode_from_camera = mode == DT_IOP_TEMP_AS_SHOT + || mode >= DT_IOP_NUM_OF_STD_TEMP_PRESETS; + + // user modified and from image area keep the flag: it is ignored in camera + // reference (_wb_from_params()), so a visit there must not lose the unticked + // flag of the 'caveats' workaround + if(is_current_reference && is_new_mode_from_camera) + { + p->late_correction = dt_dev_cat_is_active(self->dev); + } p->preset = mode; - chr->late_correction = mode == DT_IOP_TEMP_D65_LATE && self->enabled; + + if(g && g->check_late_correction) + { + // show the checkbox only in modes where user can adjust multipliers + gtk_widget_set_visible(g->check_late_correction, is_new_mode_manual); + + dt_bauhaus_toggle_set(g->check_late_correction, p->late_correction); + + _update_controls_lock(self); + } } void gui_update(dt_iop_module_t *self) { dt_iop_temperature_gui_data_t *g = self->gui_data; dt_iop_temperature_params_t *p = self->params; - dt_iop_temperature_params_t *d = self->default_params; - - d->preset = dt_is_scene_referred() ? DT_IOP_TEMP_D65_LATE : DT_IOP_TEMP_AS_SHOT; const gboolean true_monochrome = dt_image_monochrome_flags(&self->dev->image_storage) & DT_IMAGE_MONOCHROME; @@ -1190,22 +1288,27 @@ void gui_update(dt_iop_module_t *self) gboolean found = FALSE; const dt_dev_chroma_t *chr = &self->dev->chroma; - // is this a "as shot" white balance? - if(dt_dev_equal_chroma((float *)p, chr->as_shot) && (p->preset == DT_IOP_TEMP_D65_LATE)) + // a saved "camera reference" stays one, even if its coefficients (pasted, + // or from a preset or style) differ from this image's reference: relabeling + // it would enable its stored late_correction (_wb_from_params()) + if(p->preset == DT_IOP_TEMP_D65) { - dt_bauhaus_combobox_set(g->presets, DT_IOP_TEMP_D65_LATE); + dt_bauhaus_combobox_set(g->presets, DT_IOP_TEMP_D65); found = TRUE; } - else if(dt_dev_equal_chroma((float *)p, chr->as_shot)) + // is this a "as shot" white balance? + else if(dt_dev_equal_chroma((const float *)p, chr->as_shot)) { dt_bauhaus_combobox_set(g->presets, DT_IOP_TEMP_AS_SHOT); p->preset = DT_IOP_TEMP_AS_SHOT; found = TRUE; } - // is this a "D65 white balance"? - else if(dt_dev_equal_chroma((float *)p, chr->D65coeffs)) + // is this a "D65 white balance"? Only legacy params have no saved mode; + // a saved camera preset or user WB that equals the reference stays as it is + else if(p->preset == DT_IOP_TEMP_UNKNOWN + && dt_dev_equal_chroma((const float *)p, chr->wb.D65coeffs)) { dt_bauhaus_combobox_set(g->presets, DT_IOP_TEMP_D65); p->preset = DT_IOP_TEMP_D65; @@ -1227,7 +1330,7 @@ void gui_update(dt_iop_module_t *self) i++) { const dt_wb_data *wbp = dt_wb_preset(i); - if(dt_dev_equal_chroma((float *)p, wbp->channels)) + if(dt_dev_equal_chroma((const float *)p, wbp->channels)) { // got exact match! dt_bauhaus_combobox_set(g->presets, j); @@ -1289,7 +1392,7 @@ void gui_update(dt_iop_module_t *self) dt_wb_preset_interpolate(dt_wb_preset(i - 1), dt_wb_preset(i), &interpolated); - if(dt_dev_equal_chroma((float *)p, interpolated.channels)) + if(dt_dev_equal_chroma((const float *)p, interpolated.channels)) { // got exact match! @@ -1345,8 +1448,6 @@ void gui_update(dt_iop_module_t *self) p->preset == DT_IOP_TEMP_USER); gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(g->btn_d65), p->preset == DT_IOP_TEMP_D65); - gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(g->btn_d65_late), - p->preset == DT_IOP_TEMP_D65_LATE); _color_temptint_sliders(self); _color_rgb_sliders(self); @@ -1356,7 +1457,7 @@ void gui_update(dt_iop_module_t *self) "used preset", NULL, self, DT_DEVICE_NONE, NULL, NULL, "preset='%s': D65 %.3f %.3f %.3f, AS-SHOT %.3f %.3f %.3f", _preset_to_str(p->preset), - chr->D65coeffs[0], chr->D65coeffs[1], chr->D65coeffs[2], chr->as_shot[0], chr->as_shot[1], chr->as_shot[2]); + chr->wb.D65coeffs[0], chr->wb.D65coeffs[1], chr->wb.D65coeffs[2], chr->as_shot[0], chr->as_shot[1], chr->as_shot[2]); dt_gui_update_collapsible_section(&g->cs); @@ -1499,7 +1600,8 @@ void reload_defaults(dt_iop_module_t *self) dt_iop_temperature_params_t *d = self->default_params; dt_iop_temperature_params_t *p = self->params; - d->preset = dt_is_scene_referred() ? DT_IOP_TEMP_D65_LATE : DT_IOP_TEMP_AS_SHOT; + d->preset = DT_IOP_TEMP_AS_SHOT; + d->late_correction = p->late_correction = FALSE; float *dcoeffs = (float *)d; for_four_channels(k) @@ -1509,7 +1611,7 @@ void reload_defaults(dt_iop_module_t *self) if(!self->dev || !dt_is_valid_imgid(self->dev->image_storage.id)) return; - const gboolean is_raw = + const gboolean is_color_raw = dt_image_is_matrix_correction_supported(&self->dev->image_storage); const gboolean true_monochrome = dt_image_monochrome_flags(&self->dev->image_storage) & DT_IMAGE_MONOCHROME; @@ -1518,6 +1620,8 @@ void reload_defaults(dt_iop_module_t *self) if(!dt_is_scene_referred()) { + // can't use dev->chroma.adaptation: this runs before history replay, and + // it is never set without a GUI another_cat_defined = dt_history_check_module_exists(self->dev->image_storage.id, "channelmixerrgb", TRUE); @@ -1530,10 +1634,6 @@ void reload_defaults(dt_iop_module_t *self) // we want these data in all cases to keep them in dev->chroma double daylights[4] = {1.0, 1.0, 1.0, 1.0 }; - double as_shot[4] = {1.0, 1.0, 1.0, 1.0 }; - - // to have at least something and definitely not crash - _temp_array_from_params(daylights, d); if(!_calculate_bogus_daylight_wb(self, daylights)) { @@ -1560,8 +1660,10 @@ void reload_defaults(dt_iop_module_t *self) } } + double as_shot[4] = {1.0, 1.0, 1.0, 1.0 }; + // Store EXIF WB coeffs - if(is_raw) + if(is_color_raw) { _find_coeffs(self, as_shot); as_shot[0] /= as_shot[1]; @@ -1574,7 +1676,7 @@ void reload_defaults(dt_iop_module_t *self) for_four_channels(k) { chr->as_shot[k] = as_shot[k]; - chr->D65coeffs[k] = daylights[k]; + chr->wb.D65coeffs[k] = daylights[k]; } dt_print(DT_DEBUG_PARAMS, @@ -1584,19 +1686,19 @@ void reload_defaults(dt_iop_module_t *self) STR_YESNO(another_cat_defined), daylights[0], daylights[1], daylights[2], as_shot[0], as_shot[1], as_shot[2]); - d->preset = p->preset = DT_IOP_TEMP_AS_SHOT; - // White balance module doesn't need to be enabled for true_monochrome raws (like - // for leica monochrom cameras). prepare_matrices is a noop as well, as there + // for Leica 'Monochrom' cameras). prepare_matrices is a no-op as well, as there // isn't a color matrix, so we can skip that as well. if(!true_monochrome) { + // set up the matrices gui_data->XYZ_to_CAM, gui_data->CAM_to_XYZ, + // used for temp/tint adjustments if(self->gui_data) _prepare_matrices(self); - /* check if file is raw / hdr */ - if(is_raw) + /* check if file is color raw / sRaw */ + if(is_color_raw) { // raw images need wb: self->default_enabled = TRUE; @@ -1608,7 +1710,8 @@ void reload_defaults(dt_iop_module_t *self) { for_four_channels(k) dcoeffs[k] = as_shot[k]; - d->preset = p->preset = DT_IOP_TEMP_D65_LATE; + d->preset = p->preset = DT_IOP_TEMP_AS_SHOT; + d->late_correction = p->late_correction = TRUE; } else { @@ -1635,6 +1738,8 @@ void reload_defaults(dt_iop_module_t *self) dt_bauhaus_slider_set_default(g->scale_b, dcoeffs[2]); dt_bauhaus_slider_set_default(g->scale_y, dcoeffs[3]); + dt_bauhaus_toggle_set_default(g->check_late_correction, d->late_correction); + for_four_channels(k) g->mod_coeff[k] = dcoeffs[k]; @@ -1655,7 +1760,6 @@ void reload_defaults(dt_iop_module_t *self) dt_bauhaus_combobox_add(g->presets, C_("white balance", "user modified")); // old "camera neutral", reason: better matches intent dt_bauhaus_combobox_add(g->presets, C_("white balance", "camera reference")); - dt_bauhaus_combobox_add(g->presets, C_("white balance", "as shot to reference")); g->preset_cnt = DT_IOP_NUM_OF_STD_TEMP_PRESETS; memset(g->preset_num, 0, sizeof(g->preset_num)); @@ -1665,7 +1769,6 @@ void reload_defaults(dt_iop_module_t *self) _gui_sliders_update(self); dt_bauhaus_combobox_set(g->presets, p->preset); - gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(g->btn_d65_late), p->preset == DT_IOP_TEMP_D65_LATE); gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(g->btn_asshot), p->preset == DT_IOP_TEMP_AS_SHOT); gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(g->btn_user), FALSE); gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(g->btn_d65), FALSE); @@ -1725,8 +1828,10 @@ void gui_changed(dt_iop_module_t *self, GtkWidget *w, void *previous) _mul2temp(self, p, &g->mod_temp, &g->mod_tint); - dt_bauhaus_combobox_set(g->presets, DT_IOP_TEMP_USER); - _update_preset(self, DT_IOP_TEMP_USER); + if(w != g->check_late_correction) { + dt_bauhaus_combobox_set(g->presets, DT_IOP_TEMP_USER); + _update_preset(self, DT_IOP_TEMP_USER); + } } static void _btn_toggled(GtkGestureSingle *gesture, @@ -1742,7 +1847,6 @@ static void _btn_toggled(GtkGestureSingle *gesture, const int preset = togglebutton == g->btn_asshot ? DT_IOP_TEMP_AS_SHOT : togglebutton == g->btn_d65 ? DT_IOP_TEMP_D65 : - togglebutton == g->btn_d65_late ? DT_IOP_TEMP_D65_LATE : togglebutton == g->btn_user ? DT_IOP_TEMP_USER : 0; if(!gtk_toggle_button_get_active(GTK_TOGGLE_BUTTON(togglebutton))) @@ -1762,7 +1866,7 @@ static void _btn_toggled(GtkGestureSingle *gesture, "toggled preset", NULL, self, DT_DEVICE_NONE, NULL, NULL, "preset='%s': D65 %.3f %.3f %.3f, AS-SHOT %.3f %.3f %.3f", _preset_to_str(preset), - chr->D65coeffs[0], chr->D65coeffs[1], chr->D65coeffs[2], chr->as_shot[0], chr->as_shot[1], chr->as_shot[2]); + chr->wb.D65coeffs[0], chr->wb.D65coeffs[1], chr->wb.D65coeffs[2], chr->as_shot[0], chr->as_shot[1], chr->as_shot[2]); } static void _preset_tune_callback(GtkWidget *widget, dt_iop_module_t *self) @@ -1784,8 +1888,6 @@ static void _preset_tune_callback(GtkWidget *widget, dt_iop_module_t *self) pos == DT_IOP_TEMP_USER); gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(g->btn_d65), pos == DT_IOP_TEMP_D65); - gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(g->btn_d65_late), - pos == DT_IOP_TEMP_D65_LATE); gboolean show_finetune = FALSE; dt_dev_chroma_t *chr = &self->dev->chroma; @@ -1809,10 +1911,7 @@ static void _preset_tune_callback(GtkWidget *widget, dt_iop_module_t *self) _temp_params_from_array(p, g->mod_coeff); break; case DT_IOP_TEMP_D65: // camera reference d65 - _temp_params_from_array(p, chr->D65coeffs); - break; - case DT_IOP_TEMP_D65_LATE: // as shot wb just for now - _temp_params_from_array(p, chr->as_shot); + _temp_params_from_array(p, chr->wb.D65coeffs); break; default: // camera WB presets { @@ -1911,7 +2010,7 @@ static void _preset_tune_callback(GtkWidget *widget, dt_iop_module_t *self) } DT_ENTER_GUI_UPDATE(); - float *pcoeffs = (float *)p; + const float *pcoeffs = (const float *)p; dt_bauhaus_slider_set(g->scale_k, TempK); dt_bauhaus_slider_set(g->scale_tint, tint); dt_bauhaus_slider_set(g->scale_r, pcoeffs[0]); @@ -1943,7 +2042,7 @@ void color_picker_apply(dt_iop_module_t *self, GtkWidget *picker, // normalize green: const float gnormal = grayrgb[1] > 0.001f ? 1.0f / grayrgb[1] : 1.0f; for_four_channels(c) - pcoeffs[c] = fmaxf(0.0f,fminf(8.0f,(grayrgb[c] > 0.001f ? 1.0f / grayrgb[c] : 1.0f) / gnormal)); + pcoeffs[c] = fmaxf(0.0f,fminf(8.0f,(grayrgb[c] > 0.001f ? 1.0f / grayrgb[c] : 1.0f) / gnormal)); pcoeffs[1] = 1.0f; dt_bauhaus_combobox_set(g->presets, DT_IOP_TEMP_SPOT); @@ -2044,6 +2143,29 @@ static void _preference_changed(gpointer instance, dt_iop_module_t *self) _color_finetuning_slider(self); } +static void _gui_wb_proxy(const dt_iop_module_t *self, dt_dev_wb_t *wb) +{ + if(!self || !self->dev) + { + dt_dev_wb_set_neutral(wb); + return; + } + + const dt_iop_temperature_params_t *p = self->params; + + _wb_from_params(p, self->enabled, self->hide_enable_button, self->dev->chroma.wb.D65coeffs, wb); +} + +// channelmixerrgb doesn't report changes: every change of its params runs the +// preview pipe, and so do instances added, removed or switched off +static void _preview_pipe_finished(gpointer instance, dt_iop_module_t *self) +{ + if(dt_dev_cat_overrides_wb(self->dev)) + dt_iop_color_picker_reset(self, FALSE); + + _update_controls_lock(self); +} + void gui_init(dt_iop_module_t *self) { dt_iop_temperature_gui_data_t *g = IOP_GUI_ALLOC(temperature); @@ -2090,21 +2212,11 @@ void gui_init(dt_iop_module_t *self) (g->btn_d65, _("set white balance to camera reference point\nin most cases it should be D65")); - g->btn_d65_late = dt_iop_togglebutton_new(self, - N_("settings"), - N_("as shot to reference"), NULL, - G_CALLBACK(_btn_toggled), FALSE, 0, 0, - dtgtk_cairo_paint_bulb_mod, NULL); - gtk_widget_set_tooltip_text - (g->btn_d65_late, - _("set white balance to as shot and later correct to camera reference point,\nin most cases it should be D65")); - // put buttons at top. fill later. g->buttonbar = dt_gui_hbox(dt_gui_expand(g->btn_asshot), dt_gui_expand(g->colorpicker), dt_gui_expand(g->btn_user), - dt_gui_expand(g->btn_d65), - dt_gui_expand(g->btn_d65_late)); + dt_gui_expand(g->btn_d65)); dt_gui_add_class(g->buttonbar, "dt_iop_toggle"); g->presets = dt_bauhaus_combobox_new(self); @@ -2146,12 +2258,33 @@ void gui_init(dt_iop_module_t *self) (g->scale_tint, _("color tint of the image, from magenta (value < 1) to green (value > 1)")); - GtkWidget *box_enabled = dt_gui_vbox(g->buttonbar, - g->presets, - g->finetune, - temp_label_box, - g->scale_k, - g->scale_tint); + // the _from_params helper appends to self->widget, so it has to be + // box_enabled while the checkbox is created + GtkWidget *box_enabled = dt_gui_vbox(g->buttonbar); + self->widget = box_enabled; + + g->check_late_correction = dt_bauhaus_toggle_from_params(self, "late_correction"); + gtk_widget_set_tooltip_text(g->check_late_correction, + _("ensures the white balance coefficients are treated as a reference for the color calibration module.\n" + "keep this checked for non-reference white balance in the modern scene-referred workflow.")); + + g->label_locked = gtk_label_new(_("the white balance is controlled in the color calibration module")); + gtk_widget_set_halign(g->label_locked, GTK_ALIGN_START); + gtk_label_set_xalign(GTK_LABEL(g->label_locked), 0.0f); + // gtk_label_set_line_wrap() is GTK 3 only, the property exists in both + g_object_set(g->label_locked, "wrap", TRUE, NULL); + gtk_widget_set_tooltip_text(g->label_locked, + _("to use these controls, select the illuminant 'as set in white balance module' in color calibration,\n" + "or turn chromatic adaptation off there")); + + dt_gui_box_add(box_enabled, + g->label_locked, + g->presets, + g->finetune, + temp_label_box, + g->scale_k, + g->scale_tint); + dt_gui_new_collapsible_section (&g->cs, "plugins/darkroom/temperature/expand_coefficients", @@ -2184,6 +2317,7 @@ void gui_init(dt_iop_module_t *self) // update the gui when the preferences changed (i.e. colored sliders stuff) DT_CONTROL_SIGNAL_HANDLE(DT_SIGNAL_PREFERENCES_CHANGE, _preference_changed); + DT_CONTROL_SIGNAL_HANDLE(DT_SIGNAL_DEVELOP_PREVIEW_PIPE_FINISHED, _preview_pipe_finished); // start building top level widget self->widget = gtk_stack_new(); @@ -2195,6 +2329,8 @@ void gui_init(dt_iop_module_t *self) gtk_stack_add_named(GTK_STACK(self->widget), GTK_WIDGET(box_enabled), "enabled"); gtk_stack_add_named(GTK_STACK(self->widget), label_disabled, "disabled"); + + darktable.develop->proxy.gui_wb = _gui_wb_proxy; } void gui_cleanup(dt_iop_module_t *self) @@ -2205,9 +2341,9 @@ void gui_cleanup(dt_iop_module_t *self) void gui_reset(dt_iop_module_t *self) { dt_iop_temperature_gui_data_t *g = self->gui_data; - dt_iop_temperature_params_t *d = self->default_params; + dt_iop_temperature_params_t *p = self->params; - const int preset = d->preset = dt_is_scene_referred() ? DT_IOP_TEMP_D65_LATE : DT_IOP_TEMP_AS_SHOT; + const int preset = p->preset; dt_iop_color_picker_reset(self, TRUE); gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(g->btn_asshot), @@ -2216,8 +2352,6 @@ void gui_reset(dt_iop_module_t *self) preset == DT_IOP_TEMP_USER); gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(g->btn_d65), preset == DT_IOP_TEMP_D65); - gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(g->btn_d65_late), - preset == DT_IOP_TEMP_D65_LATE); _color_finetuning_slider(self); _color_rgb_sliders(self);