exposure: the published deflicker correction carries no validity marker

Found by code analysis during the #21974 exposure-proxy review, 2026-09-06.

Extended and re-checked on 2026-09-06 against branch
fix-21974-applied-exposure-disagreement at 39929f3058, and against the branch point
d63f1023a3 for the master comparison. Static analysis only; no GUI reproduction was run in
this environment.

Severity: low / inconsistency. Not a regression from master -- see Master status
below. The pipe can continue displaying pixels processed with one deflicker correction
while the exposure proxy reports a different one, or none. The mismatch reaches the AGX
"read exposure" action and can persist incorrect range values in history.

Summary

In deflicker mode the correction is computed inside the pipe, so the proxy cannot derive
it from parameters the way the manual branch does. It is published as a single float and
read back later:

• publish: _process_common_setup() computes the correction from the raw histogram and
  writes g->deflicker_computed_exposure after a preview-pipe run
  (src/iop/exposure.c:505-513)
• reset: gui_update() writes EXPOSURE_CORRECTION_UNDEFINED
  (src/iop/exposure.c:747-750), having first freed the histogram (:744-745)
• read: _exposure_proxy_get_effective_exposure() returns the scalar, mapping the sentinel
  to 0.f (src/iop/exposure.c:860-873)

The scalar records a value and nothing else. It does not record which image, which
deflicker parameters or which histogram produced it. The lock added on this branch makes
the read race-free; it does not make the value identifiable. Four consequences follow.

Failure mode 1: a presentation-only refresh discards a valid result

gui_update() is not told why it was called, and resets unconditionally. That is right when
the image or the parameters changed. It is wrong for a refresh that changes only what is
displayed.

1. Open a raw, single-channel TYPE_UINT16 image and select exposure's automatic
   (deflicker) mode.
2. Let the preview pipe process the image, so _process_common_setup() publishes a nonzero
   computed correction.
3. Open the exposure module's blending options and choose show output channels.
   Exposure supports blending (src/iop/exposure.c:140-143), so this menu is available.
4. _blendif_show_output_channels() changes only the GUI state and calls
   dt_iop_gui_update(module) (src/develop/blend_gui.c:2004-2012). It does not add a
   history item or explicitly invalidate a pipe.
5. dt_iop_gui_update() invokes module->gui_update()
   (src/develop/imageop.c:2440-2461), which blanks the label and clears the published
   correction. A normal pixelpipe cache hit then returns before exposure runs again
   (src/develop/pixelpipe_hb.c:2009-2015).

The proxy reports 0 EV, and the module's own "computed EC:" label goes blank, until a
later preview run publishes another value. The label is the more directly visible half:
it is wrong in the mode's own panel, with no other module involved.

The same block also frees and rebuilds the histogram on every such refresh, and
_deflicker_prepare_histogram() performs a blocking full-resolution mipmap fetch plus a
histogram pass (src/iop/exposure.c:744-745 and :1047-1048). A presentation-only refresh
therefore pays for a full-image histogram it did not need.

Failure mode 2: changing the target or percentile does not invalidate the result

gui_changed() (src/iop/exposure.c:1020) handles the mode combobox, the manual exposure
slider, the two compensation toggles and the black slider. It has no case for
deflicker_target_level or deflicker_percentile. A bauhaus slider change routes to
gui_changed(), not gui_update(), so nothing clears the scalar.

With a histogram whose selected percentile falls at raw 1024, raw black 0 and raw white
16384, _raw_to_ev() gives -log2(16384) + log2(1024) = -4 EV, so the correction is the
target plus 4: 4 EV at target 0, 2 EV at target -2. After moving the target from 0 to -2
the accessor keeps returning 4 until processing republishes 2. An AGX read in that
interval stores ranges derived from the previous setting.

Failure mode 3: a mode switch leaves the previous session's value in place

The mode case in gui_changed() frees and rebuilds the histogram
(src/iop/exposure.c:1029-1030 and :1047-1048) but does not touch the scalar. Switching
manual to automatic therefore exposes whatever an earlier automatic session on the same
image left behind, until the next preview run republishes.

Failure mode 4: a superseded worker can publish over a current result

The publication site writes under the GUI lock, but from d->params, the copy committed
when that pipe run started (src/iop/exposure.c:509). The GTK thread may have changed
self->params since. The lock serialises the write; it cannot reject it. A pending worker
can therefore overwrite a current value with one computed from older parameters.

This is why simply removing the reset would not be a fix on its own: it would trade
failure mode 1 for permanent reuse of stale results.

Consequence

AGX is the only current consumer of the effective-exposure proxy. It reads the value in
_adjust_relative_exposure_from_exposure_params() (src/iop/agx.c:1042-1054) and uses it to
derive the black and white relative EV ranges.

With a real deflicker correction of +2 EV and no dynamic range scaling, AGX should derive
-7 EV and +5.6 EV. Reading 0 EV instead it derives -8 EV and +4 EV. Pressing AGX's "read
exposure" action then commits those values to history
(src/iop/agx.c:1158-1167). The persistence consequence is traced through the source; no
real AGX button interaction was executed here.

This does not directly change the rendered pixels. It changes the parameters written by a
later user action, so the issue is low severity. It also needs an interactive darkroom
session in a mode intended for non-interactive timelapse batches, which narrows the
practical exposure further: dt_dev_exposure_get_effective_exposure() returns 0 outside the
darkroom (src/develop/develop.c:3673-3678), so the batch path never consults the proxy.

Master status

None of the four failure modes is a regression introduced by #21974.

On the branch point d63f1023a3, the accessor read a different field altogether:

static float _exposure_proxy_get_effective_exposure(dt_iop_module_t *self)
{
  const dt_iop_exposure_gui_data_t* const g = self->gui_data;
  return g->effective_exposure;
}

(d63f1023a3, src/iop/exposure.c:824-828: unlocked, and with no NULL guard on g.)

g->effective_exposure was written unlocked from commit_params() on any pipe thread
(d63f1023a3, src/iop/exposure.c:641), and written before the deflicker gate at :642-649,
so it only ever held the manual composition. It never carried the deflicker correction at
all. That is the broader mismatch #21974 set out to fix.

The reset block in gui_update() is byte-identical on the branch point, lock included
(d63f1023a3, src/iop/exposure.c:722-728, with the label at :725 and the scalar at :727).
#21974 did not touch it. The blank-label half of failure mode 1 is therefore a master bug.

Comparing the two revisions in automatic mode:

situation	d63f1023a3	39929f3058
freshly processed	manual composition, always wrong	computed correction, right
after a presentation-only refresh	manual composition, wrong	0 EV, wrong
after a target or percentile change	manual composition, wrong	stale correction, wrong
concurrent publication	unlocked write from any pipe, unlocked read	locked, but unidentified

The branch is not worse in any of them, and is right in the case that occurs most.

What #21974 did change is structural: g->deflicker_computed_exposure was disposable
label state on master, and the branch gave it a second, cross-module consumer without
changing its lifecycle. That coupling is new. The behaviour is not.

Suggested fix

Give the published result an identity, and validate it where it is used rather than trying
to predict every invalidation. gui_update() cannot know why it ran: its callers split into
those that really do change parameters (dt_dev_pop_history_items() at
src/develop/develop.c:1807 for undo, redo and image load; module reset and preset
autoapply at src/develop/imageop.c:2517 and :2558 and src/gui/presets.c:1116; a new
instance at src/develop/imageop.c:849; the reset label at src/dtgtk/resetlabel.c:44) and
those that only change presentation (the blend display toggles at
src/develop/blend_gui.c:1977, :2012 and :2032). An image-identity guard alone would let
undo and redo through with a stale result.

1. Record what the cached value was computed from, alongside the scalar: image id,
   deflicker percentile, deflicker target, and -- when the histogram was missing and the
   manual composition was published as the fallback -- that composition and a flag saying
   so.
2. Fill it at the publication site, inside the existing critical section, from d->params
   (the copy the pipe actually used) and piece->pipe->image.id
   (src/develop/pixelpipe_hb.h:252), which avoids a cross-thread read of
   dev->image_storage.
3. Compare it against self->params on the GTK thread in one predicate, and use that
   predicate in both _exposure_proxy_get_effective_exposure() and _show_computed(). The
   label and the proxy then agree by construction, which is what #21974 is for.
4. Make the reset block in gui_update() conditional on the same predicate.

Validating on read rather than on invalidation covers failure modes 2 and 3 without
enumerating slider cases, and covers failure mode 4 as well: a superseded worker publishes
an older stamp, which fails the comparison. No generation counter is needed, because the
comparison is over inputs rather than over ordering.

The residual gap is inputs the stamp cannot carry. rawprepare's raw_black_level and
raw_white_point live in pipe->dsc and are only filled during processing
(src/iop/rawprepare.c:297-298), so the GTK thread has nothing to compare against. They
change only through a rawprepare edit, which invalidates the pipe and republishes.

Two things this does not address, deliberately:

• The accessor still maps "no result" to 0 EV, so on a genuine cold start AGX still writes
  -8 and +4. Distinguishing unavailable from an actual 0 EV correction needs a
  validity-returning accessor and a matching change at src/iop/agx.c:1158.
• The histogram's ownership across the GTK and pipe threads is #22006, and is untouched.
  Making the reset conditional narrows that use-after-free window rather than widening it,
  since the buffer is freed less often, but it does not close it.

Related

• #21974 -- the parent exposure-proxy change, on branch
  fix-21974-applied-exposure-disagreement. It is the reason this scalar has a cross-module
  consumer at all, and its fallback fix does not close this case: returning the manual
  total for an undefined value would still disagree with a successfully computed nonzero
  correction. The evidence in Master status above is the argument that this report does
  not have to be resolved on that branch.
• #22006 -- deflicker_histogram is freed by the GTK thread while a pipe thread walks
  it. Shares the gui_update() block at src/iop/exposure.c:744-750, so any fix here touches
  the same lines. Separate defect class (memory safety, not staleness) and separate fix;
  keep them apart and cross-reference.
• #22007 -- the _show_computed() idle source can outlive the module. Shares the
  publication block at src/iop/exposure.c:505-513.
• #22009 -- AGX leaves a stale curve_gamma in history after "read exposure". Same call
  site, src/iop/agx.c:1158, but the cause there is AGX's own DT_IN_GUI_UPDATE() guard
  skipping the gamma recompute, not a wrong exposure input.

Environment

Present at 39929f3058 on branch fix-21974-applied-exposure-disagreement, and, in the forms
described under Master status, at the branch point d63f1023a3.