bauhaus: the combobox changed-state check compares an entry index with an entry value

Found by coding agents via static analysis, not verified by executing code.
Date: 2026-09-12.
Validated against commit c20f5e356615432b3ff46eba91e7b91dbc0f3e4d, branch
dev-doc/gui-data-sharing-pr (level with master, git rev-list --count HEAD..master is 0).

Summary

A combobox stores two different numbers with similar names:

• d->active is the position of the selected entry in the entries array
• d->defpos is the data value of the default entry, not its position

The code that decides whether a combobox still shows its default compares the two
directly (src/bauhaus/bauhaus.c:1433 and src/bauhaus/bauhaus.c:2120):

b->combobox.entries->len && b->combobox.active != b->combobox.defpos

For an enum whose constants are not 0, 1, 2, ... in declaration order, position and
value differ, so the answer is wrong. The result is the "changed" highlight on notebook
tabs: it is shown on a tab whose controls are all at their defaults, or withheld from a
tab that was really changed.

A concrete case in the tree is channelmixerrgb's illuminant combobox on the CAT
notebook page. Its entry positions are 0..8, but the last entry,
DT_ILLUMINANT_CAMERA, has the value 10, and that is the default darktable picks for a
raw whose camera white balance was found. active is then 8 and defpos is 10, so the
CAT tab is marked as changed on every image load.

Suggested fix: convert before comparing, using the existing
dt_bauhaus_combobox_get_from_value().

Why the two numbers are not the same

defpos holds a value

dt_bauhaus_combobox_set_default() stores its argument unchanged
(src/bauhaus/bauhaus.c:1110-1114):

void dt_bauhaus_combobox_set_default(GtkWidget *widget, const int def)
{
  dt_bauhaus_widget_t *w = DT_BAUHAUS_WIDGET(widget);
  w->combobox.defpos = def;
}

Everything that stores into defpos stores a value, and everything else that reads it
reads it as a value:

• dt_bauhaus_combobox_add_full() takes the default from the first sensitive entry's
  data pointer, which is the entry's value, not its position
  (src/bauhaus/bauhaus.c:1911-1912)

• dt_bauhaus_combobox_from_params() passes the enum value taken from default_params
  (src/develop/imageop_gui.c:182):

  dt_bauhaus_combobox_set_default(combobox, *(int*)((uint8_t *)d + f->header.offset));
  

• resetting a widget resolves it through the value lookup
  (src/bauhaus/bauhaus.c:3696):

  case DT_BAUHAUS_COMBOBOX:
    dt_bauhaus_combobox_set_from_value(widget, w->combobox.defpos);
  

For comboboxes filled with plain dt_bauhaus_combobox_add() the distinction is
invisible, because that path assigns each entry a value equal to its position
(src/bauhaus/bauhaus.c:1903-1904). Enum comboboxes built from introspection are the
ones that differ: dt_bauhaus_combobox_add_introspection() stores the enum constant's
value as the entry data (src/bauhaus/bauhaus.c:1860-1866).

active holds a position

_combobox_set() clamps its argument against the entries array and uses it to index it
(src/bauhaus/bauhaus.c:2074 and src/bauhaus/bauhaus.c:2099-2101):

d->active = CLAMP(pos, -1, (int)d->entries->len - 1);
...
*e = GPOINTER_TO_INT(_combobox_entry(d, d->active)->data);

Where the wrong comparison is made

Two sites, both feeding the notebook tab highlight:

1. src/bauhaus/bauhaus.c:1431-1433, inside _highlight_changed_notebook_tab(), which
   scans the other widgets on the same page:

   else
     is_changed = b->combobox.entries->len
       && b->combobox.active != b->combobox.defpos;
   

2. src/bauhaus/bauhaus.c:2119-2120, in _combobox_set(), which reports the widget that
   was just set:

   _highlight_changed_notebook_tab(GTK_WIDGET(w),
                                   GINT_TO_POINTER(d->active != d->defpos));
   

The similar line at src/bauhaus/bauhaus.c:1473 is not part of this defect: it is in
_toggle_set(), where toggle.active and toggle.defpos are both gboolean
(src/bauhaus/bauhaus.c:95 and src/bauhaus/bauhaus.c:1787).

When it shows

_highlight_changed_notebook_tab() is reached both when the user changes a widget
(src/bauhaus/bauhaus.c:2119) and when the widgets are synced from the parameters,
which happens on every gui_update() (src/bauhaus/bauhaus.c:1566-1568):

if(notebook)
  gtk_container_foreach(GTK_CONTAINER(notebook),
                        _highlight_changed_notebook_tab, NULL);

So an affected module shows the wrong tab state from the moment the image is opened.

Worked example: channelmixerrgb, CAT page

dt_illuminant_t is declared out of value order and with a gap
(src/common/illuminants.h):

DT_ILLUMINANT_PIPE            = 0,  // $DESCRIPTION: "same as pipeline (D50)"
...
DT_ILLUMINANT_CUSTOM          = 7,  // $DESCRIPTION: "custom" ...
DT_ILLUMINANT_CAMERA          = 10, // $DESCRIPTION: "as shot in camera" ...
DT_ILLUMINANT_LAST,
DT_ILLUMINANT_DETECT_SURFACES = 8,
DT_ILLUMINANT_DETECT_EDGES    = 9,

The three constants without a $DESCRIPTION get an empty description from the generator
(tools/introspection/ast.pm:978-983, via ast::mark_for_translation() at
tools/introspection/ast.pm:55-67), so dt_bauhaus_combobox_add_introspection() skips
them (src/bauhaus/bauhaus.c:1860-1861). The combobox therefore holds nine entries, at
positions 0 to 8, and position 8 has the value 10.

The widget is built from the parameters on the CAT notebook page
(src/iop/channelmixerrgb.c:4433 and src/iop/channelmixerrgb.c:4469), and
reload_defaults() sets its default to DT_ILLUMINANT_CAMERA for a raw whose white
balance coefficients yield a usable illuminant (src/iop/channelmixerrgb.c:3877-3880,
then src/iop/channelmixerrgb.c:3896):

dt_bauhaus_combobox_set_default(g->illuminant, d->illuminant);

At default parameters the widget shows entry 8 while defpos is 10, so the check says
"changed" and the CAT tab carries the changed CSS class
(data/themes/darktable.css:1926-1933) although nothing was changed.

The opposite error follows from the same mismatch: whenever the user selects the entry
whose position happens to equal defpos, the widget is reported as unchanged even
though the parameter differs from its default.

A second, narrower site with the same root cause

_combobox_next_sensitive() also treats defpos as a position
(src/bauhaus/bauhaus.c:236-241):

if(new_pos == -1 && d->defpos != -1)
  cur = d->defpos;
else
  cur = new_pos + step;

cur is then used to index the entries array (src/bauhaus/bauhaus.c:245-247).

The branch is taken when the combobox has no entry selected, that is d->active == -1.
That state is reached when dt_bauhaus_combobox_set_from_value() finds no entry with the
requested value and cannot re-add one from introspection either
(src/bauhaus/bauhaus.c:2167-2186), or when a caller passes -1 to
dt_bauhaus_combobox_set(). Scrolling such a combobox then starts its search from the
entry at position defpos instead of the default entry. The while loop bounds the index
(src/bauhaus/bauhaus.c:245), so this is a wrong starting entry, not an out-of-bounds
read. It is a less visible symptom than the tab highlight, but it is the same confusion
and should be fixed with it.

Suggested fix

Resolve defpos to a position before comparing it, with the lookup that already exists
(src/bauhaus/bauhaus.c:2149-2163):

int dt_bauhaus_combobox_get_from_value(GtkWidget *widget, const int value)

A small static helper keeps the three call sites in step, for example:

// defpos holds the default entry's value, not its position: for an enum
// combobox built from introspection the two differ
static gboolean _combobox_is_changed(dt_bauhaus_widget_t *w)
{
  const dt_bauhaus_combobox_data_t *d = &w->combobox;
  return d->entries->len
    && d->active != dt_bauhaus_combobox_get_from_value(GTK_WIDGET(w), d->defpos);
}

used at src/bauhaus/bauhaus.c:1433 and src/bauhaus/bauhaus.c:2120, with
_combobox_next_sensitive() converted the same way.

Comparing in the other direction, value against value, also works, but it needs a guard
for active == -1 before _combobox_entry(d, d->active) is called, so the conversion to
a position is the smaller change.

Renaming the member from defpos to something like defval would be a real improvement,
since the present name is what invites the mistake, but it touches the slider and toggle
members that share the name and are genuinely positions or states, so it is a separate
decision.

Priority

minor or medium, depending on how the glitch is rated; the table cell is the
only judgement involved.

• How it shows up: every time. For an affected module the state is recomputed on
  every gui_update(), so it is deterministic, not timing-dependent.
• What it does: the wrong changed highlight on a notebook tab. Read as cosmetic
  ("a small or short-lived UI glitch") the cell gives minor. Read as malfunction
  ("a UI glitch that stays for a long time"; also "wrong values shown that the user acts
  on", since the highlight is what tells a user which tab they touched) the cell gives
  medium.

The highlight persists for the whole editing session rather than being short-lived, so
medium (malfunction x every time) is the better fit, with the caveat that no pixel
data, history or file content is affected.

Read the combobox state logic in src/bauhaus/bauhaus.c, especially _highlight_changed_notebook_tab(), _combobox_set(), _combobox_next_sensitive(), and dt_bauhaus_combobox_get_from_value(). Trace the channelmixerrgb illuminant combobox from src/iop/channelmixerrgb.c and src/common/illuminants.h. Done when default-state highlighting and sensitive-entry navigation use the default value correctly for non-sequential enum values.