From 71c329047638450ab4bff2836753315a0ed38e33 Mon Sep 17 00:00:00 2001 From: Brandon Martin-Anderson Date: Fri, 24 Jul 2026 18:18:58 -0700 Subject: [PATCH 01/13] feat(mouse): report mouse input to applications that request it 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 --- lib/src/main/cpp/Terminal.cpp | 61 +++- lib/src/main/cpp/Terminal.h | 6 + .../org/connectbot/terminal/MouseInput.kt | 70 ++++ .../connectbot/terminal/TerminalEmulator.kt | 130 +++++++- .../org/connectbot/terminal/TerminalNative.kt | 40 +++ .../connectbot/terminal/MouseReportingTest.kt | 306 ++++++++++++++++++ 6 files changed, 591 insertions(+), 22 deletions(-) create mode 100644 lib/src/main/java/org/connectbot/terminal/MouseInput.kt create mode 100644 lib/src/test/java/org/connectbot/terminal/MouseReportingTest.kt diff --git a/lib/src/main/cpp/Terminal.cpp b/lib/src/main/cpp/Terminal.cpp index b727c1b1..16b13fbb 100644 --- a/lib/src/main/cpp/Terminal.cpp +++ b/lib/src/main/cpp/Terminal.cpp @@ -395,6 +395,14 @@ int Terminal::setDefaultColors(uint32_t fgColor, uint32_t bgColor) { } // Keyboard input handlers +static VTermModifier toVTermModifier(int modifiers) { + VTermModifier mod = VTERM_MOD_NONE; + if (modifiers & 1) mod = (VTermModifier)(mod | VTERM_MOD_SHIFT); + if (modifiers & 2) mod = (VTermModifier)(mod | VTERM_MOD_ALT); + if (modifiers & 4) mod = (VTermModifier)(mod | VTERM_MOD_CTRL); + return mod; +} + bool Terminal::dispatchKey(int modifiers, int key) { std::scoped_lock lock(mLock); @@ -402,12 +410,7 @@ bool Terminal::dispatchKey(int modifiers, int key) { return false; } - VTermModifier mod = VTERM_MOD_NONE; - if (modifiers & 1) mod = (VTermModifier)(mod | VTERM_MOD_SHIFT); - if (modifiers & 2) mod = (VTermModifier)(mod | VTERM_MOD_ALT); - if (modifiers & 4) mod = (VTermModifier)(mod | VTERM_MOD_CTRL); - - vterm_keyboard_key(mVt, (VTermKey)key, mod); + vterm_keyboard_key(mVt, (VTermKey)key, toVTermModifier(modifiers)); return true; } @@ -418,12 +421,33 @@ bool Terminal::dispatchCharacter(int modifiers, int codepoint) { return false; } - VTermModifier mod = VTERM_MOD_NONE; - if (modifiers & 1) mod = (VTermModifier)(mod | VTERM_MOD_SHIFT); - if (modifiers & 2) mod = (VTermModifier)(mod | VTERM_MOD_ALT); - if (modifiers & 4) mod = (VTermModifier)(mod | VTERM_MOD_CTRL); + vterm_keyboard_unichar(mVt, codepoint, toVTermModifier(modifiers)); + return true; +} + +// Mouse input handlers +bool Terminal::mouseMove(int row, int col, int modifiers) { + std::scoped_lock lock(mLock); + + if (!mVt) { + return false; + } + + // libvterm only emits a report here when the application asked for drag or + // motion tracking; otherwise this just records the position that a + // subsequent mouseButton() report will carry. + vterm_mouse_move(mVt, row, col, toVTermModifier(modifiers)); + return true; +} + +bool Terminal::mouseButton(int button, bool pressed, int modifiers) { + std::scoped_lock lock(mLock); + + if (!mVt) { + return false; + } - vterm_keyboard_unichar(mVt, codepoint, mod); + vterm_mouse_button(mVt, button, pressed, toVTermModifier(modifiers)); return true; } @@ -1224,6 +1248,21 @@ Java_org_connectbot_terminal_TerminalNative_nativeDispatchCharacter(JNIEnv* /* e return term->dispatchCharacter(modifiers, character); } +JNIEXPORT jboolean JNICALL +Java_org_connectbot_terminal_TerminalNative_nativeMouseMove(JNIEnv* /* env */, jobject /* thiz */, + jlong ptr, jint row, jint col, jint modifiers) { + auto* term = reinterpret_cast(ptr); + return term->mouseMove(row, col, modifiers); +} + +JNIEXPORT jboolean JNICALL +Java_org_connectbot_terminal_TerminalNative_nativeMouseButton(JNIEnv* /* env */, jobject /* thiz */, + jlong ptr, jint button, jboolean pressed, + jint modifiers) { + auto* term = reinterpret_cast(ptr); + return term->mouseButton(button, pressed, modifiers); +} + JNIEXPORT jint JNICALL Java_org_connectbot_terminal_TerminalNative_nativeGetCellRun(JNIEnv* env, jobject /* thiz */, jlong ptr, jint row, jint col, jobject runObject) { diff --git a/lib/src/main/cpp/Terminal.h b/lib/src/main/cpp/Terminal.h index 03010d9d..69978cd3 100644 --- a/lib/src/main/cpp/Terminal.h +++ b/lib/src/main/cpp/Terminal.h @@ -61,6 +61,12 @@ class Terminal { bool dispatchKey(int modifiers, int key); bool dispatchCharacter(int modifiers, int codepoint); + // Mouse input - generates escape sequences only when the application has + // requested mouse tracking (DECSET 1000/1002/1003). Encoding follows the + // protocol the application selected (X10, UTF-8, SGR or rxvt). + bool mouseMove(int row, int col, int modifiers); + bool mouseButton(int button, bool pressed, int modifiers); + // Cell data retrieval for rendering int getCellRun(JNIEnv* env, int row, int col, jobject runObject); diff --git a/lib/src/main/java/org/connectbot/terminal/MouseInput.kt b/lib/src/main/java/org/connectbot/terminal/MouseInput.kt new file mode 100644 index 00000000..f54a82f3 --- /dev/null +++ b/lib/src/main/java/org/connectbot/terminal/MouseInput.kt @@ -0,0 +1,70 @@ +/* + * ConnectBot Terminal + * Copyright 2026 Kenny Root + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.connectbot.terminal + +/** + * The level of mouse reporting the application running in the terminal has + * asked for, mirroring libvterm's `VTERM_PROP_MOUSE` values. + * + * Applications request this with DECSET; the terminal must not send mouse + * reports until they do. Full-screen programs such as vim, tmux and Claude + * Code's alternate-screen renderer enable it so they can handle scrolling and + * clicks themselves instead of relying on the terminal's own scrollback. + * + * Note that the modes are not additive in the way the escape sequences suggest: + * whichever mode was enabled last wins. Every mode other than [NONE] reports + * button presses, which includes the wheel. + */ +enum class MouseTracking { + /** No reporting. The terminal should handle gestures locally. */ + NONE, + + /** DECSET 1000: button press and release only. */ + CLICK, + + /** DECSET 1002: button events, plus motion while a button is held. */ + DRAG, + + /** DECSET 1003: button events, plus all motion whether or not a button is held. */ + MOVE, + + ; + + /** Whether the application wants mouse reports at all. */ + val isEnabled: Boolean get() = this != NONE +} + +/** + * A physical mouse button, excluding the wheel. Use + * [TerminalEmulator.scrollWheel] for wheel input. + */ +enum class MouseButton(internal val code: Int) { + LEFT(1), + MIDDLE(2), + RIGHT(3), +} + +/** + * A wheel detent direction. Terminals report wheel input as presses of buttons + * 4 through 7; there is no matching release. + */ +enum class WheelDirection(internal val code: Int) { + UP(4), + DOWN(5), + LEFT(6), + RIGHT(7), +} diff --git a/lib/src/main/java/org/connectbot/terminal/TerminalEmulator.kt b/lib/src/main/java/org/connectbot/terminal/TerminalEmulator.kt index 0f4275e1..4c2e0b06 100644 --- a/lib/src/main/java/org/connectbot/terminal/TerminalEmulator.kt +++ b/lib/src/main/java/org/connectbot/terminal/TerminalEmulator.kt @@ -108,6 +108,59 @@ sealed interface TerminalEmulator { */ fun dispatchCharacter(modifiers: Int, codepoint: Int) + /** + * The level of mouse reporting the running application has requested. + * + * While this is [MouseTracking.NONE] the mouse methods below produce no + * output, and gestures should be handled locally (scrollback, selection). + * Once an application enables tracking it expects to receive the events + * itself — a full-screen program keeps its own scrollback, so scrolling the + * terminal's copy would do nothing useful. + */ + val mouseTracking: MouseTracking + + /** + * Report the mouse moving to a cell. + * + * A motion report is only emitted when the application asked for + * [MouseTracking.DRAG] (and a button is held) or [MouseTracking.MOVE]. + * Moving to the cell the mouse already occupies is a no-op, so this is safe + * to call for every pointer sample. + * + * @param row Row index (0-based) within the visible screen + * @param col Column index (0-based) within the visible screen + * @param modifiers Bitmask: 1=Shift, 2=Alt, 4=Ctrl + */ + fun mouseMove(row: Int, col: Int, modifiers: Int = 0) + + /** + * Report a mouse button press or release at a cell. + * + * Each press must be paired with a release; applications track button state + * and a dropped release leaves them believing the button is still down. + * + * @param row Row index (0-based) within the visible screen + * @param col Column index (0-based) within the visible screen + * @param modifiers Bitmask: 1=Shift, 2=Alt, 4=Ctrl + */ + fun mouseButton(button: MouseButton, pressed: Boolean, row: Int, col: Int, modifiers: Int = 0) + + /** + * Report [steps] wheel detents at a cell. + * + * This is what lets a scroll gesture reach an application that has taken + * over the screen. Callers converting a continuous gesture into detents + * should rate-limit: applications commonly throttle or coalesce a flood of + * wheel events, so a fling turned into hundreds of detents scrolls less far + * than the same distance delivered as a few dozen. + * + * @param row Row index (0-based) within the visible screen + * @param col Column index (0-based) within the visible screen + * @param steps Number of detents to report; values below 1 send nothing + * @param modifiers Bitmask: 1=Shift, 2=Alt, 4=Ctrl + */ + fun scrollWheel(direction: WheelDirection, row: Int, col: Int, steps: Int = 1, modifiers: Int = 0) + /** * Clears the terminal emulator screen. */ @@ -343,6 +396,12 @@ internal class TerminalEmulatorImpl( private var terminalTitle = "" private var isAltScreenActive = false + // Read outside damageLock by gesture handling on the UI thread, written + // from the native callback thread. + @Volatile + override var mouseTracking: MouseTracking = MouseTracking.NONE + private set + // Scrollback buffer private val scrollback = mutableListOf() private val maxScrollbackLines = 1000 @@ -448,6 +507,35 @@ internal class TerminalEmulatorImpl( terminalNative.dispatchCharacter(modifiers, codepoint) } + /** + * Report the mouse moving to a cell. + */ + override fun mouseMove(row: Int, col: Int, modifiers: Int) { + terminalNative.mouseMove(row, col, modifiers) + } + + /** + * Report a mouse button press or release at a cell. + */ + override fun mouseButton(button: MouseButton, pressed: Boolean, row: Int, col: Int, modifiers: Int) { + terminalNative.mouseMove(row, col, modifiers) + terminalNative.mouseButton(button.code, pressed, modifiers) + } + + /** + * Report wheel detents at a cell. + */ + override fun scrollWheel(direction: WheelDirection, row: Int, col: Int, steps: Int, modifiers: Int) { + if (steps < 1) return + + terminalNative.mouseMove(row, col, modifiers) + repeat(steps) { + // Wheel buttons report a press with no matching release; libvterm + // emits one report per call. + terminalNative.mouseButton(direction.code, true, modifiers) + } + } + /** * Clears the terminal emulator screen. */ @@ -609,21 +697,41 @@ internal class TerminalEmulatorImpl( } is TerminalProperty.IntValue -> { - // Property 6 is VTERM_PROP_CURSORSHAPE (from vterm.h line 260) - if (prop == 6) { - cursorShape = when (value.value) { - 1 -> CursorShape.BLOCK + when (prop) { + // Property 6 is VTERM_PROP_CURSORSHAPE (from vterm.h line 260) + 6 -> { + cursorShape = when (value.value) { + 1 -> CursorShape.BLOCK - // VTERM_PROP_CURSORSHAPE_BLOCK - 2 -> CursorShape.UNDERLINE + // VTERM_PROP_CURSORSHAPE_BLOCK + 2 -> CursorShape.UNDERLINE - // VTERM_PROP_CURSORSHAPE_UNDERLINE - 3 -> CursorShape.BAR_LEFT + // VTERM_PROP_CURSORSHAPE_UNDERLINE + 3 -> CursorShape.BAR_LEFT - // VTERM_PROP_CURSORSHAPE_BAR_LEFT - else -> CursorShape.BLOCK + // VTERM_PROP_CURSORSHAPE_BAR_LEFT + else -> CursorShape.BLOCK + } + propertyChanged = true + } + + // Property 8 is VTERM_PROP_MOUSE (from vterm.h line 261) + 8 -> { + mouseTracking = when (value.value) { + // VTERM_PROP_MOUSE_CLICK + 1 -> MouseTracking.CLICK + + // VTERM_PROP_MOUSE_DRAG + 2 -> MouseTracking.DRAG + + // VTERM_PROP_MOUSE_MOVE + 3 -> MouseTracking.MOVE + + // VTERM_PROP_MOUSE_NONE + else -> MouseTracking.NONE + } + propertyChanged = true } - propertyChanged = true } } diff --git a/lib/src/main/java/org/connectbot/terminal/TerminalNative.kt b/lib/src/main/java/org/connectbot/terminal/TerminalNative.kt index 4aa9178e..ad509a2a 100644 --- a/lib/src/main/java/org/connectbot/terminal/TerminalNative.kt +++ b/lib/src/main/java/org/connectbot/terminal/TerminalNative.kt @@ -108,6 +108,44 @@ internal class TerminalNative(callbacks: TerminalCallbacks) : AutoCloseable { return nativeDispatchCharacter(nativePtr, modifiers, character) } + /** + * Move the mouse cursor to a cell. + * + * Records the position used by subsequent [mouseButton] reports. A motion + * report is emitted via onKeyboardInput() only when the application has + * requested drag tracking (DECSET 1002, while a button is held) or any-motion + * tracking (DECSET 1003). Moving to the cell the mouse already occupies is a + * no-op, so repeated calls at the same cell do not flood the application. + * + * @param row Row index (0-based) + * @param col Column index (0-based) + * @param modifiers Bitmask: 1=Shift, 2=Alt, 4=Ctrl + * @return true if handled + */ + fun mouseMove(row: Int, col: Int, modifiers: Int): Boolean { + checkNotClosed() + return nativeMouseMove(nativePtr, row, col, modifiers) + } + + /** + * Dispatch a mouse button press or release at the current mouse position. + * + * Nothing is emitted unless the application has enabled mouse tracking. The + * report encoding follows the protocol the application selected (X10, UTF-8, + * SGR or rxvt). + * + * @param button 1=left, 2=middle, 3=right, 4=wheel up, 5=wheel down, + * 6=wheel left, 7=wheel right + * @param pressed true for press, false for release. Wheel buttons only + * report presses; a release is not expected. + * @param modifiers Bitmask: 1=Shift, 2=Alt, 4=Ctrl + * @return true if handled + */ + fun mouseButton(button: Int, pressed: Boolean, modifiers: Int): Boolean { + checkNotClosed() + return nativeMouseButton(nativePtr, button, pressed, modifiers) + } + /** * Get a run of cells with identical formatting starting at the given position. * This is the primary method for retrieving terminal content for rendering. @@ -213,6 +251,8 @@ internal class TerminalNative(callbacks: TerminalCallbacks) : AutoCloseable { private external fun nativeResize(ptr: Long, rows: Int, cols: Int): Int private external fun nativeDispatchKey(ptr: Long, modifiers: Int, key: Int): Boolean private external fun nativeDispatchCharacter(ptr: Long, modifiers: Int, character: Int): Boolean + private external fun nativeMouseMove(ptr: Long, row: Int, col: Int, modifiers: Int): Boolean + private external fun nativeMouseButton(ptr: Long, button: Int, pressed: Boolean, modifiers: Int): Boolean private external fun nativeGetCellRun(ptr: Long, row: Int, col: Int, run: CellRun): Int private external fun nativeSetPaletteColors(ptr: Long, colors: IntArray, count: Int): Int private external fun nativeSetDefaultColors(ptr: Long, fgColor: Int, bgColor: Int): Int diff --git a/lib/src/test/java/org/connectbot/terminal/MouseReportingTest.kt b/lib/src/test/java/org/connectbot/terminal/MouseReportingTest.kt new file mode 100644 index 00000000..cb005409 --- /dev/null +++ b/lib/src/test/java/org/connectbot/terminal/MouseReportingTest.kt @@ -0,0 +1,306 @@ +/* + * ConnectBot Terminal + * Copyright 2026 Kenny Root + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.connectbot.terminal + +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith + +/** + * Tests for mouse reporting: detecting the tracking mode an application asks + * for via DECSET, and encoding the reports we send back. + * + * The escape sequences here are the ones a full-screen application actually + * emits. Claude Code's alternate-screen renderer, for example, sends + * `1000h 1002h 1003h 1006h` on startup and expects SGR wheel reports in return; + * without them a scroll gesture has nowhere to go, because the application + * keeps its own scrollback rather than the terminal's. + */ +@RunWith(AndroidJUnit4::class) +class MouseReportingTest { + + /** + * Collects everything the emulator would write back to the PTY. + * + * The emulator posts keyboard output to its Looper rather than delivering it + * on the calling thread, so both reading and clearing drain pending work + * first — otherwise a report sent before a [clear] would land after it. + */ + private class Output { + private val sb = StringBuilder() + + val text: String + get() { + drain() + return sb.toString() + } + + fun append(data: ByteArray) { + sb.append(String(data, Charsets.ISO_8859_1)) + } + + fun clear() { + drain() + sb.setLength(0) + } + + private fun drain() = InstrumentationRegistry.getInstrumentation().waitForIdleSync() + } + + private fun emulator(out: Output): TerminalEmulator = TerminalEmulatorFactory.create( + initialRows = 24, + initialCols = 80, + onKeyboardInput = { out.append(it) }, + ) + + private fun TerminalEmulator.send(s: String) = writeInput(s.toByteArray()) + + // ----------------------------------------------------------------------- + // Tracking mode detection (VTERM_PROP_MOUSE) + // ----------------------------------------------------------------------- + + @Test + fun testTrackingDefaultsToNone() = runBlocking { + assertEquals(MouseTracking.NONE, emulator(Output()).mouseTracking) + } + + @Test + fun testDecsetSelectsTrackingMode() = runBlocking { + val term = emulator(Output()) + + term.send("\u001B[?1000h") + assertEquals("DECSET 1000", MouseTracking.CLICK, term.mouseTracking) + + term.send("\u001B[?1002h") + assertEquals("DECSET 1002", MouseTracking.DRAG, term.mouseTracking) + + term.send("\u001B[?1003h") + assertEquals("DECSET 1003", MouseTracking.MOVE, term.mouseTracking) + + term.send("\u001B[?1003l") + assertEquals("DECRST 1003", MouseTracking.NONE, term.mouseTracking) + } + + @Test + fun testClaudeCodeStartupSequenceEnablesTracking() = runBlocking { + // The exact sequence Claude Code's flicker-free renderer emits. + val term = emulator(Output()) + term.send("\u001B[?1000h\u001B[?1002h\u001B[?1003h\u001B[?1006h") + + assertEquals(MouseTracking.MOVE, term.mouseTracking) + assertTrue(term.mouseTracking.isEnabled) + } + + // ----------------------------------------------------------------------- + // Wheel reporting + // ----------------------------------------------------------------------- + + @Test + fun testNoReportsWhileTrackingDisabled() = runBlocking { + val out = Output() + val term = emulator(out) + + term.scrollWheel(WheelDirection.UP, row = 3, col = 5) + term.mouseButton(MouseButton.LEFT, pressed = true, row = 3, col = 5) + term.mouseMove(row = 4, col = 6) + + assertEquals("", out.text) + } + + @Test + fun testSgrWheelEncoding() = runBlocking { + val out = Output() + val term = emulator(out) + + // SGR (1006) is what every modern application selects, because X10 + // cannot address columns beyond 223. + term.send("\u001B[?1000h\u001B[?1006h") + out.clear() + + term.scrollWheel(WheelDirection.UP, row = 9, col = 19) + + // Button 64 = wheel up; coordinates are 1-based in the report. + assertEquals("\u001B[<64;20;10M", out.text) + } + + @Test + fun testSgrWheelDownAndHorizontal() = runBlocking { + val out = Output() + val term = emulator(out) + term.send("\u001B[?1000h\u001B[?1006h") + + out.clear() + term.scrollWheel(WheelDirection.DOWN, row = 0, col = 0) + assertEquals("wheel down", "\u001B[<65;1;1M", out.text) + + out.clear() + term.scrollWheel(WheelDirection.LEFT, row = 0, col = 0) + assertEquals("wheel left", "\u001B[<66;1;1M", out.text) + + out.clear() + term.scrollWheel(WheelDirection.RIGHT, row = 0, col = 0) + assertEquals("wheel right", "\u001B[<67;1;1M", out.text) + } + + @Test + fun testMultipleStepsSendOneReportEach() = runBlocking { + val out = Output() + val term = emulator(out) + term.send("\u001B[?1000h\u001B[?1006h") + out.clear() + + term.scrollWheel(WheelDirection.DOWN, row = 0, col = 0, steps = 3) + + assertEquals("\u001B[<65;1;1M".repeat(3), out.text) + } + + @Test + fun testNonPositiveStepsSendNothing() = runBlocking { + val out = Output() + val term = emulator(out) + term.send("\u001B[?1000h\u001B[?1006h") + out.clear() + + term.scrollWheel(WheelDirection.DOWN, row = 0, col = 0, steps = 0) + term.scrollWheel(WheelDirection.DOWN, row = 0, col = 0, steps = -2) + + assertEquals("", out.text) + } + + @Test + fun testWheelModifiersAreEncoded() = runBlocking { + val out = Output() + val term = emulator(out) + term.send("\u001B[?1000h\u001B[?1006h") + out.clear() + + // Modifier bits are shifted left by 2 in the report: shift=4, alt=8, + // ctrl=16. Ctrl+wheel-up is 64|16 = 80. + term.scrollWheel(WheelDirection.UP, row = 0, col = 0, modifiers = 4) + + assertEquals("\u001B[<80;1;1M", out.text) + } + + @Test + fun testX10WheelEncodingWhenSgrNotRequested() = runBlocking { + val out = Output() + val term = emulator(out) + + // Tracking without an encoding request leaves the legacy X10 encoding. + term.send("\u001B[?1000h") + out.clear() + + term.scrollWheel(WheelDirection.UP, row = 0, col = 0) + + // CSI M, then (code|mods)+0x20, col+0x21, row+0x21. + assertEquals("\u001B[M`!!", out.text) + } + + // ----------------------------------------------------------------------- + // Buttons and motion + // ----------------------------------------------------------------------- + + @Test + fun testButtonPressAndReleaseEncoding() = runBlocking { + val out = Output() + val term = emulator(out) + term.send("\u001B[?1000h\u001B[?1006h") + out.clear() + + term.mouseButton(MouseButton.LEFT, pressed = true, row = 2, col = 7) + term.mouseButton(MouseButton.LEFT, pressed = false, row = 2, col = 7) + + // Press ends in 'M', release in 'm'; left button is code 0. + assertEquals("\u001B[<0;8;3M\u001B[<0;8;3m", out.text) + } + + @Test + fun testMiddleAndRightButtonCodes() = runBlocking { + val out = Output() + val term = emulator(out) + term.send("\u001B[?1000h\u001B[?1006h") + + out.clear() + term.mouseButton(MouseButton.MIDDLE, pressed = true, row = 0, col = 0) + assertEquals("middle", "\u001B[<1;1;1M", out.text) + + out.clear() + term.mouseButton(MouseButton.RIGHT, pressed = true, row = 0, col = 0) + assertEquals("right", "\u001B[<2;1;1M", out.text) + } + + @Test + fun testClickTrackingDoesNotReportMotion() = runBlocking { + val out = Output() + val term = emulator(out) + term.send("\u001B[?1000h\u001B[?1006h") + out.clear() + + term.mouseMove(row = 5, col = 5) + term.mouseMove(row = 6, col = 7) + + assertEquals("", out.text) + } + + @Test + fun testMoveTrackingReportsMotion() = runBlocking { + val out = Output() + val term = emulator(out) + term.send("\u001B[?1003h\u001B[?1006h") + out.clear() + + term.mouseMove(row = 5, col = 9) + + // 32 is the motion bit; with no button held libvterm reports button 4. + assertEquals("\u001B[<35;10;6M", out.text) + } + + @Test + fun testRepeatedMoveToSameCellIsSuppressed() = runBlocking { + val out = Output() + val term = emulator(out) + term.send("\u001B[?1003h\u001B[?1006h") + out.clear() + + term.mouseMove(row = 5, col = 9) + val afterFirst = out.text + term.mouseMove(row = 5, col = 9) + + assertEquals("second move suppressed", afterFirst, out.text) + } + + @Test + fun testWheelDoesNotEmitMotionUnderMoveTracking() = runBlocking { + val out = Output() + val term = emulator(out) + term.send("\u001B[?1003h\u001B[?1006h") + + // Park the pointer where the gesture is happening, then scroll there. + term.mouseMove(row = 4, col = 4) + out.clear() + + term.scrollWheel(WheelDirection.UP, row = 4, col = 4, steps = 2) + + // Only the two wheel reports — the implicit move is a no-op because the + // pointer is already on that cell. + assertEquals("\u001B[<64;5;5M".repeat(2), out.text) + } +} From 90523ac5778d13a18ca56b969a9427edb642f358 Mon Sep 17 00:00:00 2001 From: Brandon Martin-Anderson Date: Fri, 24 Jul 2026 18:19:08 -0700 Subject: [PATCH 02/13] feat(mouse): route scroll gestures to applications tracking the mouse 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 --- .../java/org/connectbot/terminal/Terminal.kt | 96 ++++++--- .../org/connectbot/terminal/WheelScroller.kt | 90 +++++++++ .../terminal/WheelScrollGestureTest.kt | 184 ++++++++++++++++++ .../connectbot/terminal/WheelScrollerTest.kt | 182 +++++++++++++++++ 4 files changed, 527 insertions(+), 25 deletions(-) create mode 100644 lib/src/main/java/org/connectbot/terminal/WheelScroller.kt create mode 100644 lib/src/test/java/org/connectbot/terminal/WheelScrollGestureTest.kt create mode 100644 lib/src/test/java/org/connectbot/terminal/WheelScrollerTest.kt diff --git a/lib/src/main/java/org/connectbot/terminal/Terminal.kt b/lib/src/main/java/org/connectbot/terminal/Terminal.kt index bbf238d6..b86d0bdd 100644 --- a/lib/src/main/java/org/connectbot/terminal/Terminal.kt +++ b/lib/src/main/java/org/connectbot/terminal/Terminal.kt @@ -1078,6 +1078,11 @@ internal fun TerminalWithAccessibility( var panAccumulator = Offset.Zero var initialScrollOffset = 0f + // Set when the running application has taken over scrolling + // via mouse tracking; null means scroll our own scrollback. + var wheelScroller: WheelScroller? = null + var wheelPanY = 0f + // 4. Main event loop try { while (true) { @@ -1114,6 +1119,20 @@ internal fun TerminalWithAccessibility( isUserScrolling = true // Adjust initialScrollOffset so (initial + panAccumulator) matches current offset initialScrollOffset = scrollOffset.value - panAccumulator.y + // Hand the gesture to the application if it asked + // for mouse reporting; it owns the viewport then. + wheelScroller = if (terminalEmulator.mouseTracking.isEnabled) { + WheelScroller( + emulator = terminalEmulator, + lineHeightPx = baseCharHeight, + anchorRow = (down.position.y / baseCharHeight).toInt() + .coerceIn(0, screenState.snapshot.rows - 1), + anchorCol = (down.position.x / baseCharWidth).toInt() + .coerceIn(0, screenState.snapshot.cols - 1), + ) + } else { + null + } // Clear any active selection when scrolling starts if (selectionManager.mode != SelectionMode.NONE) { selectionManager.clearSelection() @@ -1145,24 +1164,35 @@ internal fun TerminalWithAccessibility( } GestureType.Scroll -> { - // Update scroll offset using total pan from the start of the gesture - // to avoid stuttering from stale scrollOffset.value. - val currentMaxScroll = - screenState.snapshot.scrollback.size * baseCharHeight - val newOffset = (initialScrollOffset + panAccumulator.y) - .coerceIn(0f, currentMaxScroll) - - // Cancel any ongoing scroll or fling and snap to the new position. - // Using launch with cancel ensures the latest snap always wins. - scrollJob?.cancel() - scrollJob = launch { - scrollOffset.snapTo(newOffset) - } + val scroller = wheelScroller + if (scroller != null) { + // The application scrolls itself; our own + // scrollback and offset stay put. Feed the + // delta since the last sample, taken from the + // accumulator so the travel that satisfied + // touch slop is not dropped. + scroller.scrollBy(panAccumulator.y - wheelPanY) + wheelPanY = panAccumulator.y + } else { + // Update scroll offset using total pan from the start of the gesture + // to avoid stuttering from stale scrollOffset.value. + val currentMaxScroll = + screenState.snapshot.scrollback.size * baseCharHeight + val newOffset = (initialScrollOffset + panAccumulator.y) + .coerceIn(0f, currentMaxScroll) + + // Cancel any ongoing scroll or fling and snap to the new position. + // Using launch with cancel ensures the latest snap always wins. + scrollJob?.cancel() + scrollJob = launch { + scrollOffset.snapTo(newOffset) + } - // Update terminal buffer scrollback position - val scrolledLines = - (newOffset / baseCharHeight).toInt() - screenState.scrollBy(scrolledLines - screenState.scrollbackPosition) + // Update terminal buffer scrollback position + val scrolledLines = + (newOffset / baseCharHeight).toInt() + screenState.scrollBy(scrolledLines - screenState.scrollbackPosition) + } } else -> {} @@ -1228,16 +1258,32 @@ internal fun TerminalWithAccessibility( GestureType.Scroll -> { // Apply fling animation val velocity = velocityTracker.calculateVelocity() + val scroller = wheelScroller scrollJob?.cancel() scrollJob = launch { - scrollOffset.animateDecay( - initialVelocity = velocity.y, - animationSpec = splineBasedDecay(density), - ) { - // Update terminal buffer during animation - val scrolledLines = - (value / baseCharHeight).toInt() - screenState.scrollBy(scrolledLines - screenState.scrollbackPosition) + if (scroller != null) { + // Decay a scratch offset on the same curve the + // local scrollback uses, reporting the detents it + // passes over so a fling feels the same either way. + val flingOffset = Animatable(0f) + var lastValue = 0f + flingOffset.animateDecay( + initialVelocity = velocity.y, + animationSpec = splineBasedDecay(density), + ) { + scroller.scrollBy(value - lastValue) + lastValue = value + } + } else { + scrollOffset.animateDecay( + initialVelocity = velocity.y, + animationSpec = splineBasedDecay(density), + ) { + // Update terminal buffer during animation + val scrolledLines = + (value / baseCharHeight).toInt() + screenState.scrollBy(scrolledLines - screenState.scrollbackPosition) + } } } } diff --git a/lib/src/main/java/org/connectbot/terminal/WheelScroller.kt b/lib/src/main/java/org/connectbot/terminal/WheelScroller.kt new file mode 100644 index 00000000..bb043a03 --- /dev/null +++ b/lib/src/main/java/org/connectbot/terminal/WheelScroller.kt @@ -0,0 +1,90 @@ +/* + * ConnectBot Terminal + * Copyright 2026 Kenny Root + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.connectbot.terminal + +import kotlin.math.abs +import kotlin.math.min + +/** + * Pixels of finger travel per wheel detent, as a multiple of the line height. + * + * One detent per line makes the content track the finger for an application + * that scrolls a single line per detent, which is the common case. + */ +private const val LINES_PER_WHEEL_DETENT = 1f + +/** + * Most detents to report from a single gesture sample. + * + * A fast fling can cover dozens of lines between animation frames. Applications + * commonly throttle or coalesce a burst of wheel reports, so sending every + * detent from such a frame can scroll *less* far than sending a few — besides + * putting a pointless amount of traffic on the wire. + */ +private const val MAX_DETENTS_PER_SAMPLE = 8 + +/** + * Turns a continuous vertical scroll gesture into discrete wheel reports for an + * application that has enabled mouse tracking. + * + * Such an application keeps its own scrollback and paints the whole screen, so + * scrolling the terminal's scrollback would do nothing visible. Reporting the + * wheel instead lets the application scroll itself. + * + * All reports are sent at a fixed anchor cell — the cell the gesture started + * on. Chasing the finger would make every crossed row emit a motion report to + * an application tracking in [MouseTracking.MOVE] mode, and the anchor is what + * the gesture is aimed at anyway. + * + * @param emulator The emulator to report to + * @param lineHeightPx Height of one terminal line in pixels + * @param anchorRow Row (0-based) the gesture started on + * @param anchorCol Column (0-based) the gesture started on + */ +internal class WheelScroller( + private val emulator: TerminalEmulator, + lineHeightPx: Float, + private val anchorRow: Int, + private val anchorCol: Int, +) { + private val pixelsPerDetent = lineHeightPx * LINES_PER_WHEEL_DETENT + + /** Travel not yet worth a whole detent, carried into the next sample. */ + private var residualPx = 0f + + /** + * Report the detents covered by [deltaPx] of vertical travel. + * + * Positive values mean the finger moved down the screen, which reveals + * earlier output and so reports as wheel up. + */ + fun scrollBy(deltaPx: Float) { + if (pixelsPerDetent <= 0f || !deltaPx.isFinite()) return + + residualPx += deltaPx + val detents = (residualPx / pixelsPerDetent).toInt() + if (detents == 0) return + residualPx -= detents * pixelsPerDetent + + emulator.scrollWheel( + direction = if (detents > 0) WheelDirection.UP else WheelDirection.DOWN, + row = anchorRow, + col = anchorCol, + steps = min(abs(detents), MAX_DETENTS_PER_SAMPLE), + ) + } +} diff --git a/lib/src/test/java/org/connectbot/terminal/WheelScrollGestureTest.kt b/lib/src/test/java/org/connectbot/terminal/WheelScrollGestureTest.kt new file mode 100644 index 00000000..ad3f679d --- /dev/null +++ b/lib/src/test/java/org/connectbot/terminal/WheelScrollGestureTest.kt @@ -0,0 +1,184 @@ +/* + * ConnectBot Terminal + * Copyright 2026 Kenny Root + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.connectbot.terminal + +import androidx.activity.ComponentActivity +import androidx.compose.foundation.layout.size +import androidx.compose.ui.Modifier +import androidx.compose.ui.geometry.Offset +import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule +import androidx.compose.ui.test.onRoot +import androidx.compose.ui.test.performTouchInput +import androidx.compose.ui.unit.dp +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import org.robolectric.shadows.ShadowLog + +/** + * End-to-end tests for routing a scroll gesture to the running application when + * it has enabled mouse tracking, rather than to the terminal's own scrollback. + * + * The two cases have to stay distinct: an application that never asked for the + * mouse must keep the local scrolling behaviour it has always had. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34], qualifiers = "w1000dp-h1200dp-xhdpi") +class WheelScrollGestureTest { + @get:Rule + val composeTestRule = createAndroidComposeRule() + + private val output = StringBuilder() + + @Before + fun setUp() { + ShadowLog.stream = System.out + composeTestRule.activityRule.scenario.onActivity { activity -> + activity.window.setLayout(1000, 1200) + } + } + + /** The mouse-tracking sequence Claude Code's flicker-free renderer sends. */ + private val enableMouseTracking = "\u001B[?1000h\u001B[?1002h\u001B[?1003h\u001B[?1006h" + + private fun emulatorWithContent(): TerminalEmulator { + val emulator = TerminalEmulatorFactory.create( + initialRows = 24, + initialCols = 80, + onKeyboardInput = { output.append(String(it, Charsets.ISO_8859_1)) }, + ) + val content = (1..100).joinToString("\r\n") { "Line $it" } + emulator.writeInput(content.toByteArray()) + (emulator as? TerminalEmulatorImpl)?.processPendingUpdates() + return emulator + } + + /** Count of wheel reports in the output, ignoring any motion reports. */ + private fun wheelReports(): Int = Regex("\u001B\\[<6[4-7];").findAll(output.toString()).count() + + private fun showTerminal(emulator: TerminalEmulator): ScrollController { + var scrollController: ScrollController? = null + composeTestRule.setContent { + TerminalWithAccessibility( + terminalEmulator = emulator, + modifier = Modifier.size(800.dp, 1200.dp), + onScrollControllerAvailable = { scrollController = it }, + ) + } + + composeTestRule.waitForIdle() + (emulator as? TerminalEmulatorImpl)?.processPendingUpdates() + composeTestRule.waitForIdle() + composeTestRule.waitUntil { scrollController != null } + return scrollController!! + } + + /** + * Drag downwards, which scrolls back towards earlier output. + * + * Two moves, not one: the local scroll path anchors its offset at the moment + * the gesture is classified, so it only travels on moves after that point. + */ + private fun dragDown() { + composeTestRule.mainClock.autoAdvance = false + composeTestRule.onRoot().performTouchInput { down(0, center) } + // Past the multi-touch grace period, so the move is taken as a scroll. + composeTestRule.mainClock.advanceTimeBy(100) + + composeTestRule.onRoot().performTouchInput { + moveTo(0, center + Offset(0f, 200f)) + } + composeTestRule.mainClock.advanceTimeBy(100) + composeTestRule.waitForIdle() + + composeTestRule.onRoot().performTouchInput { + moveTo(0, center + Offset(0f, 400f)) + } + composeTestRule.mainClock.advanceTimeBy(100) + composeTestRule.waitForIdle() + + composeTestRule.onRoot().performTouchInput { up(0) } + + composeTestRule.mainClock.autoAdvance = true + composeTestRule.waitForIdle() + composeTestRule.mainClock.advanceTimeBy(1000) + composeTestRule.waitForIdle() + } + + @Test + fun testScrollGoesToApplicationWhenTrackingEnabled() { + val emulator = emulatorWithContent() + emulator.writeInput(enableMouseTracking.toByteArray()) + (emulator as? TerminalEmulatorImpl)?.processPendingUpdates() + + val controller = showTerminal(emulator) + val initialPosition = controller.scrollbackPosition + output.setLength(0) + + dragDown() + + assertTrue( + "Expected wheel reports, got: ${output.toString().replace("\u001B", "ESC")}", + wheelReports() > 0, + ) + assertEquals( + "Local scrollback must not move; the application owns the viewport", + initialPosition, + controller.scrollbackPosition, + ) + } + + @Test + fun testDragDownReportsWheelUp() { + val emulator = emulatorWithContent() + emulator.writeInput(enableMouseTracking.toByteArray()) + (emulator as? TerminalEmulatorImpl)?.processPendingUpdates() + + showTerminal(emulator) + output.setLength(0) + + dragDown() + + // Dragging down reveals earlier output, which is a wheel-up. + val text = output.toString() + assertTrue("expected wheel-up reports", text.contains("\u001B[<64;")) + assertTrue("expected no wheel-down reports", !text.contains("\u001B[<65;")) + } + + @Test + fun testScrollStaysLocalWhenTrackingDisabled() { + val emulator = emulatorWithContent() + + val controller = showTerminal(emulator) + val initialPosition = controller.scrollbackPosition + output.setLength(0) + + dragDown() + + assertEquals("no mouse reports without tracking", 0, wheelReports()) + assertTrue( + "local scrollback should have moved (initial=$initialPosition, " + + "current=${controller.scrollbackPosition})", + controller.scrollbackPosition > initialPosition, + ) + } +} diff --git a/lib/src/test/java/org/connectbot/terminal/WheelScrollerTest.kt b/lib/src/test/java/org/connectbot/terminal/WheelScrollerTest.kt new file mode 100644 index 00000000..a6312550 --- /dev/null +++ b/lib/src/test/java/org/connectbot/terminal/WheelScrollerTest.kt @@ -0,0 +1,182 @@ +/* + * ConnectBot Terminal + * Copyright 2026 Kenny Root + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.connectbot.terminal + +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import org.junit.Assert.assertEquals +import org.junit.Test +import org.junit.runner.RunWith + +/** + * Tests for turning continuous gesture travel into discrete wheel detents. + * + * These drive a real emulator with SGR mouse tracking enabled and assert on the + * bytes it would write back, so they cover the conversion and the encoding + * together. + */ +@RunWith(AndroidJUnit4::class) +class WheelScrollerTest { + + private companion object { + const val LINE_HEIGHT = 20f + const val ANCHOR_ROW = 5 + const val ANCHOR_COL = 7 + + /** A wheel-up report at the anchor cell, in 1-based SGR coordinates. */ + const val UP = "\u001B[<64;8;6M" + + /** A wheel-down report at the anchor cell. */ + const val DOWN = "\u001B[<65;8;6M" + } + + private val output = StringBuilder() + + private fun scroller(): WheelScroller { + val emulator = TerminalEmulatorFactory.create( + initialRows = 24, + initialCols = 80, + onKeyboardInput = { output.append(String(it, Charsets.ISO_8859_1)) }, + ) + emulator.writeInput("\u001B[?1000h\u001B[?1006h".toByteArray()) + drain() + output.setLength(0) + + return WheelScroller( + emulator = emulator, + lineHeightPx = LINE_HEIGHT, + anchorRow = ANCHOR_ROW, + anchorCol = ANCHOR_COL, + ) + } + + /** Output is posted to the Looper, so let it settle before asserting. */ + private fun drain() = InstrumentationRegistry.getInstrumentation().waitForIdleSync() + + private fun reported(): String { + drain() + return output.toString() + } + + @Test + fun testOneDetentPerLineOfTravel() { + val scroller = scroller() + + scroller.scrollBy(LINE_HEIGHT) + + assertEquals(UP, reported()) + } + + @Test + fun testFingerDownScrollsBackIntoHistory() { + // Dragging down reveals earlier output, which is what a wheel-up does. + val scroller = scroller() + + scroller.scrollBy(LINE_HEIGHT * 2) + + assertEquals(UP.repeat(2), reported()) + } + + @Test + fun testFingerUpScrollsForward() { + val scroller = scroller() + + scroller.scrollBy(-LINE_HEIGHT * 3) + + assertEquals(DOWN.repeat(3), reported()) + } + + @Test + fun testTravelBelowOneDetentIsHeld() { + val scroller = scroller() + + scroller.scrollBy(LINE_HEIGHT / 2) + + assertEquals("", reported()) + } + + @Test + fun testHeldTravelAccumulatesIntoADetent() { + val scroller = scroller() + + scroller.scrollBy(LINE_HEIGHT * 0.6f) + scroller.scrollBy(LINE_HEIGHT * 0.6f) + + // 1.2 line heights of travel is one detent, with the remainder held. + assertEquals(UP, reported()) + } + + @Test + fun testReversalCancelsHeldTravel() { + val scroller = scroller() + + scroller.scrollBy(LINE_HEIGHT * 0.75f) + scroller.scrollBy(-LINE_HEIGHT * 1.5f) + + // Net travel is -0.75 of a line: not yet a detent in either direction. + assertEquals("", reported()) + } + + @Test + fun testBurstIsRateLimited() { + val scroller = scroller() + + // A fast fling can cover this much between two animation frames. + scroller.scrollBy(LINE_HEIGHT * 40) + + assertEquals("capped at 8 detents", UP.repeat(8), reported()) + } + + @Test + fun testRateLimitDoesNotBacklog() { + val scroller = scroller() + + // The dropped detents are gone, not queued: a following sample of one + // line reports exactly one detent. + scroller.scrollBy(LINE_HEIGHT * 40) + drain() + output.setLength(0) + scroller.scrollBy(LINE_HEIGHT) + + assertEquals(UP, reported()) + } + + @Test + fun testNonFiniteTravelIsIgnored() { + val scroller = scroller() + + scroller.scrollBy(Float.NaN) + scroller.scrollBy(Float.POSITIVE_INFINITY) + scroller.scrollBy(LINE_HEIGHT) + + assertEquals("a bad sample must not poison the accumulator", UP, reported()) + } + + @Test + fun testNothingIsSentWhileTrackingIsOff() { + val emulator = TerminalEmulatorFactory.create( + initialRows = 24, + initialCols = 80, + onKeyboardInput = { output.append(String(it, Charsets.ISO_8859_1)) }, + ) + val scroller = WheelScroller(emulator, LINE_HEIGHT, ANCHOR_ROW, ANCHOR_COL) + + scroller.scrollBy(LINE_HEIGHT * 5) + + assertEquals("", reported()) + } +} From 6ed03a62d0a9f1cfa289ba6fde99de2b37603b4a Mon Sep 17 00:00:00 2001 From: Brandon Martin-Anderson Date: Fri, 24 Jul 2026 18:23:16 -0700 Subject: [PATCH 03/13] fix(props): correct VTermProp identifiers for title and cursor shape 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 --- .../connectbot/terminal/TerminalEmulator.kt | 71 +++++++++++-------- .../terminal/CursorAndModeEscapeTest.kt | 54 ++++++++++++++ 2 files changed, 94 insertions(+), 31 deletions(-) diff --git a/lib/src/main/java/org/connectbot/terminal/TerminalEmulator.kt b/lib/src/main/java/org/connectbot/terminal/TerminalEmulator.kt index 4c2e0b06..97890c45 100644 --- a/lib/src/main/java/org/connectbot/terminal/TerminalEmulator.kt +++ b/lib/src/main/java/org/connectbot/terminal/TerminalEmulator.kt @@ -304,6 +304,34 @@ class TerminalEmulatorFactory { } } +/** + * Property identifiers and values libvterm passes to + * [TerminalCallbacks.setTermProp], from `VTermProp` in vterm.h. + * + * These are ordinals of an unnumbered C enum, so inserting a property shifts + * every one after it. Keep them in the same order as the header. + */ +private object VTermProp { + const val CURSOR_VISIBLE = 1 // bool + const val CURSOR_BLINK = 2 // bool + const val ALT_SCREEN = 3 // bool + const val TITLE = 4 // string + const val ICON_NAME = 5 // string + const val REVERSE = 6 // bool + const val CURSOR_SHAPE = 7 // number + const val MOUSE = 8 // number + const val FOCUS_REPORT = 9 // bool + + const val CURSOR_SHAPE_BLOCK = 1 + const val CURSOR_SHAPE_UNDERLINE = 2 + const val CURSOR_SHAPE_BAR_LEFT = 3 + + const val MOUSE_NONE = 0 + const val MOUSE_CLICK = 1 + const val MOUSE_DRAG = 2 + const val MOUSE_MOVE = 3 +} + /** * Service-compatible terminal state manager. * @@ -667,8 +695,7 @@ internal class TerminalEmulatorImpl( synchronized(damageLock) { when (value) { is TerminalProperty.StringValue -> { - // Property 7 is VTERM_PROP_TITLE (from vterm.h line 257) - if (prop == 7) { + if (prop == VTermProp.TITLE) { terminalTitle = value.value propertyChanged = true } @@ -676,20 +703,17 @@ internal class TerminalEmulatorImpl( is TerminalProperty.BoolValue -> { when (prop) { - // Property 1 is VTERM_PROP_CURSORVISIBLE (from vterm.h line 254) - 1 -> { + VTermProp.CURSOR_VISIBLE -> { cursorVisible = value.value propertyChanged = true } - // Property 2 is VTERM_PROP_CURSORBLINK (from vterm.h line 255) - 2 -> { + VTermProp.CURSOR_BLINK -> { cursorBlink = value.value propertyChanged = true } - // Property 3 is VTERM_PROP_ALTSCREEN (from vterm.h line 256) - 3 -> { + VTermProp.ALT_SCREEN -> { isAltScreenActive = value.value propertyChanged = true } @@ -698,36 +722,21 @@ internal class TerminalEmulatorImpl( is TerminalProperty.IntValue -> { when (prop) { - // Property 6 is VTERM_PROP_CURSORSHAPE (from vterm.h line 260) - 6 -> { + VTermProp.CURSOR_SHAPE -> { cursorShape = when (value.value) { - 1 -> CursorShape.BLOCK - - // VTERM_PROP_CURSORSHAPE_BLOCK - 2 -> CursorShape.UNDERLINE - - // VTERM_PROP_CURSORSHAPE_UNDERLINE - 3 -> CursorShape.BAR_LEFT - - // VTERM_PROP_CURSORSHAPE_BAR_LEFT + VTermProp.CURSOR_SHAPE_BLOCK -> CursorShape.BLOCK + VTermProp.CURSOR_SHAPE_UNDERLINE -> CursorShape.UNDERLINE + VTermProp.CURSOR_SHAPE_BAR_LEFT -> CursorShape.BAR_LEFT else -> CursorShape.BLOCK } propertyChanged = true } - // Property 8 is VTERM_PROP_MOUSE (from vterm.h line 261) - 8 -> { + VTermProp.MOUSE -> { mouseTracking = when (value.value) { - // VTERM_PROP_MOUSE_CLICK - 1 -> MouseTracking.CLICK - - // VTERM_PROP_MOUSE_DRAG - 2 -> MouseTracking.DRAG - - // VTERM_PROP_MOUSE_MOVE - 3 -> MouseTracking.MOVE - - // VTERM_PROP_MOUSE_NONE + VTermProp.MOUSE_CLICK -> MouseTracking.CLICK + VTermProp.MOUSE_DRAG -> MouseTracking.DRAG + VTermProp.MOUSE_MOVE -> MouseTracking.MOVE else -> MouseTracking.NONE } propertyChanged = true diff --git a/lib/src/test/java/org/connectbot/terminal/CursorAndModeEscapeTest.kt b/lib/src/test/java/org/connectbot/terminal/CursorAndModeEscapeTest.kt index b6baf7f5..b68ff52d 100644 --- a/lib/src/test/java/org/connectbot/terminal/CursorAndModeEscapeTest.kt +++ b/lib/src/test/java/org/connectbot/terminal/CursorAndModeEscapeTest.kt @@ -276,4 +276,58 @@ class CursorAndModeEscapeTest { after.lines[0].text.trimEnd() == "protected row 0", ) } + + // ----------------------------------------------------------------------- + // Terminal properties reaching the snapshot + // ----------------------------------------------------------------------- + + @Test + fun testOsc0SetsTitle() = runBlocking { + val emulator = TerminalEmulatorFactory.create(initialRows = 10, initialCols = 40) + val impl = emulator as TerminalEmulatorImpl + + emulator.send("\u001B]0;hello") + + assertEquals("hello", getSnapshot(impl).terminalTitle) + } + + @Test + fun testOsc2SetsTitle() = runBlocking { + // OSC 2 sets the window title only; OSC 0 sets title and icon name. + val emulator = TerminalEmulatorFactory.create(initialRows = 10, initialCols = 40) + val impl = emulator as TerminalEmulatorImpl + + emulator.send("\u001B]2;window") + + assertEquals("window", getSnapshot(impl).terminalTitle) + } + + @Test + fun testDecscusrSelectsCursorShape() = runBlocking { + val emulator = TerminalEmulatorFactory.create(initialRows = 10, initialCols = 40) + val impl = emulator as TerminalEmulatorImpl + + // DECSCUSR: 4 is a steady underline, 6 a steady bar, 2 a steady block. + emulator.send("\u001B[4 q") + assertEquals("underline", CursorShape.UNDERLINE, getSnapshot(impl).cursorShape) + + emulator.send("\u001B[6 q") + assertEquals("bar", CursorShape.BAR_LEFT, getSnapshot(impl).cursorShape) + + emulator.send("\u001B[2 q") + assertEquals("block", CursorShape.BLOCK, getSnapshot(impl).cursorShape) + } + + @Test + fun testDecscusrSetsBlink() = runBlocking { + val emulator = TerminalEmulatorFactory.create(initialRows = 10, initialCols = 40) + val impl = emulator as TerminalEmulatorImpl + + // Odd DECSCUSR values blink, even ones are steady. + emulator.send("\u001B[2 q") + assertEquals("steady block", false, getSnapshot(impl).cursorBlink) + + emulator.send("\u001B[1 q") + assertEquals("blinking block", true, getSnapshot(impl).cursorBlink) + } } From 30bc2077d001efc31331ee5dcd373ba20709f129 Mon Sep 17 00:00:00 2001 From: Brandon Martin-Anderson Date: Fri, 24 Jul 2026 19:07:59 -0700 Subject: [PATCH 04/13] fix(mouse): tell the embedder when a reset clears mouse tracking 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 --- lib/src/main/cpp/libvterm/src/state.c | 6 +++ .../connectbot/terminal/MouseReportingTest.kt | 53 +++++++++++++++++-- 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/lib/src/main/cpp/libvterm/src/state.c b/lib/src/main/cpp/libvterm/src/state.c index ce8e0342..809ae806 100644 --- a/lib/src/main/cpp/libvterm/src/state.c +++ b/lib/src/main/cpp/libvterm/src/state.c @@ -2125,6 +2125,12 @@ void vterm_state_reset(VTermState *state, int hard) settermprop_bool(state, VTERM_PROP_CURSORVISIBLE, 1); settermprop_bool(state, VTERM_PROP_CURSORBLINK, 1); settermprop_int (state, VTERM_PROP_CURSORSHAPE, VTERM_PROP_CURSORSHAPE_BLOCK); + /* Local modification: clearing state->mouse_flags above disables reporting + * without telling the embedder. A terminal mirroring VTERM_PROP_MOUSE would + * then believe an application still wants the mouse and keep routing gestures + * into a vterm that silently drops them. This notifies; the assignment above + * is kept so the flags are cleared even if the callback vetoes the store. */ + settermprop_int (state, VTERM_PROP_MOUSE, VTERM_PROP_MOUSE_NONE); if(hard) { state->pos.row = 0; diff --git a/lib/src/test/java/org/connectbot/terminal/MouseReportingTest.kt b/lib/src/test/java/org/connectbot/terminal/MouseReportingTest.kt index cb005409..6eaa1bd5 100644 --- a/lib/src/test/java/org/connectbot/terminal/MouseReportingTest.kt +++ b/lib/src/test/java/org/connectbot/terminal/MouseReportingTest.kt @@ -109,6 +109,49 @@ class MouseReportingTest { assertTrue(term.mouseTracking.isEnabled) } + @Test + fun testHardResetClearsTracking() = runBlocking { + // RIS is how a user unwedges a terminal after a full-screen application + // dies without restoring modes. It stops libvterm reporting the mouse, so + // our mirror of the mode has to follow: believing an application still + // wants the mouse routes gestures nowhere and leaves the terminal with no + // scrolling at all. + val term = emulator(Output()) + term.send("\u001B[?1003h\u001B[?1006h") + + term.send("\u001Bc") + + assertEquals(MouseTracking.NONE, term.mouseTracking) + } + + @Test + fun testSoftResetClearsTracking() = runBlocking { + // DECSTR, the same story by a different route. + val term = emulator(Output()) + term.send("\u001B[?1003h\u001B[?1006h") + + term.send("\u001B[!p") + + assertEquals(MouseTracking.NONE, term.mouseTracking) + } + + @Test + fun testNoReportsAfterReset() = runBlocking { + // The mode flag and the reporting have to agree: whatever mouseTracking + // says, a reset terminal emits nothing. + val out = Output() + val term = emulator(out) + term.send("\u001B[?1003h\u001B[?1006h") + term.send("\u001Bc") + out.clear() + + term.scrollWheel(WheelDirection.UP, row = 3, col = 5) + term.mouseButton(MouseButton.LEFT, row = 3, col = 5, pressed = true) + term.mouseMove(row = 4, col = 6) + + assertEquals("", out.text) + } + // ----------------------------------------------------------------------- // Wheel reporting // ----------------------------------------------------------------------- @@ -119,7 +162,7 @@ class MouseReportingTest { val term = emulator(out) term.scrollWheel(WheelDirection.UP, row = 3, col = 5) - term.mouseButton(MouseButton.LEFT, pressed = true, row = 3, col = 5) + term.mouseButton(MouseButton.LEFT, row = 3, col = 5, pressed = true) term.mouseMove(row = 4, col = 6) assertEquals("", out.text) @@ -225,8 +268,8 @@ class MouseReportingTest { term.send("\u001B[?1000h\u001B[?1006h") out.clear() - term.mouseButton(MouseButton.LEFT, pressed = true, row = 2, col = 7) - term.mouseButton(MouseButton.LEFT, pressed = false, row = 2, col = 7) + term.mouseButton(MouseButton.LEFT, row = 2, col = 7, pressed = true) + term.mouseButton(MouseButton.LEFT, row = 2, col = 7, pressed = false) // Press ends in 'M', release in 'm'; left button is code 0. assertEquals("\u001B[<0;8;3M\u001B[<0;8;3m", out.text) @@ -239,11 +282,11 @@ class MouseReportingTest { term.send("\u001B[?1000h\u001B[?1006h") out.clear() - term.mouseButton(MouseButton.MIDDLE, pressed = true, row = 0, col = 0) + term.mouseButton(MouseButton.MIDDLE, row = 0, col = 0, pressed = true) assertEquals("middle", "\u001B[<1;1;1M", out.text) out.clear() - term.mouseButton(MouseButton.RIGHT, pressed = true, row = 0, col = 0) + term.mouseButton(MouseButton.RIGHT, row = 0, col = 0, pressed = true) assertEquals("right", "\u001B[<2;1;1M", out.text) } From 3ad7810aadc797188d0ed9915fa42d741e941cf9 Mon Sep 17 00:00:00 2001 From: Brandon Martin-Anderson Date: Fri, 24 Jul 2026 19:08:27 -0700 Subject: [PATCH 05/13] fix(mouse): bound the wheel fling 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 --- .../java/org/connectbot/terminal/Terminal.kt | 6 +++ .../org/connectbot/terminal/WheelScroller.kt | 17 +++++++ .../connectbot/terminal/WheelScrollerTest.kt | 51 +++++++++++++++++-- 3 files changed, 71 insertions(+), 3 deletions(-) diff --git a/lib/src/main/java/org/connectbot/terminal/Terminal.kt b/lib/src/main/java/org/connectbot/terminal/Terminal.kt index b86d0bdd..aea82787 100644 --- a/lib/src/main/java/org/connectbot/terminal/Terminal.kt +++ b/lib/src/main/java/org/connectbot/terminal/Terminal.kt @@ -1265,7 +1265,13 @@ internal fun TerminalWithAccessibility( // Decay a scratch offset on the same curve the // local scrollback uses, reporting the detents it // passes over so a fling feels the same either way. + // Bounded because, unlike the local path, there is + // no scrollback size to run out of. val flingOffset = Animatable(0f) + flingOffset.updateBounds( + lowerBound = -scroller.maxFlingTravelPx, + upperBound = scroller.maxFlingTravelPx, + ) var lastValue = 0f flingOffset.animateDecay( initialVelocity = velocity.y, diff --git a/lib/src/main/java/org/connectbot/terminal/WheelScroller.kt b/lib/src/main/java/org/connectbot/terminal/WheelScroller.kt index bb043a03..bdff19f4 100644 --- a/lib/src/main/java/org/connectbot/terminal/WheelScroller.kt +++ b/lib/src/main/java/org/connectbot/terminal/WheelScroller.kt @@ -37,6 +37,17 @@ private const val LINES_PER_WHEEL_DETENT = 1f */ private const val MAX_DETENTS_PER_SAMPLE = 8 +/** + * Most detents a single fling may report, as a bound on its decay distance. + * + * The local scrollback path is bounded by the scrollback it has; the wheel path + * has no comparable limit, because the terminal cannot know where the + * application's own history begins or ends. Without a bound, a hard fling keeps + * emitting detents long after the application has hit its top, so this caps the + * decay well above what an ordinary fling covers. + */ +private const val MAX_DETENTS_PER_FLING = 200 + /** * Turns a continuous vertical scroll gesture into discrete wheel reports for an * application that has enabled mouse tracking. @@ -63,6 +74,12 @@ internal class WheelScroller( ) { private val pixelsPerDetent = lineHeightPx * LINES_PER_WHEEL_DETENT + /** + * Travel a fling may cover before it should be stopped, as a bound for the + * decay animation driving [scrollBy]. + */ + val maxFlingTravelPx = pixelsPerDetent * MAX_DETENTS_PER_FLING + /** Travel not yet worth a whole detent, carried into the next sample. */ private var residualPx = 0f diff --git a/lib/src/test/java/org/connectbot/terminal/WheelScrollerTest.kt b/lib/src/test/java/org/connectbot/terminal/WheelScrollerTest.kt index a6312550..f474faaf 100644 --- a/lib/src/test/java/org/connectbot/terminal/WheelScrollerTest.kt +++ b/lib/src/test/java/org/connectbot/terminal/WheelScrollerTest.kt @@ -46,8 +46,11 @@ class WheelScrollerTest { private val output = StringBuilder() + /** The emulator behind the most recent [scroller], for tests that drive it directly. */ + private lateinit var emulator: TerminalEmulator + private fun scroller(): WheelScroller { - val emulator = TerminalEmulatorFactory.create( + emulator = TerminalEmulatorFactory.create( initialRows = 24, initialCols = 80, onKeyboardInput = { output.append(String(it, Charsets.ISO_8859_1)) }, @@ -168,15 +171,57 @@ class WheelScrollerTest { @Test fun testNothingIsSentWhileTrackingIsOff() { - val emulator = TerminalEmulatorFactory.create( + val untracked = TerminalEmulatorFactory.create( initialRows = 24, initialCols = 80, onKeyboardInput = { output.append(String(it, Charsets.ISO_8859_1)) }, ) - val scroller = WheelScroller(emulator, LINE_HEIGHT, ANCHOR_ROW, ANCHOR_COL) + val scroller = WheelScroller(untracked, LINE_HEIGHT, ANCHOR_ROW, ANCHOR_COL) scroller.scrollBy(LINE_HEIGHT * 5) assertEquals("", reported()) } + + @Test + fun testTrackingDisabledMidGestureStopsReports() { + // An application can drop mouse tracking while a finger is still down. + // The scroller keeps converting travel, but nothing may reach the wire. + val scroller = scroller() + + scroller.scrollBy(LINE_HEIGHT) + drain() + assertEquals("before DECRST", UP, output.toString()) + + emulator.writeInput("\u001B[?1000l".toByteArray()) + drain() + output.setLength(0) + scroller.scrollBy(LINE_HEIGHT * 3) + + assertEquals("", reported()) + } + + // ----------------------------------------------------------------------- + // Fling bound + // ----------------------------------------------------------------------- + + @Test + fun testMaxFlingTravelConvertsToTheDetentCap() { + // The bound handed to the fling animation has to mean what it says: the + // travel it permits is exactly the detent cap, fed one detent at a time + // so the per-sample rate limit does not mask it. + val scroller = scroller() + + var reports = 0 + var travelled = 0f + while (travelled < scroller.maxFlingTravelPx) { + scroller.scrollBy(LINE_HEIGHT) + travelled += LINE_HEIGHT + drain() + reports += Regex(Regex.escape(UP)).findAll(output.toString()).count() + output.setLength(0) + } + + assertEquals("detents permitted by the fling bound", 200, reports) + } } From ad31fc5df34413a1a0f94a5c657ee845abfebfc5 Mon Sep 17 00:00:00 2001 From: Brandon Martin-Anderson Date: Fri, 24 Jul 2026 19:08:27 -0700 Subject: [PATCH 06/13] refactor(mouse): emit position and button under one native lock 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 --- lib/src/main/cpp/Terminal.cpp | 40 ++++++++++++++++--- lib/src/main/cpp/Terminal.h | 3 +- .../connectbot/terminal/TerminalEmulator.kt | 20 +++++----- .../org/connectbot/terminal/TerminalNative.kt | 34 ++++++++++++++-- 4 files changed, 76 insertions(+), 21 deletions(-) diff --git a/lib/src/main/cpp/Terminal.cpp b/lib/src/main/cpp/Terminal.cpp index 16b13fbb..c77f7c17 100644 --- a/lib/src/main/cpp/Terminal.cpp +++ b/lib/src/main/cpp/Terminal.cpp @@ -440,14 +440,36 @@ bool Terminal::mouseMove(int row, int col, int modifiers) { return true; } -bool Terminal::mouseButton(int button, bool pressed, int modifiers) { +bool Terminal::mouseButton(int row, int col, int button, bool pressed, int modifiers) { std::scoped_lock lock(mLock); if (!mVt) { return false; } - vterm_mouse_button(mVt, button, pressed, toVTermModifier(modifiers)); + // Position and button are set under one lock: libvterm carries the position + // recorded by the move into the button report, so a concurrent move for a + // different gesture must not be able to land between the two. + VTermModifier mod = toVTermModifier(modifiers); + vterm_mouse_move(mVt, row, col, mod); + vterm_mouse_button(mVt, button, pressed, mod); + return true; +} + +bool Terminal::scrollWheel(int row, int col, int button, int steps, int modifiers) { + std::scoped_lock lock(mLock); + + if (!mVt || steps < 1) { + return false; + } + + VTermModifier mod = toVTermModifier(modifiers); + vterm_mouse_move(mVt, row, col, mod); + for (int i = 0; i < steps; i++) { + // Wheel buttons report a press with no matching release; libvterm emits + // one report per call. + vterm_mouse_button(mVt, button, true, mod); + } return true; } @@ -1257,10 +1279,18 @@ Java_org_connectbot_terminal_TerminalNative_nativeMouseMove(JNIEnv* /* env */, j JNIEXPORT jboolean JNICALL Java_org_connectbot_terminal_TerminalNative_nativeMouseButton(JNIEnv* /* env */, jobject /* thiz */, - jlong ptr, jint button, jboolean pressed, - jint modifiers) { + jlong ptr, jint row, jint col, jint button, + jboolean pressed, jint modifiers) { + auto* term = reinterpret_cast(ptr); + return term->mouseButton(row, col, button, pressed, modifiers); +} + +JNIEXPORT jboolean JNICALL +Java_org_connectbot_terminal_TerminalNative_nativeScrollWheel(JNIEnv* /* env */, jobject /* thiz */, + jlong ptr, jint row, jint col, jint button, + jint steps, jint modifiers) { auto* term = reinterpret_cast(ptr); - return term->mouseButton(button, pressed, modifiers); + return term->scrollWheel(row, col, button, steps, modifiers); } JNIEXPORT jint JNICALL diff --git a/lib/src/main/cpp/Terminal.h b/lib/src/main/cpp/Terminal.h index 69978cd3..4a5dce53 100644 --- a/lib/src/main/cpp/Terminal.h +++ b/lib/src/main/cpp/Terminal.h @@ -65,7 +65,8 @@ class Terminal { // requested mouse tracking (DECSET 1000/1002/1003). Encoding follows the // protocol the application selected (X10, UTF-8, SGR or rxvt). bool mouseMove(int row, int col, int modifiers); - bool mouseButton(int button, bool pressed, int modifiers); + bool mouseButton(int row, int col, int button, bool pressed, int modifiers); + bool scrollWheel(int row, int col, int button, int steps, int modifiers); // Cell data retrieval for rendering int getCellRun(JNIEnv* env, int row, int col, jobject runObject); diff --git a/lib/src/main/java/org/connectbot/terminal/TerminalEmulator.kt b/lib/src/main/java/org/connectbot/terminal/TerminalEmulator.kt index 97890c45..3cf40a5b 100644 --- a/lib/src/main/java/org/connectbot/terminal/TerminalEmulator.kt +++ b/lib/src/main/java/org/connectbot/terminal/TerminalEmulator.kt @@ -141,9 +141,10 @@ sealed interface TerminalEmulator { * * @param row Row index (0-based) within the visible screen * @param col Column index (0-based) within the visible screen + * @param pressed true for a press, false for a release * @param modifiers Bitmask: 1=Shift, 2=Alt, 4=Ctrl */ - fun mouseButton(button: MouseButton, pressed: Boolean, row: Int, col: Int, modifiers: Int = 0) + fun mouseButton(button: MouseButton, row: Int, col: Int, pressed: Boolean, modifiers: Int = 0) /** * Report [steps] wheel detents at a cell. @@ -545,9 +546,8 @@ internal class TerminalEmulatorImpl( /** * Report a mouse button press or release at a cell. */ - override fun mouseButton(button: MouseButton, pressed: Boolean, row: Int, col: Int, modifiers: Int) { - terminalNative.mouseMove(row, col, modifiers) - terminalNative.mouseButton(button.code, pressed, modifiers) + override fun mouseButton(button: MouseButton, row: Int, col: Int, pressed: Boolean, modifiers: Int) { + terminalNative.mouseButton(row, col, button.code, pressed, modifiers) } /** @@ -556,12 +556,7 @@ internal class TerminalEmulatorImpl( override fun scrollWheel(direction: WheelDirection, row: Int, col: Int, steps: Int, modifiers: Int) { if (steps < 1) return - terminalNative.mouseMove(row, col, modifiers) - repeat(steps) { - // Wheel buttons report a press with no matching release; libvterm - // emits one report per call. - terminalNative.mouseButton(direction.code, true, modifiers) - } + terminalNative.scrollWheel(row, col, direction.code, steps, modifiers) } /** @@ -733,13 +728,16 @@ internal class TerminalEmulatorImpl( } VTermProp.MOUSE -> { + // No propertyChanged here: this is read straight off + // the volatile field by gesture handling and does not + // appear in the snapshot, so rebuilding one would + // produce an identical value at real cost. mouseTracking = when (value.value) { VTermProp.MOUSE_CLICK -> MouseTracking.CLICK VTermProp.MOUSE_DRAG -> MouseTracking.DRAG VTermProp.MOUSE_MOVE -> MouseTracking.MOVE else -> MouseTracking.NONE } - propertyChanged = true } } } diff --git a/lib/src/main/java/org/connectbot/terminal/TerminalNative.kt b/lib/src/main/java/org/connectbot/terminal/TerminalNative.kt index ad509a2a..91414f94 100644 --- a/lib/src/main/java/org/connectbot/terminal/TerminalNative.kt +++ b/lib/src/main/java/org/connectbot/terminal/TerminalNative.kt @@ -128,12 +128,18 @@ internal class TerminalNative(callbacks: TerminalCallbacks) : AutoCloseable { } /** - * Dispatch a mouse button press or release at the current mouse position. + * Dispatch a mouse button press or release at a cell. + * + * The move to [row]/[col] and the button report are made under a single + * native lock, so a concurrent report for another gesture cannot land + * between the two and send this button at the wrong position. * * Nothing is emitted unless the application has enabled mouse tracking. The * report encoding follows the protocol the application selected (X10, UTF-8, * SGR or rxvt). * + * @param row Row index (0-based) + * @param col Column index (0-based) * @param button 1=left, 2=middle, 3=right, 4=wheel up, 5=wheel down, * 6=wheel left, 7=wheel right * @param pressed true for press, false for release. Wheel buttons only @@ -141,9 +147,28 @@ internal class TerminalNative(callbacks: TerminalCallbacks) : AutoCloseable { * @param modifiers Bitmask: 1=Shift, 2=Alt, 4=Ctrl * @return true if handled */ - fun mouseButton(button: Int, pressed: Boolean, modifiers: Int): Boolean { + fun mouseButton(row: Int, col: Int, button: Int, pressed: Boolean, modifiers: Int): Boolean { + checkNotClosed() + return nativeMouseButton(nativePtr, row, col, button, pressed, modifiers) + } + + /** + * Dispatch [steps] presses of a wheel button at a cell. + * + * Equivalent to [steps] calls to [mouseButton] with a wheel button, but the + * whole burst is emitted under a single native lock so it cannot be + * interleaved with another gesture's reports. + * + * @param row Row index (0-based) + * @param col Column index (0-based) + * @param button 4=wheel up, 5=wheel down, 6=wheel left, 7=wheel right + * @param steps Number of detents to report; values below 1 send nothing + * @param modifiers Bitmask: 1=Shift, 2=Alt, 4=Ctrl + * @return true if handled + */ + fun scrollWheel(row: Int, col: Int, button: Int, steps: Int, modifiers: Int): Boolean { checkNotClosed() - return nativeMouseButton(nativePtr, button, pressed, modifiers) + return nativeScrollWheel(nativePtr, row, col, button, steps, modifiers) } /** @@ -252,7 +277,8 @@ internal class TerminalNative(callbacks: TerminalCallbacks) : AutoCloseable { private external fun nativeDispatchKey(ptr: Long, modifiers: Int, key: Int): Boolean private external fun nativeDispatchCharacter(ptr: Long, modifiers: Int, character: Int): Boolean private external fun nativeMouseMove(ptr: Long, row: Int, col: Int, modifiers: Int): Boolean - private external fun nativeMouseButton(ptr: Long, button: Int, pressed: Boolean, modifiers: Int): Boolean + private external fun nativeMouseButton(ptr: Long, row: Int, col: Int, button: Int, pressed: Boolean, modifiers: Int): Boolean + private external fun nativeScrollWheel(ptr: Long, row: Int, col: Int, button: Int, steps: Int, modifiers: Int): Boolean private external fun nativeGetCellRun(ptr: Long, row: Int, col: Int, run: CellRun): Int private external fun nativeSetPaletteColors(ptr: Long, colors: IntArray, count: Int): Int private external fun nativeSetDefaultColors(ptr: Long, fgColor: Int, bgColor: Int): Int From a535e5863db765a9a08646e818610ace541f4b42 Mon Sep 17 00:00:00 2001 From: Brandon Martin-Anderson Date: Fri, 24 Jul 2026 19:08:37 -0700 Subject: [PATCH 07/13] build(api): record the mouse reporting API and tighten gesture tests 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 --- lib/README.md | 5 ++ lib/api.txt | 28 ++++++++ .../terminal/WheelScrollGestureTest.kt | 68 ++++++++++++++----- 3 files changed, 83 insertions(+), 18 deletions(-) diff --git a/lib/README.md b/lib/README.md index a1840901..172ca4f5 100644 --- a/lib/README.md +++ b/lib/README.md @@ -42,6 +42,11 @@ Terminal( ``` PTY/SSH → TerminalEmulator.writeInput() → libvterm → Callbacks → TerminalEmulator → Terminal Keyboard → TerminalEmulator.dispatchKey() → libvterm → onKeyboardInput() → PTY/SSH +Mouse → TerminalEmulator.scrollWheel() → libvterm → onKeyboardInput() → PTY/SSH ``` +Mouse reports are only emitted once the running application asks for them with +DECSET 1000/1002/1003; check `TerminalEmulator.mouseTracking` to know whether a +gesture belongs to the application or to the terminal's own scrollback. + **Important**: Callbacks must not call back into Terminal methods (causes deadlock). Defer work to avoid reentrancy. diff --git a/lib/api.txt b/lib/api.txt index 5dae1006..900f7359 100644 --- a/lib/api.txt +++ b/lib/api.txt @@ -30,6 +30,21 @@ package org.connectbot.terminal { method public boolean isShiftActive(); } + public enum MouseButton { + enum_constant public static final org.connectbot.terminal.MouseButton LEFT; + enum_constant public static final org.connectbot.terminal.MouseButton MIDDLE; + enum_constant public static final org.connectbot.terminal.MouseButton RIGHT; + } + + public enum MouseTracking { + method @InaccessibleFromKotlin public boolean isEnabled(); + property public boolean isEnabled; + enum_constant public static final org.connectbot.terminal.MouseTracking CLICK; + enum_constant public static final org.connectbot.terminal.MouseTracking DRAG; + enum_constant public static final org.connectbot.terminal.MouseTracking MOVE; + enum_constant public static final org.connectbot.terminal.MouseTracking NONE; + } + public enum ProgressState { enum_constant public static final org.connectbot.terminal.ProgressState DEFAULT; enum_constant public static final org.connectbot.terminal.ProgressState ERROR; @@ -106,8 +121,12 @@ package org.connectbot.terminal { method @InaccessibleFromKotlin public boolean getBoldAsBright(); method @InaccessibleFromKotlin public org.connectbot.terminal.TerminalDimensions getDimensions(); method public String? getLastCommandOutput(); + method @InaccessibleFromKotlin public org.connectbot.terminal.MouseTracking getMouseTracking(); method public java.util.List getUrls(optional org.connectbot.terminal.UrlScanScope scope); + method public void mouseButton(org.connectbot.terminal.MouseButton button, int row, int col, boolean pressed, optional int modifiers); + method public void mouseMove(int row, int col, optional int modifiers); method public void resize(int newRows, int newCols); + method public void scrollWheel(org.connectbot.terminal.WheelDirection direction, int row, int col, optional int steps, optional int modifiers); method public int setAnsiPalette(int[] ansiColors); method public int setDefaultColors(int foreground, int background); method public void writeInput(byte[] data, optional int offset, optional int length); @@ -115,6 +134,7 @@ package org.connectbot.terminal { property public abstract boolean autoDetectUrls; property public abstract boolean boldAsBright; property public abstract org.connectbot.terminal.TerminalDimensions dimensions; + property public abstract org.connectbot.terminal.MouseTracking mouseTracking; } public final class TerminalEmulatorFactory { @@ -259,4 +279,12 @@ package org.connectbot.terminal { field public static final int UP = 5; // 0x5 } + public enum WheelDirection { + enum_constant public static final org.connectbot.terminal.WheelDirection DOWN; + enum_constant public static final org.connectbot.terminal.WheelDirection LEFT; + enum_constant public static final org.connectbot.terminal.WheelDirection RIGHT; + enum_constant public static final org.connectbot.terminal.WheelDirection UP; + } + } + diff --git a/lib/src/test/java/org/connectbot/terminal/WheelScrollGestureTest.kt b/lib/src/test/java/org/connectbot/terminal/WheelScrollGestureTest.kt index ad3f679d..7d7f8aa7 100644 --- a/lib/src/test/java/org/connectbot/terminal/WheelScrollGestureTest.kt +++ b/lib/src/test/java/org/connectbot/terminal/WheelScrollGestureTest.kt @@ -57,6 +57,15 @@ class WheelScrollGestureTest { } } + private companion object { + /** Longer than WAIT_FOR_SECOND_TOUCH_MS, so a move counts as a scroll. */ + const val GRACE_PERIOD_MS = 100L + + const val DRAG_STEPS = 4 + const val DRAG_STEP_MS = 16L + const val DRAG_STEP_PX = 100f + } + /** The mouse-tracking sequence Claude Code's flicker-free renderer sends. */ private val enableMouseTracking = "\u001B[?1000h\u001B[?1002h\u001B[?1003h\u001B[?1006h" @@ -95,26 +104,30 @@ class WheelScrollGestureTest { /** * Drag downwards, which scrolls back towards earlier output. * - * Two moves, not one: the local scroll path anchors its offset at the moment - * the gesture is classified, so it only travels on moves after that point. + * Several moves, not one: the local scroll path anchors its offset at the + * moment the gesture is classified, so it only travels on moves after that + * point. */ - private fun dragDown() { + private fun dragDown(sample: () -> Int = { 0 }): DragSamples { composeTestRule.mainClock.autoAdvance = false composeTestRule.onRoot().performTouchInput { down(0, center) } // Past the multi-touch grace period, so the move is taken as a scroll. - composeTestRule.mainClock.advanceTimeBy(100) - - composeTestRule.onRoot().performTouchInput { - moveTo(0, center + Offset(0f, 200f)) + composeTestRule.onRoot().performTouchInput { advanceEventTime(GRACE_PERIOD_MS) } + composeTestRule.mainClock.advanceTimeBy(GRACE_PERIOD_MS) + + // Event time, not just the frame clock, has to advance across the moves: + // it is what the velocity tracker reads, and a fling needs real velocity. + repeat(DRAG_STEPS) { step -> + composeTestRule.onRoot().performTouchInput { + advanceEventTime(DRAG_STEP_MS) + moveTo(0, center + Offset(0f, DRAG_STEP_PX * (step + 1))) + } + composeTestRule.mainClock.advanceTimeBy(DRAG_STEP_MS) + composeTestRule.waitForIdle() } - composeTestRule.mainClock.advanceTimeBy(100) - composeTestRule.waitForIdle() - composeTestRule.onRoot().performTouchInput { - moveTo(0, center + Offset(0f, 400f)) - } - composeTestRule.mainClock.advanceTimeBy(100) - composeTestRule.waitForIdle() + // Sampled before the finger lifts, so the fling cannot contribute. + val afterDrag = sample() composeTestRule.onRoot().performTouchInput { up(0) } @@ -122,8 +135,22 @@ class WheelScrollGestureTest { composeTestRule.waitForIdle() composeTestRule.mainClock.advanceTimeBy(1000) composeTestRule.waitForIdle() + + return DragSamples(afterDrag = afterDrag, afterFling = sample()) } + /** + * A measurement taken at the end of the drag and again once the fling settles. + * + * The two are equal in practice here: injected touch input does not carry + * enough velocity through this harness for a decay animation to run, on + * either the wheel path or the local one. Sampling at the end of the drag is + * still what makes these tests specific — without it, reports produced only + * by a fling would be indistinguishable from reports produced by the drag. + * The fling's own conversion and bound are covered in WheelScrollerTest. + */ + private data class DragSamples(val afterDrag: Int, val afterFling: Int) + @Test fun testScrollGoesToApplicationWhenTrackingEnabled() { val emulator = emulatorWithContent() @@ -134,11 +161,12 @@ class WheelScrollGestureTest { val initialPosition = controller.scrollbackPosition output.setLength(0) - dragDown() + val reports = dragDown { wheelReports() } assertTrue( - "Expected wheel reports, got: ${output.toString().replace("\u001B", "ESC")}", - wheelReports() > 0, + "Expected wheel reports from the drag itself, got: " + + output.toString().replace("\u001B", "ESC"), + reports.afterDrag > 0, ) assertEquals( "Local scrollback must not move; the application owns the viewport", @@ -172,9 +200,13 @@ class WheelScrollGestureTest { val initialPosition = controller.scrollbackPosition output.setLength(0) - dragDown() + val lines = dragDown { controller.scrollbackPosition } assertEquals("no mouse reports without tracking", 0, wheelReports()) + assertTrue( + "the drag itself should scroll, not just the fling", + lines.afterDrag > initialPosition, + ) assertTrue( "local scrollback should have moved (initial=$initialPosition, " + "current=${controller.scrollbackPosition})", From bd12411e7d64e7ac5c19c16cd11a8adbc68e1938 Mon Sep 17 00:00:00 2001 From: Brandon Martin-Anderson Date: Sat, 25 Jul 2026 23:42:09 -0700 Subject: [PATCH 08/13] feat(mouse): report clicks and tighten the mouse invariants 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 --- lib/README.md | 14 +- lib/api.txt | 3 +- lib/src/main/cpp/Terminal.cpp | 54 +++++-- lib/src/main/cpp/Terminal.h | 21 +++ lib/src/main/cpp/libvterm/src/state.c | 32 ++-- .../org/connectbot/terminal/MouseInput.kt | 2 +- .../java/org/connectbot/terminal/Terminal.kt | 46 +++--- .../connectbot/terminal/TerminalEmulator.kt | 93 +++++++---- .../org/connectbot/terminal/TerminalNative.kt | 26 ++- .../org/connectbot/terminal/WheelScroller.kt | 86 ++++++++-- .../connectbot/terminal/MouseReportingTest.kt | 148 +++++++++++++++++- .../terminal/WheelScrollGestureTest.kt | 82 +++++++++- .../connectbot/terminal/WheelScrollerTest.kt | 148 ++++++++++++++++-- 13 files changed, 637 insertions(+), 118 deletions(-) diff --git a/lib/README.md b/lib/README.md index 172ca4f5..6956a3c1 100644 --- a/lib/README.md +++ b/lib/README.md @@ -47,6 +47,18 @@ Mouse → TerminalEmulator.scrollWheel() → libvterm → onKeyboardInput() Mouse reports are only emitted once the running application asks for them with DECSET 1000/1002/1003; check `TerminalEmulator.mouseTracking` to know whether a -gesture belongs to the application or to the terminal's own scrollback. +gesture belongs to the application or to the terminal's own scrollback. It is +Compose state, so reading it in a composable subscribes to it. + +While tracking is on, `Terminal` routes a tap to the application as a click and a +scroll as wheel detents. Long-press selection stays local — it remains the way to +copy text out of a full-screen application. + +The public surface is `mouseClick` and `scrollWheel` only — a click is always +delivered with its release, and there is no way to report a bare press or bare +pointer motion, neither of which a touch gesture produces. Coordinates are +clamped to the screen and a single `scrollWheel` call reports a bounded number of +detents, so no caller can put a malformed report or an unbounded burst on the +wire. **Important**: Callbacks must not call back into Terminal methods (causes deadlock). Defer work to avoid reentrancy. diff --git a/lib/api.txt b/lib/api.txt index 900f7359..adea20d9 100644 --- a/lib/api.txt +++ b/lib/api.txt @@ -123,8 +123,7 @@ package org.connectbot.terminal { method public String? getLastCommandOutput(); method @InaccessibleFromKotlin public org.connectbot.terminal.MouseTracking getMouseTracking(); method public java.util.List getUrls(optional org.connectbot.terminal.UrlScanScope scope); - method public void mouseButton(org.connectbot.terminal.MouseButton button, int row, int col, boolean pressed, optional int modifiers); - method public void mouseMove(int row, int col, optional int modifiers); + method public void mouseClick(org.connectbot.terminal.MouseButton button, int row, int col, optional int modifiers); method public void resize(int newRows, int newCols); method public void scrollWheel(org.connectbot.terminal.WheelDirection direction, int row, int col, optional int steps, optional int modifiers); method public int setAnsiPalette(int[] ansiColors); diff --git a/lib/src/main/cpp/Terminal.cpp b/lib/src/main/cpp/Terminal.cpp index c77f7c17..aa428483 100644 --- a/lib/src/main/cpp/Terminal.cpp +++ b/lib/src/main/cpp/Terminal.cpp @@ -426,6 +426,19 @@ bool Terminal::dispatchCharacter(int modifiers, int codepoint) { } // Mouse input handlers +VTermModifier Terminal::positionMouseLocked(int row, int col, int modifiers) { + VTermModifier mod = toVTermModifier(modifiers); + + // libvterm only emits a report here when the application asked for drag or + // motion tracking; otherwise this just records the position that a + // subsequent button report will carry. + vterm_mouse_move(mVt, + std::clamp(row, 0, mRows > 0 ? mRows - 1 : 0), + std::clamp(col, 0, mCols > 0 ? mCols - 1 : 0), + mod); + return mod; +} + bool Terminal::mouseMove(int row, int col, int modifiers) { std::scoped_lock lock(mLock); @@ -433,10 +446,7 @@ bool Terminal::mouseMove(int row, int col, int modifiers) { return false; } - // libvterm only emits a report here when the application asked for drag or - // motion tracking; otherwise this just records the position that a - // subsequent mouseButton() report will carry. - vterm_mouse_move(mVt, row, col, toVTermModifier(modifiers)); + positionMouseLocked(row, col, modifiers); return true; } @@ -450,9 +460,25 @@ bool Terminal::mouseButton(int row, int col, int button, bool pressed, int modif // Position and button are set under one lock: libvterm carries the position // recorded by the move into the button report, so a concurrent move for a // different gesture must not be able to land between the two. - VTermModifier mod = toVTermModifier(modifiers); - vterm_mouse_move(mVt, row, col, mod); - vterm_mouse_button(mVt, button, pressed, mod); + vterm_mouse_button(mVt, button, pressed, positionMouseLocked(row, col, modifiers)); + return true; +} + +bool Terminal::mouseClick(int row, int col, int button, int modifiers) { + std::scoped_lock lock(mLock); + + if (!mVt) { + return false; + } + + // Press and release under one lock. An application tracks button state from + // these reports, so a press whose release is lost - dropped by a caller, or + // separated from it by a reset that clears the button state in between - + // leaves the application believing the button is still down. Emitting the + // pair as one operation means a click cannot be left half-delivered. + VTermModifier mod = positionMouseLocked(row, col, modifiers); + vterm_mouse_button(mVt, button, true, mod); + vterm_mouse_button(mVt, button, false, mod); return true; } @@ -463,9 +489,9 @@ bool Terminal::scrollWheel(int row, int col, int button, int steps, int modifier return false; } - VTermModifier mod = toVTermModifier(modifiers); - vterm_mouse_move(mVt, row, col, mod); - for (int i = 0; i < steps; i++) { + VTermModifier mod = positionMouseLocked(row, col, modifiers); + int bounded = std::min(steps, MAX_WHEEL_STEPS_PER_CALL); + for (int i = 0; i < bounded; i++) { // Wheel buttons report a press with no matching release; libvterm emits // one report per call. vterm_mouse_button(mVt, button, true, mod); @@ -1285,6 +1311,14 @@ Java_org_connectbot_terminal_TerminalNative_nativeMouseButton(JNIEnv* /* env */, return term->mouseButton(row, col, button, pressed, modifiers); } +JNIEXPORT jboolean JNICALL +Java_org_connectbot_terminal_TerminalNative_nativeMouseClick(JNIEnv* /* env */, jobject /* thiz */, + jlong ptr, jint row, jint col, jint button, + jint modifiers) { + auto* term = reinterpret_cast(ptr); + return term->mouseClick(row, col, button, modifiers); +} + JNIEXPORT jboolean JNICALL Java_org_connectbot_terminal_TerminalNative_nativeScrollWheel(JNIEnv* /* env */, jobject /* thiz */, jlong ptr, jint row, jint col, jint button, diff --git a/lib/src/main/cpp/Terminal.h b/lib/src/main/cpp/Terminal.h index 4a5dce53..25d8e1f2 100644 --- a/lib/src/main/cpp/Terminal.h +++ b/lib/src/main/cpp/Terminal.h @@ -64,10 +64,18 @@ class Terminal { // Mouse input - generates escape sequences only when the application has // requested mouse tracking (DECSET 1000/1002/1003). Encoding follows the // protocol the application selected (X10, UTF-8, SGR or rxvt). + // + // Coordinates are clamped to the screen; see positionMouseLocked(). bool mouseMove(int row, int col, int modifiers); bool mouseButton(int row, int col, int button, bool pressed, int modifiers); + bool mouseClick(int row, int col, int button, int modifiers); bool scrollWheel(int row, int col, int button, int steps, int modifiers); + // Most wheel detents one scrollWheel() call may report. The loop runs with + // mLock held and emits a report per iteration, so the bound belongs here + // rather than with any particular caller. + static constexpr int MAX_WHEEL_STEPS_PER_CALL = 32; + // Cell data retrieval for rendering int getCellRun(JNIEnv* env, int row, int col, jobject runObject); @@ -121,6 +129,19 @@ class Terminal { static bool cellStyleEqual(const VTermScreenCell& a, const VTermScreenCell& b); void resolveColor(const VTermColor& color, uint8_t& r, uint8_t& g, uint8_t& b); + // The single path by which any mouse report reaches libvterm. Clamps + // row/col to the screen and positions the pointer, returning the translated + // modifiers for the caller's own report. Every public mouse method goes + // through it, so none of them can report at a coordinate off the screen - + // libvterm's X10 encoder clamps only the high end and would otherwise emit + // a control byte into the PTY stream for a negative coordinate. + // + // Clamping against mRows/mCols under mLock, rather than against a size the + // caller believes, means a resize racing a gesture cannot slip through. + // + // Caller must hold mLock and must have checked mVt. + VTermModifier positionMouseLocked(int row, int col, int modifiers); + // libvterm state VTerm* mVt; VTermScreen* mVts; diff --git a/lib/src/main/cpp/libvterm/src/state.c b/lib/src/main/cpp/libvterm/src/state.c index 809ae806..f27a7e01 100644 --- a/lib/src/main/cpp/libvterm/src/state.c +++ b/lib/src/main/cpp/libvterm/src/state.c @@ -56,6 +56,20 @@ static void erase(VTermState *state, VTermRect rect, int selective) return; } +/* Local modification: every field describing the mouse lives here, so that + * creating a state and resetting one cannot drift apart. Upstream initialised + * these in vterm_state_new() and cleared only mouse_flags in + * vterm_state_reset(), which left the report encoding and any held button + * surviving a reset. */ +static void reset_mouse_state(VTermState *state) +{ + state->mouse_col = 0; + state->mouse_row = 0; + state->mouse_buttons = 0; + state->mouse_flags = 0; + state->mouse_protocol = MOUSE_X10; +} + static VTermState *vterm_state_new(VTerm *vt) { VTermState *state = vterm_allocator_malloc(vt, sizeof(VTermState)); @@ -65,11 +79,7 @@ static VTermState *vterm_state_new(VTerm *vt) state->rows = vt->rows; state->cols = vt->cols; - state->mouse_col = 0; - state->mouse_row = 0; - state->mouse_buttons = 0; - - state->mouse_protocol = MOUSE_X10; + reset_mouse_state(state); state->callbacks = NULL; state->cbdata = NULL; @@ -2087,7 +2097,7 @@ void vterm_state_reset(VTermState *state, int hard) state->mode.bracketpaste = 0; state->mode.report_focus = 0; - state->mouse_flags = 0; + reset_mouse_state(state); state->vt->mode.ctrl8bit = 0; @@ -2125,11 +2135,11 @@ void vterm_state_reset(VTermState *state, int hard) settermprop_bool(state, VTERM_PROP_CURSORVISIBLE, 1); settermprop_bool(state, VTERM_PROP_CURSORBLINK, 1); settermprop_int (state, VTERM_PROP_CURSORSHAPE, VTERM_PROP_CURSORSHAPE_BLOCK); - /* Local modification: clearing state->mouse_flags above disables reporting - * without telling the embedder. A terminal mirroring VTERM_PROP_MOUSE would - * then believe an application still wants the mouse and keep routing gestures - * into a vterm that silently drops them. This notifies; the assignment above - * is kept so the flags are cleared even if the callback vetoes the store. */ + /* Local modification: reset_mouse_state() above disables reporting without + * telling the embedder. A terminal mirroring VTERM_PROP_MOUSE would then + * believe an application still wants the mouse and keep routing gestures into + * a vterm that silently drops them. This notifies; the reset above is kept so + * the state is cleared even if the callback vetoes the store. */ settermprop_int (state, VTERM_PROP_MOUSE, VTERM_PROP_MOUSE_NONE); if(hard) { diff --git a/lib/src/main/java/org/connectbot/terminal/MouseInput.kt b/lib/src/main/java/org/connectbot/terminal/MouseInput.kt index f54a82f3..78a2cb4f 100644 --- a/lib/src/main/java/org/connectbot/terminal/MouseInput.kt +++ b/lib/src/main/java/org/connectbot/terminal/MouseInput.kt @@ -1,6 +1,6 @@ /* * ConnectBot Terminal - * Copyright 2026 Kenny Root + * Copyright 2026 Termlib contributors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/lib/src/main/java/org/connectbot/terminal/Terminal.kt b/lib/src/main/java/org/connectbot/terminal/Terminal.kt index aea82787..b5df5523 100644 --- a/lib/src/main/java/org/connectbot/terminal/Terminal.kt +++ b/lib/src/main/java/org/connectbot/terminal/Terminal.kt @@ -1262,24 +1262,10 @@ internal fun TerminalWithAccessibility( scrollJob?.cancel() scrollJob = launch { if (scroller != null) { - // Decay a scratch offset on the same curve the - // local scrollback uses, reporting the detents it - // passes over so a fling feels the same either way. - // Bounded because, unlike the local path, there is - // no scrollback size to run out of. - val flingOffset = Animatable(0f) - flingOffset.updateBounds( - lowerBound = -scroller.maxFlingTravelPx, - upperBound = scroller.maxFlingTravelPx, - ) - var lastValue = 0f - flingOffset.animateDecay( - initialVelocity = velocity.y, - animationSpec = splineBasedDecay(density), - ) { - scroller.scrollBy(value - lastValue) - lastValue = value - } + // The application scrolls itself, so the fling is + // reported rather than animated; the scroller owns + // the decay and the bound that stops it. + scroller.fling(velocity.y, splineBasedDecay(density)) } else { scrollOffset.animateDecay( initialVelocity = velocity.y, @@ -1303,15 +1289,29 @@ internal fun TerminalWithAccessibility( GestureType.Undetermined -> { // This is a tap. If a selection is active, clear it. - // Otherwise, check for hyperlink or forward the tap. + // Otherwise the application gets it if it asked for the + // mouse; failing that, check for a hyperlink and forward. + val tapCol = (down.position.x / baseCharWidth).toInt() + .coerceIn(0, screenState.snapshot.cols - 1) + val tapRow = (down.position.y / baseCharHeight).toInt() + .coerceIn(0, screenState.snapshot.rows - 1) + if (selectionManager.mode != SelectionMode.NONE) { selectionManager.clearSelection() + } else if (terminalEmulator.mouseTracking.isEnabled) { + // The application owns the viewport, so it owns the + // click too — including on anything that looks like a + // link, which it drew and will handle itself. Sent as + // one click so the release cannot go missing. + // Long-press selection stays local: it remains the way + // to copy text out of a full-screen application. + terminalEmulator.mouseClick(MouseButton.LEFT, tapRow, tapCol) + if (keyboardEnabled) { + focusRequester.requestFocus() + } + currentOnTerminalTap() } else { // Check if tap is on a hyperlink - val tapCol = (down.position.x / baseCharWidth).toInt() - .coerceIn(0, screenState.snapshot.cols - 1) - val tapRow = (down.position.y / baseCharHeight).toInt() - .coerceIn(0, screenState.snapshot.rows - 1) val hyperlinkUrl = screenState.getHyperlinkUrlAt( tapRow, tapCol, diff --git a/lib/src/main/java/org/connectbot/terminal/TerminalEmulator.kt b/lib/src/main/java/org/connectbot/terminal/TerminalEmulator.kt index 3cf40a5b..7660cbb4 100644 --- a/lib/src/main/java/org/connectbot/terminal/TerminalEmulator.kt +++ b/lib/src/main/java/org/connectbot/terminal/TerminalEmulator.kt @@ -23,6 +23,9 @@ import android.os.Looper import android.util.Log import android.view.Choreographer import androidx.annotation.VisibleForTesting +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.setValue import androidx.compose.ui.graphics.Color import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -116,35 +119,31 @@ sealed interface TerminalEmulator { * Once an application enables tracking it expects to receive the events * itself — a full-screen program keeps its own scrollback, so scrolling the * terminal's copy would do nothing useful. + * + * This is Compose snapshot state: reading it inside a composition subscribes + * to it, so UI that changes with the tracking mode recomposes when an + * application enables or disables it. */ val mouseTracking: MouseTracking /** - * Report the mouse moving to a cell. + * Report a complete click — a press and its matching release — at a cell. * - * A motion report is only emitted when the application asked for - * [MouseTracking.DRAG] (and a button is held) or [MouseTracking.MOVE]. - * Moving to the cell the mouse already occupies is a no-op, so this is safe - * to call for every pointer sample. + * The pair is emitted as one operation, so the release cannot be lost and + * the application cannot be left believing the button is still down. * - * @param row Row index (0-based) within the visible screen - * @param col Column index (0-based) within the visible screen - * @param modifiers Bitmask: 1=Shift, 2=Alt, 4=Ctrl - */ - fun mouseMove(row: Int, col: Int, modifiers: Int = 0) - - /** - * Report a mouse button press or release at a cell. - * - * Each press must be paired with a release; applications track button state - * and a dropped release leaves them believing the button is still down. + * There is deliberately no way to report a press without its release, or to + * report bare pointer motion. Both are only meaningful for input that holds + * a button down across events or hovers without one — neither of which a + * touch gesture produces — and both are easy to leave half-delivered. If + * physical mouse or stylus support needs them later, they can be added then, + * against a real caller. * - * @param row Row index (0-based) within the visible screen - * @param col Column index (0-based) within the visible screen - * @param pressed true for a press, false for a release + * @param row Row index (0-based); clamped to the visible screen + * @param col Column index (0-based); clamped to the visible screen * @param modifiers Bitmask: 1=Shift, 2=Alt, 4=Ctrl */ - fun mouseButton(button: MouseButton, row: Int, col: Int, pressed: Boolean, modifiers: Int = 0) + fun mouseClick(button: MouseButton, row: Int, col: Int, modifiers: Int = 0) /** * Report [steps] wheel detents at a cell. @@ -153,10 +152,12 @@ sealed interface TerminalEmulator { * over the screen. Callers converting a continuous gesture into detents * should rate-limit: applications commonly throttle or coalesce a flood of * wheel events, so a fling turned into hundreds of detents scrolls less far - * than the same distance delivered as a few dozen. + * than the same distance delivered as a few dozen. A single call reports a + * bounded number of detents however large [steps] is, so no caller can make + * one call occupy the terminal for an unbounded time. * - * @param row Row index (0-based) within the visible screen - * @param col Column index (0-based) within the visible screen + * @param row Row index (0-based); clamped to the visible screen + * @param col Column index (0-based); clamped to the visible screen * @param steps Number of detents to report; values below 1 send nothing * @param modifiers Bitmask: 1=Shift, 2=Alt, 4=Ctrl */ @@ -425,10 +426,13 @@ internal class TerminalEmulatorImpl( private var terminalTitle = "" private var isAltScreenActive = false - // Read outside damageLock by gesture handling on the UI thread, written - // from the native callback thread. - @Volatile - override var mouseTracking: MouseTracking = MouseTracking.NONE + // Read outside damageLock by gesture handling on the UI thread, written from + // the native callback thread. Snapshot state rather than @Volatile so that + // reading it in a composition subscribes to it: an embedder whose UI depends + // on the tracking mode gets recomposed instead of having to poll. Compose + // state supports writes from any thread and carries the same visibility + // guarantee @Volatile did. + override var mouseTracking: MouseTracking by mutableStateOf(MouseTracking.NONE) private set // Scrollback buffer @@ -538,18 +542,49 @@ internal class TerminalEmulatorImpl( /** * Report the mouse moving to a cell. + * + * Not part of the public API: bare motion is only meaningful for a device + * that can hover, and nothing in the library produces one yet. Kept because + * it is how the tracking modes that report motion — [MouseTracking.DRAG] and + * [MouseTracking.MOVE] — are exercised, and because it is the natural + * primitive if physical mouse support arrives. + * + * A motion report is only emitted when the application asked for + * [MouseTracking.DRAG] (and a button is held) or [MouseTracking.MOVE]. + * Moving to the cell the mouse already occupies is a no-op. + * + * @param row Row index (0-based); clamped to the visible screen + * @param col Column index (0-based); clamped to the visible screen + * @param modifiers Bitmask: 1=Shift, 2=Alt, 4=Ctrl */ - override fun mouseMove(row: Int, col: Int, modifiers: Int) { + internal fun mouseMove(row: Int, col: Int, modifiers: Int = 0) { terminalNative.mouseMove(row, col, modifiers) } /** * Report a mouse button press or release at a cell. + * + * Not part of the public API, for the same reason as [mouseMove]: a bare + * press is only meaningful for input that holds a button down across events, + * and a caller that loses the release leaves the application believing the + * button is still down. [mouseClick] is the form that cannot be misused. + * + * @param row Row index (0-based); clamped to the visible screen + * @param col Column index (0-based); clamped to the visible screen + * @param pressed true for a press, false for a release + * @param modifiers Bitmask: 1=Shift, 2=Alt, 4=Ctrl */ - override fun mouseButton(button: MouseButton, row: Int, col: Int, pressed: Boolean, modifiers: Int) { + internal fun mouseButton(button: MouseButton, row: Int, col: Int, pressed: Boolean, modifiers: Int = 0) { terminalNative.mouseButton(row, col, button.code, pressed, modifiers) } + /** + * Report a complete click at a cell. + */ + override fun mouseClick(button: MouseButton, row: Int, col: Int, modifiers: Int) { + terminalNative.mouseClick(row, col, button.code, modifiers) + } + /** * Report wheel detents at a cell. */ diff --git a/lib/src/main/java/org/connectbot/terminal/TerminalNative.kt b/lib/src/main/java/org/connectbot/terminal/TerminalNative.kt index 91414f94..11ecba64 100644 --- a/lib/src/main/java/org/connectbot/terminal/TerminalNative.kt +++ b/lib/src/main/java/org/connectbot/terminal/TerminalNative.kt @@ -117,8 +117,8 @@ internal class TerminalNative(callbacks: TerminalCallbacks) : AutoCloseable { * tracking (DECSET 1003). Moving to the cell the mouse already occupies is a * no-op, so repeated calls at the same cell do not flood the application. * - * @param row Row index (0-based) - * @param col Column index (0-based) + * @param row Row index (0-based); clamped to the screen natively + * @param col Column index (0-based); clamped to the screen natively * @param modifiers Bitmask: 1=Shift, 2=Alt, 4=Ctrl * @return true if handled */ @@ -152,12 +152,31 @@ internal class TerminalNative(callbacks: TerminalCallbacks) : AutoCloseable { return nativeMouseButton(nativePtr, row, col, button, pressed, modifiers) } + /** + * Dispatch a press and its matching release at a cell. + * + * Both reports are emitted under a single native lock, so a click cannot be + * left half-delivered — an application that saw the press always sees the + * release, whatever else is happening on other threads. + * + * @param row Row index (0-based) + * @param col Column index (0-based) + * @param button 1=left, 2=middle, 3=right + * @param modifiers Bitmask: 1=Shift, 2=Alt, 4=Ctrl + * @return true if handled + */ + fun mouseClick(row: Int, col: Int, button: Int, modifiers: Int): Boolean { + checkNotClosed() + return nativeMouseClick(nativePtr, row, col, button, modifiers) + } + /** * Dispatch [steps] presses of a wheel button at a cell. * * Equivalent to [steps] calls to [mouseButton] with a wheel button, but the * whole burst is emitted under a single native lock so it cannot be - * interleaved with another gesture's reports. + * interleaved with another gesture's reports. The native layer bounds the + * burst, so no caller can hold that lock for an arbitrary length of time. * * @param row Row index (0-based) * @param col Column index (0-based) @@ -278,6 +297,7 @@ internal class TerminalNative(callbacks: TerminalCallbacks) : AutoCloseable { private external fun nativeDispatchCharacter(ptr: Long, modifiers: Int, character: Int): Boolean private external fun nativeMouseMove(ptr: Long, row: Int, col: Int, modifiers: Int): Boolean private external fun nativeMouseButton(ptr: Long, row: Int, col: Int, button: Int, pressed: Boolean, modifiers: Int): Boolean + private external fun nativeMouseClick(ptr: Long, row: Int, col: Int, button: Int, modifiers: Int): Boolean private external fun nativeScrollWheel(ptr: Long, row: Int, col: Int, button: Int, steps: Int, modifiers: Int): Boolean private external fun nativeGetCellRun(ptr: Long, row: Int, col: Int, run: CellRun): Int private external fun nativeSetPaletteColors(ptr: Long, colors: IntArray, count: Int): Int diff --git a/lib/src/main/java/org/connectbot/terminal/WheelScroller.kt b/lib/src/main/java/org/connectbot/terminal/WheelScroller.kt index bdff19f4..99ccc782 100644 --- a/lib/src/main/java/org/connectbot/terminal/WheelScroller.kt +++ b/lib/src/main/java/org/connectbot/terminal/WheelScroller.kt @@ -1,6 +1,6 @@ /* * ConnectBot Terminal - * Copyright 2026 Kenny Root + * Copyright 2026 Termlib contributors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,9 @@ */ package org.connectbot.terminal +import androidx.compose.animation.core.AnimationState +import androidx.compose.animation.core.DecayAnimationSpec +import androidx.compose.animation.core.animateDecay import kotlin.math.abs import kotlin.math.min @@ -38,13 +41,13 @@ private const val LINES_PER_WHEEL_DETENT = 1f private const val MAX_DETENTS_PER_SAMPLE = 8 /** - * Most detents a single fling may report, as a bound on its decay distance. + * Most detents a single fling may report. * * The local scrollback path is bounded by the scrollback it has; the wheel path * has no comparable limit, because the terminal cannot know where the * application's own history begins or ends. Without a bound, a hard fling keeps - * emitting detents long after the application has hit its top, so this caps the - * decay well above what an ordinary fling covers. + * emitting detents long after the application has hit its top, so this caps a + * fling well above what an ordinary one covers. */ private const val MAX_DETENTS_PER_FLING = 200 @@ -74,34 +77,83 @@ internal class WheelScroller( ) { private val pixelsPerDetent = lineHeightPx * LINES_PER_WHEEL_DETENT - /** - * Travel a fling may cover before it should be stopped, as a bound for the - * decay animation driving [scrollBy]. - */ - val maxFlingTravelPx = pixelsPerDetent * MAX_DETENTS_PER_FLING - /** Travel not yet worth a whole detent, carried into the next sample. */ private var residualPx = 0f /** - * Report the detents covered by [deltaPx] of vertical travel. + * Report the detents covered by [deltaPx] of vertical travel while a finger + * is down. * * Positive values mean the finger moved down the screen, which reveals * earlier output and so reports as wheel up. + * + * Travel beyond the per-sample limit is discarded rather than queued. While + * the finger is down the gesture is a position, not a distance: a backlog + * would keep reporting after the finger has stopped, which reads as the + * content sliding out from under it. */ fun scrollBy(deltaPx: Float) { - if (pixelsPerDetent <= 0f || !deltaPx.isFinite()) return + report(deltaPx, limit = MAX_DETENTS_PER_SAMPLE, carryExcess = false) + } + + /** + * Report the detents covered by a fling, decaying [initialVelocityPx] on + * [decaySpec] — the same curve the local scrollback path flings on, so the + * gesture feels the same whoever ends up handling it. + * + * The decay lives here rather than in the gesture handler because it is + * inseparable from the conversion: the detent budget that stops a hard fling + * is spent by the reports, not by the distance travelled. Keeping both in + * one place also means the fling can be driven by a test frame clock instead + * of only by a real one. + * + * Unlike [scrollBy] this carries travel past the per-sample limit into the + * following frames rather than dropping it. A fling is a distance, and the + * decay's slow tail gives the carried detents somewhere to go — so how far a + * fling scrolls does not depend on how many frames the device managed to + * draw during it. + */ + suspend fun fling(initialVelocityPx: Float, decaySpec: DecayAnimationSpec) { + if (pixelsPerDetent <= 0f || !initialVelocityPx.isFinite()) return + + var budget = MAX_DETENTS_PER_FLING + var lastValue = 0f + AnimationState(initialValue = 0f, initialVelocity = initialVelocityPx) + .animateDecay(decaySpec) { + budget -= report( + deltaPx = value - lastValue, + limit = min(MAX_DETENTS_PER_SAMPLE, budget), + carryExcess = true, + ) + lastValue = value + if (budget <= 0) cancelAnimation() + } + } + + /** + * Convert [deltaPx] of travel into at most [limit] detents and report them, + * returning how many were sent. + * + * When [carryExcess] is true, travel past [limit] stays in the residual for + * the next call; otherwise it is dropped. + */ + private fun report(deltaPx: Float, limit: Int, carryExcess: Boolean): Int { + if (pixelsPerDetent <= 0f || !deltaPx.isFinite() || limit < 1) return 0 residualPx += deltaPx - val detents = (residualPx / pixelsPerDetent).toInt() - if (detents == 0) return - residualPx -= detents * pixelsPerDetent + val wanted = (residualPx / pixelsPerDetent).toInt() + if (wanted == 0) return 0 + + val sent = min(abs(wanted), limit) + val consumed = if (carryExcess) sent else abs(wanted) + residualPx -= if (wanted > 0) consumed * pixelsPerDetent else -consumed * pixelsPerDetent emulator.scrollWheel( - direction = if (detents > 0) WheelDirection.UP else WheelDirection.DOWN, + direction = if (wanted > 0) WheelDirection.UP else WheelDirection.DOWN, row = anchorRow, col = anchorCol, - steps = min(abs(detents), MAX_DETENTS_PER_SAMPLE), + steps = sent, ) + return sent } } diff --git a/lib/src/test/java/org/connectbot/terminal/MouseReportingTest.kt b/lib/src/test/java/org/connectbot/terminal/MouseReportingTest.kt index 6eaa1bd5..1bc85053 100644 --- a/lib/src/test/java/org/connectbot/terminal/MouseReportingTest.kt +++ b/lib/src/test/java/org/connectbot/terminal/MouseReportingTest.kt @@ -1,6 +1,6 @@ /* * ConnectBot Terminal - * Copyright 2026 Kenny Root + * Copyright 2026 Termlib contributors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -65,11 +65,16 @@ class MouseReportingTest { private fun drain() = InstrumentationRegistry.getInstrumentation().waitForIdleSync() } - private fun emulator(out: Output): TerminalEmulator = TerminalEmulatorFactory.create( + /** + * The concrete type, not the [TerminalEmulator] interface: motion and bare + * button presses are internal to the library, so only the implementation + * exposes them. + */ + private fun emulator(out: Output): TerminalEmulatorImpl = TerminalEmulatorFactory.create( initialRows = 24, initialCols = 80, onKeyboardInput = { out.append(it) }, - ) + ) as TerminalEmulatorImpl private fun TerminalEmulator.send(s: String) = writeInput(s.toByteArray()) @@ -346,4 +351,141 @@ class MouseReportingTest { // pointer is already on that cell. assertEquals("\u001B[<64;5;5M".repeat(2), out.text) } + + // ----------------------------------------------------------------------- + // Clicks + // ----------------------------------------------------------------------- + + @Test + fun testClickEmitsPressAndRelease() = runBlocking { + val out = Output() + val term = emulator(out) + term.send("\u001B[?1000h\u001B[?1006h") + out.clear() + + term.mouseClick(MouseButton.LEFT, row = 2, col = 7) + + // Exactly what a press followed by its release encodes to, and nothing + // an application could mistake for a button still being held. + assertEquals("\u001B[<0;8;3M\u001B[<0;8;3m", out.text) + } + + @Test + fun testClickReleasesEveryButton() = runBlocking { + val out = Output() + val term = emulator(out) + term.send("\u001B[?1000h\u001B[?1006h") + + for (button in MouseButton.entries) { + out.clear() + term.mouseClick(button, row = 0, col = 0) + + val reports = out.text + assertTrue( + "$button click should end in a release, got: " + reports.replace("\u001B", "ESC"), + reports.endsWith("m"), + ) + } + } + + @Test + fun testClickIsSilentWhileTrackingDisabled() = runBlocking { + val out = Output() + val term = emulator(out) + + term.mouseClick(MouseButton.LEFT, row = 2, col = 7) + + assertEquals("", out.text) + } + + // ----------------------------------------------------------------------- + // Coordinates outside the screen + // ----------------------------------------------------------------------- + + @Test + fun testCoordinatesAreClampedToTheScreen() = runBlocking { + // libvterm's X10 encoder clamps only the high end, so a negative + // coordinate would otherwise put a control byte on the wire. Clamping + // happens natively, against the size the terminal actually has. + val out = Output() + val term = emulator(out) + term.send("\u001B[?1000h\u001B[?1006h") + + out.clear() + term.mouseClick(MouseButton.LEFT, row = -5, col = -9) + assertEquals("clamped to the first cell", "\u001B[<0;1;1M\u001B[<0;1;1m", out.text) + + out.clear() + term.mouseClick(MouseButton.LEFT, row = 9999, col = 9999) + // 24x80 terminal, so the last cell is row 23, col 79, 1-based in SGR. + assertEquals("clamped to the last cell", "\u001B[<0;80;24M\u001B[<0;80;24m", out.text) + } + + @Test + fun testWheelCoordinatesAreClampedToTheScreen() = runBlocking { + val out = Output() + val term = emulator(out) + term.send("\u001B[?1000h\u001B[?1006h") + out.clear() + + term.scrollWheel(WheelDirection.UP, row = Int.MIN_VALUE, col = Int.MIN_VALUE) + + assertEquals("\u001B[<64;1;1M", out.text) + } + + @Test + fun testWheelBurstIsBoundedNatively() = runBlocking { + // The native loop emits a report per step while holding the terminal + // lock, so an absurd step count must not translate into an absurd number + // of reports - whatever a caller asks for. + val out = Output() + val term = emulator(out) + term.send("\u001B[?1000h\u001B[?1006h") + out.clear() + + term.scrollWheel(WheelDirection.DOWN, row = 0, col = 0, steps = Int.MAX_VALUE) + + val reports = Regex(Regex.escape("\u001B[<65;1;1M")).findAll(out.text).count() + assertTrue("bounded burst, got $reports reports", reports in 1..64) + } + + // ----------------------------------------------------------------------- + // Reset clears the rest of the mouse state, not just the mode + // ----------------------------------------------------------------------- + + @Test + fun testResetClearsReportEncoding() = runBlocking { + // A reset that leaves the SGR encoding selected would answer a later + // plain DECSET 1000 in a protocol that application never asked for. + val out = Output() + val term = emulator(out) + term.send("\u001B[?1000h\u001B[?1006h") + + term.send("\u001Bc") + term.send("\u001B[?1000h") + out.clear() + + term.scrollWheel(WheelDirection.UP, row = 0, col = 0) + + // X10 again: CSI M, then (code|mods)+0x20, col+0x21, row+0x21. + assertEquals("\u001B[M`!!", out.text) + } + + @Test + fun testResetClearsHeldButtons() = runBlocking { + // A press whose release never came leaves libvterm believing a button is + // down, which makes DRAG tracking report motion with nothing held. + val out = Output() + val term = emulator(out) + term.send("\u001B[?1000h\u001B[?1006h") + term.mouseButton(MouseButton.LEFT, row = 0, col = 0, pressed = true) + + term.send("\u001Bc") + term.send("\u001B[?1002h\u001B[?1006h") + out.clear() + + term.mouseMove(row = 5, col = 5) + + assertEquals("no button is held, so a drag reports nothing", "", out.text) + } } diff --git a/lib/src/test/java/org/connectbot/terminal/WheelScrollGestureTest.kt b/lib/src/test/java/org/connectbot/terminal/WheelScrollGestureTest.kt index 7d7f8aa7..6595979c 100644 --- a/lib/src/test/java/org/connectbot/terminal/WheelScrollGestureTest.kt +++ b/lib/src/test/java/org/connectbot/terminal/WheelScrollGestureTest.kt @@ -1,6 +1,6 @@ /* * ConnectBot Terminal - * Copyright 2026 Kenny Root + * Copyright 2026 Termlib contributors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -84,6 +84,29 @@ class WheelScrollGestureTest { /** Count of wheel reports in the output, ignoring any motion reports. */ private fun wheelReports(): Int = Regex("\u001B\\[<6[4-7];").findAll(output.toString()).count() + /** Count of left-button press/release pairs in the output. */ + private fun clickReports(): Int = Regex("\u001B\\[<0;\\d+;\\d+M" + "\u001B\\[<0;\\d+;\\d+m").findAll(output.toString()).count() + + /** + * Tap once in the middle of the terminal. + * + * Event time is advanced well past the double-tap timeout first, so + * consecutive calls stay separate taps rather than becoming a word + * selection, and the touch is released quickly enough not to become a long + * press. + */ + private fun tap() { + composeTestRule.onRoot().performTouchInput { + advanceEventTime(GRACE_PERIOD_MS * 10) + down(0, center) + advanceEventTime(DRAG_STEP_MS) + up(0) + } + composeTestRule.waitForIdle() + composeTestRule.mainClock.advanceTimeBy(1000) + composeTestRule.waitForIdle() + } + private fun showTerminal(emulator: TerminalEmulator): ScrollController { var scrollController: ScrollController? = null composeTestRule.setContent { @@ -147,7 +170,11 @@ class WheelScrollGestureTest { * either the wheel path or the local one. Sampling at the end of the drag is * still what makes these tests specific — without it, reports produced only * by a fling would be indistinguishable from reports produced by the drag. - * The fling's own conversion and bound are covered in WheelScrollerTest. + * + * That the fling is invisible here is why WheelScroller owns its decay: it + * can then be driven directly from a test frame clock, which is what + * WheelScrollerTest does. This harness covers the routing decision; it is + * not the place the fling itself gets tested. */ private data class DragSamples(val afterDrag: Int, val afterFling: Int) @@ -213,4 +240,55 @@ class WheelScrollGestureTest { controller.scrollbackPosition > initialPosition, ) } + + @Test + fun testTapReportsAClickWhenTrackingEnabled() { + val emulator = emulatorWithContent() + emulator.writeInput(enableMouseTracking.toByteArray()) + (emulator as? TerminalEmulatorImpl)?.processPendingUpdates() + + showTerminal(emulator) + output.setLength(0) + + tap() + + assertEquals( + "a tap should reach the application as one complete click, got: " + + output.toString().replace("\u001B", "ESC"), + 1, + clickReports(), + ) + } + + @Test + fun testTapStaysLocalWhenTrackingDisabled() { + val emulator = emulatorWithContent() + + showTerminal(emulator) + output.setLength(0) + + tap() + + assertEquals("no mouse reports without tracking", 0, clickReports()) + assertEquals("", output.toString()) + } + + @Test + fun testTapDoesNotLeaveAButtonHeld() { + // Every press the application sees has to be followed by its release, or + // it spends the rest of the session believing the button is down. + val emulator = emulatorWithContent() + emulator.writeInput(enableMouseTracking.toByteArray()) + (emulator as? TerminalEmulatorImpl)?.processPendingUpdates() + + showTerminal(emulator) + output.setLength(0) + + repeat(3) { tap() } + + val presses = Regex("\u001B\\[<0;\\d+;\\d+M").findAll(output.toString()).count() + val releases = Regex("\u001B\\[<0;\\d+;\\d+m").findAll(output.toString()).count() + assertEquals("every press is released", presses, releases) + assertEquals(3, presses) + } } diff --git a/lib/src/test/java/org/connectbot/terminal/WheelScrollerTest.kt b/lib/src/test/java/org/connectbot/terminal/WheelScrollerTest.kt index f474faaf..a42b6886 100644 --- a/lib/src/test/java/org/connectbot/terminal/WheelScrollerTest.kt +++ b/lib/src/test/java/org/connectbot/terminal/WheelScrollerTest.kt @@ -1,6 +1,6 @@ /* * ConnectBot Terminal - * Copyright 2026 Kenny Root + * Copyright 2026 Termlib contributors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,9 +16,14 @@ */ package org.connectbot.terminal +import androidx.compose.animation.core.exponentialDecay +import androidx.compose.runtime.MonotonicFrameClock import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.platform.app.InstrumentationRegistry +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withContext import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue import org.junit.Test import org.junit.runner.RunWith @@ -37,6 +42,9 @@ class WheelScrollerTest { const val ANCHOR_ROW = 5 const val ANCHOR_COL = 7 + /** Detents one fling may report, mirroring MAX_DETENTS_PER_FLING. */ + const val FLING_DETENT_CAP = 200 + /** A wheel-up report at the anchor cell, in 1-based SGR coordinates. */ const val UP = "\u001B[<64;8;6M" @@ -44,6 +52,30 @@ class WheelScrollerTest { const val DOWN = "\u001B[<65;8;6M" } + /** + * A frame clock that runs animation frames back to back without waiting for + * a real one. + * + * A decay animation is driven by whatever [MonotonicFrameClock] is in the + * coroutine context, so supplying one makes a fling completely deterministic + * and as fast as the arithmetic — no Compose harness, no injected velocity + * that the device might not honour, and no dependence on how many frames a + * real device would have managed to draw. + */ + private class ImmediateFrameClock( + private val frameNanos: Long, + private val maxFrames: Int = 10_000, + ) : MonotonicFrameClock { + private var now = 0L + private var frames = 0 + + override suspend fun withFrameNanos(onFrame: (Long) -> R): R { + check(++frames <= maxFrames) { "animation did not settle within $maxFrames frames" } + now += frameNanos + return onFrame(now) + } + } + private val output = StringBuilder() /** The emulator behind the most recent [scroller], for tests that drive it directly. */ @@ -75,6 +107,23 @@ class WheelScrollerTest { return output.toString() } + /** How many times [report] appears in what has been sent so far. */ + private fun countOf(report: String): Int = Regex(Regex.escape(report)).findAll(reported()).count() + + /** Run [WheelScroller.fling] to completion on a deterministic frame clock. */ + private fun fling( + scroller: WheelScroller, + velocityPx: Float, + frameNanos: Long = 16_000_000L, + ) = runBlocking { + withContext(ImmediateFrameClock(frameNanos)) { + scroller.fling( + initialVelocityPx = velocityPx, + decaySpec = exponentialDecay(frictionMultiplier = 1f, absVelocityThreshold = 1f), + ) + } + } + @Test fun testOneDetentPerLineOfTravel() { val scroller = scroller() @@ -202,26 +251,93 @@ class WheelScrollerTest { } // ----------------------------------------------------------------------- - // Fling bound + // Fling // ----------------------------------------------------------------------- @Test - fun testMaxFlingTravelConvertsToTheDetentCap() { - // The bound handed to the fling animation has to mean what it says: the - // travel it permits is exactly the detent cap, fed one detent at a time - // so the per-sample rate limit does not mask it. + fun testFlingReportsInTheDirectionOfTravel() { val scroller = scroller() - var reports = 0 - var travelled = 0f - while (travelled < scroller.maxFlingTravelPx) { - scroller.scrollBy(LINE_HEIGHT) - travelled += LINE_HEIGHT - drain() - reports += Regex(Regex.escape(UP)).findAll(output.toString()).count() - output.setLength(0) - } + fling(scroller, velocityPx = 4000f) + + // A downward fling reveals earlier output, so it reports wheel up only. + assertTrue("expected wheel-up reports", countOf(UP) > 0) + assertEquals("no reports in the opposite direction", 0, countOf(DOWN)) + } + + @Test + fun testFlingUpwardsReportsWheelDown() { + val scroller = scroller() + + fling(scroller, velocityPx = -4000f) + + assertTrue("expected wheel-down reports", countOf(DOWN) > 0) + assertEquals("no reports in the opposite direction", 0, countOf(UP)) + } + + @Test + fun testFlingDistanceFollowsVelocity() { + val gentle = scroller() + fling(gentle, velocityPx = 1000f) + val gentleReports = countOf(UP) + + output.setLength(0) + val hard = scroller() + fling(hard, velocityPx = 8000f) + + assertTrue( + "a harder fling should travel further (gentle=$gentleReports, hard=${countOf(UP)})", + countOf(UP) > gentleReports, + ) + } + + @Test + fun testFlingIsBoundedByTheDetentCap() { + // Unlike the local path there is no scrollback to run out of, so an + // enormous velocity has to stop somewhere. + val scroller = scroller() - assertEquals("detents permitted by the fling bound", 200, reports) + fling(scroller, velocityPx = 5_000_000f) + + assertEquals("detents permitted by a single fling", FLING_DETENT_CAP, countOf(UP)) + } + + @Test + fun testFlingDistanceDoesNotDependOnFrameRate() { + // The per-sample limit caps how many detents leave in one frame. If that + // limit dropped the excess, a device drawing half as many frames would + // scroll a fling half as far. Carrying the excess is what makes these + // two runs agree. + val smooth = scroller() + fling(smooth, velocityPx = 6000f, frameNanos = 8_000_000L) + val smoothReports = countOf(UP) + + output.setLength(0) + val janky = scroller() + fling(janky, velocityPx = 6000f, frameNanos = 48_000_000L) + + assertEquals("same fling, fewer frames", smoothReports, countOf(UP)) + } + + @Test + fun testFlingSendsNothingWhileTrackingIsOff() { + val untracked = TerminalEmulatorFactory.create( + initialRows = 24, + initialCols = 80, + onKeyboardInput = { output.append(String(it, Charsets.ISO_8859_1)) }, + ) + + fling(WheelScroller(untracked, LINE_HEIGHT, ANCHOR_ROW, ANCHOR_COL), velocityPx = 4000f) + + assertEquals("", reported()) + } + + @Test + fun testNonFiniteFlingVelocityIsIgnored() { + val scroller = scroller() + + fling(scroller, velocityPx = Float.NaN) + + assertEquals("", reported()) } } From edac8c409e546218513bc12703c4878df89a5666 Mon Sep 17 00:00:00 2001 From: Brandon Martin-Anderson Date: Sat, 25 Jul 2026 23:59:21 -0700 Subject: [PATCH 09/13] refactor(mouse): remove duplication left by the mouse changes 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 --- .../java/org/connectbot/terminal/Terminal.kt | 25 +++++----- .../org/connectbot/terminal/WheelScroller.kt | 9 ++-- .../connectbot/terminal/MouseReportingTest.kt | 50 +++++++++++-------- 3 files changed, 46 insertions(+), 38 deletions(-) diff --git a/lib/src/main/java/org/connectbot/terminal/Terminal.kt b/lib/src/main/java/org/connectbot/terminal/Terminal.kt index b5df5523..f3e6e4cd 100644 --- a/lib/src/main/java/org/connectbot/terminal/Terminal.kt +++ b/lib/src/main/java/org/connectbot/terminal/Terminal.kt @@ -1125,10 +1125,8 @@ internal fun TerminalWithAccessibility( WheelScroller( emulator = terminalEmulator, lineHeightPx = baseCharHeight, - anchorRow = (down.position.y / baseCharHeight).toInt() - .coerceIn(0, screenState.snapshot.rows - 1), - anchorCol = (down.position.x / baseCharWidth).toInt() - .coerceIn(0, screenState.snapshot.cols - 1), + anchorRow = (down.position.y / baseCharHeight).toInt(), + anchorCol = (down.position.x / baseCharWidth).toInt(), ) } else { null @@ -1296,6 +1294,14 @@ internal fun TerminalWithAccessibility( val tapRow = (down.position.y / baseCharHeight).toInt() .coerceIn(0, screenState.snapshot.rows - 1) + // Request focus when terminal is tapped to show keyboard + fun forwardTap() { + if (keyboardEnabled) { + focusRequester.requestFocus() + } + currentOnTerminalTap() + } + if (selectionManager.mode != SelectionMode.NONE) { selectionManager.clearSelection() } else if (terminalEmulator.mouseTracking.isEnabled) { @@ -1306,10 +1312,7 @@ internal fun TerminalWithAccessibility( // Long-press selection stays local: it remains the way // to copy text out of a full-screen application. terminalEmulator.mouseClick(MouseButton.LEFT, tapRow, tapCol) - if (keyboardEnabled) { - focusRequester.requestFocus() - } - currentOnTerminalTap() + forwardTap() } else { // Check if tap is on a hyperlink val hyperlinkUrl = screenState.getHyperlinkUrlAt( @@ -1322,11 +1325,7 @@ internal fun TerminalWithAccessibility( // User tapped on a hyperlink currentOnHyperlinkClick(hyperlinkUrl) } else { - // Request focus when terminal is tapped to show keyboard - if (keyboardEnabled) { - focusRequester.requestFocus() - } - currentOnTerminalTap() + forwardTap() } } // Record tap for double-tap detection diff --git a/lib/src/main/java/org/connectbot/terminal/WheelScroller.kt b/lib/src/main/java/org/connectbot/terminal/WheelScroller.kt index 99ccc782..8073e594 100644 --- a/lib/src/main/java/org/connectbot/terminal/WheelScroller.kt +++ b/lib/src/main/java/org/connectbot/terminal/WheelScroller.kt @@ -66,8 +66,8 @@ private const val MAX_DETENTS_PER_FLING = 200 * * @param emulator The emulator to report to * @param lineHeightPx Height of one terminal line in pixels - * @param anchorRow Row (0-based) the gesture started on - * @param anchorCol Column (0-based) the gesture started on + * @param anchorRow Row (0-based) the gesture started on; the emulator clamps it + * @param anchorCol Column (0-based) the gesture started on; the emulator clamps it */ internal class WheelScroller( private val emulator: TerminalEmulator, @@ -144,12 +144,13 @@ internal class WheelScroller( val wanted = (residualPx / pixelsPerDetent).toInt() if (wanted == 0) return 0 + val up = wanted > 0 val sent = min(abs(wanted), limit) val consumed = if (carryExcess) sent else abs(wanted) - residualPx -= if (wanted > 0) consumed * pixelsPerDetent else -consumed * pixelsPerDetent + residualPx -= (if (up) consumed else -consumed) * pixelsPerDetent emulator.scrollWheel( - direction = if (wanted > 0) WheelDirection.UP else WheelDirection.DOWN, + direction = if (up) WheelDirection.UP else WheelDirection.DOWN, row = anchorRow, col = anchorCol, steps = sent, diff --git a/lib/src/test/java/org/connectbot/terminal/MouseReportingTest.kt b/lib/src/test/java/org/connectbot/terminal/MouseReportingTest.kt index 1bc85053..de4861b2 100644 --- a/lib/src/test/java/org/connectbot/terminal/MouseReportingTest.kt +++ b/lib/src/test/java/org/connectbot/terminal/MouseReportingTest.kt @@ -78,6 +78,14 @@ class MouseReportingTest { private fun TerminalEmulator.send(s: String) = writeInput(s.toByteArray()) + private companion object { + /** DECSET 1000 and 1006: click tracking, SGR encoding. */ + const val CLICK_TRACKING_SGR = "\u001B[?1000h\u001B[?1006h" + + /** DECSET 1003 and 1006: all-motion tracking, SGR encoding. */ + const val MOVE_TRACKING_SGR = "\u001B[?1003h\u001B[?1006h" + } + // ----------------------------------------------------------------------- // Tracking mode detection (VTERM_PROP_MOUSE) // ----------------------------------------------------------------------- @@ -122,7 +130,7 @@ class MouseReportingTest { // wants the mouse routes gestures nowhere and leaves the terminal with no // scrolling at all. val term = emulator(Output()) - term.send("\u001B[?1003h\u001B[?1006h") + term.send(MOVE_TRACKING_SGR) term.send("\u001Bc") @@ -133,7 +141,7 @@ class MouseReportingTest { fun testSoftResetClearsTracking() = runBlocking { // DECSTR, the same story by a different route. val term = emulator(Output()) - term.send("\u001B[?1003h\u001B[?1006h") + term.send(MOVE_TRACKING_SGR) term.send("\u001B[!p") @@ -146,7 +154,7 @@ class MouseReportingTest { // says, a reset terminal emits nothing. val out = Output() val term = emulator(out) - term.send("\u001B[?1003h\u001B[?1006h") + term.send(MOVE_TRACKING_SGR) term.send("\u001Bc") out.clear() @@ -180,7 +188,7 @@ class MouseReportingTest { // SGR (1006) is what every modern application selects, because X10 // cannot address columns beyond 223. - term.send("\u001B[?1000h\u001B[?1006h") + term.send(CLICK_TRACKING_SGR) out.clear() term.scrollWheel(WheelDirection.UP, row = 9, col = 19) @@ -193,7 +201,7 @@ class MouseReportingTest { fun testSgrWheelDownAndHorizontal() = runBlocking { val out = Output() val term = emulator(out) - term.send("\u001B[?1000h\u001B[?1006h") + term.send(CLICK_TRACKING_SGR) out.clear() term.scrollWheel(WheelDirection.DOWN, row = 0, col = 0) @@ -212,7 +220,7 @@ class MouseReportingTest { fun testMultipleStepsSendOneReportEach() = runBlocking { val out = Output() val term = emulator(out) - term.send("\u001B[?1000h\u001B[?1006h") + term.send(CLICK_TRACKING_SGR) out.clear() term.scrollWheel(WheelDirection.DOWN, row = 0, col = 0, steps = 3) @@ -224,7 +232,7 @@ class MouseReportingTest { fun testNonPositiveStepsSendNothing() = runBlocking { val out = Output() val term = emulator(out) - term.send("\u001B[?1000h\u001B[?1006h") + term.send(CLICK_TRACKING_SGR) out.clear() term.scrollWheel(WheelDirection.DOWN, row = 0, col = 0, steps = 0) @@ -237,7 +245,7 @@ class MouseReportingTest { fun testWheelModifiersAreEncoded() = runBlocking { val out = Output() val term = emulator(out) - term.send("\u001B[?1000h\u001B[?1006h") + term.send(CLICK_TRACKING_SGR) out.clear() // Modifier bits are shifted left by 2 in the report: shift=4, alt=8, @@ -270,7 +278,7 @@ class MouseReportingTest { fun testButtonPressAndReleaseEncoding() = runBlocking { val out = Output() val term = emulator(out) - term.send("\u001B[?1000h\u001B[?1006h") + term.send(CLICK_TRACKING_SGR) out.clear() term.mouseButton(MouseButton.LEFT, row = 2, col = 7, pressed = true) @@ -284,7 +292,7 @@ class MouseReportingTest { fun testMiddleAndRightButtonCodes() = runBlocking { val out = Output() val term = emulator(out) - term.send("\u001B[?1000h\u001B[?1006h") + term.send(CLICK_TRACKING_SGR) out.clear() term.mouseButton(MouseButton.MIDDLE, row = 0, col = 0, pressed = true) @@ -299,7 +307,7 @@ class MouseReportingTest { fun testClickTrackingDoesNotReportMotion() = runBlocking { val out = Output() val term = emulator(out) - term.send("\u001B[?1000h\u001B[?1006h") + term.send(CLICK_TRACKING_SGR) out.clear() term.mouseMove(row = 5, col = 5) @@ -312,7 +320,7 @@ class MouseReportingTest { fun testMoveTrackingReportsMotion() = runBlocking { val out = Output() val term = emulator(out) - term.send("\u001B[?1003h\u001B[?1006h") + term.send(MOVE_TRACKING_SGR) out.clear() term.mouseMove(row = 5, col = 9) @@ -325,7 +333,7 @@ class MouseReportingTest { fun testRepeatedMoveToSameCellIsSuppressed() = runBlocking { val out = Output() val term = emulator(out) - term.send("\u001B[?1003h\u001B[?1006h") + term.send(MOVE_TRACKING_SGR) out.clear() term.mouseMove(row = 5, col = 9) @@ -339,7 +347,7 @@ class MouseReportingTest { fun testWheelDoesNotEmitMotionUnderMoveTracking() = runBlocking { val out = Output() val term = emulator(out) - term.send("\u001B[?1003h\u001B[?1006h") + term.send(MOVE_TRACKING_SGR) // Park the pointer where the gesture is happening, then scroll there. term.mouseMove(row = 4, col = 4) @@ -360,7 +368,7 @@ class MouseReportingTest { fun testClickEmitsPressAndRelease() = runBlocking { val out = Output() val term = emulator(out) - term.send("\u001B[?1000h\u001B[?1006h") + term.send(CLICK_TRACKING_SGR) out.clear() term.mouseClick(MouseButton.LEFT, row = 2, col = 7) @@ -374,7 +382,7 @@ class MouseReportingTest { fun testClickReleasesEveryButton() = runBlocking { val out = Output() val term = emulator(out) - term.send("\u001B[?1000h\u001B[?1006h") + term.send(CLICK_TRACKING_SGR) for (button in MouseButton.entries) { out.clear() @@ -409,7 +417,7 @@ class MouseReportingTest { // happens natively, against the size the terminal actually has. val out = Output() val term = emulator(out) - term.send("\u001B[?1000h\u001B[?1006h") + term.send(CLICK_TRACKING_SGR) out.clear() term.mouseClick(MouseButton.LEFT, row = -5, col = -9) @@ -425,7 +433,7 @@ class MouseReportingTest { fun testWheelCoordinatesAreClampedToTheScreen() = runBlocking { val out = Output() val term = emulator(out) - term.send("\u001B[?1000h\u001B[?1006h") + term.send(CLICK_TRACKING_SGR) out.clear() term.scrollWheel(WheelDirection.UP, row = Int.MIN_VALUE, col = Int.MIN_VALUE) @@ -440,7 +448,7 @@ class MouseReportingTest { // of reports - whatever a caller asks for. val out = Output() val term = emulator(out) - term.send("\u001B[?1000h\u001B[?1006h") + term.send(CLICK_TRACKING_SGR) out.clear() term.scrollWheel(WheelDirection.DOWN, row = 0, col = 0, steps = Int.MAX_VALUE) @@ -459,7 +467,7 @@ class MouseReportingTest { // plain DECSET 1000 in a protocol that application never asked for. val out = Output() val term = emulator(out) - term.send("\u001B[?1000h\u001B[?1006h") + term.send(CLICK_TRACKING_SGR) term.send("\u001Bc") term.send("\u001B[?1000h") @@ -477,7 +485,7 @@ class MouseReportingTest { // down, which makes DRAG tracking report motion with nothing held. val out = Output() val term = emulator(out) - term.send("\u001B[?1000h\u001B[?1006h") + term.send(CLICK_TRACKING_SGR) term.mouseButton(MouseButton.LEFT, row = 0, col = 0, pressed = true) term.send("\u001Bc") From e14bb80be9464fd14465154bb46453eead355353 Mon Sep 17 00:00:00 2001 From: Brandon Martin-Anderson Date: Sun, 26 Jul 2026 00:03:43 -0700 Subject: [PATCH 10/13] perf(mouse): deliver a report burst in one upcall 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 --- lib/src/main/cpp/Terminal.cpp | 22 +++++++- lib/src/main/cpp/Terminal.h | 28 ++++++++++ .../connectbot/terminal/MouseReportingTest.kt | 56 +++++++++++++++++++ 3 files changed, 105 insertions(+), 1 deletion(-) diff --git a/lib/src/main/cpp/Terminal.cpp b/lib/src/main/cpp/Terminal.cpp index aa428483..32f2faf5 100644 --- a/lib/src/main/cpp/Terminal.cpp +++ b/lib/src/main/cpp/Terminal.cpp @@ -475,7 +475,9 @@ bool Terminal::mouseClick(int row, int col, int button, int modifiers) { // these reports, so a press whose release is lost - dropped by a caller, or // separated from it by a reset that clears the button state in between - // leaves the application believing the button is still down. Emitting the - // pair as one operation means a click cannot be left half-delivered. + // pair as one operation means a click cannot be left half-delivered, and + // coalescing sends the pair in one trip rather than two. + CoalescedOutput out(this); VTermModifier mod = positionMouseLocked(row, col, modifiers); vterm_mouse_button(mVt, button, true, mod); vterm_mouse_button(mVt, button, false, mod); @@ -489,6 +491,9 @@ bool Terminal::scrollWheel(int row, int col, int button, int steps, int modifier return false; } + // One report per detent leaves libvterm, so the whole burst is collected + // and delivered in a single upcall rather than one per detent. + CoalescedOutput out(this); VTermModifier mod = positionMouseLocked(row, col, modifiers); int bounded = std::min(steps, MAX_WHEEL_STEPS_PER_CALL); for (int i = 0; i < bounded; i++) { @@ -664,9 +669,24 @@ int Terminal::termSbClear(void* user) { void Terminal::termOutput(const char* s, size_t len, void* user) { auto* term = static_cast(user); + if (term->mOutputSink) { + term->mOutputSink->append(s, len); + return; + } term->invokeKeyboardOutput(s, len); } +Terminal::CoalescedOutput::CoalescedOutput(Terminal* term) : mTerm(term) { + mTerm->mOutputSink = &mBuffer; +} + +Terminal::CoalescedOutput::~CoalescedOutput() { + mTerm->mOutputSink = nullptr; + if (!mBuffer.empty()) { + mTerm->invokeKeyboardOutput(mBuffer.data(), mBuffer.size()); + } +} + // OSC sequence fallback handler // Handles fragmented OSC sequences by accumulating data across callbacks int Terminal::termOscFallback(int command, VTermStringFragment frag, void* user) { diff --git a/lib/src/main/cpp/Terminal.h b/lib/src/main/cpp/Terminal.h index 25d8e1f2..5a92b3c2 100644 --- a/lib/src/main/cpp/Terminal.h +++ b/lib/src/main/cpp/Terminal.h @@ -142,6 +142,34 @@ class Terminal { // Caller must hold mLock and must have checked mVt. VTermModifier positionMouseLocked(int row, int col, int modifiers); + // Collects everything libvterm emits within its scope and delivers it as a + // single keyboard-output callback. + // + // libvterm has no output buffer installed, so each report it generates + // upcalls into Java on its own - a jbyteArray, a JNI call and a Message for + // six bytes. That is fine for a keystroke but wasteful for a wheel burst, + // which produces one report per detent. Concatenating them changes nothing + // about the bytes reaching the PTY, only how many trips they take. + // + // Construct under mLock, after checking mVt. + class CoalescedOutput { + public: + explicit CoalescedOutput(Terminal* term); + ~CoalescedOutput(); + + CoalescedOutput(const CoalescedOutput&) = delete; + CoalescedOutput& operator=(const CoalescedOutput&) = delete; + + private: + Terminal* mTerm; + std::string mBuffer; + }; + + // Where termOutput() sends bytes while a CoalescedOutput is in scope; null + // means upcall immediately. Guarded by mLock like the rest of the output + // path. + std::string* mOutputSink{nullptr}; + // libvterm state VTerm* mVt; VTermScreen* mVts; diff --git a/lib/src/test/java/org/connectbot/terminal/MouseReportingTest.kt b/lib/src/test/java/org/connectbot/terminal/MouseReportingTest.kt index de4861b2..8ba27b60 100644 --- a/lib/src/test/java/org/connectbot/terminal/MouseReportingTest.kt +++ b/lib/src/test/java/org/connectbot/terminal/MouseReportingTest.kt @@ -496,4 +496,60 @@ class MouseReportingTest { assertEquals("no button is held, so a drag reports nothing", "", out.text) } + + // ----------------------------------------------------------------------- + // Delivery shape + // ----------------------------------------------------------------------- + + @Test + fun testWheelBurstArrivesAsOneCallback() = runBlocking { + // libvterm emits one report per detent, and each one reaching Java on + // its own costs a byte array, a JNI call and a main-thread message. The + // bytes are identical either way, so only the callback count can show + // the burst is coalesced. + var callbacks = 0 + val sb = StringBuilder() + val term = TerminalEmulatorFactory.create( + initialRows = 24, + initialCols = 80, + onKeyboardInput = { + callbacks++ + sb.append(String(it, Charsets.ISO_8859_1)) + }, + ) + term.writeInput(CLICK_TRACKING_SGR.toByteArray()) + InstrumentationRegistry.getInstrumentation().waitForIdleSync() + callbacks = 0 + sb.setLength(0) + + term.scrollWheel(WheelDirection.DOWN, row = 0, col = 0, steps = 5) + InstrumentationRegistry.getInstrumentation().waitForIdleSync() + + assertEquals("five detents, one delivery", 1, callbacks) + assertEquals("\u001B[<65;1;1M".repeat(5), sb.toString()) + } + + @Test + fun testClickArrivesAsOneCallback() = runBlocking { + var callbacks = 0 + val sb = StringBuilder() + val term = TerminalEmulatorFactory.create( + initialRows = 24, + initialCols = 80, + onKeyboardInput = { + callbacks++ + sb.append(String(it, Charsets.ISO_8859_1)) + }, + ) + term.writeInput(CLICK_TRACKING_SGR.toByteArray()) + InstrumentationRegistry.getInstrumentation().waitForIdleSync() + callbacks = 0 + sb.setLength(0) + + term.mouseClick(MouseButton.LEFT, row = 2, col = 7) + InstrumentationRegistry.getInstrumentation().waitForIdleSync() + + assertEquals("press and release, one delivery", 1, callbacks) + assertEquals("\u001B[<0;8;3M\u001B[<0;8;3m", sb.toString()) + } } From ecb2e3dbf84abdf0c2f032ae4f1e188cfa4cc6d3 Mon Sep 17 00:00:00 2001 From: Brandon Martin-Anderson Date: Sun, 26 Jul 2026 00:10:04 -0700 Subject: [PATCH 11/13] refactor(props): translate VTermProp at the boundary, not by hand 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 --- lib/src/main/cpp/Terminal.cpp | 19 +++++++++++- lib/src/main/cpp/Terminal.h | 31 +++++++++++++++++++ .../java/org/connectbot/terminal/Terminal.kt | 11 +++++-- .../connectbot/terminal/TerminalEmulator.kt | 16 ++++++---- 4 files changed, 68 insertions(+), 9 deletions(-) diff --git a/lib/src/main/cpp/Terminal.cpp b/lib/src/main/cpp/Terminal.cpp index 32f2faf5..9bcde93a 100644 --- a/lib/src/main/cpp/Terminal.cpp +++ b/lib/src/main/cpp/Terminal.cpp @@ -824,6 +824,22 @@ void Terminal::invokeMoveCursor(int row, int col, int oldRow, int oldCol, bool v JNI_CHECK_EXCEPTION(env); } +Terminal::PropCode Terminal::toPropCode(VTermProp prop) { + switch (prop) { + case VTERM_PROP_CURSORVISIBLE: return PropCode::CursorVisible; + case VTERM_PROP_CURSORBLINK: return PropCode::CursorBlink; + case VTERM_PROP_ALTSCREEN: return PropCode::AltScreen; + case VTERM_PROP_TITLE: return PropCode::Title; + case VTERM_PROP_ICONNAME: return PropCode::IconName; + case VTERM_PROP_REVERSE: return PropCode::Reverse; + case VTERM_PROP_CURSORSHAPE: return PropCode::CursorShape; + case VTERM_PROP_MOUSE: return PropCode::Mouse; + case VTERM_PROP_FOCUSREPORT: return PropCode::FocusReport; + case VTERM_N_PROPS: break; + } + return PropCode::Unknown; +} + void Terminal::invokeSetTermProp(VTermProp prop, VTermValue* val) { if (!mSetTermPropMethod) { return; @@ -866,7 +882,8 @@ void Terminal::invokeSetTermProp(VTermProp prop, VTermValue* val) { } if (propValue.get()) { - env->CallIntMethod(mCallbacks, mSetTermPropMethod, prop, propValue.get()); + env->CallIntMethod(mCallbacks, mSetTermPropMethod, + static_cast(toPropCode(prop)), propValue.get()); JNI_CHECK_EXCEPTION(env); } } diff --git a/lib/src/main/cpp/Terminal.h b/lib/src/main/cpp/Terminal.h index 5a92b3c2..356c6afa 100644 --- a/lib/src/main/cpp/Terminal.h +++ b/lib/src/main/cpp/Terminal.h @@ -113,6 +113,37 @@ class Terminal { static int termSelectionSet(VTermSelectionMask mask, VTermStringFragment frag, void* user); static int termSelectionQuery(VTermSelectionMask mask, void* user); + // Property identifiers as sent to Java. + // + // VTermProp is an unnumbered C enum, so its ordinals shift whenever + // upstream inserts a property. Translating by name here, where the compiler + // checks the cases against vterm.h, means Java holds identifiers this repo + // defines rather than a hand-copied snapshot of upstream's numbering. + // Getting that copy wrong is not hypothetical: TITLE and CURSORSHAPE were + // read at the wrong ordinals until 6ed03a6, which left OSC 0/2 and DECSCUSR + // silently doing nothing. + // + // These are the wrapper's own protocol. Keep them in step with VTermProp in + // TerminalEmulator.kt -- never with vterm.h. + enum class PropCode : jint { + Unknown = 0, + CursorVisible = 1, + CursorBlink = 2, + AltScreen = 3, + Title = 4, + IconName = 5, + Reverse = 6, + CursorShape = 7, + Mouse = 8, + FocusReport = 9, + }; + + // Deliberately has no default case: a property added upstream then draws a + // -Wswitch warning here, naming the enumerator, rather than silently + // arriving in Java as the wrong identifier. (Warning, not error -- the + // build sets no -Werror.) + static PropCode toPropCode(VTermProp prop); + // Java callback invocation helpers void invokeDamage(int startRow, int endRow, int startCol, int endCol); int invokeMoverect(VTermRect dest, VTermRect src); diff --git a/lib/src/main/java/org/connectbot/terminal/Terminal.kt b/lib/src/main/java/org/connectbot/terminal/Terminal.kt index f3e6e4cd..40f0de3e 100644 --- a/lib/src/main/java/org/connectbot/terminal/Terminal.kt +++ b/lib/src/main/java/org/connectbot/terminal/Terminal.kt @@ -945,6 +945,13 @@ internal fun TerminalWithAccessibility( val down = awaitFirstDown(requireUnconsumed = false) scrollJob?.cancel() + // Who this gesture belongs to is decided once, here, and + // read by both the scroll and tap paths below. An + // application that enables or drops mouse tracking + // mid-gesture should not take a gesture that began as + // ours, or abandon one halfway through. + val appOwnsPointer = terminalEmulator.mouseTracking.isEnabled + // 1a. Check for double-tap to start word selection val isDoubleTap = (down.uptimeMillis - tapTracker.lastTimestamp) < viewConfiguration.doubleTapTimeoutMillis && (down.position - tapTracker.lastPosition).getDistanceSquared() < touchSlopSquared @@ -1121,7 +1128,7 @@ internal fun TerminalWithAccessibility( initialScrollOffset = scrollOffset.value - panAccumulator.y // Hand the gesture to the application if it asked // for mouse reporting; it owns the viewport then. - wheelScroller = if (terminalEmulator.mouseTracking.isEnabled) { + wheelScroller = if (appOwnsPointer) { WheelScroller( emulator = terminalEmulator, lineHeightPx = baseCharHeight, @@ -1304,7 +1311,7 @@ internal fun TerminalWithAccessibility( if (selectionManager.mode != SelectionMode.NONE) { selectionManager.clearSelection() - } else if (terminalEmulator.mouseTracking.isEnabled) { + } else if (appOwnsPointer) { // The application owns the viewport, so it owns the // click too — including on anything that looks like a // link, which it drew and will handle itself. Sent as diff --git a/lib/src/main/java/org/connectbot/terminal/TerminalEmulator.kt b/lib/src/main/java/org/connectbot/terminal/TerminalEmulator.kt index 7660cbb4..bdd952f8 100644 --- a/lib/src/main/java/org/connectbot/terminal/TerminalEmulator.kt +++ b/lib/src/main/java/org/connectbot/terminal/TerminalEmulator.kt @@ -307,11 +307,16 @@ class TerminalEmulatorFactory { } /** - * Property identifiers and values libvterm passes to - * [TerminalCallbacks.setTermProp], from `VTermProp` in vterm.h. + * Property identifiers and values passed to [TerminalCallbacks.setTermProp]. * - * These are ordinals of an unnumbered C enum, so inserting a property shifts - * every one after it. Keep them in the same order as the header. + * These are the native wrapper's own identifiers, assigned by `PropCode` in + * Terminal.h. They are deliberately not libvterm's `VTermProp` ordinals: that + * is an unnumbered C enum whose values shift when upstream inserts a property, + * and transcribing them here is what once left the title and cursor shape being + * read at the wrong numbers. Terminal.cpp translates by name instead, where the + * compiler checks it against the header. + * + * Keep these in step with `PropCode`, which is the definition. */ private object VTermProp { const val CURSOR_VISIBLE = 1 // bool @@ -589,8 +594,7 @@ internal class TerminalEmulatorImpl( * Report wheel detents at a cell. */ override fun scrollWheel(direction: WheelDirection, row: Int, col: Int, steps: Int, modifiers: Int) { - if (steps < 1) return - + // Bounds on steps belong with the loop that spends them, in Terminal.cpp. terminalNative.scrollWheel(row, col, direction.code, steps, modifiers) } From fcc1f1936712ddb5e923192bc2c4865fbbeb2a27 Mon Sep 17 00:00:00 2001 From: Brandon Martin-Anderson Date: Sun, 26 Jul 2026 01:13:52 -0700 Subject: [PATCH 12/13] fix(props): reassemble a string property before delivering it 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 --- lib/src/main/cpp/Terminal.cpp | 42 ++++++++++-- lib/src/main/cpp/Terminal.h | 11 +++ .../terminal/CursorAndModeEscapeTest.kt | 68 +++++++++++++++++++ 3 files changed, 115 insertions(+), 6 deletions(-) diff --git a/lib/src/main/cpp/Terminal.cpp b/lib/src/main/cpp/Terminal.cpp index 9bcde93a..571dfc0f 100644 --- a/lib/src/main/cpp/Terminal.cpp +++ b/lib/src/main/cpp/Terminal.cpp @@ -861,14 +861,44 @@ void Terminal::invokeSetTermProp(VTermProp prop, VTermValue* val) { propValue = ScopedLocalRef(env, env->NewObject(mTerminalPropertyIntClass, mTerminalPropertyIntConstructor, val->number)); break; - case VTERM_VALUETYPE_STRING: - if (val->string.str) { - char* utf8_str = mutf8_to_utf8(val->string.str, val->string.len, nullptr); - ScopedLocalRef str(env, env->NewStringUTF(utf8_str)); - propValue = ScopedLocalRef(env, env->NewObject(mTerminalPropertyStringClass, mTerminalPropertyStringConstructor, str.get())); - free(utf8_str); + case VTERM_VALUETYPE_STRING: { + // libvterm hands a string property over in fragments, one per input + // buffer, so a title that straddles a PTY read arrives in pieces and + // the sequence ends with a fragment that is often empty. Forwarding + // each fragment on its own would let the last one win: a title split + // across two reads would arrive truncated to its tail, and a + // terminator arriving on its own would clear the title outright. + // + // So accumulate here and deliver once, the same shape as + // termOscFallback() and termSelectionSet(). Java then only ever sees + // whole values. + // Keyed by property rather than a single buffer: OSC 0 sets the icon + // name and the title from the same fragment, so two values are in + // flight at once and one buffer would interleave them. + std::string& buffer = mStringPropData[prop]; + + if (val->string.initial) { + buffer.clear(); } + if (val->string.str && val->string.len > 0) { + // Bounded because the payload is remote input and the sequence + // that ends it may never arrive. Excess is dropped rather than + // the value abandoned: an over-long title is still worth showing + // truncated, and a real one is a line at most. + size_t room = MAX_STRING_PROP_BYTES - std::min(buffer.size(), MAX_STRING_PROP_BYTES); + buffer.append(val->string.str, std::min(static_cast(val->string.len), room)); + } + if (!val->string.final) { + break; + } + + char* utf8_str = mutf8_to_utf8(buffer.data(), buffer.size(), nullptr); + ScopedLocalRef str(env, env->NewStringUTF(utf8_str)); + propValue = ScopedLocalRef(env, env->NewObject(mTerminalPropertyStringClass, mTerminalPropertyStringConstructor, str.get())); + free(utf8_str); + buffer.clear(); break; + } case VTERM_VALUETYPE_COLOR: { uint8_t r, g, b; diff --git a/lib/src/main/cpp/Terminal.h b/lib/src/main/cpp/Terminal.h index 356c6afa..93d60299 100644 --- a/lib/src/main/cpp/Terminal.h +++ b/lib/src/main/cpp/Terminal.h @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -218,6 +219,16 @@ class Terminal { int mOscCommand{-1}; // Current OSC command being accumulated VTermPos mOscCursorPos{0, 0}; // Cursor position when OSC sequence started + // Accumulates a string-valued property (title, icon name) across the + // fragments libvterm delivers it in; see invokeSetTermProp(). Keyed by + // property because OSC 0 sets two of them from the same fragment. + // + // Most a single string property may accumulate. The payload is remote input + // and nothing guarantees the terminator ever arrives, so the buffer needs a + // ceiling; this one is far above any real title. + static constexpr size_t MAX_STRING_PROP_BYTES = 4096; + std::map mStringPropData; + // Terminal dimensions int mRows; int mCols; diff --git a/lib/src/test/java/org/connectbot/terminal/CursorAndModeEscapeTest.kt b/lib/src/test/java/org/connectbot/terminal/CursorAndModeEscapeTest.kt index b68ff52d..08c413e2 100644 --- a/lib/src/test/java/org/connectbot/terminal/CursorAndModeEscapeTest.kt +++ b/lib/src/test/java/org/connectbot/terminal/CursorAndModeEscapeTest.kt @@ -302,6 +302,74 @@ class CursorAndModeEscapeTest { assertEquals("window", getSnapshot(impl).terminalTitle) } + @Test + fun testOscTitleTerminatedByStringTerminator() = runBlocking { + // ST rather than BEL. Both end the sequence; either may show up. + val emulator = TerminalEmulatorFactory.create(initialRows = 10, initialCols = 40) + val impl = emulator as TerminalEmulatorImpl + + emulator.send("\u001B]2;window\u001B\\") + + assertEquals("window", getSnapshot(impl).terminalTitle) + } + + @Test + fun testOscTitleSplitAcrossWrites() = runBlocking { + // libvterm hands the payload over one fragment per input buffer, so a + // title straddling a PTY read arrives in pieces. They have to be joined, + // not overwritten by whichever lands last. + val emulator = TerminalEmulatorFactory.create(initialRows = 10, initialCols = 40) + val impl = emulator as TerminalEmulatorImpl + + emulator.send("\u001B]0;hel") + emulator.send("lo, wo") + emulator.send("rld\u0007") + + assertEquals("hello, world", getSnapshot(impl).terminalTitle) + } + + @Test + fun testOscTitleTerminatorArrivingAlone() = runBlocking { + // The terminator lands in its own read, so the sequence ends on an empty + // fragment. That must complete the title, not blank it. + val emulator = TerminalEmulatorFactory.create(initialRows = 10, initialCols = 40) + val impl = emulator as TerminalEmulatorImpl + + emulator.send("\u001B]0;hello") + emulator.send("\u0007") + + assertEquals("hello", getSnapshot(impl).terminalTitle) + } + + @Test + fun testUnterminatedOscTitleIsNotDelivered() = runBlocking { + // Until the sequence ends there is no value to report: the terminal + // cannot know whether more of the title is still coming. + val emulator = TerminalEmulatorFactory.create(initialRows = 10, initialCols = 40) + val impl = emulator as TerminalEmulatorImpl + + emulator.send("\u001B]2;first\u0007") + assertEquals("first", getSnapshot(impl).terminalTitle) + + emulator.send("\u001B]2;partial") + + assertEquals("the previous title stands", "first", getSnapshot(impl).terminalTitle) + } + + @Test + fun testSuccessiveOscTitlesDoNotAccumulate() = runBlocking { + // The buffer belongs to one sequence: a second title replaces the first + // rather than being appended to what the first left behind. + val emulator = TerminalEmulatorFactory.create(initialRows = 10, initialCols = 40) + val impl = emulator as TerminalEmulatorImpl + + emulator.send("\u001B]0;first\u0007") + emulator.send("\u001B]0;sec") + emulator.send("ond\u0007") + + assertEquals("second", getSnapshot(impl).terminalTitle) + } + @Test fun testDecscusrSelectsCursorShape() = runBlocking { val emulator = TerminalEmulatorFactory.create(initialRows = 10, initialCols = 40) From 17f0378198cf786ce6818745836564d5e0f77e09 Mon Sep 17 00:00:00 2001 From: Brandon Martin-Anderson Date: Sun, 26 Jul 2026 01:14:04 -0700 Subject: [PATCH 13/13] build(libvterm): track the local state.c modification as a patch 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 --- lib/README.md | 13 +++ .../0001-reset-full-mouse-state.patch | 107 ++++++++++++++++++ lib/src/main/cpp/libvterm/src/state.c | 6 +- 3 files changed, 125 insertions(+), 1 deletion(-) create mode 100644 lib/src/main/cpp/libvterm-patches/0001-reset-full-mouse-state.patch diff --git a/lib/README.md b/lib/README.md index 6956a3c1..02fe90d0 100644 --- a/lib/README.md +++ b/lib/README.md @@ -62,3 +62,16 @@ detents, so no caller can put a malformed report or an unbounded burst on the wire. **Important**: Callbacks must not call back into Terminal methods (causes deadlock). Defer work to avoid reentrancy. + +## Local libvterm modifications + +`src/main/cpp/libvterm/` is vendored, and is **not** pristine upstream. Each +divergence is kept as a patch in `src/main/cpp/libvterm-patches/` as well as +being applied in tree, so that it survives a libvterm bump — CMake compiles the +vendored sources directly, so a bump that overwrites them drops the change +silently, and the symptom is a behavioural regression rather than a build +failure. After bumping, re-apply each patch and re-run the tests it names. + +| Patch | File | Why | +| --- | --- | --- | +| `0001-reset-full-mouse-state.patch` | `src/state.c` | `vterm_state_reset()` cleared `mouse_flags` but left the report encoding and any held button stale, and switched reporting off without a `VTERM_PROP_MOUSE` callback — so an embedder mirroring that property kept routing gestures into a vterm that drops them. Not yet submitted upstream. | diff --git a/lib/src/main/cpp/libvterm-patches/0001-reset-full-mouse-state.patch b/lib/src/main/cpp/libvterm-patches/0001-reset-full-mouse-state.patch new file mode 100644 index 00000000..07167d2d --- /dev/null +++ b/lib/src/main/cpp/libvterm-patches/0001-reset-full-mouse-state.patch @@ -0,0 +1,107 @@ +Subject: [PATCH] state: reset the whole mouse state, and say so + +vterm_state_reset() cleared mouse_flags but left mouse_protocol and +mouse_buttons untouched, and told the embedder nothing. + +Two consequences: + + * A reset left the previously negotiated report encoding in place, so an + application that afterwards asked for plain DECSET 1000 got its reports + in whichever protocol the *previous* application had selected. Likewise + a button pressed before the reset stayed held, which makes DECSET 1002 + report motion with nothing actually down. + + * Reporting is switched off without a VTERM_PROP_MOUSE callback. An + embedder mirroring that property still believes an application wants the + mouse, and keeps routing input into a vterm that silently drops it. In a + terminal that hands scroll gestures to the application while tracking is + on, that strands scrolling entirely: the gestures go to an application + that is not listening, and the terminal's own scrollback is not reached + either. RIS is exactly what a user types to unwedge a terminal after a + full-screen application dies without restoring its modes, so this is the + moment it matters most. + +Move every mouse field into reset_mouse_state(), call it from both +vterm_state_new() and vterm_state_reset() so the two cannot drift, and +notify VTERM_PROP_MOUSE_NONE from the reset. The direct field reset is kept +alongside the notification so the state is cleared even if the callback +declines to store the property. + +Status: not yet submitted upstream. + +-- + +Applies to lib/src/main/cpp/libvterm/src/state.c, which is vendored. The +change is already applied in tree; this file exists so the divergence is +visible and so it can be re-applied after a libvterm bump: + + git apply lib/src/main/cpp/libvterm-patches/0001-reset-full-mouse-state.patch + +Covered by testHardResetClearsTracking, testSoftResetClearsTracking, +testNoReportsAfterReset, testResetClearsReportEncoding and +testResetClearsHeldButtons in MouseReportingTest. + +diff --git a/lib/src/main/cpp/libvterm/src/state.c b/lib/src/main/cpp/libvterm/src/state.c +index ce8e034..7b6587b 100644 +--- a/lib/src/main/cpp/libvterm/src/state.c ++++ b/lib/src/main/cpp/libvterm/src/state.c +@@ -56,6 +56,24 @@ static void erase(VTermState *state, VTermRect rect, int selective) + return; + } + ++/* Local modification: every field describing the mouse lives here, so that ++ * creating a state and resetting one cannot drift apart. Upstream initialised ++ * these in vterm_state_new() and cleared only mouse_flags in ++ * vterm_state_reset(), which left the report encoding and any held button ++ * surviving a reset. ++ * ++ * Also kept as libvterm-patches/0001-reset-full-mouse-state.patch. Re-apply it ++ * after bumping libvterm; a bump overwrites this file without failing the ++ * build. */ ++static void reset_mouse_state(VTermState *state) ++{ ++ state->mouse_col = 0; ++ state->mouse_row = 0; ++ state->mouse_buttons = 0; ++ state->mouse_flags = 0; ++ state->mouse_protocol = MOUSE_X10; ++} ++ + static VTermState *vterm_state_new(VTerm *vt) + { + VTermState *state = vterm_allocator_malloc(vt, sizeof(VTermState)); +@@ -65,11 +83,7 @@ static VTermState *vterm_state_new(VTerm *vt) + state->rows = vt->rows; + state->cols = vt->cols; + +- state->mouse_col = 0; +- state->mouse_row = 0; +- state->mouse_buttons = 0; +- +- state->mouse_protocol = MOUSE_X10; ++ reset_mouse_state(state); + + state->callbacks = NULL; + state->cbdata = NULL; +@@ -2087,7 +2101,7 @@ void vterm_state_reset(VTermState *state, int hard) + state->mode.bracketpaste = 0; + state->mode.report_focus = 0; + +- state->mouse_flags = 0; ++ reset_mouse_state(state); + + state->vt->mode.ctrl8bit = 0; + +@@ -2125,6 +2139,12 @@ void vterm_state_reset(VTermState *state, int hard) + settermprop_bool(state, VTERM_PROP_CURSORVISIBLE, 1); + settermprop_bool(state, VTERM_PROP_CURSORBLINK, 1); + settermprop_int (state, VTERM_PROP_CURSORSHAPE, VTERM_PROP_CURSORSHAPE_BLOCK); ++ /* Local modification: reset_mouse_state() above disables reporting without ++ * telling the embedder. A terminal mirroring VTERM_PROP_MOUSE would then ++ * believe an application still wants the mouse and keep routing gestures into ++ * a vterm that silently drops them. This notifies; the reset above is kept so ++ * the state is cleared even if the callback vetoes the store. */ ++ settermprop_int (state, VTERM_PROP_MOUSE, VTERM_PROP_MOUSE_NONE); + + if(hard) { + state->pos.row = 0; diff --git a/lib/src/main/cpp/libvterm/src/state.c b/lib/src/main/cpp/libvterm/src/state.c index f27a7e01..7b6587b6 100644 --- a/lib/src/main/cpp/libvterm/src/state.c +++ b/lib/src/main/cpp/libvterm/src/state.c @@ -60,7 +60,11 @@ static void erase(VTermState *state, VTermRect rect, int selective) * creating a state and resetting one cannot drift apart. Upstream initialised * these in vterm_state_new() and cleared only mouse_flags in * vterm_state_reset(), which left the report encoding and any held button - * surviving a reset. */ + * surviving a reset. + * + * Also kept as libvterm-patches/0001-reset-full-mouse-state.patch. Re-apply it + * after bumping libvterm; a bump overwrites this file without failing the + * build. */ static void reset_mouse_state(VTermState *state) { state->mouse_col = 0;