Skip to content

Accumulate VALUE120 high-res discrete scrolling events - #9128

Merged
kovidgoyal merged 1 commit into
kovidgoyal:masterfrom
evan-goode:evan-goode/accumulate-value120
Oct 21, 2025
Merged

Accumulate VALUE120 high-res discrete scrolling events#9128
kovidgoyal merged 1 commit into
kovidgoyal:masterfrom
evan-goode:evan-goode/accumulate-value120

Conversation

@evan-goode

Copy link
Copy Markdown
Contributor

Fixes a bug where VALUE120 scroll events with value < 1 line would always scroll a full line, leading to strange-feeling scrolling behavior on smooth-scrolling input devices.

Explanation:

        // VALUE120 events are "discrete" in that they scroll lines, not pixels.
        // But each VALUE120 event of value `v` represents a fraction of a
        // line, specifically `v` 120ths of a line.

        // GLFWscrollfun does not handle non-highres scroll events with
        // non-integer value. We could use `highres` for VALUE120 events, but
        // that would be wrong since the units should be interpreted as
        // fractional lines, not pixels.

        // So instead, we accumulate discrete events until we have at least one
        // full line to scroll, and then call the GLFWscrollfun to scroll entire
        // lines.

@kovidgoyal

Copy link
Copy Markdown
Owner

The correct fix for this would be to pass the fact that these are 120th of line units in the flags parameter of GLFWscrollfun() and then change the code in mouse.c that handles them to use that information to actually scroll by 120th of a line. IIRC only the first four bits of flags is currently used so you can use bit 5 for it.

@evan-goode
evan-goode force-pushed the evan-goode/accumulate-value120 branch 2 times, most recently from 5d25a94 to 65cdd9e Compare October 19, 2025 15:17
@evan-goode

Copy link
Copy Markdown
Contributor Author

The correct fix for this would be to pass the fact that these are 120th of line units in the flags parameter of GLFWscrollfun() and then change the code in mouse.c that handles them to use that information to actually scroll by 120th of a line. IIRC only the first four bits of flags is currently used so you can use bit 5 for it.

Thanks, done. I didn't look at first and I assumed the GLFWscrollfun was some library code.

I realized that we currently do not handle the case when the two scroll axes are different types, e.g. one is continuous and another is discrete. But that would only be a problem with some very small minority of pointing devices.

@kovidgoyal

Copy link
Copy Markdown
Owner

I dont quite follow the logic. In mouse.c you are treating a VALUE120
value as pixels (the logic is identical for high res and value120
values? For value120 value shouldnt this be

s = offset * cell_height / 120

rather than

s = offset / cell_height

In other words a value of 120 should cause a scroll by one line.

@evan-goode
evan-goode force-pushed the evan-goode/accumulate-value120 branch from 65cdd9e to 80a9bdb Compare October 20, 2025 13:33
@evan-goode

Copy link
Copy Markdown
Contributor Author

You're right, my bad. How's that?

@kovidgoyal
kovidgoyal merged commit 8630f21 into kovidgoyal:master Oct 21, 2025
francislavoie added a commit to francislavoie/kitty that referenced this pull request Aug 2, 2026
On a high resolution wheel one physical detent is delivered as several
VALUE120 fragments, and those fragments do not reliably total 120 units.
Measured on a Logitech MX Master 3 under GNOME/Wayland, single detents ranged
from 96 to 152 units, with 96% of them falling between 0.80 and 1.40 lines.

scale_scroll() carries pending_scroll_pixels across events and never resets
it, so residual left over from a previous detent, possibly in the opposite
direction, has to be cancelled before any line can be emitted. A detent is
therefore often consumed entirely and does not scroll at all.

Measured over 96 detents, against a terminal program that had requested mouse
events, 47% of detents that reversed direction scrolled nothing, as did 14% of
detents continuing in the same direction.

Treat an idle gap or a direction reversal as the start of a new detent,
discard the stale residual, and emit a line as soon as the detent is
unambiguous rather than waiting for a full line to accumulate. Both decisions
are taken on an incoming fragment rather than on a timer, so no latency is
added.

This preserves the accumulating behaviour added in kovidgoyal#9128 for devices that
stream many small VALUE120 events: within a continuous same-direction stream
nothing changes, and fragments too small to be a detent still scroll nothing.

Measured after the change, over 118 detents, no detent failed to scroll, in
either direction.
francislavoie added a commit to francislavoie/kitty that referenced this pull request Aug 2, 2026
On a high resolution wheel one physical detent is delivered as several
VALUE120 fragments, and those fragments do not reliably total 120 units.
Measured on a Logitech MX Master 3 under GNOME/Wayland, single detents ranged
from 96 to 152 units, with 96% of them falling between 0.80 and 1.40 lines.

scale_scroll() carries pending_scroll_pixels across events and never resets
it, so residual left over from a previous detent, possibly in the opposite
direction, has to be cancelled before any line can be emitted. A detent is
therefore often consumed entirely and does not scroll at all.

Measured over 96 detents, against a terminal program that had requested mouse
events, 47% of detents that reversed direction scrolled nothing, as did 14% of
detents continuing in the same direction.

Treat an idle gap or a direction reversal as the start of a new detent,
discard the stale residual, and emit a line as soon as the detent is
unambiguous rather than waiting for a full line to accumulate. Both decisions
are taken on an incoming fragment rather than on a timer, so no latency is
added.

This preserves the accumulating behaviour added in kovidgoyal#9128 for devices that
stream many small VALUE120 events: within a continuous same-direction stream
nothing changes, and fragments too small to be a detent still scroll nothing.

Measured after the change, over 118 detents, no detent failed to scroll, in
either direction.
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