`channelmixerrgb`: the colour-checker profiler inverts the exposure transform wrongly, from the wrong thread

Found by bots via code analysis

Severity: low impact, but deterministic. The wrong values reach only a GUI label, not
the profile. See Blast radius — that was checked carefully, three times,
because it decides how much this is worth.

Summary

_extract_patches() (src/iop/channelmixerrgb.c:1427) undoes the exposure module's
transform on the sampled patches so the reported normalization values are independent of
the user's exposure setting. It gets this wrong in three independent ways, and it
asks the question from the wrong thread.

Defect 1 — the wrong exposure quantity

  const float user_exposure = exp2f(dt_dev_exposure_get_exposure(darktable.develop));  // :1561
  const float user_black = dt_dev_exposure_get_black(darktable.develop);               // :1562
  ...
  RGB_test[c] = RGB_test[c] / user_exposure / exposure + user_black;                   // :1583

dt_dev_exposure_get_exposure() resolves to _exposure_proxy_get_exposure()
(src/iop/exposure.c:787-799), which returns the raw p->exposure. The pipe applies
the effective exposure — raw minus EXIF bias, plus highlight-preservation bias
(_effective_manual_exposure(), src/iop/exposure.c:614-629).

compensate_hilite_pres defaults to TRUE for the first instance
(src/iop/exposure.c:73, :369), and the highlight bias is clamped to [-1, 4] EV
(:606), so on any camera carrying that tag the inversion is wrong by up to 4 EV.

Defect 2 — the wrong formula

The exposure module applies (src/iop/exposure.c:516-517, :559-575):

white = exp2f(-E_eff)
scale = 1 / (white - black)
out   = (in - black) * scale

so the exact inverse is in = out * (white - black) + black. The code computes
out * white + black — it drops black from the scale denominator. Exact only when
black == 0, and the scene-referred default is black = -0.000244140625f
(src/iop/exposure.c:358), so it is never exactly zero in the default workflow.

Defect 3 — in deflicker mode it inverts with a target, not a correction

_exposure_proxy_get_exposure() returns p->deflicker_target_level when
mode == EXPOSURE_MODE_DEFLICKER (src/iop/exposure.c:790-793). That is the level
deflicker aims at, not the correction it applied — which is computed inside the pipe from
the raw histogram. Unrelated numbers.

Defect 4 — read from the preview pipe thread

Both accessors read the exposure module's live params, which are owned by the GTK
thread (widget callbacks, dt_dev_pop_history_items(), presets write them there).
_extract_patches() runs on the preview pipe thread: reached from
_extract_color_checker() (:1657) / _validate_color_checker() (:1949), both called
from process() (:2163, :2287).

A data race by construction, and gui_lock cannot fix it — nothing takes that lock when
params are written.

_dev_exposure_proxy_available() (src/develop/develop.c:3627) additionally reads
dev->proxy.exposure.module and calls dt_view_get_current() from that pipe thread. The
pointer is rebound from the GTK thread — dt_iop_request_focus()
(src/develop/imageop.c:2732), preset apply (src/gui/presets.c:1129, :1307, :1339),
module move (src/develop/imageop.c:717, :753) — with no pipe quiescing. This is not
a use-after-free: every path that destroys an instance is quiesced by
dt_dev_pixelpipe_stop_and_lock_all() (src/develop/develop.c:611, taken on instance
delete src/develop/imageop.c:571-629, undo/redo src/libs/history.c:585, and darkroom
leave()). The pointee is alive; the symptom is "answers about the wrong instance".

Defect 5 — undefined multi-instance semantics

The pipe applies every enabled exposure instance. The proxy reports exactly one, and
the two entry points disagree about which: get_exposure/get_black answer for
dev->proxy.exposure.module (bound with FILTER_PREFER,
src/develop/imageop.c:4096-4098), while dt_dev_exposure_get_effective_exposure()
re-resolves with FILTER_REQUIRE + prefer-unmasked + prefer-first
(src/develop/develop.c:3641-3675). Inverting one of N is wrong regardless.

Blast radius

Display only. user_exposure/user_black are used solely inside the if(XYZ_to_CAM)
block (:1563-1632) to build a local RGB_test for the least-squares fit that yields
extraction_result_t {black, exposure} (:1639). The patches array is read and never
written in that block — verified by scanning :1556-1642 for any assignment to
patches[...]: there are none, only two reads into a local (:1575, :1605).

extraction_result reaches only g_strdup_printf() for g->delta_E_label_text
(:1911-1935, :1968-1977), whose sole consumer is gtk_label_set_markup() in
_preview_pipe_finished_callback() (:3029-3038).

The profile itself is unaffected: g->mix is solved by pseudo_solve_gaussian() /
repack_double3x3_to_3xSSE() (:1855-1858) from LMS_test, which derives from the
untouched patches, and _commit_profile_callback() (:2943-2953) copies g->mix into
p->red/green/blue. user_exposure never enters that chain.

So the user-visible symptom is: the "Normalization values — exposure compensation / black
offset" line in the profile quality report shows wrong numbers.

Suggested fix — answer the question in the pipe

The thing channelmixerrgb needs is not "the exposure module's parameter" but the
linear transform the pipe applied upstream of me. That is per-pipe data, already
computed, and asking for it on the pipe thread is then natural rather than a violation.

Publish per node what the module committed:

// on dt_dev_pixelpipe_iop_t
typedef struct dt_dev_pipe_linear_t
{
  gboolean valid;   // this node published an invertible linear transform
  float offset;     // subtracted before scaling
  float scale;      // multiplied after
} dt_dev_pipe_linear_t;

• exposure fills it in commit_params() (manual mode) and refreshes it in
  _process_common_setup() (deflicker).

• channelmixerrgb walks pipe->nodes from the head, stopping at its own node —
  everything before is upstream in this pipe, by construction. Do not compare live
  piece->module->iop_order: the pipe holds its own topology snapshot
  (pipe->iop_order_list = dt_ioppr_iop_order_copy_deep(...),
  src/develop/pixelpipe_hb.c:519) while the GTK thread can change the live order.
  This also matters because no rule pins exposure before channelmixerrgb —
  dt_ioppr_get_iop_order_rules() (src/common/iop_order.c:801-821) only pins
  colorin → channelmixerrgb, so a DT_IOP_ORDER_CUSTOM order can invert them.

• Three accumulator states, not two. Omission is not neutrality: silently skipping an
  un-invertible node composes it as the identity, which is wrong.

  state	meaning	action
  identity	no enabled exposure upstream	invert with {0, 1}
  known	all upstream enabled exposures invertible	invert exactly
  unknown	≥1 upstream exposure masked/blended or record invalid	refuse to answer; say so in the report

• Blending gates validity. Blending runs after process()
  (dt_develop_blend_process(), src/develop/pixelpipe_hb.c:1797, :2884) whenever
  d->mask_mode & DEVELOP_MASK_ENABLED (:1500-1509). A masked instance applies only
  inside its mask — spatially varying, not one {offset, scale}. Check
  piece->blendop_data on the read side, where it is populated.

• init_pipe() must set valid = FALSE, so a node that has never committed cannot
  present uninitialised data. commit_params() is not called for every node on every
  synch: DT_DEV_PIPE_TOP_CHANGED routes to dt_dev_pixelpipe_synch_top(), which synchs
  one history item (src/develop/pixelpipe_hb.c:798-820, :834-846). That is fine — the
  record persists in the node and is refreshed whenever the value can change — but it
  makes correct initialisation mandatory.

• Deflicker must be cache-aware. Its record can only be published from
  _process_common_setup(), which a pixelpipe cache hit skips
  (src/develop/pixelpipe_hb.c:1991-2019; downstream invalidation can preserve an
  upstream cache line, src/develop/pixelpipe_cache.c:370-382). Invalidate at
  commit_params() and re-validate only in _process_common_setup(), so a node that did
  not recompute reports unknown rather than a stale number.

Publishing {offset, scale} rather than "exposure in EV" fixes defects 1, 2 and 3 for
free: it is what the pipe applies, so the inverse in = out/scale + offset is exact.

Do not do these

• Do not simply switch to dt_dev_exposure_get_effective_exposure().
  _extract_color_checker() is called with channelmixerrgb's own gui_lock held
  (:2162-2167), and that accessor takes the exposure module's gui_lock in deflicker
  mode (src/iop/exposure.c:843-847) — a new cross-module lock edge
  channelmixerrgb.gui_lock → exposure.gui_lock. No cycle today, but do not create it.
• Do not make the existing proxy thread-safe by publishing a locked snapshot. That is
  what the original #21974 code was: a value one commit stale. Locking removes the UB and
  keeps the wrong answer.

Also worth fixing while here

channelmixerrgb passes the global darktable.develop (:1561-1562), not self->dev.
Currently masked — the profiling branch needs self->dev->gui_attached && g (:2155) and
the pinned second-window dev's cloned modules have gui_data == NULL
(_clone_module(), src/develop/develop.c:347-371) — but it is wrong on principle. Note
dt_dev_exposure_get_effective_exposure()'s own dev argument is already decorative:
_find_preferred_instance() hardcodes darktable.develop->iop
(src/develop/imageop.c:4007).

Environment

Present in 56898c27b8. Static analysis only — no display in the analysis container,
nothing verified at runtime.

Start in src/iop/channelmixerrgb.c at _extract_patches() and trace the callers through process(), then read the exposure transform and proxy code in src/iop/exposure.c and the pixelpipe lifecycle in src/develop/pixelpipe_hb.c. Review how per-node data is initialized, committed, refreshed, and invalidated. Done means the normalization report uses the upstream pipe transform without GTK-thread access and reports unknown for masked, blended, or stale transforms; runtime verification is unavailable in the stated environment.