toneequal: the pipe clears histogram_valid without resetting max_histogram

Found by code analysis, 2026-09-03.

Severity: low, cosmetic but persistent. g->max_histogram is the divisor the
equalizer graph normalises its inset histogram by. Since 2021 the function that
produces it only ever raises it, so it behaves as a high-water mark over the life of
the module GUI. Every path that invalidates the histogram resets it except the one the
pipe uses, so after an upstream edit the histogram is recomputed correctly and then
drawn against the largest peak the module has seen since it was opened. The curve keeps
its shape and loses its height. No pixels, no parameters and no crash are involved --
only the graph the user reads to decide where to place the nodes.

The defect

src/iop/toneequal.c. max_histogram (declared :226) has exactly one reader, the
inset histogram in area_draw():

2648:      const float y_temp = (float)(g->histogram[k]) / (float)(g->max_histogram) * 0.96;

and three writers: two that set it to 1, and one that raises it.

 631:  g->max_histogram = 1;                     // invalidate_luminance_cache()
1346:  g->max_histogram = 1;                     // gui_cache_init(), from gui_init()

1398: static inline void compute_log_histogram_and_stats(const float *const restrict luminance,
1400:                                                    const size_t num_elem,
1401:                                                    int *max_histogram,
...
1406:   memset(histogram, 0, sizeof(int) * UI_SAMPLES);      // the bins, not the out-parameter
...
1468:     // store the max numbers of elements in bins for later normalization
1469:     *max_histogram = histogram[i] > *max_histogram ? histogram[i] : *max_histogram;

:1406 is the only initialisation in the function and it clears the bin array. The
out-parameter is never written before :1469 reads it, and :1469 is a max, so the
value carried in from the caller survives. Its only caller passes the live field:

1479:   if(!g->histogram_valid && g->luminance_valid)
1480:   {
1481:     const size_t num_elem = g->pd.height * g->pd.width;
1482:     compute_log_histogram_and_stats(g->pd.buf, g->histogram, num_elem,
1483:                                     &g->max_histogram,
1484:                                     &g->histogram_first_decile, &g->histogram_last_decile);

So update_histogram() (:1473-1489) refreshes the bins and the two deciles from
scratch, and merges the new peak into the old one.

The clears disagree about it. Four places set histogram_valid = FALSE; two reset
the divisor beside it and two do not.

site	context	resets max_histogram?
invalidate_luminance_cache:633	GUI, mask parameters	yes, :631
gui_cache_init:1353	gui_init()	yes, :1346
toneeq_process:1042	pipe worker, module changed pipe order	no
toneeq_process:1130	pipe worker, preview recompute	no

:1130 is the one that matters: it sits in the preview-pipe branch, inside
if(saved_hash != hash || !luminance_valid), so it fires on every upstream change that
alters this module's input. The next area_draw() calls update_histogram() at :2616,
which recomputes the bins because histogram_valid is FALSE and normalises them at
:2648 against a divisor that was never brought down.

(The two auto-tune quads also clear the flag without resetting the divisor, :1819 and
:1881, but each ends by calling invalidate_luminance_cache() -- :1844, :1922 --
so they repair it before the next draw, and the values they compute are the deciles,
which are assigned rather than maxed. They are not part of this defect.)

What the user sees

The histogram is drawn short by exactly the ratio between the stale peak and the true
one, with its shape intact, and it stays that way until something calls
invalidate_luminance_cache().

The pronounced case comes from the edge bins. compute_log_histogram_and_stats() builds
an extended histogram spanning [-10, +6] EV and then folds it into the 256 bins the graph
shows, which span [-8, 0] EV:

1461:     const float EV = 16.0 * (float)k / (float)(TEMP_SAMPLES - 1) - 10.0;
1462:     const int i =
1463:       CLAMP((int)(((EV + 8.0f) / 8.0f) * (float)UI_SAMPLES),
1464:             0, UI_SAMPLES - 1);
1465:     histogram[i] += temp_hist[k];

Everything at or above 0 EV lands in bin 255 and everything below -8 EV in bin 0. Raising
the exposure upstream by a couple of EV therefore piles a large part of the mask into a
single bin and sets a high-water mark far above any peak an ordinary distribution
produces; lowering it again leaves that mark in place and the histogram is drawn at a
fraction of its correct height. Any upstream edit that spreads the mask -- more contrast,
a different white balance -- produces the same effect in a milder form.

Not reproduced. This is read off the code; nothing here was run. Two things that
would look like the same bug and are not: the modern crop module runs after
toneequal in every default order (iop_order.c, v50_order: toneequal 24.0 at
:332, crop 24.5 at :334), so cropping does not change this module's pixel count,
and the preview pipe is processed from DT_MIPMAP_F (develop.c:697-699) at
scale = 1.0f with window_width / window_height left at G_MAXINT (:803-805,
the if(port) block at :809 being skipped for the preview pipe), so :863-864 size
it from the input rather than from the viewport; resizing the darkroom does not change
it either. Only the distribution moves, which is what makes the edge-bin case the
interesting one. (The one module that does change the count is the deprecated clipping,
which sits at 17.0, ahead of toneequal, in the same table.)

Recovery. Any of these resets the divisor and the graph is correct again:

• touching one of the controls gui_changed() routes to it: method, blending,
  feathering, iterations, quantization (:1746), details (:1750), or either
  boost slider (:1756)
• either auto-tune quad (:1844, :1922)
• anything that runs the module's gui_update(), which calls
  invalidate_luminance_cache() at :1729: an image switch, and undo or redo, since
  dt_dev_pop_history_items() updates every module (develop.c:1780, :1795)
• leaving and re-entering the darkroom, which recreates the module GUI and so re-runs
  gui_init() -> gui_cache_init:1346

Adjusting an upstream module does none of these, which is why the wrong scale persists.

Why it is a regression rather than a design choice

The out-parameter was initialised for the first three years of the module's life, and the
two pipe-side clears were written while it still was.

commit	date	what it did
5f78447450	2018-12-16	initial commit: accumulates into a local temp_max_histogram = 0, assigns *max_histogram at the end
dce60377a8	2019-09-06	rewrite, same local-and-assign shape; adds g->max_histogram = 1 to invalidate_luminance_cache
7ef8429158	2020-02-17	"fix race condition in computing histogram": moves the max search to its own loop, opening it with *max_histogram = 0;
354a4000fc	2021-09-14	merges histogram and deciles into compute_log_histogram_and_stats(), carrying *max_histogram = 0; over
22fbe0d8fe	2021-09-20	"change buttons icons and optimizations": folds the max search back into the remap loop as a ternary and drops *max_histogram = 0; in the same hunk

The hunk in 22fbe0d8fe:

-  *max_histogram = 0;
   // remap the extended histogram into the normal one
   // bins between [-8; 0] EV remapped between [0 ; UI_SAMPLES]
   for(size_t k = 0; k < TEMP_SAMPLES; ++k)
   {
     ...
     // store the max numbers of elements in bins for later normalization
-    if(histogram[i] > *max_histogram)
-      *max_histogram = histogram[i];
+    *max_histogram = histogram[i] > *max_histogram ? histogram[i] : *max_histogram;
   }

Nothing in that commit's message or diff suggests the change of contract was intended; it
reads as a mechanical rewrite of the comparison. The callers were not adjusted, and both
pipe-side clears already existed: :1130 since dce60377a8 (2019-09-06) and :1042
since e22b5b89ab (2020-11-02). The g->max_histogram = 1 at :631 was belt and braces
when it was written and has been load-bearing since.

This is not a locking bug

Worth stating, because the finding comes out of a lock-discipline sweep and row 53 of
that sweep is a confirmed false positive. max_histogram is written only from GTK
threads: :631 and :1346 under the module GUI lock, and :1469 from
update_histogram(), which holds it across :1478-1488. The pipe worker writes
histogram_valid (:1042, :1130, both inside critical sections) and never touches
the divisor. The unlocked read at :2648 is on the same thread as the only write that
can reach it, area_draw() having just called update_histogram() at :2616. There is
no race here, only a missing reset. The module has no process_cl(), so there is no
second pipe path to keep in sync.

Suggested fix

Restore the initialisation in the producer, which repairs every caller at once and puts
the contract back where it was until 2021:

   // remap the extended histogram into the normal one
   // bins between [-8; 0] EV remapped between [0 ; UI_SAMPLES]
+  *max_histogram = 1;
+
   for(size_t k = 0; k < TEMP_SAMPLES; ++k)

1 rather than the historical 0 keeps the divisor at :2648 non-zero by construction,
which is the invariant the two g->max_histogram = 1 sites already assert; the result is
identical whenever a histogram exists, since at least one bin holds at least one pixel
(toneeq_process:1025 rejects a zero-sized ROI before any of this).

The alternative -- adding g->max_histogram = 1; beside the clears at :1042 and
:1130 -- also works, but it leaves an out-parameter that silently accumulates, so the
next caller added has to know to reset it first. It would also have to be repeated at
:1819 and :1881 to be complete.

Either way the resets at :631 and :1346 should stay: they cover the window between
gui_init() and the first histogram.

Related reports

• #22068, #22091, #22121, #22133 and #22136 are the other toneequal
  reports in the corpus. The first four are all luminance_valid / gui_lock /
  commit_params() interactions; #22136 is the graph geometry cache. None reads or
  writes max_histogram, and none shares a fix with this one.
• Not checked against upstream -- no gh in this environment, so a pre-existing
  GitHub issue cannot be ruled out.
• If this is filed as a PR rather than an issue, it changes what a user sees and so needs
  a RELEASE_NOTES.md entry under Bug Fixes, per AGENTS.md.

Environment

Verified line by line against 3c73bf2aaa (branch dt-lockcheck; src/ matches the
2026-09-03 upstream rebase). Line numbers agree with row 53 of
discipline-gap-validated.md, which was written against 4d9e40f30e: :631, :1042,
:1130, :1346, :1406, :1469, :1483, :2616, :2648 are unchanged between the
two commits. Static analysis and git log only -- nothing here was built or run.

Start in src/iop/toneequal.c with compute_log_histogram_and_stats() and the pipe-side histogram invalidation sites at toneeq_process():1042 and :1130. Trace update_histogram() through area_draw() and verify that an upstream edit redraws the histogram using the current peak rather than a previous high-water mark. The issue notes that no test or build was run, so validate the change with the described preview-pipe scenario and a darktable build.