Skip to content

gtk4-prep: finish the event-controller migration and cleanup sweep - #21787

Draft
Arecsu wants to merge 25 commits into
darktable-org:masterfrom
Arecsu:gtk4-prep/rebase-master
Draft

gtk4-prep: finish the event-controller migration and cleanup sweep#21787
Arecsu wants to merge 25 commits into
darktable-org:masterfrom
Arecsu:gtk4-prep/rebase-master

Conversation

@Arecsu

@Arecsu Arecsu commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

I'm finding 2010 bugs and fixing them, it's incredible :)

This is the tail end of the event-controller migration. The part of the gtk4-prep series that moves the GTK3-only signal handlers (button-press / scroll / motion / key-press) onto the event controllers GTK4 ships.

It is not the end of gtk4-prep. We're still ways off: the GtkMenu→GtkPopover migration, and the remaining GTK3-only subsystems (open-URL, map-view DnD, the GDK-level keymap filter) are still ahead. And possibly other things I'm missing as well.

This branch finishes the controllers: dialogs' Enter key handling, menu-item press handlers as gestures, the lighttable center viewport pan/zoom/pinch, the panel-scroll emulation, the synthetic-event paths, and the last ~40 gtk_get_current_event() stragglers.

And because once you touch a hundred call sites you start noticing things that were copy-pasted, comments that were wrong, and occasionally a bug from 2010 that nobody ever read (!!) ((to be fair a work around has been used this whole time, but yeah, in my port to gtk4, it exploded in my face)), the same passes left the code cleaner than we found it:

  • One builder instead of two. The darkroom iop preset menu and the lib (modulegroups / header) preset menu were building the same ~80-line menu: query walk, writeprotect separators, hierarchy insertion, active highlight, edit/delete/store/update tail. They now share a single builder parameterized by a small ops struct; each caller keeps its own SQL and callbacks.
  • Named-argument factories. 74 dtgtk_button_new and 35 dtgtk_togglebutton_new sites went from the six-line "create → tooltip → action define → connect clicked" block to one constructor taking a config struct with C99 designated initializers. The config covers tooltip / markup / action / clicked / toggled; layout and styling calls (name, classes, sensitive) stay as explicit one-liners next to it, on purpose.
  • Every place where the migration forced a choice carries a comment: why the factory connects via g_signal_connect_data instead of the type-checking macro, why a gesture can't veto a menu close, why passing DT_ACTION(self) through dt_action_define covers the iop modules too. This should help other developers and AI systems to work with the code much faster

I've set this to Draft as I want to test this much more. Although the number of lines being changed is scary, it should not be as code-breaking as the first Event Controllers PR. Still, testing help would be GREATLY appreciated.

Related: #15920 #20433

@Arecsu Arecsu changed the title gtk4-prep: finish the event-controller migration and cleanup sweep (stacked on unmerged #21783 #21784 #21785, will rebase when they land) gtk4-prep: finish the event-controller migration and cleanup sweep Aug 10, 2026
@Arecsu

Arecsu commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

@kofa73 if you can do your dark magic on this, highly appreciated. Taking this into account: https://docs.gtk.org/gtk4/migrating-3to4.html and also comparing gtk3 and gtk4 actual source code of the toolkit to fact check the code. I think you already did this last time, just wanted to be sure. If possible to look for more opportunities when it comes to DRY patterns somewhere else in the UI code, memory to be freed up, etc, that would be neat. Thank you!

@kofa73

kofa73 commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Important (accepted)

  • src/bauhaus/bauhaus.c:755 — The new GTK4-only branches call gtk_widget_get_surface(), which is
    not public API in GTK 4.23, so those files cannot compile against GTK4.
    — ⚠ detail contested: severity (recorded medium; the panel did not converge on lowering it to
    low, see qualifiers)
    • Evidence:
      • src/bauhaus/bauhaus.c:755 — the GTK4-only outside-popup guard calls
        gtk_widget_get_surface(), which is absent from GTK4's public headers.
      • src/develop/imageop.c:2871 — the GTK4-only IOP header guard repeats the same unavailable call.
      • src/libs/lib.c:1043 — the GTK4-only lib header guard repeats the same unavailable call.
      • src/develop/imageop.c:2871 — the documented replacement
        gtk_native_get_surface(gtk_widget_get_native(w)) returns the widget's toplevel surface, which
        is also what gdk_event_get_surface() returns for events delivered to that toplevel, so the
        rewritten guard would compare a surface with itself and never trigger. Impact: an API-correct
        port of this branch still would not reproduce the GTK3 "ignore clicks on header buttons" test
        at src/develop/imageop.c:2874.
      • src/develop/imageop.c:2871git blame attributes all three GTK4-only branches
        (bauhaus.c:755, imageop.c:2871, lib.c:1043) to commit 1e299f89bbc inside the reviewed
        range, while their GTK3 counterparts pre-date it, so the unavailable call is introduced by this
        change rather than inherited.
      • src/CMakeLists.txt:855 — normal (non-source-package) builds add -Werror, making the
        undeclared GTK4 call a fatal diagnostic.
    • Qualifiers raised during debate (these are what the severity dispute is about):
      • gtk_widget_get_surface() exists in GTK 4.23.3 only in gtk/gtkwidgetprivate.h:234 (not
        installed; gtk/meson.build installs gtkwidget.h), and migrating-3to4.md:1185 documents its
        removal; no local shim defines it.
      • src/CMakeLists.txt:312 does find_package(GTK3 3.24.15 REQUIRED) and there is no gtk+-4.0
        configure path, so no currently buildable configuration compiles those branches — the impact
        is a future compile error, not present runtime behaviour. This is the argument for low; the
        panel did not unanimously adopt it, so the recorded severity stays medium and the
        disagreement is handed to you.

Minor (accepted)

  • src/views/map.c:2070 — The converted map-view and histogram scroll handlers leak the owned
    GdkEvent copy returned by dt_gui_get_current_event() on every matching scroll under GTK3.

    • Evidence:
      • src/gui/gtk.h:294dt_gui_get_current_event() documents that its GTK3 result is an owned
        gtk_get_current_event() copy which callers must free.
      • src/views/map.c:2070_view_map_scroll_cb() takes an owned event copy, but the
        gdk_event_free(event) the pre-change code had after gdk_event_get_state() was deleted and
        not replaced by a #if !GTK_CHECK_VERSION(4,0,0)-guarded free; no other exit path frees it.
        Precondition: every scroll over the map view under GTK3. Impact: one leaked GdkEvent (plus its
        GdkWindow/GdkDevice references) per scroll event.
      • src/views/map.c:2070 — the callback has no GTK3 free on its normal path or on any later early
        return; repeated map scrolling accumulates event copies and their referenced objects.
      • src/libs/histogram.c:485 — the histogram callback acquires its own owned copy before the
        Shift+Alt forwarding branch.
      • src/libs/histogram.c:493_eventbox_scroll_callback() returns immediately after
        dt_gui_forward_scroll(self, s->scope_draw) with the comment "dt_gui_forward_scroll() already
        freed the event", but that refers to a different copy, so the caller's own copy taken at
        src/libs/histogram.c:485 is leaked. Precondition: Shift+Alt scroll over the scope event box
        (the resize path).
      • src/gui/gtk.c:4894dt_gui_forward_scroll() acquires and frees a separate current-event
        copy, so returning at src/libs/histogram.c:494 leaves the histogram callback's original copy
        leaked.
    • Note: recorded as low/performance; one seat argued medium/correctness at Round 0. The
      panel converged on the current values in debate, so this is not flagged as contested.
  • src/gui/gtk.c:4277 — The double-click-on-tab "reset all widgets on this page" feature is dead
    under GTK3 because the new BUBBLE-phase click gesture on a GtkNotebook is never dispatched.

    • Evidence:
      • src/gui/gtk.c:4277dt_ui_notebook_page() now wires the reset via
        dt_gui_connect_click(GTK_WIDGET(notebook), _notebook_button_press_callback, NULL, NULL);
        dt_gui_connect_click() leaves the gesture at the default GTK_PHASE_BUBBLE
        (gtk_event_controller_init sets priv->phase = GTK_PHASE_BUBBLE,
        gtk-3.24.52/gtk/gtkeventcontroller.c:196). Impact: the callback
        _notebook_button_press_callback (src/gui/gtk.c:4236) never runs.
      • src/gui/gtk.c:4236 — on GTK3, BUBBLE-phase controllers are only run for button events from
        gtk_widget_real_button_event() (gtk-3.24.52/gtk/gtkwidget.c:7211), the GtkWidget class
        default handler, while GtkNotebook installs its own
        widget_class->button_press_event = gtk_notebook_button_press
        (gtk-3.24.52/gtk/gtknotebook.c:723) which never chains up to the parent handler.
        Precondition: any GtkNotebook created by dt_ui_notebook_new()/dt_ui_notebook_page().
      • src/gui/gtk.c:4277 — the replaced code connected the handler to the notebook's
        button-press-event signal, which runs before the class handler and did receive
        GDK_2BUTTON_PRESS, so double-clicking a module tab did reset that page's bauhaus widgets before
        this change. Impact: only the shortcut path (_action_process_tabs, DT_ACTION_EFFECT_RESET,
        src/gui/gtk.c:4138) still reaches _reset_all_bauhaus().
      • src/gui/gtk.c:5267dt_gui_connect_motion() explicitly sets GTK_PHASE_TARGET, which GTK3
        dispatches from gtk_widget_event_internal() regardless of any class-handler override, while
        dt_gui_connect_click() leaves the default BUBBLE phase. Impact: the notebook motion callback
        wired on the same widget still fires, which isolates the propagation phase as the cause and as
        the smallest correction.
  • src/gui/accelerators.c:280 — Shortcut effects "on", "off", "ctrl-on", "right-toggle" and
    "right-on" became silent no-ops for every toggle button that has no stored
    DT_ACTION_GESTURE_KEY gesture.

    • Evidence:
      • src/gui/accelerators.c:280 — the no-gesture branch now activates only for
        DT_ACTION_EFFECT_TOGGLE (0) and DT_ACTION_EFFECT_TOGGLE_CTRL (3); ON=1, OFF=2,
        ON_CTRL=4, TOGGLE_RIGHT=5, ON_RIGHT=6 (src/common/action.h:98-104) fall through and do
        nothing. Impact: the shortcut runs but the button state does not change.
      • src/gui/accelerators.c:280 — the base revision c19e88f8 ran
        g_signal_emit_by_name(target, "button-press-event", event, &handled); if(!handled) gtk_button_clicked(GTK_BUTTON(target));
        for all of these effects; since the converted widgets no longer have any button-press-event
        handler, handled was FALSE and gtk_button_clicked() flipped the toggle. Impact: a behavioural
        regression relative to the base commit, not merely a GTK4-prep no-op.
      • src/gui/accelerators.c:151dt_action_effect_toggle[] offers
        "on"/"off"/"ctrl-on"/"right-toggle"/"right-on" as user-selectable effects in the shortcuts
        dialog, and _action_fallbacks_toggle (src/gui/accelerators.c:392) maps a right-button or
        long-press shortcut to DT_ACTION_EFFECT_TOGGLE_RIGHT. Precondition: only widgets without
        DT_ACTION_GESTURE_KEY are affected — i.e. everything except the ashift, masks-shape and
        dt_iop_togglebutton_new buttons (e.g. overexposed, gamut check, softproof, colour assessment,
        high quality processing, modulegroups tabs, global toolbox help/shortcuts).
      • src/gui/accelerators.c:323 — the sibling _action_process_button() has the same gap for plain
        buttons: only DT_ACTION_EFFECT_ACTIVATE and ACTIVATE_CTRL call gtk_widget_activate(), while
        the base delivered ACTIVATE_RIGHT as a synthetic secondary button-press via
        gtk_widget_event(), which reached the then-still-connected button-press-event handlers.
        Impact: the "right-click" shortcut effect on buttons without a stored gesture is also a silent
        no-op now.

Rejected (raised then dropped)

  • src/gui/gtk.c:4221 — "On GTK3, modifier-scroll can no longer switch notebook tabs when sidebar
    scrolling is the default because the event is gated twice after its modifier is cleared."
    • Raised because: the shared scroll proxy calls dt_gui_ignore_scroll() for controls inside a side
      panel (src/gui/gtk.c:5316); that gate clears sidebar_scroll_mask from the mutable event
      (src/gui/gtk.c:481); the notebook callback then gates the same event again (src/gui/gtk.c:4221),
      would see no modifier, and return before changing pages.
    • Dropped because: the second gate never sees the cleared modifier — it reads a different event
      copy.
      dt_gui_ignore_scroll() (src/gui/gtk.c:473-485) has exactly one caller, _scroll_sidebar
      (src/gui/gtk.c:5316), and it clears the mask only in the owned copy that _scroll_proxy_real
      obtained via dt_gui_get_current_event() (src/gui/gtk.c:5361; the GTK3 branch of
      src/gui/gtk.h:297 is gtk_get_current_event(), which returns a gdk_event_copy() per
      gtk-3.24.52/gtk/gtkmain.c:2469-2476) and frees at src/gui/gtk.c:5405. The notebook gate
      dt_gui_ignore_scroll_controller (src/gui/gtk.c:487-491) instead calls
      dt_gui_get_current_event_state(), which takes a fresh gtk_get_current_event() copy at
      src/gui/gtk.c:499 that still carries the mask; _dt_gui_ignore_scroll then returns
      !sidebar_scroll_default = FALSE (src/gui/gtk.c:468-469), so _notebook_scroll_callback falls
      through to _action_process_tabs and modifier-scroll still switches pages.
    • Raised by: Codex · independent Round-0 support: 1. It survived one round on a split vote, then was
      rejected unanimously in round 2 (including by the seat that raised it).

Process notes

  • Notable field mutations / evidence added during debate:
    • gtk_widget_get_surface() issue: gained the "the documented gtk_native_get_surface()
      replacement would compare a surface with itself" point, the git blame attribution to commit
      1e299f89bbc inside the reviewed range, and the -Werror point. Severity stayed medium but is
      marked detail-contested — the panel's later reasoning argues low on the grounds that no
      configure path builds against GTK4 today, and that reasoning was not unanimously adopted.
    • Notebook double-click issue: gained the dt_gui_connect_motion() / GTK_PHASE_TARGET contrast,
      which both isolates the cause and points at the smallest fix.
    • Toggle-shortcut issue: gained a second affected site_action_process_button()
      (src/gui/accelerators.c:323) has the same gap, so the "right-click" effect on plain buttons is
      also a silent no-op.
    • Notebook modifier-scroll issue: accumulated four independent counter-analyses and moved
      open → rejected.
  • Author guidance coverage: the seats worked against the GTK 3.24.52 and 4.23.3 trees and the 3→4
    migration guide as requested. On the second aim (further DRY / memory-freeing / non-idiomatic-code
    opportunities), no seat raised a new finding in either debate round beyond the event-copy leak
    above — that is the panel's result, not evidence that none exist; the panel was directed at defects,
    and pure refactoring opportunities are outside the finding contract (a finding must describe
    something wrong).

Arecsu added 24 commits August 10, 2026 15:41
…ick popups

The right-click-popup idiom (dt_gui_connect_click + manual
gtk_gesture_single_set_button(..., GDK_BUTTON_SECONDARY)) was repeated by
hand at every call site.  Add the one-line wrapper encoding the intent, and
convert the four modulegroups popup connections to it.  Since the gesture's
button filter already restricts to the right button, the redundant
get_current_button() guards in the four popup handlers go away.
dt_handle_dialog_enter was connected as a classic key-press-event handler
(GtkWidget*, GdkEventKey*) at seven call sites.  Switch it to the
controller signature (GtkEventControllerKey*, keyval, keycode, state) and
wire all dialogs through dt_gui_connect_key, which already bridges to the
GTK4 key-pressed signal.  The dialog widget is resolved via
gtk_event_controller_get_widget() instead of being passed in.
src/gui/gtk.c:
- side panel empty box right-click -> dt_gui_connect_click_secondary
  (the handler's own button check was the whole point of the wrapper)
- log/toast message press-to-hide -> click controllers; the two
  identical handlers collapse into one
- outer border press -> click controller
- collapsible section header press -> click controller (primary check
  via gtk_gesture_single_get_current_button)
- panel resize handles (left/right/bottom): the duplicated
  press/release/motion/enter/leave blocks become one click + one motion
  controller per handle; double-click hide is n_press == 2; the
  gtk_widget_set_events masks are no longer needed

src/bauhaus/bauhaus.c:
- popup key-press-event wrapper deleted; dt_gui_connect_key now connects
  the controller-style _popup_key_press directly (the wrapper was a
  leftover from the earlier migration)
Add dt_gui_get_current_event_state(): modifier state for controller
callbacks, from gtk_event_controller_get_current_event_state() on GTK4 and
the dispatched event on GTK3 (which lacks the controller accessor).

Split the shared ignore_scroll decision out of dt_gui_ignore_scroll() and
expose it for controllers: the GdkEvent flavor keeps its modifier-consuming
side effect (gradientslider still uses it), the controller flavor reads the
state without mutation.

src/gui/gtk.c notebook: motion/scroll/button-press signals replaced by
motion/scroll/click controllers.  The scroll controller uses the DISCRETE
flag, whose per-controller accumulation replaces
dt_gui_get_scroll_unit_deltas()'s global static accumulator; the
gtk_get_event_widget() target gates disappear because TARGET-phase
controllers fire only for the widget itself (not tab labels), and the
double-click reset becomes n_press == 2.

src/bauhaus/bauhaus.c: popup window motion-notify-event replaced by a
motion controller, using dt_gui_get_current_root_coords() and the new
state helper.
The module preset menu (gui/presets.c), lib preset menu (libs/lib.c) and
style menus (dtgtk/stylemenu.c + export/print_settings/darkroom callers)
all connected classic button-press/release/motion-event handlers whose
return values carried meaning (apply-on-press, keep the menu open).

Gestures do not consume mouse events (verified in gtkgesture.c: only
touch sequences claimed in CAPTURE phase are consumed), so the menu shell
still activates items and closes the menu normally:

- module presets: a click gesture applies the preset on press (n_press == 1
  replaces the _click_time > event_time double-click gate) and handles the
  secondary duplicate/copy-lua on release.  The old 'keep menu open on long
  press' return value has no controller equivalent and is dropped.
- lib presets: the secondary-only gesture runs in CAPTURE phase and claims
  the sequence (new shared dt_gui_gesture_claim helper, same pattern as the
  picker buttons) so the shell never sees the press or release - the menu
  stays open after copying the preset as lua, exactly like before.  Primary
  clicks still apply via the activate signal.
- style menus: the button callbacks never vetoed (always returned FALSE),
  so the conversion is lossless; the closure-notify data ownership is kept
  by connecting the gesture's pressed signal with g_signal_connect_data.
Phase 6 of the event-controller migration: no GdkEvent synthesis remains.

- _action_process_toggle/_action_process_button: the non-gesture fallback
  no longer builds GdkButtonPress/Release events.  A plain GTK button's
  internal press gesture is primary-button-only, so the primary/ctrl
  variants reduce to gtk_widget_activate() and the right variants were
  no-ops anyway (a secondary press is ignored by the button); the widget's
  own secondary-click gesture is not reachable from keyboard shortcuts in
  GTK4 and stays a no-op.
- dt_gui_simulate_button_event deleted; the colorpicker lib sample copy
  (point/box) now calls the picker activation entry directly.
- temperature.c 'spot' preset calls dt_iop_color_picker_toggle() instead
  of emitting button-press-event on the picker; the former static
  _color_picker_widget_toggle is now public and shared by clicks,
  shortcuts and programmatic activation.
- Menu activate handlers distinguish keyboard activation without
  gtk_get_current_event(): press gestures mark the item
  (dt_gui_menuitem_mark_pressed) and the activate handlers skip
  release-time mouse activation via dt_gui_menuitem_activated_by_keyboard().
  Covers presets and the three style menus.
- Shortcuts-dialog Delete/BackSpace interceptor: GTK_PHASE_CAPTURE key
  controller on GTK4 (same before-the-focus-widget position), classic
  signal as the GTK3 bridge.
Phase 7: the last widget with old-style class-handler overrides
(enter/leave/button-press/button-release/motion/scroll/key-press vfuncs,
all GTK3-only) now uses the shared dt_gui_connect_* controller helpers:

- button press/release: click gesture; double-click reset via n_press == 2
  (replaces the GDK_2BUTTON_PRESS check); the gesture also distinguishes
  primary (select+drag) from secondary (toggle selection) directly.
- motion/enter/leave: motion controller; drag + marker hover unchanged.
- scroll: DISCRETE vertical scroll controller replaces the unit-delta
  accumulator; the dt_gui_ignore_scroll() check is dropped because the
  scroll proxy's _scroll_sidebar() already makes the panel-vs-control
  decision centrally; modifier state comes from the controller.
- keys: dt_gui_connect_key (GTK3 bridge keeps the classic signal).

Also swept the remaining non-gesture GDK_BUTTON_SECONDARY uses:
_gui_multiinstance_callback's GdkEventButton parameter was dead (both
callers pass NULL; the gesture already handles secondary/middle buttons),
so the event branches are removed.  All other GDK_BUTTON_SECONDARY uses
are gesture- or view-API-driven.
Phase 4: the center viewport's input handlers are now controller-based,
and touchpad pinch uses GtkGestureZoom everywhere (verified present in
both GTK 3.24.x and GTK4, handling 2-finger touchpad pinch in both):

- motion (moved/enter/leave), click (press/release), scroll: converted to
  dt_gui_connect_motion/click/scroll.  n_press replaces the
  GDK_2BUTTON_PRESS/3BUTTON_PRESS event-type distinction; pen pressure
  reads the controller's current event (new dt_gui_get_current_event
  helper: GTK4 borrowed / GTK3 owned); the released handler guards the
  synthetic cancel release (button 0).
- _scrolled became a GtkEventControllerScroll callback; it reads the raw
  event for the pan-vs-zoom dispatch (device/direction/stop/state/coords)
  exactly as before.  The toast message box that shared _scrolled is
  converted alongside.
- touchpad pinch: the old "event" signal handler (GTK3-only) is replaced
  by GtkGestureZoom begin/scale-changed/end ("end" also fires on cancel,
  so the view's END/CANCEL reset still runs; verified in gtkgesture.c).
  dx/dy come from the last event, not the zoom API; touchscreen pinches
  (recognized by GtkGestureZoom but never handled before) are ignored for
  parity.  GtkGestureSwipe records the touchpad device for the follow-up
  scroll-stream pan routing, replacing the old event switch.
- scroll helpers (dt_gui_get_scroll_deltas, _unit_deltas, _delta,
  _unit_delta, dt_gui_scroll_should_pan, dt_gui_scroll_zoom_delta) now
  take the opaque GdkEvent and work on GTK4 via gdk_scroll_event_*; all
  callers updated (culling, thumbtable, navigation, accelerators,
  darkroom second window).
- gdk_event_utils.h: gdk_event_get_state/source_device/scroll accessors
  and new touchpad-pinch accessors get GTK4 branches.
- culling.c _event_gesture and the darkroom second-window pinch (same
  pattern) converted to GtkGestureZoom as well.

GTK4 caveats (no public root-coords API in GTK4): pinch focal points fall
back to surface-relative positions; the consumers' gdk_window_get_origin()
conversion is GTK3-only and needs a coordinate decision on the GTK4 port.
configure-event / focus-in-out / delete-event connects and the
gtk_main_do_event-based shortcut-mapping dispatcher are separate GTK3-only
subsystems, tracked in TODO.md.
Phase 5: the documented scroll-emulation blockers get GTK4 branches,
with the GTK3 paths kept byte-identical.

- scroll proxy: _scroll_proxy_real and _scroll_sidebar now compile on
  GTK4 (controller current event; the panel-scroll decision is taken
  from the modifiers and the panel's adjustment is scrolled directly
  instead of forwarding the raw event via gtk_widget_event()).
- panel-center scroll gating: on GTK4 the panel's content box gets a
  BUBBLE scroll controller that implements the sidebar_scroll_mask /
  sidebar_scroll_default decision.  A child control with its own scroll
  handling claims the event at TARGET phase and never reaches it; the
  scrolled window's internal controller does the actual scrolling when
  the gate propagates.  The GTK3 signal handler is unchanged.
- border scroll: on GTK4 a scroll controller on the border scrolls the
  panel's adjustment directly with the same gate (GTK3 still forwards
  the raw event to the scrolled window).
- resize-wrap cluster: motion/button/enter/leave converted to click
  gesture + BUBBLE motion controller (the crossing detail/mode come
  from the controller's current event).  The scroll handlers get GTK4
  controller versions: _resize_wrap_scroll returns PROPAGATE when the
  inner scrolled window cannot move, replacing the GTK3
  gtk_propagate_event() manual pass-through; _scroll_wrap_height
  returns PROPAGATE so the enclosing scrolled window scrolls.  Both use
  the DISCRETE flag so GTK4 accumulates unit steps natively.
- bauhaus popup: the last leave-notify-event connect becomes a motion
  controller leave.

GTK4 caveats (need interactive validation): the GTK4 panel/border gate
semantics (capture/bubble ordering, GtkScrolledWindow's internal
controller) and the resize-wrap smooth-scroll passthrough were designed
against the GTK4 sources but not exercised; the gtk_main_do_event-based
shortcut-mapping dispatcher and configure-event/focus/delete connects
remain separate GTK3-only subsystems, tracked in TODO.md.
…ger scroll path

The scroll-controller conversion (739cdbe) left the early-return free in
the filemanager scroll branch unguarded, while Phase 6b guarded the other
free in the same function.  dt_gui_get_current_event() returns an owned copy
on GTK3 and a borrowed event on GTK4: freeing the borrowed event is a
use-after-free on the GTK4 port.

Related: darktable-org#15920 darktable-org#20433
The lighttable center viewport and the culling widget both converted the
old GTK3-only "event" signal handler (raw GDK_TOUCHPAD_PINCH forwarding)
into the same GtkGestureZoom begin/scale-changed/end triplet, with the same
touchscreen-pinch filter, touchpad_gestures_enabled pref check and active
flag -- ~60 lines of near-identical boilerplate duplicated (the TODO even
flagged culling as "the same pattern").

dt_gui_connect_pinch() now owns all of that: the gesture setup, the
per-gesture active tracking (g_object_set_data_full, so several pinch
gestures coexist without statics), the phase -> begin/update/end mapping
and the dx/dy, scale, state and focal-point parsing (root coords on GTK3,
surface-relative on GTK4).  Handlers only receive a parsed
dt_gui_pinch_event_t and forward it (e.g. to
dt_view_manager_gesture_pinch); the center viewport keeps its
_record_touchpad_device() call for the follow-up scroll pan routing.

Behavior is unchanged: touchscreen pinches are still ignored, "end" still
fires on cancel, and END arrives with no event data (deltas zero, scale 1).

Related: darktable-org#15920 darktable-org#20433
The alt+scroll channel-tab switching restored by bd7fc8c was copy-pasted
verbatim across four modules (atrous, colorzones, denoiseprofile,
rawdenoise): the same gtk_get_current_event() + gtk_widget_event() dance,
unguarded, with the same "GTK4: reimplement as a controller on the
notebook" note -- inconsistent with the Phase 6b convention of keeping
GTK3-only event consumption behind #if !GTK_CHECK_VERSION guards.

dt_gui_forward_scroll() in gtk.c now owns that forwarding (raw event via
gtk_widget_event() on GTK3, guarded; no-op stub on GTK4 with the migration
note), and the four scroll handlers are down to one call each, differing
only in the target notebook.  dt_bauhaus_widget_show_popup() gets the same
guard for its gtk_get_current_event() time stamp.

Behavior is unchanged on GTK3; on GTK4 the forwarding is explicitly a no-op
until the notebooks get their own scroll controllers.

Related: darktable-org#15920 darktable-org#20433
dt_lib_gui_get_expander()'s non-expandable branch (marked FIXME upstream)
re-connects module->presets_button's 'clicked' signal on every view switch
(view.c calls it for non-expandable libs in switch_to_view).  For the
modulegroups lib that button is the presets icon at the right of the group
tab bar, so each lighttable<->darkroom round trip stacked one more
connection: clicking it then popped the lib presets menu once per stacked
connection (growing with every view switch).

Guard the connection with a per-button data flag so it is made exactly
once, when the button is first shown outside its expander.
The old FIXME block in dt_lib_gui_get_expander()'s non-expandable branch
connected module->presets_button's "clicked" signal on every view switch
and never registered the button with the lib actions.  The previous commit
guarded the connect; this one addresses the FIXME itself:

- register the button via dt_action_define() (NULL label), matching the
  expandable header path, so shortcuts and the action fallback
  (DT_ACTION_ELEMENT_PRESETS in _action_process) can reach it;
- do both under one per-button data flag, since the widget -- unlike an
  expander -- survives view switches;
- replace the FIXME with a note that GtkButton::clicked is unchanged on
  GTK4 and dt_action_define() is qdata-guarded, so no
  #if GTK_CHECK_VERSION split is needed here.

One popup per click, no matter how many lighttable<->darkroom round
trips happen.
…d_scroll()

colorequal's area scroll and histogram's scope-eventbox scroll were the only
remaining hand-rolled gtk_widget_event()+free blocks with a #if !GTK4
split and a dead GTK4 stub.  Both now pick their target via
dt_gui_get_current_event_state() (0 on GTK4) and forward through the shared
helper, which owns the event/free and the GTK4 no-op stub.

histogram's early return keeps the event owned by exactly one consumer: the
helper frees it on the shift+alt path, the common tail still frees it for
the highlight/scope branches.
_dt_lib_presets_popup_menu_show (libs/lib.c) and
dt_gui_presets_popup_menu_show_for_module (gui/presets.c) built the same
menu with the same ~80-line skeleton: the query walk, writeprotect
separators, dt_insert_preset_in_menu_hierarchy() insertion, active-preset
highlight (with the _active_menu_item weak pointer and the menu-path walk)
and the edit/delete/store-new/update-preset tail.

Both now go through one builder, dt_gui_presets_popup_menu_show(),
parameterized by a small ops struct (query/bind, per-row evaluation and
wiring, the manage/edit/delete/store/update handlers and the optional
trailing prefs section).  The two callers keep their own SQL and callbacks
-- the darkroom variant filters by image (format flags, camera/lens/ISO/
exposure/focal-length) and checks default_params + blend params + enabled,
the lib variant matches the exact params/op_version and adds the manage
window and set_preferences hooks.

Behavior unchanged, plus two incidental fixes:
- the lib menu leaked menu_path/prev_split on every popup (the iop builder
  freed them, the lib one did not);
- the ops struct lives in gui/presets.h; sqlite3_stmt is now an explicit
  include there.
…ites

dtgtk_button_new_full() wraps the dominant plain-button idiom
(dtgtk_button_new + gtk_widget_set_tooltip_text + dt_action_define +
g_signal_connect "clicked") in a single constructor; any part may be
NULL.  It also covers the IOP modules: passing DT_ACTION(self) with a
section routes through dt_action_define()'s existing IOP-instance
dispatch, so the dt_action_define_iop() call sites (agx, channelmixerrgb)
collapse too.

37 of the 95 dtgtk_button_new sites are converted; the rest stay on the
base constructor because they need a press/release gesture, reuse the
action's return value (global_toolbox preferences, filters/colors
operator), have a #ifdef-gated tooltip (lut3d), or carry no tooltip/
action/click at all (splash clock, ashift auto-fit, watermark refresh,
the modulegroups editor icon pickers, filtering close buttons).

No behavior change: the factory calls the same three functions in the
same order the converted sites did (tooltip first, then action, then
clicked), so tooltip/action side effects (has-tooltip, hover-clear
motion) are unchanged.
The four right-click tab/basic-button menus (and the two editor popup
builders they share) all created a plain GtkMenu named
"modulegroups-popup" and popped it with the same anchors.  Factor that
into _manage_popup_new()/_manage_popup_show() so the css name and the
gravity pairing exist once and cannot drift between the builders.

The _manage_direct_module_popup popup still anchors at the pointer (it
pops the submenu _build_menu_from_actions() returned, not the wrapper),
so it only uses _manage_popup_new().
The seven optional wiring arguments of dtgtk_button_new_full() were
positional, so call sites read as runs of NULLs and the reader had to
count arguments to know which slot was set.  Replace them with a
dtgtk_button_config_t filled with C99 designated initializers -- the C
equivalent of named arguments: callers name the fields they use and
everything left out stays off.  The mandatory paint/flags/paintdata stay
positional, matching dtgtk_button_new().

All 69 call sites are converted mechanically; the field values are the
verbatim argument expressions from the previous positional form.
Four sites were left on dtgtk_button_new() for mechanical reasons that
turn out to be solvable:

- global_toolbox preferences and filters/colors operator reused the
  dt_action_define() return value; the factory defines the action on the
  button and dt_action_define() binds it as the widget "dt_action" qdata,
  so the callers now fetch it back with dt_action_widget() and the
  shortcut registration is unchanged.
- lut3d: the tooltip was selected by #ifdef HAVE_GMIC, so the string is
  now hoisted into a local before the factory call; the #ifdef no longer
  interleaves with widget construction.
- filtering sort close button: tooltip via the factory, the press
  gesture (dt_gui_connect_click_all) stays as the follow-up line.

The 21 sites left on the base constructor genuinely have nothing for the
factory to wire (no tooltip/action/click), need tooltip markup, or share
the button between branches.
The colorlabels quick-filter buttons show computed Pango markup, so the
config gets a .tooltip_markup sibling (plain text wins if both are set)
and the last plain-button site converts to the factory.  Like the other
return-value sites, its action is fetched back with dt_action_widget()
for the F1-F5 shortcut registration; each button's qdata is the same
action object dt_action_define() used to return.
The togglebutton sites repeat the same cluster as the buttons did
(tooltip + action define + "toggled"/"clicked" callback), so the config
struct gains .toggled_cb/.toggled_data and dtgtk_togglebutton_new_full()
mirrors dtgtk_button_new_full(); both callback fields are honored so the
toggles that use a plain click (darkroom quickbuttons, snapshots,
geotagging, global_toolbox) work too.

35 of 63 sites converted across 17 files: iop header on/off (imageop),
masks shape adders (6), modulegroups tabs (3), lighttable layout buttons
(6), darkroom quickbuttons (9), histogram modes, global_toolbox
help/shortcuts, plus basicadj/colorzones/rgblevels/geotagging/
neural_restore/snapshots/filtering-adjacent.  Action return values come
back through dt_action_widget() where shortcuts are registered; the
secondary-click gestures (modulegroups, masks) stay as follow-up lines.

The remaining 28 sites stay on the base constructor: no tooltip/action/
callback at all, "toggled" via g_signal_connect_data with a stored
handler id (map_locations, vectorscope, histogram), a query-tooltip
mechanism (imageop mask_indicator), conditional tooltips, or the
tagging.c NEW_TOGGLE_BUTTON macro which already wraps the cluster.
When the active preset is a shipped/writeprotect default, the tail logic
took the "active preset" branch but appended nothing (edit/delete are
hidden for writeprotect presets), leaving the separator that follows the
preset list with no items after it -- and no store/update items either.

The pre-merge iop builder fell through to "store new preset" / "update
preset" in that case; the shared builder inherited the lib version's
branch shape (and its FIXME).  Fold the writeprotect check into the
branch condition so a writeprotect active preset lands in the else
branch again, restoring the old iop menu and fixing the lib FIXME case.
@Arecsu
Arecsu force-pushed the gtk4-prep/rebase-master branch from 58dfb43 to d0709c0 Compare August 10, 2026 18:41
@Arecsu

Arecsu commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Thank you @kofa73!

Besides, what are the chances of applying for this? https://claude.com/contact-sales/claude-for-oss

I've been doing all this port and general bug-fixing with Deepseek v4 Flash out of my pocket. I didn't spend that much in terms of API cost. It's been more about my human hardwork than anything else to be honest but, if this could be sponsored in some way, it would be super great. We are close to get into compiling trials with GTK4.

CC to @masterpiga because I recall you having mentioned this claude-for-oss, or I might be completely off on this.

Also, now that we are on-topic. What would be the way to proceed with GTK4 trials, tests, etc? A new branch here in darktable-org, in my own fork, or where? Because that one will require some deep changes and dependency changes AFAIK when it lands.

Four accepted findings from the automated review, fact-checked against the
GTK 3.24.52 / 4.23.3 sources:

- gtk_widget_get_surface() is not public GTK4 API (gtkwidgetprivate.h only,
  migrating-3to4.md:1185 documents removal).  bauhaus.c popup guard is now
  GTK3-only (GTK4 has no gtk_grab_add, so foreign clicks never reach the
  gesture); imageop.c/lib.c header guards use a rethought GTK4 check --
  gtk_widget_pick() the release point and ignore releases on/inside buttons
  (the naive surface-compare would never fire).  Latent error today (no GTK4
  configure path), but a blind API swap would have silently dropped the
  button-ignoring behavior.

- map.c _view_map_scroll_cb leaked the owned GTK3 current-event copy on every
  scroll (the pre-change gdk_event_free was deleted by 86ae2b6, not
  guarded); restored as a GTK3-only free after x/y/state extraction.
  histogram.c _eventbox_scroll_callback leaked its copy on the Shift+Alt
  forward path (dt_gui_forward_scroll() frees only its own copy); freed
  before the return, misleading comment corrected.

- notebook double-click tab reset was dead under GTK3: GtkNotebook overrides
  button_press_event and never chains up to gtk_widget_real_button_event(),
  so the BUBBLE-phase click gesture never dispatched.  Set GTK_PHASE_TARGET
  (GTK3-only), which gtk_widget_event_internal() runs for the event widget.

- accelerators.c _action_process_toggle: the no-gesture branch only handled
  TOGGLE/TOGGLE_CTRL; ON/OFF/ON_CTRL/TOGGLE_RIGHT/ON_RIGHT silently did
  nothing.  All 7 effects now flip via gtk_widget_activate() (the
  DT_ACTION_TOGGLE_NEEDED gate already restricts ON/OFF to the needed state
  change).  ACTIVATE_RIGHT stays a no-op on plain buttons -- the
  button-press-event handlers the base revision reached are all converted to
  gestures now (comment corrected).  Right-button shortcuts on gesture-wired
  widgets are restored: dt_gui_connect_click_secondary() stores its gesture
  under DT_ACTION_GESTURE_KEY, modulegroups tab handlers flip on a shortcut
  primary press (dt_gui_current_button()) and open the popup on secondary,
  and the other secondary handlers guard on GDK_BUTTON_SECONDARY.
@kofa73

kofa73 commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

I tried applying for Anthropic and OpenAI/Codex (https://developers.openai.com/community/codex-for-oss), nothing. But maybe a PR is not the best place to discuss this. :)

I can run a separate check, though, to look for refactoring opportunities, as the review prompt specifically says to only report broken code. Let me start it.

@Arecsu

Arecsu commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Got it, thanks! Just let me know whenever you want where would be a proper way to discuss all this, if given the case. Also, beware, I've fixed the things your previous write up flagged, just to be sure. Awesome if it can search for refactoring opportunities

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants