fix(linux): round a scroll to the nearest notch instead of truncating it (#1618)

Author: y3owk1n

docs/CROSS_PLATFORM.md

@@ -221,11 +221,22 @@ event, and the XTEST pointer has no scroll valuator for the smooth XI2 path.
 Windows sits with Wayland: `MOUSEEVENTF_WHEEL` counts 120ths of `WHEEL_DELTA`,
 so the animator steps in 120ths of a notch.
 
-**So X11 animates in notches, and a scroll worth one notch is not animated at
-all.** The default `scroll.scroll_step` of 50 pixels is exactly one notch
-there, so a plain `scroll_down` on X11 arrives as the single wheel click it
-always did. From two notches up the same eased curve applies as everywhere
-else.
+**What a `scroll_step` number means differs per platform.** macOS posts
+pixel-unit wheel events, so a step of 50 scrolls 50 px. Windows sends
+`WHEEL_DELTA` units at 4 per pixel (120 per 30 px notch), so 50 px is 200
+units, delivered exactly, and an application accumulates the fraction the
+way it does for a high-resolution wheel. Linux converts at 30 px per notch on
+each path that sends notches (uinput, the wlroots virtual pointer, XTest) and
+rounds to the nearest, never fewer than one. 44 px is one notch, 45 px and
+the default 50 px are two, 500 px is seventeen. Rounding rather than
+truncation is what keeps a 59 px step from travelling no further than a
+30 px one. On KDE, libei carries the pixel delta as is.
+
+**So X11 animates in notches, and a scroll that rounds to one notch is not
+animated at all.** A `scroll_step` under 45 pixels on X11 arrives as the
+single wheel click it always did. From two notches up, the default included,
+the same eased curve applies as everywhere else, and the animated scroll
+travels the same notch count as the unanimated one.
 
 Neru sends the same distance on every backend. On Wayland the animated path
 spends that distance as a continuous delta where the unanimated one spends it

internal/adapter/accessibility/native/linux/element.go

@@ -429,6 +429,16 @@ func MouseUp(button action.MouseButton) error {
 // different distance depending on which backend answered.
 const scrollPixelsPerNotch = 30
 
+// scrollNotches is how many wheel notches a scroll of delta pixels is: the
+// nearest whole notch, and never fewer than one so a step shorter than a notch
+// still moves. Truncating instead left a default 50 px step one notch short
+// of the two it rounds to, and a 59 px step no further than a 30 px one.
+// Every unanimated Linux path and the animated total share this, so the two
+// travel the same distance.
+func scrollNotches(delta int) int {
+	return max((abs(delta)+scrollPixelsPerNotch/2)/scrollPixelsPerNotch, 1)
+}
+
 // ScrollAtCursor scrolls at the cursor, presenting modifiers as held.
 //
 // Linux has no event-flags concept, so a modifier is a real key press around
@@ -533,9 +543,6 @@ func scrollAtCursorNow(deltaX, deltaY int, modifiers action.Modifiers) error {
 			waitForWaylandModifierPress()
 		}
 
-		// Scale factor: each uinput scroll event approximates ~1 line.
-		const scrollScale = scrollPixelsPerNotch
-
 		// maxBatchEvents caps the number of uinput events sent per
 		// write/flush to avoid overflowing the kernel evdev buffer (~8192
 		// bytes) or the Wayland socket buffer.  Each batch is kept small so
@@ -551,10 +558,7 @@ func scrollAtCursorNow(deltaX, deltaY int, modifiers action.Modifiers) error {
 				return 0
 			}
 
-			totalNotches := abs(delta) / scrollScale
-			if totalNotches == 0 {
-				totalNotches = 1
-			}
+			totalNotches := scrollNotches(delta)
 
 			remainingNotches := totalNotches
 			batch := make([]int, 0, maxBatchEvents)
@@ -584,7 +588,9 @@ func scrollAtCursorNow(deltaX, deltaY int, modifiers action.Modifiers) error {
 				}
 			}
 
-			return uinputScrollRemainder(delta, totalNotches, remainingNotches, scrollScale)
+			return uinputScrollRemainder(
+				delta, totalNotches, remainingNotches, scrollPixelsPerNotch,
+			)
 		}
 
 		remainY := sendScaledScroll(uinputScrollAxisVertical, deltaY)
@@ -618,10 +624,12 @@ func scrollAtCursorNow(deltaX, deltaY int, modifiers action.Modifiers) error {
 // send, in the caller's pixels, for the virtual pointer to finish.
 //
 // It is zero whenever every notch went out, even when the delta was not a
-// whole number of notches: a 50 px step is one 30 px notch, and the 20 px
-// left over is rounding, not a lost scroll. Handing it on used to add a
-// second, opposite-signed notch to every press on Hyprland, which Chromium
-// and Electron clients summed to nothing.
+// whole number of notches: a 50 px step is two 30 px notches, and the 10 px
+// left over is rounding, not a lost scroll. Handing it on used to add an
+// extra, opposite-signed notch to every press on Hyprland, which Chromium
+// and Electron clients summed to nothing. When some notches remain, the
+// pixels handed on round to exactly the unsent count, so the virtual pointer
+// finishes the same scroll rather than a re-rounded one.
 func uinputScrollRemainder(delta, totalNotches, remainingNotches, scale int) int {
 	if remainingNotches == 0 {
 		return 0

internal/adapter/accessibility/native/linux/element_wayland_wlroots_cgo.go

@@ -206,12 +206,10 @@ func wlrootsMouseUp(button action.MouseButton) error {
 	return nil
 }
 
-// wlrootsScrollScale mirrors the uinput scroll scaling constant so
-// that both backends produce comparable scroll behavior from the same
-// pixel-level delta values supplied by the scroll service
-// (e.g. ScrollStep=50, ScrollStepHalf=500, ScrollStepFull=1000000).
+// wlrootsScrollStep is the pixel value one virtual-pointer notch carries,
+// the same figure the uinput path counts notches in, so the two backends
+// travel the same distance from the same scroll service delta.
 const (
-	wlrootsScrollScale     = scrollPixelsPerNotch
 	wlrootsScrollMaxEvents = 50
 	wlrootsScrollStep      = scrollPixelsPerNotch // pixels per notch
 )
@@ -340,10 +338,7 @@ func (s *waylandScrollSession) close() {
 // Application  convention: positive delta = scroll up (axis 0) / right (axis 1).
 // Vertical axis sign is negated to convert between the two.
 func wlrootsScrollAxis(axis int, delta int) error {
-	totalNotches := abs(delta) / wlrootsScrollScale
-	if totalNotches == 0 {
-		totalNotches = 1
-	}
+	totalNotches := scrollNotches(delta)
 
 	step, disc := wlrootsScrollNotch(axis, delta, wlrootsScrollStep)
 

internal/adapter/accessibility/native/linux/element_x11_cgo.go

@@ -230,20 +230,10 @@ func x11ScrollAtCursor(deltaX, deltaY int, modifiers action.Modifiers) error {
 	// (e.g. ScrollStep=50, ScrollStepHalf=500, ScrollStepFull=1000000).
 	// We scale them to a capped number of clicks to avoid flooding X11
 	// with tens of thousands of button events on large scrolls.
-	const (
-		scale     = scrollPixelsPerNotch
-		maxClicks = maxScrollUnitsPerRequest
-	)
+	const maxClicks = maxScrollUnitsPerRequest
 
 	if deltaY != 0 {
-		yClicks := abs(deltaY) / scale
-		if yClicks == 0 {
-			yClicks = 1
-		}
-
-		if yClicks > maxClicks {
-			yClicks = maxClicks
-		}
+		yClicks := min(scrollNotches(deltaY), maxClicks)
 
 		for range yClicks {
 			const mouseButtonVerticalScroll = 4
@@ -261,14 +251,7 @@ func x11ScrollAtCursor(deltaX, deltaY int, modifiers action.Modifiers) error {
 	}
 
 	if deltaX != 0 {
-		xClicks := abs(deltaX) / scale
-		if xClicks == 0 {
-			xClicks = 1
-		}
-
-		if xClicks > maxClicks {
-			xClicks = maxClicks
-		}
+		xClicks := min(scrollNotches(deltaX), maxClicks)
 
 		for range xClicks {
 			const mouseButtonHorizontalScrollRight = 7

internal/adapter/accessibility/native/linux/scroll_animator.go

@@ -155,18 +155,26 @@ func scrollChunks(delta float64, steps int, granularity float64, maxUnits int) [
 
 	total := delta
 
-	if granularity > 0 && maxUnits > 0 {
-		ceiling := float64(maxUnits) * granularity
-		total = math.Max(math.Min(total, ceiling), -ceiling)
+	// A granular backend travels the nearest whole number of units, never
+	// fewer than one, exactly as the unanimated path counts them in
+	// scrollNotches. The animation spreads the same notches over time, it
+	// does not shorten the scroll. Rounding up front rather than per step
+	// is what lets the eased chunks below truncate and still add up to it.
+	if granularity > 0 {
+		units := math.Max(math.Round(math.Abs(total)/granularity), 1)
+		if maxUnits > 0 {
+			units = math.Min(units, float64(maxUnits))
+		}
+
+		total = math.Copysign(units*granularity, total)
 	}
 
-	// A scroll worth one unit or less on a granular backend is not an
-	// animation: whatever the curve says, exactly one event goes out. Putting
-	// it anywhere but the first step would deliver the same single wheel click
-	// the unanimated path sends, only later — pure added latency, which is the
-	// one thing this must not buy. X11 with the default scroll_step of 50
-	// pixels is exactly that case.
-	if granularity > 0 && math.Trunc(math.Abs(total)/granularity) <= 1 {
+	// A scroll worth one unit on a granular backend is not an animation:
+	// whatever the curve says, exactly one event goes out. Putting it anywhere
+	// but the first step would deliver the same single wheel click the
+	// unanimated path sends, only later — pure added latency, which is the one
+	// thing this must not buy. A scroll_step under 45 pixels is that case.
+	if granularity > 0 && math.Abs(total) == granularity {
 		unit := granularity
 		if total < 0 {
 			unit = -granularity

internal/adapter/accessibility/native/linux/scroll_animator_test.go

@@ -134,23 +134,23 @@ func TestScrollChunks_KeepsAGranularBackendOnWholeUnits(t *testing.T) {
 		}
 	}
 
-	// The unanimated X11 path sends abs(delta)/30 clicks, truncated: 16 here.
-	// Rounding per step would lose several of them.
-	if got, want := sum(chunks), -16*float64(notch); got != want {
-		t.Errorf("chunks total %v, want %v (16 notches)", got, want)
+	// The unanimated X11 path sends scrollNotches(delta) clicks, which is 500
+	// px rounded to the nearest notch, 17. Rounding per step would lose several.
+	if got, want := sum(chunks), -17*float64(notch); got != want {
+		t.Errorf("chunks total %v, want %v (17 notches)", got, want)
 	}
 }
 
 // TestScrollChunks_SendsAOneUnitScrollImmediately covers the case that is not
-// an animation at all, and the one the default configuration hits on X11: a
-// scroll_step of 50 pixels is a single wheel notch there. One event goes out
-// either way, so scheduling it anywhere but the first step would deliver the
-// same scroll later — added latency and nothing else.
+// an animation at all: a scroll that rounds to a single wheel notch on X11.
+// One event goes out either way, so scheduling it anywhere but the first step
+// would deliver the same scroll later — added latency and nothing else.
 func TestScrollChunks_SendsAOneUnitScrollImmediately(t *testing.T) {
 	const notch = scrollPixelsPerNotch
 
-	// Shorter than a notch, exactly a notch, and the shipped scroll_step.
-	for _, delta := range []float64{10, -10, 1, -1, notch, -notch, 50, -50} {
+	// Shorter than a notch, exactly a notch, and the last step that still
+	// rounds down to one.
+	for _, delta := range []float64{10, -10, 1, -1, notch, -notch, 44, -44} {
 		chunks := scrollChunks(delta, 20, notch, maxScrollUnitsPerRequest)
 
 		want := float64(notch)
@@ -535,14 +535,15 @@ func TestScrollAnimator_Animate_HeldRepeatOnAGranularBackend(t *testing.T) {
 		time.Sleep(scrollTestRepeatInterval)
 	}
 
+	// What ten discrete presses send: each rounds to the nearest notch.
 	var (
-		asked     = float64(ticks * -scrollTestHalfPage)
+		asked     = float64(ticks * scrollNotches(scrollTestHalfPage) * notch)
 		tolerance = float64(ticks * notch)
 	)
 
 	// The last tick is still draining, so the floor is what has to be waited
 	// for; the ceiling holds at every instant, since no schedule can send more
-	// than was asked for.
+	// notches than the presses would.
 	waitFor(t, func() bool { return -log.traveled() >= asked-tolerance })
 
 	time.Sleep(60 * time.Millisecond)

internal/adapter/accessibility/native/linux/scroll_notch_test.go

@@ -4,9 +4,51 @@ package linux
 
 import "testing"
 
+// TestScrollNotches_RoundsToTheNearestNotch pins the pixel-to-notch conversion
+// every Linux path shares: nearest whole notch, never fewer than one, and the
+// animated schedule lands on the same count so switching smooth_scroll on
+// changes when a scroll arrives, never how far it goes.
+func TestScrollNotches_RoundsToTheNearestNotch(t *testing.T) {
+	const notch = scrollPixelsPerNotch
+
+	tests := []struct {
+		pixels int
+		want   int
+	}{
+		{pixels: 1, want: 1},
+		{pixels: 29, want: 1},
+		{pixels: 30, want: 1},
+		{pixels: 44, want: 1},
+		{pixels: 45, want: 2},
+		{pixels: 50, want: 2},
+		{pixels: 59, want: 2},
+		{pixels: 60, want: 2},
+		{pixels: 500, want: 17},
+	}
+
+	for _, testCase := range tests {
+		for _, delta := range []int{testCase.pixels, -testCase.pixels} {
+			if got := scrollNotches(delta); got != testCase.want {
+				t.Errorf("scrollNotches(%d) = %d, want %d", delta, got, testCase.want)
+			}
+
+			want := float64(testCase.want * notch)
+			if delta < 0 {
+				want = -want
+			}
+
+			animated := sum(scrollChunks(float64(delta), 20, notch, maxScrollUnitsPerRequest))
+			if animated != want {
+				t.Errorf("animated %d px travels %v, want %v (the unanimated %d notches)",
+					delta, animated, want, testCase.want)
+			}
+		}
+	}
+}
+
 // TestUinputScrollRemainder_WholeSendLeavesNothing is the Hyprland regression:
-// a default 50 px step is one 30 px notch, and the 20 px of rounding left over
-// must not go out again on the virtual pointer as a second notch.
+// a default 50 px step is two 30 px notches, and the 10 px of rounding left
+// over must not go out again on the virtual pointer as a third notch.
 func TestUinputScrollRemainder_WholeSendLeavesNothing(t *testing.T) {
 	const scale = 30
 
@@ -17,9 +59,10 @@ func TestUinputScrollRemainder_WholeSendLeavesNothing(t *testing.T) {
 		remaining int
 		want      int
 	}{
-		{name: "every notch sent, rounding left over", delta: -50, total: 1, remaining: 0, want: 0},
+		{name: "every notch sent, rounding left over", delta: -50, total: 2, remaining: 0, want: 0},
+		{name: "one of two sent hands on the rest", delta: 50, total: 2, remaining: 1, want: 20},
 		{name: "every notch sent, exact", delta: 60, total: 2, remaining: 0, want: 0},
-		{name: "nothing sent keeps the whole delta", delta: -50, total: 1, remaining: 1, want: -50},
+		{name: "nothing sent keeps the whole delta", delta: -50, total: 2, remaining: 2, want: -50},
 		{name: "half sent keeps the unsent half", delta: 120, total: 4, remaining: 2, want: 60},
 	}