gui: fix flaky cursor indicators on GTK3 backends

[Arecsu]

Related to #21846

The cursor indicators in darkroom were flaky on some GTK3 backends. Several controls were changing one shared cursor, while widget-local cursors could also pin or clear the wrong window. The resize wrappers additionally kept their hover and drag state in shared globals.

This keeps the cursor close to the widget that owns the interaction:

• Panel handles, range selectors, and timeline endpoints use their own widget cursors.
• Resize wrappers keep hover and drag state per instance, so metadata separators cannot clear or retain another separator's cursor.
• Legacy view-level cursors stay on the toplevel window, where crop and the other darkroom tools already expect them.
• Widget-local cursors are cleared with NULL, so busy, help, and shortcut-mapping cursors can still inherit correctly.
• Tone and color equalizer cursor changes now go through the same GTK3/GTK4 compatibility helpers instead of writing to GDK windows directly.

The changes stay within the existing GTK3/GTK4 compatibility boundary and do not add platform-specific cursor workarounds.

I manually tested shortcut mapping, the histogram grab cursor, panel handles, metadata handles, range selectors, the timeline, crop cursor zones, busy/help cursor inheritance, and both equalizer cursor paths. These are now behaving consistently on the tested systems.

Two GTK3/macOS limitations are intentionally left outside this PR. Native window-frame resizing is handled by AppKit/GTK outside the normal widget hierarchy and can still leave a frame cursor stuck. Also, while actively dragging a metadata separator, GTK3 can still show the text-entry cursor when the pointer crosses an entry child window. The handle itself and the resize operation work; the child-window cursor issue was left alone rather than shipping recursive cursor overrides or a seat-grab workaround.

[wpferguson]

First test:

• clicked the shortcut key (keyboard at top left of center view in lighttable).
• cursor changed to keyboard
• cursor responded appropriately when mousing over things that could be assigned (keyboard) and things that couldn't (circle with slash through it)
• right clicked to exit mapping mode and cursor changed to circle with slash through it instead of back to arrow.
• couldn't get rid of circle with slash

Second test:

• in darkroom hover over the histogram and cursor doesn't change to hand

Third test

• started crop module
• hovered at right side and cursor change to horizontal move
• went to corner and cursor changed to diagonal move
• went to stop and cursor did not change, still diagonal
• went to center and cursor did not change to 4 way, stayed as diagonal
• moved cursor to center of image and circled and the cursor stepped through all the different cursors (vertical, both diagonals, horizontal and 4 way) in the center 10% of the image.

[Arecsu]

@kofa73 I need your skynet agent to dig further on this 🙏, it's gets difficult for me as I'm at the point where I don't have ways to reproduce some of them on my end. Good report @wpferguson by the way, it definitely helps

[kofa73]

@Arecsu I'll be able to provide feedback later this afternoon, currently over the limit.

[kofa73]

Sorry, I was on the master branch when I ran them. I think they picked up the right branch automatically, but I'll re-run the analysis just to be sure. Watch this space.

[Arecsu]

Nice! I was just about to dig into it, just finished reading the message. Thanks for the heads up
By the way, is the workflow capable of making a PR to my repo so I can incorporate whatever suggestions it could have to the same branch of this PR? In the shape of code commit I mean. That would be amazing, because usually the recommendations it gives are accurate and requires none to minimal changes

[kofa73]

The workflow is a strict code review, it does not support investigations. Right now, I'm manually coordinating Claude and Gemini. :-) I can surely have them generate patches and send to you via email, if that's OK.

[Arecsu]

Yes of course, patches is what I'm looking for more than anything, thanks! And of course the write up is useful in the same level already

[kofa73]

Updated based on this branch. It appears master has changes that will be relevant (@Christian-Bouhon , see issue 0: your changes bypassing cursor helpers).

---

@Arecsu @wpferguson — here is what the three test failures come down to, plus one thing
that will bite on rebase and one correction about which of them is actually a regression.

Short version: tests 1 and 2 are real regressions from this PR and share a single root
cause. Test 3 is not a regression at all — it is deliberate crop.c behaviour from 2023.
And there is a fourth problem that does not exist on this branch yet but will appear the
moment the branch is rebased.

Two different bases are referenced below, because the branch has drifted:

• branch = bf95fe448a (this PR's head), based on e4a02be705
• master = 216aa37105, which is 8 commits ahead of the branch point

---

0. Rebase hazard: tone equalizer and colour equalizer

This is the one I would look at first, because it is invisible from the branch.

8a26f35f92 ("IOP Color EQ: Add interactive editing mode for color adjustments in image",
#21397) landed on master after this branch was cut. It reworked toneequal.c and
colorequal.c, and in doing so introduced raw gdk_window_set_cursor() writes that
bypass the cursor helpers entirely:

// master, src/iop/toneequal.c, switch_cursors()
GtkWidget *widget = dt_ui_main_window(darktable.gui->ui);
...
  else if(g->cursor_valid)
  {
    dt_control_change_cursor("none");              // -> centre canvas after this PR
    ...
  }
  else if(!g->cursor_valid)
  {
    GdkCursor *const cursor =
      gdk_cursor_new_from_name(gdk_display_get_default(), "default");
    gdk_window_set_cursor(gtk_widget_get_window(widget), cursor);   // -> toplevel

On master those raw writes are at toneequal.c:1983, 2018, 2029, 2096 and
colorequal.c:2332, 2350; the matching dt_control_change_cursor("none") calls are at
toneequal.c:2005 and colorequal.c:2345.

On the branch as it stands this is harmless — grep -rn gdk_window_set_cursor src/ outside
src/gui/gtk.c returns nothing there, and switch_cursors() uses
dt_control_change_cursor() on every branch, so both halves agree. After a rebase they no
longer agree: the hide goes to the centre canvas and every restore goes to the toplevel, so
the mouse pointer stays invisible over the whole centre area whenever tone equalizer or
colour equalizer has focus and the pointer leaves the preview, or a mask is being edited.

Worth noting independently of this PR: those six raw writes are a step backwards from the
helper API this PR is trying to consolidate, and probably want cleaning up either way.

---

1. Root cause shared by tests 1 and 2: two cursor targets that never agree

The PR retargets dt_control_change_cursor() from the toplevel to the centre drawing area
(src/control/control.c:268-277 and :367 on the branch):

static GtkWidget *_cursor_target(void)
{
  if(!darktable.gui || !darktable.gui->ui) return NULL;
  GtkWidget *widget = dt_ui_center(darktable.gui->ui);
  return widget ? widget : dt_ui_main_window(darktable.gui->ui);
}

dt_ui_center() is the GtkDrawingArea created in darktable_gui_init()
(src/gui/gtk.c:2228, local cda). In GTK3 a GtkDrawingArea owns its own GdkWindow —
gtk_drawing_area_init() never calls gtk_widget_set_has_window(FALSE), so the
else branch of gtk_drawing_area_realize() runs and creates a real GDK_WINDOW_CHILD
(gtk-3.24.52/gtk/gtkdrawingarea.c).

Meanwhile dt_control_set_temp_cursor() / dt_control_clear_temp_cursor()
(control.c:327 and :343) were not changed and still hardcode dt_ui_main_window(), and
several call sites write the toplevel directly. So there are now two genuinely disjoint
cursor targets, and anything that sets on one and expects a later call to clear on the
other is broken.

Test 1 — mapping mode leaves "circle with slash" stuck

_set_mapping_mode_cursor() (global_toolbox.c:649) applies the cursor directly to the
main window at :682, bypassing dt_control_change_cursor(). The exit path
(_lib_keymap_button_clicked(), :814) clears it with the retargeted function at :826.
The right-click exit toggles the button off and lands in exactly that branch.

So "not-allowed" stays on the toplevel forever, and every widget without a cursor of its
own inherits it.

There is an extra twist that explains why it looked completely unshakeable rather than
"fixed only over the centre": in lighttable the thumbnail grid is not drawn on cda. It is
added as an overlay child of dt_ui_center_base() — a sibling stacked above cda with
its own GdkWindow (dt_thumbtable_set_parent(), src/dtgtk/thumbtable.c:3205-3209). The
pointer is therefore never over cda in lighttable, so the one rectangle the exit path does
reset is covered up.

Note the help button next to it (_lib_help_button_clicked()) is fine, because it uses
set_temp / clear_temp for both halves and those still both target the toplevel. That
asymmetry is the whole bug.

Test 2 — histogram no longer shows the hand cursor

src/libs/histogram.c is untouched by the PR and still calls the retargeted function at
:319, :333, :395 and :410. s->scope_draw lives in a side panel, not in cda's
subtree, so the "grab" cursor is now painted on a widget the pointer is nowhere near.

Trap in the obvious fix: s->scope_draw is itself created by dt_ui_resize_wrap()
(histogram.c:770), and this PR makes _resize_wrap_set_handle_hover() write
"ns-resize" / NULL to that same widget. Switching the four histogram calls to
dt_gui_cursor_set(s->scope_draw, …) would give one widget two independent cursor state
machines that clear each other. Whatever the fix is, it needs an explicit rule for who wins
in the bottom DT_RESIZE_HANDLE_SIZE pixels. d->preview_area
(src/libs/neural_restore.c:4492) is a second widget in the same situation.

---

2. Test 3 is not a regression — it is deliberate behaviour from 2023

moved cursor to center of image and circled and the cursor stepped through all the
different cursors (vertical, both diagonals, horizontal and 4 way) in the center 10% of
the image.

That "center 10%" is exact, and it falls straight out of _gui_get_grab()
(src/iop/crop.c:1484-1487):

float h_border = border / wd;
float v_border = border / ht;
if(!(g->clip_x || g->clip_y || g->clip_w != 1.0f || g->clip_h != 1.0f))
  h_border = v_border = 0.45;

When the crop box still covers the whole image — the state right after opening crop on an
uncropped picture — the grab borders widen to 0.45 of the image in each axis. With
clip_x = clip_y = 0, clip_w = clip_h = 1:

• GRAB_LEFT for x < 0.45, GRAB_RIGHT for x > 0.55
• GRAB_TOP for y < 0.45, GRAB_BOTTOM for y > 0.55

GRAB_CENTER is 0 (crop.c:72) and is the only value that reaches the else branch in
mouse_moved() that sets the 4-way "move" cursor (crop.c:1839). So the 4-way appears
only for x ∈ [0.45, 0.55] and y ∈ [0.45, 0.55] — the middle 10 % × 10 %. Circling in
that spot crosses the two horizontal zones, the two vertical zones, all four corner
combinations and the centre: precisely the reported sequence.

git log -L1484,1487:src/iop/crop.c dates this to f1a35d1e59 (2023-06-05), "when
uncropped&unmovable, implicitly expand crop zones". The intent is reasonable — with a
full-frame box there is nothing to move, so everything is made grabbable as a border — but
the cursor feedback reads as erratic.

The earlier steps in the same test fit too: right side → e-resize, corner → se-resize,
then moving up along x > 0.55 gives ne-resize, still a diagonal, which is easy to read as
"did not change".

This PR does not touch crop.c and does not change crop's cursor target, so it neither
causes nor worsens this. If the behaviour should change, that is a separate UX decision
about crop.c.

---

3. The original #21846 crop freeze — narrowed, still unproven

Separate from all of the above, and still the thing the issue was actually filed about:

sometimes you get the cursor indicators ... and other times you only get an arrow that
doesn't change no matter where you hover in the image. The crop module still works,
just not the indicators. This will persist for an image or two, and then start working
correctly again.

The only guard that can produce "indicators dead, crop still works" is the second half of
crop.c:1682:

if(!g->preview_ready || self->dev->preview_pipe->loading) return 0;

g->preview_ready can be excluded: button_pressed() (crop.c:1884) and
button_released() (crop.c:1858) bail on !g->preview_ready as well, so if that flag
were stuck the module would not work at all. dev->preview_pipe->loading gates only
mouse_moved() and _set_max_clip() (crop.c:1521) — button handling is unaffected. That
matches the report exactly, and loading being set for all three pipes on every image load
(develop.c:1007, :1079) matches "persists for an image or two".

So the working hypothesis is: dev->preview_pipe->loading is sometimes never cleared.
Candidate paths are the early returns in dt_dev_process_image_job() that leave it
untouched — notably develop.c:652, where a preview job bails out while the raw is still
loading and relies on being re-triggered later.

This is code reading, not runtime evidence. One dt_print at crop.c:1682 logging both
flags on every rejected mouse_moved() would settle it in a single session. The cursor side
is already instrumented — dt_control_cursor_debug() logs owner / target / temp / locked to
darktable -d input, which would be useful to have from @wpferguson alongside it.

Incidentally, the loading half of that guard is crop-only: clipping.c has the
preview_ready guards but not the loading one, and ashift.c has neither.

---

4. Three more things worth checking before merge

a. A cursor requested for the canvas can end up permanently on the toplevel.
_change_cursor_with_fallback() keeps its "a temp cursor is active, remember the request
for later" branch (control.c:309-315), and dt_control_clear_temp_cursor() then applies
_prev_cursor to dt_ui_main_window() (control.c:359). Before the PR both ends were the
toplevel. Now a cursor requested for the centre canvas is saved and later stamped onto the
toplevel — a crop se-resize can become the window-wide cursor.

b. The busy / wait cursor is probably now invisible where the user is looking.
dt_gui_cursor_set_busy() goes through dt_control_set_temp_cursor("wait"), hardcoded to
the toplevel. But after this PR the centre canvas nearly always carries an explicit cursor
of its own, because every clear path calls dt_control_change_cursor("default") rather than
clearing to NULL — and a window with its own cursor does not inherit its parent's. Same
for the view-switch wait cursor (views/view.c:307, cleared at :465). Not verified at
runtime, but it follows from the GTK3 inheritance rule.

A rule that would fix both: clearing a widget-local cursor means NULL, not "default".
NULL means "inherit from the parent" in GTK3 and in GTK4, so the canvas falls back to
whatever the toplevel shows, which is what busy / mapping / help modes want. This PR already
does it correctly in _resize_wrap_set_handle_hover(), and src/libs/neural_restore.c is
existing precedent. An explicit "default" on a widget then means something specific —
"deliberately override the toplevel here" — which is exactly how
src/libs/backgroundjobs.c:139 uses it.

c. _cursor_target() is main-window-only. The second darkroom window is a separate
toplevel with its own canvas. Nothing hits this today (all second-window cursor calls go
through dt_gui_cursor_set(dev->second_wnd, …), and that window is pan/zoom only), and it
is not a regression — on master those calls would have painted the main toplevel, which is
equally wrong. Worth a comment so it does not get inherited silently later.

---

What the PR gets right

The per-instance dt_resize_wrap_state_t genuinely fixes the original stuck-ns-resize
symptom. The shared _resize_wrap_dragging / _resize_wrap_handle_hover /
_resize_wrap_hovered statics plus the single-slot global _prev_cursor meant that moving
between two wrappers could leave a set_temp without its clear_temp; _prev_cursor then
stays non-NULL, which makes every subsequent dt_control_change_cursor() a no-op
(control.c:309). That is why @wpferguson's "hover the histogram and it resets" works — the
histogram is a dt_ui_resize_wrap (histogram.c:770), so entering it calls
dt_control_clear_temp_cursor() and unsticks the whole mechanism.

The direction is also right for GTK4 — per-widget cursors are GTK4's model. What seems to be
missing is a stated ownership rule so setters and clearers cannot drift apart again,
something like:

1. Toplevel — application-modal states only: busy / wait, shortcut mapping, help mode.
   Set and cleared through the temp mechanism.
2. Centre canvas — on-canvas interaction: crop, ashift, clipping, vignette, colorequal,
   toneequal, channelmixerrgb, views/darkroom.c, views/tethering.c, the print layout
   preview, dt_iop_request_focus().
3. Panel-local — the widget sets its own cursor: histogram, range select, timeline,
   panel handles, resize wrappers.
4. Clearing means NULL, never "default".

Also worth deciding explicitly which of the current dt_control_change_cursor() callers
genuinely mean "the whole application" and should keep the toplevel — views/view.c:307
and :465, common/darktable.c (whose own comment says it is a whole-window cursor-theming
workaround), and global_toolbox.c:826 look like the three.

[Arecsu]

Nice, thanks, and thanks for the email! Cursors work good on my end (macos) except window resize one as mentioned, and metadata grab handlers dont remain active while resizing. But wont stay stuck either, its a minor thing I guess. Couldnt solve that one, seems to be a profound gtk3 thing under macos as well... Needs further testing on other platforms