Skip to content

scroll: Stop a smooth-scrolling wheel at the edge on macOS - #3781

Merged
huacnlee merged 3 commits into
longbridge:mainfrom
insd47:scroll-phaseless-wheel
Nov 4, 2026
Merged

huacnlee merged 3 commits into
longbridge:mainfrom
insd47:scroll-phaseless-wheel

Conversation

@insd47

@insd47 insd47 commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Description

Follows up #3316. Smooth-scrolling mouse drivers (Logi Options+, Mos, Mac Mouse Fix and similar) send precise deltas with no phase, which GPUI reports in the same shape as trackpad momentum. #3316 fixed the list freezing after such a wheel bounced, but the wheel still bounces at every edge. NSScrollView does not bounce it.

I logged the raw NSEvents on macOS 27 (4,064 events):

  • Mac Mouse Fix wheel: 606 packets, all with phase and momentumPhase none.
  • Trackpad: momentum (momentumPhase began) started 0.1 to 13 ms after the finger's ended, in all 15 flings.
  • A plain NSScrollView scrolled to its end by the Mac Mouse Fix wheel never left its bounds. Trackpad flings stretched it.

GPUI does not pass momentumPhase on, but the stream tells the two apart: a trackpad sends a touch (Started) and a lift (Ended) around its momentum, and a smooth-scrolling wheel sends neither. On macOS, Overscroll now treats phaseless packets as momentum only once the stream has had a touch or a lift. A pause of GESTURE_GAP ends the stream:

 phaseless packet, not dragging
-  scroll; throw the edge if it runs into it
+  after a touch or a lift: scroll; throw the edge if it runs into it
+  otherwise (macOS): scroll, stop at the edge

ScrollBounce (wheel mode) and Input hand Overscroll only packets that lean vertical, so a swipe whose first packet leans sideways never showed it a touch. Both now pass on the phase of the packets they keep, through Overscroll::touch_phase; without that, such a swipe that turned vertical at an end stopped flat. Neither changes what it scrolls or passes on.

A driver that imitates a trackpad still bounces, as in NSScrollView: Mac Mouse Fix's trackpad mode sends began, ended, then momentum 0.1 ms later. #3316's pause and inward rules are unchanged. Other platforms are unchanged, since I could not check whether their touchpads report a lift. ScrollBounce with .scroll_wheel(false) uses its own physics rather than Overscroll, so a smooth-scrolling wheel still bounces there.

How to Test

overscroll::tests::a_smooth_scrolling_wheel_stops_at_the_edge (macOS) sends ten phaseless -40 packets 8 ms apart into 30 px of text.

  • Before: the first packet that runs past the end throws the edge (assertion failed: !overscroll.packet(-40., TouchPhase::Moved, ...)).
  • After: the text scrolls 30 px and the edge stays at 0.

Two more macOS tests, each failing without its part of the fix:

  • a_wheel_after_a_pause_stops_at_the_edge: a lift, then a wheel GESTURE_GAP later. Fails without the reset in start_gesture.
  • a_swipe_that_touches_sideways_still_bounces: the touch arrives through touch_phase, then vertical packets run into the edge. Fails if touch_phase does not mark the stream.

Four existing Overscroll tests fed momentum with no lift before it. They now send the Ended that precedes momentum on macOS; their assertions are unchanged.

Checks run:

  • cargo test -p gpui-base --features test-support --lib on macOS: 1550 passed.
  • cargo test -p gpui-component --lib: 752 passed.
  • cargo clippy -p gpui-base --features test-support --all-targets -- --deny warnings: clean.
  • cargo fmt --check: clean.
  • A throwaway probe comparing this branch with its base over 1,200 simulated scenarios (child and wheel ScrollBounce, bounce on and off, four kinds of parent, three start offsets, 25 gestures): only the four where a phaseless wheel runs into the top edge differ, and there it now stops instead of stretching 4.5 px.

Verified by hand on macOS with a trackpad and a Mac Mouse Fix wheel, in a TextView and in a markdown Input: the wheel stops at the ends; a drag and a fling at an end stretch and bounce; and a swipe that starts leaning sideways and turns vertical at an end stretches. A wheel used within 250 ms of a trackpad lift can still bounce once, as your review noted.

Not run: Logitech hardware, a Magic Mouse, the story gallery, Windows and Linux.

The diagnosis, fix and tests were written with Claude Code.

Checklist

  • I have read the CONTRIBUTING document and followed the guidelines.
  • Reviewed the changes in this PR and confirmed AI generated code (If any) is accurate.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@huacnlee

huacnlee commented Nov 4, 2026

Copy link
Copy Markdown
Member

Review of e4efec93 (T-0254): no blocking findings. The heuristic is sound for the evidence available, and non-macOS behaviour is unchanged. There is one test gap worth closing, plus three minor notes. Real macOS and driver hardware could not be tested on this Linux host.

Findings

P2: the two resets of lifted have no test (crates/base/src/overscroll.rs:83, :148)
With the macOS branch forced on, I removed each reset in turn, and all 22 overscroll tests still passed. The start_gesture reset is what makes a mouse stop at the edge after the trackpad has been used. Without it, one trackpad lift would bring the bounce back for every later wheel stream. Suggested test: Ended, then a pause of at least GESTURE_GAP, then phaseless packets into the edge, asserting no stretch. It could sit next to a_smooth_scrolling_wheel_stops_at_the_edge. This is non-blocking, because the code is correct today.

P3: a stale lifted lasts as long as the stream has no gap (overscroll.rs:194)
The state is only cleared by a pause of at least 250 ms or a Started packet. Simulated results for a wheel following a trackpad Ended:

Wheel starts Result
100 or 249 ms after Ended, at the edge bounces once
250 or 1000 ms after Ended, at the edge stops at the edge
100 ms after Ended, reaching the edge 7000 px later bounces once, on arrival
during live momentum bounces once

This needs a hand switch inside the gap or during a fling. The worst case is one bounce, which is what every wheel does before this PR, and spent stops any repeat. I would accept it.

P3: a stall of 250 ms or more loses the bounce of a real fling (overscroll.rs:194, via the gap check at :128)
With a simulated stall of 250 or 400 ms between Ended and the first momentum packet, or in the middle of momentum, start_gesture clears lifted. The fling then stops flat at the edge. At 249 ms it still bounces, and before this PR it bounced in every case. This is cosmetic: the text keeps scrolling and nothing freezes. It is the opposite of the failure in #3316. It could still show on a very slow frame, which the SPEED_SPAN comment already expects near fling ends in long lists.

Note: .scroll_wheel(false) is not covered (crates/base/src/scroll_bounce.rs:397)
That path uses its own #3316 physics and does not go through Overscroll, so a smooth-scrolling wheel still bounces there. In this repo only crates/base/examples/native/src/bin/scroll_bounce.rs uses it, but it is a public option. Any downstream caller that sets it keeps the old behaviour. The PR description could say so.

Note: Cancelled has no effect on macOS (overscroll.rs:141)
gpui-pre-macos and gpui-fast-macos events.rs map NSEventPhaseCancelled to TouchPhase::Moved, so the Cancelled arm cannot fire there. I only read one pinned source of each (gpui-pre-macos 0.3.8, gpui-fast-macos 0.1.2), not the versions in Cargo.lock (0.3.11, 0.1.13). A momentum stream after a macOS cancel would count as a wheel and not bounce. I don't expect AppKit to send one, but I could not confirm that here. Harmless, but the PR text and field comment read slightly wider than what happens.

Heuristic and API

  • Heuristic: ordering is a reasonable stand-in, because GPUI does not expose momentumPhase. The PR's event log (15 of 15 flings with a lift first, 606 wheel packets with no phase) supports it, and a misclassification only ever gains or loses one bounce. It never blocks scrolling, so the scroll_bounce: Let phaseless wheels scroll again after bouncing at an edge #3316 freeze cannot return through this path. If GPUI ever exposes momentumPhase, that would be the better signal.
  • API and naming: no public API change. lifted is private and says what it means.

What I ran

All on Linux, on a worktree verified at e4efec93c3e63471a29e54b6c36536a750dfc449:

  • cargo test -p gpui-base --features test-support --lib: 1550 passed.
  • cargo clippy -p gpui-base --features test-support --all-targets -- --deny warnings: clean.
  • cargo fmt -p gpui-base --check: clean.
  • Simulated macOS (the cfg! forced on and the new test enabled, throwaway edit): 1559 passed, including 8 local probes.
  • Reverting the fix under simulation: a_smooth_scrolling_wheel_stops_at_the_edge fails, so the test covers the fix.
  • Never setting lifted: 5 tests fail, so the momentum path is covered.
  • Throwaway probes, unmodified Linux build vs simulated macOS:
Scenario Linux Simulated macOS
Pure wheel into the edge bounces stops, offset 0
Wheel reverses at the edge bounces, then scrolls back 100 px no bounce, scrolls back 100 px
Fling with a lift throws throws
Wheel while the edge returns: outward re-throws no change
Wheel while the edge returns: inward scrolls the text scrolls the text
Edge settles after that offset 0 offset 0
  • macOS CI: the job log for Test (aarch64-apple-darwin, macos-latest) shows a_smooth_scrolling_wheel_stops_at_the_edge ... ok.

Not verified

  • Real macOS hardware, or any real driver: Mac Mouse Fix, Logi Options+, Mos, SmoothScroll, Magic Mouse, trackpad. I rely on the PR's event log and manual test, and did not reproduce them.
  • The macOS behaviour above comes from forcing the cfg! on Linux with synthetic packets, not from AppKit events.
  • Whether Logitech and other drivers really send no phase. The PR did not run Logitech hardware either.
  • The story gallery, and TextInput / Tree end to end. I only read the Input and ScrollBounce callers.
  • Windows, beyond CI being green.

🤖 Noop (reviewer) · AI agent acting for @huacnlee

insd47 and others added 2 commits November 4, 2026 15:01
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@insd47

insd47 commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

Thanks. Added the test for the pause in 04dded1, and while checking further I found and fixed a gap in 579e248.

The gap. ScrollBounce (wheel mode) and Input hand Overscroll only packets that lean vertical. When a trackpad swipe's first packet leans sideways, Overscroll never sees the Started, so the vertical packets that follow look like a wheel, and on this PR's first commit such a swipe stopped flat at an end where it used to stretch. I found it by running a throwaway probe over 1,200 simulated scenarios on the base and on the branch.

The fix. The rule is now "a touch or a lift", not only a lift: a smooth-scrolling wheel sends neither. Both callers pass on the phase of the packets they keep, through Overscroll::touch_phase; neither changes what it scrolls or passes on. With that, the probe differs from the base only where a phaseless wheel runs into an edge.

The reset on Started went away with the new rule, since a touch now marks the stream instead of clearing it. So the tests are:

  • a_wheel_after_a_pause_stops_at_the_edge: fails without the reset in start_gesture.
  • a_swipe_that_touches_sideways_still_bounces: fails if touch_phase does not mark the stream.

I agree with leaving the two P3s. I updated the description for your two notes (.scroll_wheel(false) keeps the old behavior, and Cancelled does not occur on macOS, which the new rule no longer depends on) and for the new rule.

Checked by hand on macOS with a trackpad and a Mac Mouse Fix wheel, in a TextView and in a markdown Input: the wheel stops at the ends, drags and flings bounce, and a swipe that starts sideways and turns vertical at an end stretches again. Also ran cargo test -p gpui-base --features test-support --lib (1550 passed), cargo test -p gpui-component --lib (752 passed), clippy and fmt, all clean.

@huacnlee

huacnlee commented Nov 4, 2026

Copy link
Copy Markdown
Member

Re-review of 579e2482 (T-0259): the earlier P2 is closed, but the new sideways fix has one remaining gap.

Finding

P3, small regression: a swipe that leans sideways for about 250 ms or more still stops flat at an end (crates/base/src/overscroll.rs:118-123, :151)

touch_phase only calls follow for Started/Ended. Sideways Moved packets therefore do not refresh last_packet_at. The first vertical packet then sees a gap of at least GESTURE_GAP. Because this path never called physics.begin, is_dragging() is false, so start_gesture clears touched and the swipe is treated as a wheel. The same happens to a lift that leaves sideways momentum.

Probe with cfg! forced on, on Linux: touch_phase(Started), sideways touch_phase(Moved) every 8 ms, then vertical -40 packets into 30 px of text.

Sideways lead Stretches now With the change below
100 ms yes yes
240 ms yes not run
248 ms no yes
600 ms no yes

Ended followed by sideways momentum gives the same result: it stretches at 100 ms and does not at 248 ms or more. The base has no such condition, from reading its code, though I did not run this probe on it.

The result is cosmetic, because the text still scrolls and nothing freezes. But it is the same symptom 579e248 set out to remove, and your 1,200-scenario probe may not have covered a lead this long.

Change I tried. With only the cfg! forced on, the 14 Overscroll tests not gated to macOS still pass. I did not rerun the three macOS-only tests with this change:

pub(crate) fn touch_phase(&mut self, phase: TouchPhase, now: Instant) {
    self.follow(now);
    if phase != TouchPhase::Moved {
        self.touched = true;
    }
}

The method doc would then need to say it notes any packet not handed to packet. A test with a sideways lead longer than GESTURE_GAP would pin this down. I probed Overscroll directly, not through ScrollBounce or Input. ScrollBounce also applies the axis lock, which needs y >= 1.9 * x to release, so that path needs a clear turn. Input does not lock the delta before this check.

Earlier findings

  • Untested lifted resets: closed by a_wheel_after_a_pause_stops_at_the_edge. The Started reset no longer exists under the new rule.
  • The two P3s are unchanged and accepted.
  • Description: it now covers .scroll_wheel(false) and Cancelled, and matches the code.

API and naming

There is no public API change. touch_phase is pub(crate), and touched describes the new rule.

What I ran

On Linux at 579e24824cda:

  • cargo test -p gpui-base --features test-support --lib: 1550 passed.
  • cargo clippy -p gpui-base --features test-support --all-targets -- --deny warnings: clean.
  • cargo fmt -p gpui-base --check: clean.
  • With the macOS cfg! forced on, all 16 Overscroll tests passed. Removing the start_gesture reset fails only a_wheel_after_a_pause_stops_at_the_edge. Removing the mark in touch_phase fails only a_swipe_that_touches_sideways_still_bounces.

Not verified

Real macOS and driver hardware, the story gallery, and end-to-end ScrollBounce/Input. gpui-component tests were not run locally. CI at this head was still running when I wrote this (12 passed, 7 pending).

🤖 Noop (reviewer) · AI agent acting for @huacnlee

@huacnlee
huacnlee merged commit ca95672 into longbridge:main Nov 4, 2026
19 checks passed
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