From 770546979d045fa76e8210401c4b233d42fca173 Mon Sep 17 00:00:00 2001 From: Mike Bannister Date: Tue, 1 Sep 2026 07:45:22 -0400 Subject: [PATCH] fix: scale the XTest scroll fallback to wheel notches When the xf86-input-neko driver is enabled, scroll deltas are in its scroll units, 120 per wheel notch (pkg/xinput/types.go). If the write to the driver socket fails, Scroll falls back to XTest, which clicks a wheel button once per unit, so a single notch replays as 120 clicks and a coalesced gesture as thousands. Convert the delta to whole notches on that fallback path only, through a new xorg.ScrollUnits that carries the sub-notch remainder so slow scrolling still accumulates. xorg.Scroll keeps upstream's semantics, where the delta is a click count, and still serves deployments that run without the driver, so their scrolling is unchanged. Page scrolling and Control-held zooming accumulate separately, so a remainder left by one cannot discharge as a notch of the other, and ResetKeys discards both. The arithmetic is plain Go and unit tested. The fallback path itself needs a running X server and stays uncovered. Co-Authored-By: Claude Fable 5 --- server/internal/desktop/xorg.go | 3 +- server/pkg/xorg/scroll_units.go | 57 +++++++++++++++ server/pkg/xorg/scroll_units_test.go | 102 +++++++++++++++++++++++++++ server/pkg/xorg/xorg.go | 21 ++++++ 4 files changed, 182 insertions(+), 1 deletion(-) create mode 100644 server/pkg/xorg/scroll_units.go create mode 100644 server/pkg/xorg/scroll_units_test.go diff --git a/server/internal/desktop/xorg.go b/server/internal/desktop/xorg.go index 907f1abc1..3e56ee158 100644 --- a/server/internal/desktop/xorg.go +++ b/server/internal/desktop/xorg.go @@ -28,7 +28,8 @@ func (manager *DesktopManagerCtx) Scroll(deltaX, deltaY int, controlKey bool) { } if err := manager.input.Scroll(int32(deltaX), int32(deltaY)); err != nil { manager.logger.Warn().Err(err).Msg("xinput scroll failed, falling back to XTest") - xorg.Scroll(deltaX, deltaY, false) + // the driver's deltas are scroll units, not wheel clicks + xorg.ScrollUnits(deltaX, deltaY, controlKey) } } else { // XTest fallback — handles controlKey atomically under a single X11 lock diff --git a/server/pkg/xorg/scroll_units.go b/server/pkg/xorg/scroll_units.go new file mode 100644 index 000000000..f3035faa4 --- /dev/null +++ b/server/pkg/xorg/scroll_units.go @@ -0,0 +1,57 @@ +package xorg + +// scrollNotchUnits is the number of scroll units that make up one wheel notch. +// The xf86-input-neko driver registers its scroll valuators with this +// increment (SCROLL_INCREMENT), so a delta bound for the driver means the same +// thing when the XTest fallback has to replay it. See pkg/xinput. +const scrollNotchUnits = 120 + +// Plain and Control-held scrolling accumulate separately. Sub-notch motion +// left over from a page scroll must not discharge as a notch while Control is +// held, because the browser applies that as a zoom step rather than scrolling. +// Both are guarded by mu, like the debounce maps in xorg.go. +var ( + scrollResidual scrollAccumulator + scrollResidualCtrl scrollAccumulator +) + +// scrollResidualFor returns the accumulator owning scrolls with or without +// Control held. +func scrollResidualFor(controlKey bool) *scrollAccumulator { + if controlKey { + return &scrollResidualCtrl + } + return &scrollResidual +} + +// resetScrollResiduals discards sub-notch motion pending on either accumulator. +func resetScrollResiduals() { + scrollResidual.reset() + scrollResidualCtrl.reset() +} + +// scrollAccumulator turns scroll deltas into whole wheel notches, carrying the +// sub-notch remainder between calls. XTest can only emit discrete wheel button +// clicks, so a delta has to be divided into notches before it is replayed, and +// carrying the remainder keeps slow scrolling from being rounded away. +type scrollAccumulator struct { + x, y int // pending units, always within (-scrollNotchUnits, scrollNotchUnits) +} + +// add accumulates a delta in scroll units and returns the whole notches now +// due on each axis. Notches are truncated toward zero, so the remainder keeps +// the sign of the pending motion and a reversal cancels it before emitting a +// notch in the new direction. +func (a *scrollAccumulator) add(deltaX, deltaY int) (notchesX, notchesY int) { + a.x += deltaX + a.y += deltaY + + notchesX, a.x = a.x/scrollNotchUnits, a.x%scrollNotchUnits + notchesY, a.y = a.y/scrollNotchUnits, a.y%scrollNotchUnits + return notchesX, notchesY +} + +// reset discards any pending sub-notch motion. +func (a *scrollAccumulator) reset() { + a.x, a.y = 0, 0 +} diff --git a/server/pkg/xorg/scroll_units_test.go b/server/pkg/xorg/scroll_units_test.go new file mode 100644 index 000000000..fbce426d9 --- /dev/null +++ b/server/pkg/xorg/scroll_units_test.go @@ -0,0 +1,102 @@ +package xorg + +import "testing" + +func TestScrollAccumulatorAdd(t *testing.T) { + // each step feeds delta on the vertical axis and checks the notches + // returned and the residual left behind for the next call + type step struct { + delta, notches, residual int + } + + tests := []struct { + name string + steps []step + }{ + {"whole notch", []step{{120, 1, 0}}}, + {"half notches accumulate across calls", []step{{60, 0, 60}, {60, 1, 0}}}, + {"negative delta truncates toward zero", []step{{-180, -1, -60}}}, + {"reversal cancels the residual first", []step{{90, 0, 90}, {-120, 0, -30}, {-90, -1, 0}}}, + {"coalesced gesture emits one click per notch", []step{{1500, 12, 60}}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var acc scrollAccumulator + for i, s := range tt.steps { + notchesX, notchesY := acc.add(0, s.delta) + if notchesX != 0 || acc.x != 0 { + t.Fatalf("step %d: horizontal axis moved: %d notches, residual %d", i, notchesX, acc.x) + } + if notchesY != s.notches || acc.y != s.residual { + t.Fatalf("step %d: add(0, %d) = %d notches, residual %d; want %d notches, residual %d", + i, s.delta, notchesY, acc.y, s.notches, s.residual) + } + } + }) + } +} + +func TestScrollAccumulatorAxesAreIndependent(t *testing.T) { + var acc scrollAccumulator + + notchesX, notchesY := acc.add(130, -250) + if notchesX != 1 || notchesY != -2 { + t.Fatalf("add(130, -250) = (%d, %d), want (1, -2)", notchesX, notchesY) + } + if acc.x != 10 || acc.y != -10 { + t.Fatalf("residual = (%d, %d), want (10, -10)", acc.x, acc.y) + } +} + +func TestScrollAccumulatorReset(t *testing.T) { + var acc scrollAccumulator + acc.add(100, -100) + acc.reset() + + if acc.x != 0 || acc.y != 0 { + t.Fatalf("residual after reset = (%d, %d), want (0, 0)", acc.x, acc.y) + } + + // the discarded motion must not contribute to the next notch + if notchesX, notchesY := acc.add(20, -20); notchesX != 0 || notchesY != 0 { + t.Fatalf("add(20, -20) after reset = (%d, %d), want (0, 0)", notchesX, notchesY) + } +} + +func TestScrollResidualsDoNotCrossModifiers(t *testing.T) { + resetScrollResiduals() + defer resetScrollResiduals() + + // a page scroll that has not yet reached a whole notch + if _, notchesY := scrollResidualFor(false).add(0, 110); notchesY != 0 { + t.Fatalf("plain add(0, 110) = %d notches, want 0", notchesY) + } + + // a Control-held scroll must not discharge it, which the browser would + // apply as a zoom step instead of scrolling the page + if _, notchesY := scrollResidualFor(true).add(0, 30); notchesY != 0 { + t.Fatalf("control add(0, 30) = %d notches, want 0", notchesY) + } + + // the page scroll is still pending and completes on its own + if _, notchesY := scrollResidualFor(false).add(0, 10); notchesY != 1 { + t.Fatalf("plain add(0, 10) = %d notches, want 1", notchesY) + } +} + +func TestResetScrollResidualsClearsBoth(t *testing.T) { + resetScrollResiduals() + defer resetScrollResiduals() + + scrollResidualFor(false).add(0, 100) + scrollResidualFor(true).add(0, 100) + resetScrollResiduals() + + if _, notchesY := scrollResidualFor(false).add(0, 20); notchesY != 0 { + t.Fatalf("plain add(0, 20) after reset = %d notches, want 0", notchesY) + } + if _, notchesY := scrollResidualFor(true).add(0, 20); notchesY != 0 { + t.Fatalf("control add(0, 20) after reset = %d notches, want 0", notchesY) + } +} diff --git a/server/pkg/xorg/xorg.go b/server/pkg/xorg/xorg.go index 8d5ee050d..f69484bc9 100644 --- a/server/pkg/xorg/xorg.go +++ b/server/pkg/xorg/xorg.go @@ -101,6 +101,25 @@ func Scroll(deltaX, deltaY int, controlKey bool) { C.XScroll(C.int(deltaX), C.int(deltaY)) } +// ScrollUnits scrolls by a delta expressed in the xf86-input-neko driver's +// scroll units rather than in wheel clicks, for the XTest fallback taken when +// the driver is enabled but unreachable. XTest can only click whole notches, +// so the sub-notch remainder is carried to the next call instead of being +// replayed once per unit. Its caller latches the Control modifier around the +// driver attempt, so controlKey here only keeps zoom and page scrolling in +// separate accumulators. +func ScrollUnits(deltaX, deltaY int, controlKey bool) { + mu.Lock() + defer mu.Unlock() + + notchesX, notchesY := scrollResidualFor(controlKey).add(deltaX, deltaY) + if notchesX == 0 && notchesY == 0 { + return + } + + C.XScroll(C.int(notchesX), C.int(notchesY)) +} + func ButtonDown(code uint32) error { mu.Lock() defer mu.Unlock() @@ -170,6 +189,8 @@ func ResetKeys() { C.XKey(C.KeySym(code), C.int(0)) delete(debounce_key, code) } + + resetScrollResiduals() } func CheckKeys(duration time.Duration) {