Fix 21974 applied exposure disagreement

[kofa73]

exposure: use one definition of the exposure the module applies

Fixes #21974.

TL;DR:

• publication of the exposure value via the proxy (consumed by AgX):
  • initial idea was to lock
  • but the value was only computed in commit_params, so could be lagging behind (unlikely)
  • manual mode: now the value is recomputed (cheap) in _exposure_proxy_get_effective_exposure
  • automatic (deflicker) mode reads the preview pipe’s computed correction under the GUI lock, returning 0 EV when unavailable; previously it reported the manual value even in automatic mode.
• fixed 2 rare possible image corruptions
  • in auto mode, if histogram was unavailable, the image would turn black
  • in manual mode, if black > 0, the image could be inverted, because the additional (in-camera exposure correction dialled in by the user + camera highlight preservation underexposure) were not taken into account. This was a problem because black must remain below effective (all manual + bias corrections applied) white, but the calculation that checked the limits only took the manual part into account.
• finally, when using the picker, if the sampled area was black or almost black, we could get division by zero (or by tiny number). I also disabled this "exposure matching" if the target is (almost) black -- no one would want to "exposure match" an image to black.
• some helper functions for clarity and consistency, so the back-and-forth calculation between "total exposure correction = user slider + compensation for in-camera biases" and "manual slider level to set = total exposure correction - compensation for in-camera biases" pair always remains consistent.
• also, some minor renaming, e.g. the result of 2^exposure is not 'white' but 'gain'

(^^^^^ That's not AI, it's my genuine, natural stupidity and bad writing ^^^^^)

ˇˇˇˇˇ AI from here on ˇˇˇˇˇ

Background

The exposure module does not apply the exposure slider alone. It applies the
slider plus two corrections read from EXIF:

• compensate camera exposure: the exposure correction the user dialed into the
  camera (clamped to -5 .. +5 EV)
• compensate camera highlight preservation: the underexposure the camera
  applied by itself in an HDR / DR-boost / HLG mode (clamped to 0 .. +4 EV, on by
  default for the first instance of the module)

The sum is what commit_params() stores and what process() turns into black
and scale. This PR calls that sum the total adjustment.

Three places in the module used some other value where the total adjustment was
meant. This PR gives the module a single definition of it and routes every user
of it through that definition. Sharing that definition also exposed a fourth
defect, in the area exposure mapping tool, which is fixed here as well.

What was fixed

1. The shared effective_exposure field: data race and stale value

This is the defect the issue was opened for.

commit_params() cached the applied exposure in a plain float inside
gui_data, from a pipe thread, with no lock.
dt_dev_exposure_get_effective_exposure() read the same field from the GTK
thread, also with no lock and with no NULL check on gui_data. agx reads it
in its "read exposure" button and writes the result into history.

Locking it would have been the wrong fix. The value was only ever produced by
commit_params(), so a caller asking before the pipe had re-committed got the
previous exposure. A lock removes the undefined behaviour and keeps the wrong
answer.

The field is now gone. In manual mode the accessor derives the value from the
parameters, on the caller's thread, exactly like the neighbouring
_exposure_proxy_get_exposure() and _exposure_proxy_get_black() already did.
There is no shared field left to race on.

Automatic (deflicker) mode is the exception: its correction is computed from the
raw histogram inside the pipe, so it has to be published rather than derived.
That branch now takes the module's GUI lock, checks gui_data for NULL, and
returns 0 EV while the value is still undefined, which is the case before the
preview pipe has run once. Before this PR the accessor returned the manual
composition in automatic mode as well, which is a quantity the module never
applied there.

The thread contract of the whole proxy is now written down on
dt_dev_proxy_exposure_t in develop.h: these accessors read live GTK-owned
state, so they may only be called from the GTK thread.

2. Automatic mode rendered a fully black image when the histogram was missing

_compute_correction() (renamed to _compute_deflicker_correction()) wrote its
"undefined" sentinel, -FLT_MAX, before its early return. The caller had
already seeded the same variable with the manual fallback, so the sentinel
destroyed it. The pipe then computed white = exp2f(FLT_MAX), which is
infinity, so scale = 1 / (inf - black) = 0 and every output pixel was 0.

This is deterministic, not a race: _deflicker_prepare_histogram() returns
without a histogram when the image is not single-channel TYPE_UINT16, or when
the raw buffer cannot be fetched. In that state automatic mode rendered black on
every pipe run.

The function now leaves the caller's value alone when it cannot compute
anything. The caller seeds it with the manual composition, and that same value
is both applied by the pipe and published to the proxy, so the two agree on what
the fallback was.

3. The black point was limited against the wrong white point, which could invert the image

The GUI keeps black below the white point so that white - black, and
therefore scale, stays positive. All the limit checks compared against the
white point derived from the raw slider value, while the pipe derives the
white point from the total adjustment. The two differ by the compensations,
so by up to 9 EV.

This was reachable with ordinary slider positions, and highlight-preservation
compensation is on by default. Reproduced in the GUI: camera exposure bias -2 EV
compensated, exposure slider +4 EV, so the pipe applies +6 EV and the real white
point is 2^-6 = 0.0156. Moving black to 0.0160 passed the old check, because
0.0160 < 2^-4 = 0.0625, and produced scale = 1 / (0.0156 - 0.0160) = -2667:
the image inverts.

black 0.0150, before the fix

black 0.0160, before the fix

With the fix, moving black to 0.0160 pulls the exposure slider down to
+3.265 EV, so the applied total is +5.265 EV and the white point stays above
black:

What changed in the code:

• all the limit checks now compare p->black against
  exposure2white(_total_adjustment_ev(self, p)), which is the white point the
  pipe will actually use
• _exposure_set_white() converts back through
  _required_exposure_slider_ev(), so writing the slider really produces the
  requested white point instead of missing it by the compensation offset
• the check also runs when a compensation checkbox is toggled, and when the
  module returns to manual mode. The second case matters because black can be
  edited in automatic mode, where the manual exposure is not applied, so a
  configuration that was valid in automatic mode can be invalid the moment
  manual mode comes back
• the checks run in manual mode only. In automatic mode the manual controls are
  hidden but still reachable through keyboard shortcuts, and a white point that
  the pipe does not use must not move the black point that it does use

4. Area exposure mapping could store an out-of-range exposure

With the area picker active in "correct" mode, moving the target lightness to 0
made the module store an exposure of about +66 EV. The parameter's declared
range is -18 .. +18 EV, the slider cannot show such a value, and a history item
was recorded for it, so the image blew out until the target was set back.

The tool divides the sampled luminance by the target luminance to get the white
point. A target lightness of 0 is inside the lightness slider's range, and
dt_Lab_to_XYZ() converts it back to a Y of either exactly zero or, where the
compiler contracts 116 * x - 16 into an FMA, slightly negative. The ratio is
then infinite or large and negative, and white2exposure() floors its argument
at 1e-20, which is where +66 EV comes from.

The module now declines the correction when the sampled or the target luminance
is at or below 1e-5: no parameter is written and no history item is pushed.

Two separate reasons put the threshold at 1e-5 rather than at zero:

• it is below the finest value a raw can carry. rawprepare emits
  (in - black) / (white - black), so the raw white point becomes 1.0 and the
  quantization step is 1 / (white - black): 1.53e-5 for a 16-bit raw with a
  zero black level, and coarser for everything else, such as 7.0e-5 for a
  14-bit raw with a black level of 2048. White balance multiplies by
  coefficients of order 1, and the camera matrix is normalized so that neutral
  maps to neutral, so the sampled luminance stays in that same scale
• it keeps every accepted match representable. A sample at 1e-5 needs
  +16.6 EV to reach the brightest possible target and +14.2 EV to reach
  mid-gray, both inside the declared range. At 1e-6 the same match would need
  19.9 EV, so the module would store an exposure outside its own limits again

A note for anyone bisecting this series: the simplification described below
removed a round trip that had incidentally clamped the ratio to a small positive
number, so between 9195d35176 and 362a3b8288 a negative ratio was also
stored as the black point, at about -3.8e9, which survived setting the target
back. Both halves are closed by the same check.

Smaller changes in the same commits

• new helpers in exposure.c, all static inline:
  _exposure_compensation_ev() (the two EXIF corrections),
  _total_adjustment_ev() (slider plus corrections) and
  _required_exposure_slider_ev() (the inverse, used when the GUI knows the
  white point it wants and has to write the slider)
• the area exposure mapping "measure" branch used to repeat the compensation
  arithmetic and then apply it as exposure2white(-expo), which is
  exp2f(expo) written in a confusing way. It now calls the shared helper and
  multiplies by a plain gain
• the "correct" branch had the same duplicated arithmetic followed by a
  round trip through white2exposure/exposure2white. It now passes the
  measured ratio straight to _exposure_set_white(), which does the inverse
  conversion in one place, guarded by the luminance check from section 4
• renames and comments: _compute_correction() is now
  _compute_deflicker_correction(), US spelling fixes in the two comments that
  were touched anyway, and a note explaining what white means in
  exposure2white

Behaviour, and compatibility with existing edits

• rendering does not change for manual mode, and does not change for automatic
  mode when the histogram is available: d->params.exposure, d->black and
  d->scale are computed exactly as before
• no parameter change, no module version bump, no legacy_params() work
• the limit checks live in GUI callbacks only. Loading an existing edit that is
  already in the invalid state does not rewrite it, so old images still render
  as they did. Making the pipe itself reject an invalid transform is a separate
  decision, see the follow-ups
• what a user will notice: the black slider now stops at the real white point;
  automatic mode with an unusable raw histogram renders the manual fallback
  instead of a black frame; agx's "read exposure" gets the correct value in
  automatic mode; setting the area mapping target lightness to 0 now does
  nothing instead of writing an exposure far outside the parameter's declared
  range of -18 .. +18 EV and pushing a history item for it

Files touched

• src/iop/exposure.c: all of the above
• src/develop/develop.h: the thread contract on dt_dev_proxy_exposure_t and
  on the four accessors, plus a note about the one known violation
• RELEASE_NOTES.md

Not fixed here

Reviewing this branch turned up more problems in the same area. All of them
exist on master today, none is a regression from this PR, and each one needs a
decision that does not belong in this change.

Filed:

• channelmixerrgb: the colour-checker profiler inverts the exposure transform wrongly, from the wrong thread #22005: channelmixerrgb's ColorChecker profiler reads exposure's
  GTK-owned parameters from the preview pipe thread, and inverts the exposure
  transform with the wrong quantity and the wrong formula. Only marked with a
  TODO here
• exposure: deflicker_histogram is freed by the GTK thread while a pipe thread walks it #22006: the deflicker histogram is freed and reallocated by the GTK thread
  while a pipe thread is reading it
• exposure: _show_computed() idle source can outlive the module #22007: the idle source that shows the computed correction can outlive the
  module; teardown removes one source, but one is queued per preview pipe run
• dt_dev_exposure_handle_event() has no darkroom guard, unlike its three sibling getters #22008: dt_dev_exposure_handle_event() has no darkroom guard, unlike the
  three getters next to it, and it is the only one that writes parameters
• agx: "read exposure" leaves a stale curve_gamma in history #22009: agx leaves curve_gamma stale in history after its "read
  exposure" button
• exposure: the published deflicker correction carries no validity marker #22172: the published automatic correction carries no validity marker. A
  GUI refresh that is not an edit, for example opening the blending "display
  output channels" menu, resets it, and changing the target level or percentile
  does not invalidate it. agx can then store range values that belong to a
  different correction. A lock does not fix this; the published result needs to
  say which settings it describes
• Exposure processing accepts a zero or negative black-to-white range #22182: the pipe does not check white - black at all. GUI limits cannot
  cover automatic mode, pasted or imported history, or edits made before this
  PR. A guard there changes how existing edits render, so it needs an explicit
  compatibility decision
• Exposure area mapping ignores black correction #22183: the area exposure mapping tool models exposure as a pure gain and
  ignores black, in both measurement and correction, while the pipe applies
  (in - black) * scale
• _auto_set_exposure() is manual-mode logic kept out of automatic mode by a reset call, not a guard #22184: _auto_set_exposure() (the area exposure mapping tool) is
  manual-mode logic with no mode check. Normal mode changes reset the picker,
  but a user-assigned shortcut can re-activate it in automatic mode, where it
  would report and write values the pipe ignores

Open point

The comment in develop.h refers to dev-doc/GUI_Threading.md, which is added
by #21912 and is still a draft.

What was tested

• the branch builds clean, with no new warnings. Each commit was checked out and
  built on its own, so git bisect stays usable
• no rendering change on existing edits. Headless CPU exports of the
  0001-exposure integration image, with its original version-5 manual history
  and with a version-7 automatic-mode variant, were compared between the branch
  point and the branch: ImageMagick reports 0 differing pixels for both
• parameter sweep. A harness that compiles the real exposure.c and calls
  the real commit_params() and process() covered 6,722 valid manual
  parameter combinations: both compensation switches, EXIF bias -5 .. +5 EV,
  highlight preservation 0/1/2/4 EV, exposure -18 .. +18 EV, and negative, zero
  and positive black. No parameter bytes were rewritten. Of 26,888 output
  floats, 76 differed, by at most 1.19e-07 absolute and 2.11e-07 relative, which
  is the reassociation of the same arithmetic, not a behaviour change
• defect 3, the inversion: reproduced in the GUI before the fix and confirmed
  gone after it, with the settings and screenshots above. The same case in the
  harness: scale goes from -2666.66 to +100, with the slider moved to
  +3.26534 EV
• defect 4, the unusable exposure: confirmed in the harness, which drives the
  module's real color_picker_apply(). With a sample luminance of 0.25 and the
  target moved to 0 and then back to 0.5, the branch point stored +66.4386 EV and
  one history item at the zero target; the branch stores nothing and pushes no
  history item, and the restored target gives the correct 0.5 at the sample on
  both. Between 9195d35176 and 362a3b8288 the same sequence also left
  black at -3.7887e9 and the sample at 1.0
• defect 2, the black frame: confirmed in the harness. With a missing
  histogram and a +2 EV manual fallback, scale was 0 and the output 0 before
  the fix; after it, scale is 4, an input of 0.18 becomes 0.72, and the proxy
  reports +2 EV, so the pipe and the proxy agree. Not reproduced in a GUI
  session, which would need a raw whose histogram cannot be built
• defect 1, the race: traced in the source only. Removing a shared field is
  not something a test run shows
• the integration test suite was not run: this container has no opencv,
  numpy or colour-science for its comparison script. The export comparison
  above uses the same image but compares the two revisions against each other,
  not against the suite's stored reference
• process_cl() was not exercised: OpenCL initialisation finds 0 platforms
  here, and an export that requested OpenCL fell back to the CPU. The OpenCL
  path is unchanged and consumes the same d->black and d->scale that the CPU
  path does, both produced by the shared _process_common_setup(); the kernel
  arithmetic in data/kernels/basic.cl:241 is untouched
• not covered anywhere: a real GTK session for the proxy paths, forced tiling,
  and a build with OpenCL disabled at configure time

PR written by Claude Code.

[TurboGit]

@kofa73 : I'm a human :) And frankly such a long report with AI wording is not helping me. I'm ok to have this as a "long version" of the report but a self contain description should be first and written for a human. I suppose that the text above is what the AI has written, fine, but to me this is for the dev to asses the validity of the fix.

[kofa73]

The TL;DR was me, with my own fingers. :-D

[kofa73]

Expanded the TL;DR. I think it has enough details now. If wording is unclear, let me know.

The following is a better-worded AI summary, but it covers the same:

The exposure proxy cached a value written by a pipe thread and read by the
GTK thread without locking. It could also return an old value, or the manual
exposure while automatic mode was active.

This PR removes that cache and fixes three related problems in exposure.

Changes

• Exposure proxy: in manual mode, derive the effective exposure from the
  current parameters, including both camera compensations. commit_params()
  uses the same helper. In automatic mode, read the preview pipe's computed
  correction under the GUI lock; return 0 EV if no value is available.
  Document that exposure proxy calls require the GTK thread.

• Missing histogram: keep the manual exposure, including compensations,
  as the fallback when deflicker cannot compute a correction. Previously,
  the undefined sentinel replaced the fallback and produced a black image.
  The preview pipe publishes the fallback value to the proxy too.

• Black-point limits: compare black against the white point derived from
  the exposure including camera compensations. The old checks used the slider
  alone, so white - black could become negative and invert the image.
  Apply the checks when changing either compensation and when returning to
  manual mode. Convert the requested white point back to the slider value
  through the same compensation helper.

• Area exposure mapping: skip correction when sample or target luminance
  is at or below 1e-5. A zero target could previously store about +66 EV,
  outside the declared -18 to +18 EV range. Rejected corrections leave
  parameters and history unchanged. Measurement and correction also use the
  shared exposure conversions.

Compatibility and remaining limits

No parameter layout or module version changes. Existing valid edits retain
the same rendering, apart from small floating-point rounding differences.
Automatic mode with a missing histogram intentionally changes to the fallback.

The black-point checks run in GUI callbacks. They do not repair invalid
values loaded from history or protect the automatic-mode transform; that is
tracked in #22182. Automatic correction validity after GUI refreshes or
parameter changes remains tracked in #22172. The worker-thread proxy calls
in channelmixerrgb remain tracked in #22005.

[TurboGit]

Ok, looks good. Thanks!

[TurboGit]

Merged manually with conflict resolutions.

[kofa73]

Thanks. I'll be more considerate with the PR message next time.