feat(mouse): report mouse input and route scroll gestures to applications - #259
Open
bmander wants to merge 13 commits into
Open
feat(mouse): report mouse input and route scroll gestures to applications#259bmander wants to merge 13 commits into
bmander wants to merge 13 commits into
Conversation
Expose libvterm's mouse reporting so the terminal can tell a running application about wheel, button and motion input. Applications that take over the screen -- vim, tmux, Claude Code's alternate-screen renderer -- enable tracking with DECSET 1000/1002/1003 and handle scrolling and clicks themselves; without this there is nowhere to send the input. Adds mouseMove/mouseButton/scrollWheel to TerminalEmulator, along with a mouseTracking property reflecting VTERM_PROP_MOUSE so callers can tell whether the application wants the input at all. Encoding is left to libvterm, which follows the protocol the application selected (X10, UTF-8, SGR or rxvt) and stays silent while tracking is off. Note that mouse.c was already in the CMake source list, but nothing referenced it, so the linker dropped the archive member and the symbols were missing from the shipped library. Calling it from the JNI layer pulls it back in. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A full-screen application keeps its own scrollback and paints the whole screen, so scrolling the terminal's scrollback does nothing visible -- the gesture appears dead. Report the wheel instead when the application has enabled mouse tracking, and leave the local scrollback path untouched when it has not. WheelScroller converts continuous travel into detents at one detent per line, holding sub-detent travel between samples. Two details are load bearing: Reports go to a fixed anchor cell, the one the gesture started on. Chasing the finger would emit a motion report for every row crossed to an application tracking in MOVE mode (DECSET 1003), which Claude Code enables. Bursts are capped at 8 detents per sample. A fast fling covers dozens of lines between animation frames, and applications commonly throttle a flood of wheel reports, so sending every detent can scroll less far than sending a few. Dropped detents are discarded rather than queued. The fling decays a scratch offset on the same spline the local scrollback uses, so a fling feels the same whichever path it takes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The title was read from property 7 and the cursor shape from property 6, but vterm.h numbers TITLE as 4 and CURSORSHAPE as 7 (6 is REVERSE). Both paths were therefore dead: OSC 0/2 never reached terminalTitle, and DECSCUSR never changed the cursor shape. Replace the magic numbers with a VTermProp object mirroring the header. These are ordinals of an unnumbered C enum, so a property inserted upstream shifts everything after it -- naming them makes the next libvterm bump a readable diff rather than a silent misread. Adds coverage for OSC 0, OSC 2 and DECSCUSR shape and blink, which are the observable ends of the two properties that were broken. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
vterm_state_reset() zeroed state->mouse_flags directly and re-emitted only the cursor properties, so RIS (ESC c) and DECSTR (CSI ! p) stopped mouse reporting without notifying anyone. A terminal mirroring VTERM_PROP_MOUSE was left believing the application still wanted the mouse. The consequence was worse than a stale flag. With tracking apparently on, every scroll gesture built a WheelScroller and fed reports into a vterm that silently dropped them, while the local scrollback branch never ran -- so scrolling did nothing at all. Recovery needed some later program to set and then clear mouse mode. `reset` after a full-screen application dies is exactly the situation likely to have tracking on in the first place. Route the clear through settermprop_int so the callback fires. The direct assignment stays: vterm_state_set_termprop() skips its store when the embedder's callback returns falsy, and the flags must be cleared either way. Covers RIS, DECSTR, and that a reset terminal emits nothing regardless of what the mirrored mode says. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The local scroll path's decay is clamped to the scrollback it actually has. The wheel path decayed an unbounded scratch Animatable, because the terminal cannot know where the application's own history ends -- so a hard fling kept emitting detents long after the application had hit its top. Bound the animation with Animatable.updateBounds() at a travel the scroller derives from a detent cap, well above what an ordinary fling covers. Expressing it as a bound on the animation rather than a counter in WheelScroller keeps it out of the per-gesture state that would then need resetting. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mouseButton() and scrollWheel() drove the position and the button reports through separate JNI calls, each taking mLock on its own. libvterm carries the position recorded by the move into the button report, so a concurrent report for another gesture could land between the two and send a button at the wrong cell. Only the UI thread calls these today, but the pairing is an invariant of the API rather than of its current callers. Move both into Terminal.cpp so each pair, and a whole wheel burst, is emitted under a single lock. While here, reorder mouseButton() to (button, row, col, pressed): the coordinates now read the same way as in mouseMove() and scrollWheel() rather than being split by the press flag. Drop propertyChanged from the VTERM_PROP_MOUSE branch. The value is read straight off the volatile field and never appears in the snapshot, so the rebuild it requested could only ever produce an identical one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lib/api.txt was never regenerated for the mouse API, so metalavaCheckCompatibilityRelease -- which :lib:check depends on -- failed with four AddedAbstractMethod errors. Regenerate it, and note the new entry point in the library README. WheelScrollGestureTest asserted only that some wheel report appeared by the end of the gesture, which a fling alone would have satisfied. Sample before the finger lifts so the drag's own reports are what is being measured, and assert the same for the local path. The fling itself stays uncovered at this level: injected touch input does not carry enough velocity through the Robolectric harness for a decay animation to run, on the wheel path or the local one. Recorded on DragSamples so the next reader does not mistake it for a wheel-path bug; the fling's conversion and bound are covered in WheelScrollerTest. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A tap now reaches an application that asked for the mouse, through a new mouseClick() that emits the press and its release as one native operation so it cannot be left half-delivered. While tracking is on the hyperlink path is skipped, since the application drew whatever looks like a link and will handle the click itself. Long-press selection stays local, so copying text out of a full-screen program still works. Every mouse operation now positions the pointer through Terminal::positionMouseLocked(), which clamps row/col against mRows/mCols under mLock. libvterm's X10 encoder clamps only the high end, so a negative coordinate previously put a control byte on the wire; clamping natively also means a resize racing a gesture cannot slip past a bound the caller believed. The wheel burst is bounded in the same place, where the loop holding the lock actually is -- steps = Int.MAX_VALUE used to mean two billion reports under it. Narrow the public surface to mouseClick() and scrollWheel(). A bare press with no release leaves an application believing a button is held, and neither that nor bare motion is something a touch gesture produces. Both stay as internal members of TerminalEmulatorImpl, still tested, ready to be made public against a real caller if physical mouse support lands. reset_mouse_state() is now called by both vterm_state_new() and vterm_state_reset(), so creation and reset cannot drift. Upstream reset only mouse_flags, leaving the report encoding and any held button alive across a reset: after RIS an application enabling 1000h without 1006h got SGR reports it never asked for, and an unreleased press made 1002h report motion with nothing held. WheelScroller now owns its decay via fling(), driven by whatever MonotonicFrameClock is in context. The decay previously lived in the gesture handler, where injected touch input carries too little velocity for it to run at all, so the most intricate code here had no coverage. It is also where the detent budget belongs, being spent by reports rather than by distance. Fixing the seam fixed a bug behind it: the fling carries detents past the per-sample cap into later frames instead of dropping them, so its distance no longer depends on how many frames the device drew -- a drag still drops them, being a position rather than a distance. Make mouseTracking snapshot state rather than @volatile so reading it in a composition subscribes to it. Same visibility guarantee and same API shape, without an embedder having to poll. Each fix is pinned by a test that fails when it is reverted, checked one at a time. The step bound is the exception: removing it does not fail its test, it hangs the suite. Suite goes from 401 to 423, and ./gradlew build now passes including :test-app and lint. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
WheelScroller.report() inlined the direction into a duplicated multiply when consuming the residual. Lift the sign into one `up` value and use it for both the residual and the reported direction. The tap handler grew a second copy of the request-focus-then-notify pair when the mouse-tracking branch landed beside the hyperlink one. Hoist it into a local forwardTap(). Drop the coerceIn() on the wheel anchor. The emulator clamps coordinates against the size it holds under its own lock, so clamping again here only adds a second bound derived from a snapshot that can be staler than the authoritative one. Name the two DECSET sequences the mouse tests enable tracking with, rather than repeating the literals nineteen times between them. Which mode a test runs under is the interesting part and was previously spelled 1000h or 1003h at each site; expected output stays literal, since that is what the tests exist to pin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
libvterm has no output buffer installed, so every report it generates upcalls into Java on its own: a jbyteArray, a JNI call and a Handler message for six bytes. A keystroke produces one report and does not care. A wheel burst produces one per detent -- up to eight per touch-move frame while dragging, and two hundred across a fling -- and all of it lands on the main thread while it is also rendering. Add a scoped sink that termOutput() appends to instead of upcalling, and hold one open across the detent loop and across the press/release pair in mouseClick(). The bytes reaching the PTY are unchanged, being the same reports concatenated in the same order; only the number of trips changes. The sink is scoped so it cannot be left armed, is touched only under mLock, and is null everywhere else, so the keyboard path is untouched. Only the callback count can show this, the bytes being identical either way, so the two new tests assert on that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
setTermProp sent libvterm's raw VTermProp ordinal across JNI, and Kotlin decoded it against a hand-copied snapshot of an unnumbered upstream enum. That copy has already been wrong once: until 6ed03a6 the title was read at 7 and the cursor shape at 6, so OSC 0/2 and DECSCUSR silently did nothing. Naming the constants made the numbers legible but left the transcription, and mouse tracking now rides the same wire. Translate in Terminal.cpp instead, through a PropCode enum the wrapper owns, switching on VTERM_PROP_* by name where the compiler resolves them against the header. The switch has no default case, so a property added upstream draws a -Wswitch warning naming the enumerator rather than silently arriving in Java as something else. Kotlin's identifiers are unchanged in value but now belong to this repo rather than to vterm.h. Decide once per gesture whether the application owns the pointer. The scroll path resolved it at touch-slop crossing and the tap path re-read it at finger-up, so one question had two answers that could disagree within a single gesture if tracking changed in between. Read it at finger-down and have both consult that. Drop the steps < 1 guard duplicated in TerminalEmulatorImpl. Bounds on steps belong with the loop that spends them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
libvterm hands a string-valued property over in fragments, one per input buffer, because a title can straddle a read. invokeSetTermProp() forwarded each fragment to Java on its own, where setTermProp assigns rather than appends, so the last fragment won. The sequence also ends on a fragment that is frequently empty, which passed the str null check and arrived as an empty string. Two ways for a title to be wrong, both confirmed against the old code: ESC ]0; "hel" + "lo" BEL -> "lo" ESC ]0; "hello" + BEL -> "" Dormant until 6ed03a6, which corrected the property identifier and made the title path live for the first time. Blanking a title an embedder is rendering is worse than the nothing that shipped before. Accumulate in Terminal.cpp and deliver once, on the final fragment, the same shape termOscFallback() and termSelectionSet() already use for the same reason. Buffers are keyed by property because OSC 0 sets the icon name and the title from one fragment, so two values are in flight and a single buffer would interleave them. Accumulation is bounded: the payload is remote input and nothing guarantees the terminator arrives. Excess is dropped rather than the value abandoned, an over-long title still being worth showing truncated. Four of the five new tests fail against the old code. The two existing title tests are unchanged and always passed -- they write the whole sequence in one call, so no fragment boundary ever falls inside it, which is why this went unnoticed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The vendored libvterm is no longer pristine: vterm_state_reset() was changed to clear the whole mouse state and to notify VTERM_PROP_MOUSE, so that a reset cannot leave a stale report encoding or a held button behind, and cannot leave this library believing an application still wants the mouse. There is no supported alternative -- libvterm exposes neither a reset callback nor a getter for that property -- but nothing recorded the divergence. That is the dangerous part. CMake compiles the vendored sources directly, so a libvterm bump overwrites them without failing the build. The change would disappear silently and the symptom would be a behavioural regression: after RIS or DECSTR, scroll gestures routed to an application that stopped listening, with the local scrollback not reached either. Keep the change applied in tree, since that is what gets compiled, and add it as a patch under libvterm-patches/ with a lib/README.md section naming the divergence and the tests that cover it. The patch header explains the upstream bug so it can be sent on; it has not been submitted yet. state.c points back at the patch, so a reader arriving from either direction finds the other. Verified by round trip: state.c reverted to pristine at the merge base, the patch applied forward, result byte-identical to the tree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Human header
I wanted to be able to finger-scroll in full-screen apps like Claude Code; this PR wires up those gestures when fullscreen mode is requested. Tested by hand and code reviewed until convergence.
-BMA
Why
A finger-scroll gesture does nothing in a full-screen application. Programs that take over the screen — vim, tmux, Claude Code's flicker-free renderer — keep their own scrollback and repaint everything, so scrolling our scrollback moves a buffer nothing is looking at. The gesture just appears dead.
Those programs ask for the input instead: they enable mouse tracking with DECSET 1000/1002/1003 and handle the wheel themselves. We never had a way to send it to them.
What this does
Three layers, plus two things found on the way.
Reporting mouse input —
feat(mouse): report mouse input to applications that request it, and the commits that tighten itExposes libvterm's mouse reporting through JNI and adds
mouseClick/scrollWheeltoTerminalEmulator, plus amouseTrackingproperty reflectingVTERM_PROP_MOUSEso callers can tell whether the application wants the input at all. Motion and bare button presses stay internal: both are only meaningful for input that hovers or holds a button across events, neither of which a touch gesture produces, and both are easy to leave half-delivered. A click is always emitted with its release.Encoding is left entirely to libvterm, which follows whichever protocol the application selected (X10, UTF-8, SGR, rxvt) and stays silent while tracking is off — so a caller cannot leak input into a shell that never asked for it. Coordinates are clamped natively, against the size the terminal holds under its own lock; libvterm's X10 encoder clamps only the high end and would otherwise put a control byte on the wire for a negative coordinate. A wheel burst is bounded and delivered in a single upcall rather than one per detent.
One thing worth knowing:
mouse.cwas already in the CMake source list, but nothing referenced it, so the linker dropped the archive member andvterm_mouse_button/vterm_mouse_movewere simply missing from the shipped.so. Calling them from JNI pulls them back in; no build change was needed.Routing the gesture —
feat(mouse): route scroll gestures to applications tracking the mouseForks the existing scroll branch in
Terminal.ktonmouseTracking.isEnabled, decided once at finger-down so a gesture cannot change owner halfway through. When tracking is on,scrollOffsetandscreenStateare left completely alone and travel is converted to wheel detents; when it is off, the existing local path runs unchanged. A tap forks the same way — to the application as a click, or through the existing hyperlink check. Long-press selection stays local in both cases, since it remains the way to copy text out of a full-screen application.WheelScrollerdoes the conversion. Two decisions in it are load bearing:The other tunable is one detent per line of finger travel, which makes content track the finger for an application that scrolls a line per detent.
Property identifiers, and the bug underneath them —
fix(props)× 3A drive-by, found while working in
setTermProp. The title was read from property 7 and the cursor shape from property 6, butvterm.hnumbersTITLEas 4 andCURSORSHAPEas 7 (6 isREVERSE). Both paths were dead: OSC 0/2 never reachedterminalTitle, and DECSCUSR never changed the cursor shape.VTermPropis an unnumbered C enum, so a property inserted upstream shifts everything after it — presumably how the drift happened. Rather than re-copy the numbering,Terminal.cppnow translates by name into aPropCodeenum the wrapper owns, where the compiler checks the cases against the header, and Java holds identifiers this repo defines. The switch has no default case on purpose, so a property added upstream draws a-Wswitchwarning naming the enumerator instead of silently arriving as something else.Making the title path live exposed a second bug sitting under it. libvterm hands a string property over in fragments, one per input buffer, because a title can straddle a read — and
invokeSetTermPropforwarded each fragment on its own, wheresetTermPropassigns rather than appends. So the last fragment won:The second is the worse one — the sequence ends on an empty fragment, which blanked the title outright. Fragments are now accumulated and delivered once, on the final one, the same shape
termOscFallbackandtermSelectionSetalready used. Buffers are keyed by property, because OSC 0 sets the icon name and the title from the same fragment and one buffer would interleave them, and bounded, because the payload is remote input and nothing guarantees the terminator arrives.Happy to split the whole
fix(props)thread into its own PR if you would rather keep it separate — it stands on its own, and it has grown since I first offered.The vendored libvterm is no longer pristine —
fix(mouse): tell the embedder when a reset clears mouse tracking, tracked bybuild(libvterm): ...vterm_state_reset()clearedmouse_flagsbut left the negotiated report encoding and any held button behind, and switched reporting off without aVTERM_PROP_MOUSEcallback. That last part matters here: a terminal mirroring the property keeps believing an application wants the mouse and routes gestures into a vterm that silently drops them. After RIS — which is what a user types to unwedge a terminal when a full-screen application dies without restoring its modes — scrolling would be dead in both directions at once. libvterm exposes neither a reset callback nor a getter for the property, so there is no wrapper-side alternative.Since CMake compiles the vendored sources directly, a version bump would overwrite the change without failing the build, and the symptom would be a behavioural regression rather than an error. So it is also kept as a patch under
lib/src/main/cpp/libvterm-patches/, with a section inlib/README.mdnaming the divergence and the tests that cover it, andstate.cpoints back at the patch. Verified by round trip: reverted to pristine at the merge base, applied forward, byte-identical to the tree. Not yet submitted upstream — it is a genuine libvterm bug and should go back.Testing
Suite is at 430 tests, all passing.
MouseReportingTest(29) — DECSET mode detection, SGR and X10 wheel encoding, direction and modifier and step encoding, button press/release pairing, motion suppression, coordinate clamping, reset clearing the encoding and held buttons, and burst coalescing asserted on callback count. Includes the literal1000h 1002h 1003h 1006hsequence Claude Code sends.WheelScrollerTest(18) — detent conversion, sub-detent accumulation, reversal cancelling held travel, the rate limit and its no-backlog property, non-finite samples, silence when tracking is off, and a fling driven from a deterministic frame clock so the same fling at 8ms and 48ms frame intervals travels the same distance.WheelScrollGestureTest(6) — drives the real Compose gesture pipeline through Robolectric. A downward drag produces wheel-up reports and leavesscrollbackPositionuntouched; with tracking off it produces zero mouse reports and moves local scrollback as before; a tap arrives as one complete click and never leaves a button held.CursorAndModeEscapeTest(20, nine new) — OSC 0, OSC 2, DECSCUSR shape and blink, all failing against the old property constants; plus five for fragment reassembly, four of which fail against the pre-fix wrapper.Verified end to end on a physical device: ConnectBot built against this library, SSH into a host running Claude Code with the fullscreen renderer, and the transcript scrolls under a finger drag.
Notes for review
CI is green. The full
./gradlew build --no-configuration-cachepasses on my fork on JDK 17 —:liband:test-appunit tests,lint, and both metalava compatibility checks (run). I ranspotlessChecklocally as well. One thing worth knowing if you build this locally: every local Gradle invocation reportsBUILD FAILEDfrom the pre-existing configuration-cache incompatibility in thenet.researchgate.releaseplugin, with or without this branch — which is presumably why CI passes--no-configuration-cache.The two constants in
WheelScroller.ktare feel choices —LINES_PER_WHEEL_DETENTandMAX_DETENTS_PER_SAMPLE. They felt right on a Pixel 7 Pro against Claude Code; they may want adjusting against other applications.Behaviour change to be aware of: the title property now actually flows into
TerminalSnapshot.terminalTitlefor the first time. Anything rendering that will start seeing it change where it previously sat static.One motion report per gesture under DECSET 1003. Every wheel call positions the pointer first, so the first report of a gesture moves it from wherever it last sat to the anchor, which an application tracking all motion sees. One report at the start of a gesture, not one per row crossed — but it is not zero, and the docs could be read as promising zero.
Open question, not addressed here: while tracking is on, local scrollback is unreachable by gesture. Under a fullscreen renderer that is correct, since the local scrollback is empty — but for something like
lessthere is an argument for an escape hatch, such as reserving a two-finger drag for local scrollback. Left out deliberately as a separate decision.There is a reachable stuck state inside that: scroll back locally, then have the application enable tracking, and the offset is frozen where it stands — the wheel path never touches it and the auto-scroll-to-bottom effect only fires at
scrollbackPosition == 0. A cheap mitigation independent of the escape-hatch decision is to snap local scrollback to the bottom once when a gesture classifies as a scroll and the application owns the pointer. Say the word and I will add it here rather than leave it for the follow-up.🤖 Generated with Claude Code