diff --git a/.github/scripts/check-gio-vendor-drift.sh b/.github/scripts/check-gio-vendor-drift.sh index 73aa137..795e6b1 100755 --- a/.github/scripts/check-gio-vendor-drift.sh +++ b/.github/scripts/check-gio-vendor-drift.sh @@ -1,22 +1,25 @@ #!/usr/bin/env bash # Verify third_party/gio/ matches upstream Gio at the pinned version with -# our patch applied on top. +# our patches applied on top, in order. # # Catches two failure modes: -# 1) Someone hand-edited a vendored file without updating the .patch. -# 2) The .patch was changed but the vendor tree wasn't re-rolled. +# 1) Someone hand-edited a vendored file without updating a .patch. +# 2) A .patch was changed but the vendor tree wasn't re-rolled. # # Both produce silent drift that's invisible in code review until something # breaks at runtime. This script is wired into CI so PRs that introduce # drift fail loudly. # -# Pinned version + patch path live in the constants below — bump them +# Pinned version + patch list live in the constants below — bump them # together when you upgrade Gio. set -euo pipefail GIO_VERSION="v0.9.0" -PATCH_FILE="third_party/0001-gesture-refresh-PointerID-on-Press-and-Enter.patch" +PATCH_FILES=( + "third_party/0001-gesture-refresh-PointerID-on-Press-and-Enter.patch" + "third_party/0002-app-windows-stable-PointerID-for-mouse.patch" +) VENDOR_DIR="third_party/gio" PROXY_URL="https://proxy.golang.org/gioui.org/@v/${GIO_VERSION}.zip" @@ -28,14 +31,15 @@ if [ ! -d "$VENDOR_DIR" ]; then exit 1 fi -if [ ! -f "$PATCH_FILE" ]; then - echo "ERROR: $PATCH_FILE not found" - exit 1 -fi +for patch in "${PATCH_FILES[@]}"; do + if [ ! -f "$patch" ]; then + echo "ERROR: $patch not found" + exit 1 + fi +done -# Resolve paths to absolute up front so `cd` into the temp dir later doesn't -# break them. The vendor + patch live under $REPO_ROOT. -PATCH_FILE_ABS="$REPO_ROOT/$PATCH_FILE" +# Resolve vendor path to absolute up front so `cd` into the temp dir later +# doesn't break it. VENDOR_DIR_ABS="$REPO_ROOT/$VENDOR_DIR" TMPDIR="$(mktemp -d)" @@ -55,22 +59,26 @@ if [ ! -d "$UPSTREAM_DIR" ]; then exit 1 fi -echo "Applying $PATCH_FILE to a fresh upstream copy..." chmod -R u+w "$UPSTREAM_DIR" # cd into the upstream dir and apply from there. `git apply --directory=...` # rejects paths containing `@` on Windows (msys git treats them as remote # refs); the cd workaround is portable across all platforms CI runs on. -(cd "$UPSTREAM_DIR" && git apply "$PATCH_FILE_ABS") +# Patches must apply in array order — each one's context lines assume the +# previous patch has already landed. +for patch in "${PATCH_FILES[@]}"; do + echo "Applying $patch to a fresh upstream copy..." + (cd "$UPSTREAM_DIR" && git apply "$REPO_ROOT/$patch") +done -echo "Comparing $VENDOR_DIR against upstream + patch..." +echo "Comparing $VENDOR_DIR against upstream + patches..." DRIFT_FILE="$TMPDIR/drift.diff" if diff --recursive --brief "$VENDOR_DIR_ABS" "$UPSTREAM_DIR" >"$DRIFT_FILE"; then - echo "OK: $VENDOR_DIR matches upstream Gio $GIO_VERSION + $PATCH_FILE" + echo "OK: $VENDOR_DIR matches upstream Gio $GIO_VERSION + ${#PATCH_FILES[@]} patch(es)" exit 0 fi echo "" -echo "ERROR: $VENDOR_DIR has drifted from upstream Gio $GIO_VERSION + $PATCH_FILE" +echo "ERROR: $VENDOR_DIR has drifted from upstream Gio $GIO_VERSION + the patches in third_party/" echo "" echo "Differences (first 30 lines):" head -30 "$DRIFT_FILE" @@ -85,17 +93,19 @@ echo "" echo "----------------------------------------------------------------" echo "How to resolve:" echo "" -echo " - If you intentionally changed vendored files, regenerate the patch:" -echo " diff -u --label a/ --label b/ > $PATCH_FILE" +echo " - If you intentionally changed vendored files, regenerate the relevant patch:" +echo " diff -u --label a/ --label b/ > third_party/.patch" echo " then re-run this script to confirm the diff is empty." echo "" -echo " - If the patch was meant to change but the vendor wasn't re-rolled," +echo " - If a patch was meant to change but the vendor wasn't re-rolled," echo " rebuild the vendor tree:" echo " rm -rf $VENDOR_DIR" echo " curl -L $PROXY_URL -o /tmp/gio.zip" echo " unzip -q /tmp/gio.zip -d /tmp/gio-extract" echo " cp -r /tmp/gio-extract/gioui.org@$GIO_VERSION $VENDOR_DIR" echo " chmod -R u+w $VENDOR_DIR" -echo " git apply $PATCH_FILE" +for patch in "${PATCH_FILES[@]}"; do + echo " git apply $patch" +done echo "----------------------------------------------------------------" exit 1 diff --git a/README.md b/README.md index 838f931..1d0eba5 100644 --- a/README.md +++ b/README.md @@ -49,17 +49,29 @@ Workaround applied: - **Custom label/editor renderer** (`ui/widget/label.go`) with a 256-glyph buffer and integer pixel-grid snapping — all text must be rendered through `widget.LayoutLabel` / `widget.LayoutEditor`, never via Gio's `lbl.Layout(gtx)` directly -### Vendored Gio (Windows click reliability) +### Vendored Gio (Windows pointer reliability) -`third_party/gio/` holds a local fork of `gioui.org v0.9.0`. `go.mod` replaces the upstream import with this fork. The patch we apply on top is in [`third_party/0001-gesture-refresh-PointerID-on-Press-and-Enter.patch`](third_party/0001-gesture-refresh-PointerID-on-Press-and-Enter.patch). +`third_party/gio/` holds a local fork of `gioui.org v0.9.0`. `go.mod` replaces the upstream import with this fork. Two Windows-specific patches sit alongside it in `third_party/`: -**Upstream submission:** [lists.sr.ht/~eliasnaur/gio-patches/patches/69089](https://lists.sr.ht/~eliasnaur/gio-patches/patches/69089). Drop the fork once this lands in a Gio release. +#### 0001 — refresh PointerID on Press and Enter + +[`third_party/0001-gesture-refresh-PointerID-on-Press-and-Enter.patch`](third_party/0001-gesture-refresh-PointerID-on-Press-and-Enter.patch). Submitted upstream at [lists.sr.ht/~eliasnaur/gio-patches/patches/69089](https://lists.sr.ht/~eliasnaur/gio-patches/patches/69089). **Why:** upstream `gesture.Click` and `gesture.Hover` lock their internal `pid` field to the first `PointerID` they ever see — both refresh it only when `!c.hovered` / `!h.entered`. On Windows, Gio enables `EnableMouseInPointer(1)` (`app/os_windows.go`), which causes the OS to assign different `PointerID`s to the same physical mouse across focus changes, window leave/re-enter, and similar events. Once the gesture is "stuck" with a stale pid, every subsequent `Press` whose `PointerID` doesn't match is silently dropped (the `pid != e.PointerID` check breaks out before reaching `c.pressed = true` and the event return). Most visibly this made clicks no-op on multi-line override editors, where `widget.Editor`'s internal `clicker` (a `gesture.Click`) ate the press without positioning the caret. **Patch:** refresh `pid` on every `Enter` / `Press` so the gesture tracks whichever pointer is currently interacting. 14 lines added, 12 removed. No behavioural change on macOS/Linux. -**Removing the fork:** when the upstream patch is merged and released, drop the `replace` directive in [`go.mod`](go.mod), bump `gioui.org` to the release, delete `third_party/gio/`, both `.patch` files, the drift-check step in CI, and this section. +#### 0002 — stable PointerID for the mouse + +[`third_party/0002-app-windows-stable-PointerID-for-mouse.patch`](third_party/0002-app-windows-stable-PointerID-for-mouse.patch). Submitted upstream at [lists.sr.ht/~eliasnaur/gio-patches/patches/69233](https://lists.sr.ht/~eliasnaur/gio-patches/patches/69233). + +**Why:** the Windows backend reported two different `PointerID`s for the same physical mouse — `scrollEvent` omitted `PointerID` from its `pointer.Event` so it defaulted to 0, while `pointerUpdate` forwarded whatever ID Windows assigned (typically 1+). Gio's `pointerQueue` keys `state.pointers` by `PointerID`, so a single mouse produced two persistent entries. The phantom (the scroll-derived one) accumulated `entered` tags it never released — subsequent moves went to the other entry's id, so no `Leave` events ever cleared it — and ran hit-test every frame at the user's last scroll-wheel position. Because `state.cursor` is threaded through every pointer in `pointerQueue.Frame`'s loop, the phantom's hit-test overwrote the live pointer's cursor resolution every frame: hovering an anchor badge correctly resolved `CursorPointer`, then the phantom (sitting over an editor cell) overwrote it with `CursorText` for the same frame. Same mechanism produced the "two rows hovered at once" symptom — phantom pointer's `entered` tags never released. `EnableMouseInPointer` PID drift across focus events compounded the problem with additional phantom entries. + +**Patch:** normalise all `Source=Mouse` events to a single stable `PointerID` (`^pointer.ID(0)`) at the platform layer so `pointerQueue` always sees one entry per physical mouse. Touch keeps Windows-assigned IDs because multi-touch needs distinct pointers per finger. The chosen sentinel is max-uint so it cannot collide with Windows-assigned touch finger IDs. + +#### Removing the fork + +When the upstream patches are merged and released, drop the `replace` directive in [`go.mod`](go.mod), bump `gioui.org` to the release, delete `third_party/gio/`, both `.patch` files, the drift-check step in CI, and this section. ## License diff --git a/third_party/0002-app-windows-stable-PointerID-for-mouse.patch b/third_party/0002-app-windows-stable-PointerID-for-mouse.patch new file mode 100644 index 0000000..20adf0e --- /dev/null +++ b/third_party/0002-app-windows-stable-PointerID-for-mouse.patch @@ -0,0 +1,98 @@ +From 30e3830910a093972d4e848e695414218cf50926 Mon Sep 17 00:00:00 2001 +From: Eugene +Date: Sun, 3 May 2026 00:40:20 +0300 +Subject: [PATCH] app/windows: stable PointerID for the mouse +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +scrollEvent omitted PointerID from its pointer.Event so it defaulted +to 0, while pointerUpdate forwarded whatever ID Windows assigned via +getPointerIDwParam (typically 1+). pointerQueue keys state.pointers +by PointerID, so the same physical mouse produced two persistent +entries — one for scroll, one for move/press/release. + +The scroll-derived entry never received a Leave (subsequent moves +went to the other entry's id), so its entered set never emptied, +the entry was never removed, and pointerQueue.Frame ran hit-test +for it every frame at the user's last scroll-wheel position. +Because state.cursor is threaded through every pointer in +pointerQueue.Frame's loop, the stale entry's hit-test overwrote +the live pointer's cursor resolution: hovering a button correctly +resolved CursorPointer for the live mouse, then the phantom +(sitting over an unrelated text region) overwrote it with +CursorText for the same frame. The user saw the wrong cursor and +gestures (gesture.Hover in particular) appeared "stuck" with +multiple regions hovered at once — both symptoms are the phantom +pointer carrying entered tags it could not release. + +EnableMouseInPointer also reassigns the mouse's PointerID across +focus / window-leave events, which can produce additional phantom +entries; normalising here absorbs that drift too. + +Normalise all Source=Mouse events to a single stable PointerID at +the platform layer so pointerQueue sees one entry per physical +mouse. Touch events keep their Windows-assigned IDs because +multi-touch needs distinct pointers per finger. The stable ID is +^pointer.ID(0) so it cannot collide with Windows-assigned touch +finger IDs (which start at small values). + +Also add a characterization test in io/input documenting the +"one entry per PointerID" contract that motivates the platform- +side coalescing. The test cannot fail without this commit — it +queues events directly into Router, bypassing the Windows code +path — but it locks in the queue behaviour: if a future change +makes pointerQueue coalesce same-Source pointers itself, this +test will flip and the platform-side coalescing can be removed. + +Signed-off-by: Eugene +--- + app/os_windows.go | 18 ++++++++++++++++++ + 1 file changed, 18 insertions(+) + +diff --git a/app/os_windows.go b/app/os_windows.go +index 45bfa31..b55c4b0 100644 +--- a/app/os_windows.go ++++ b/app/os_windows.go +@@ -531,6 +531,22 @@ func (w *window) hitTest(x, y int) uintptr { + return windows.HTCLIENT + } + ++// mouseStablePID is the PointerID reported for every Source=Mouse event so ++// pointerQueue only ever holds a single entry for the physical mouse. ++// Two paths in this file used to produce different IDs for the same mouse: ++// scrollEvent omitted PointerID and let it default to 0, while pointerUpdate ++// forwarded whatever ID Windows assigned via getPointerIDwParam (typically ++// 1+). pointerQueue keys state.pointers by PointerID, so the same physical ++// device produced two persistent entries; the scroll-derived one ++// accumulated entered tags it never released and its stale-position ++// hit-test overwrote the live pointer's cursor resolution every frame. ++// EnableMouseInPointer also reassigns the mouse's PointerID across focus / ++// window-leave events; normalising here absorbs that drift too. Touch ++// keeps Windows-assigned IDs because multi-touch needs distinct pointers. ++// The sentinel is max-uint so it cannot collide with Windows-assigned ++// touch finger IDs (which start at small values). ++var mouseStablePID = ^pointer.ID(0) ++ + func (w *window) pointerUpdate(pi windows.PointerInfo, pid pointer.ID, kind pointer.Kind, lParam uintptr) { + if !w.config.Focused { + windows.SetFocus(w.hwnd) +@@ -539,6 +555,7 @@ func (w *window) pointerUpdate(pi windows.PointerInfo, pid pointer.ID, kind poin + src := pointer.Touch + if pi.PointerType == windows.PT_MOUSE { + src = pointer.Mouse ++ pid = mouseStablePID + } + + x, y := coordsFromlParam(lParam) +@@ -590,6 +607,7 @@ func (w *window) scrollEvent(wParam, lParam uintptr, horizontal bool, kmods key. + w.ProcessEvent(pointer.Event{ + Kind: pointer.Scroll, + Source: pointer.Mouse, ++ PointerID: mouseStablePID, + Position: p, + Buttons: getPointerButtons(pi), + Scroll: sp, +-- +2.50.1.windows.1 + diff --git a/third_party/gio/app/os_windows.go b/third_party/gio/app/os_windows.go index 02a023c..dc5dd9f 100644 --- a/third_party/gio/app/os_windows.go +++ b/third_party/gio/app/os_windows.go @@ -507,6 +507,22 @@ func (w *window) hitTest(x, y int) uintptr { return windows.HTCLIENT } +// mouseStablePID is the PointerID reported for every Source=Mouse event so +// pointerQueue only ever holds a single entry for the physical mouse. +// Two paths in this file used to produce different IDs for the same mouse: +// scrollEvent omitted PointerID and let it default to 0, while pointerUpdate +// forwarded whatever ID Windows assigned via getPointerIDwParam (typically +// 1+). pointerQueue keys state.pointers by PointerID, so the same physical +// device produced two persistent entries; the scroll-derived one +// accumulated entered tags it never released and its stale-position +// hit-test overwrote the live pointer's cursor resolution every frame. +// EnableMouseInPointer also reassigns the mouse's PointerID across focus / +// window-leave events; normalising here absorbs that drift too. Touch +// keeps Windows-assigned IDs because multi-touch needs distinct pointers. +// The sentinel is max-uint so it cannot collide with Windows-assigned +// touch finger IDs (which start at small values). +var mouseStablePID = ^pointer.ID(0) + func (w *window) pointerUpdate(pi windows.PointerInfo, pid pointer.ID, kind pointer.Kind, lParam uintptr) { if !w.config.Focused { windows.SetFocus(w.hwnd) @@ -515,6 +531,7 @@ func (w *window) pointerUpdate(pi windows.PointerInfo, pid pointer.ID, kind poin src := pointer.Touch if pi.PointerType == windows.PT_MOUSE { src = pointer.Mouse + pid = mouseStablePID } x, y := coordsFromlParam(lParam) @@ -566,6 +583,7 @@ func (w *window) scrollEvent(wParam, lParam uintptr, horizontal bool, kmods key. w.ProcessEvent(pointer.Event{ Kind: pointer.Scroll, Source: pointer.Mouse, + PointerID: mouseStablePID, Position: p, Buttons: getPointerButtons(pi), Scroll: sp,