diff options
| author | Can Celik <ogulcancelik@gmail.com> | 2026-08-19 04:43:28 +0300 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2026-08-19 04:43:28 +0300 |
| commit | a5c69beabfc82d9c3f9563eb821139b2e0f3e14f (patch) | |
| tree | 9b7049e4629913dcab40d985df399acee262b67b | |
| parent | 3667151744e379ac5c8f76d57203b675de8a6ebc (diff) | |
fix(perf): eliminate redundant terminal wake work (#2962)
* fix(perf): eliminate redundant terminal wake work
* fix(perf): harden release benchmark gate
* test(perf): strengthen performance guardrails
28 files changed, 773 insertions, 149 deletions
@@ -58,7 +58,10 @@ Inside pane-scaled render and layout loops: Prefer deterministic operation or architecture tests to wall-clock CI limits. Performance benchmarks are supporting evidence, not substitutes for behavioral -coverage. +coverage. Before a stable release, `just bench-release-smoke` must compare the +candidate with the current stable binary under hidden and visible output. When +the result moves materially or when validating performance work, repeat it with +`HERDR_PERF_SAMPLE_SECONDS=60` and investigate the affected scenario. ### Runtime/client boundary guardrail diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d35b46c..55e129b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,6 @@ ## Unreleased -## [0.8.1] - 2026-08-18 - ### Added - CLI help now points coding agents to Herdr's plain-text guide, documentation index, and built-in control skill. - Added Qwen Code detection for idle, working, and user-confirmation states, plus optional native session restore. (#2730, #2743) @@ -28,6 +26,7 @@ - Experimental pane graphics now support bounded named layers, acknowledged full-RGBA primary-layer direct file frames on audited local terminals, owned BGRA fallback, exact pixel mouse input, and placement-only resize replay. ### Fixed +- High-rate output from many hidden panes no longer floods the server loop with redundant wakeups, and terminal input-mode synchronization no longer formats pane scrollback to read one keyboard flag. - Chinese IME commits now reach panes on macOS when the focused application requests printable key-release events. (#2924) - Windows now recognizes `Ctrl+1` through `Ctrl+9` keybindings instead of decoding those key records as control characters. (#2910) - PowerShell panes now keep their process-reported working directory synchronized with the shell's logical location. (#2879, thanks @Pimpmuckl) diff --git a/docs/next/CHANGELOG.md b/docs/next/CHANGELOG.md index 6d35b46c..55e129b0 100644 --- a/docs/next/CHANGELOG.md +++ b/docs/next/CHANGELOG.md @@ -2,8 +2,6 @@ ## Unreleased -## [0.8.1] - 2026-08-18 - ### Added - CLI help now points coding agents to Herdr's plain-text guide, documentation index, and built-in control skill. - Added Qwen Code detection for idle, working, and user-confirmation states, plus optional native session restore. (#2730, #2743) @@ -28,6 +26,7 @@ - Experimental pane graphics now support bounded named layers, acknowledged full-RGBA primary-layer direct file frames on audited local terminals, owned BGRA fallback, exact pixel mouse input, and placement-only resize replay. ### Fixed +- High-rate output from many hidden panes no longer floods the server loop with redundant wakeups, and terminal input-mode synchronization no longer formats pane scrollback to read one keyboard flag. - Chinese IME commits now reach panes on macOS when the focused application requests printable key-release events. (#2924) - Windows now recognizes `Ctrl+1` through `Ctrl+9` keybindings instead of decoding those key records as control characters. (#2910) - PowerShell panes now keep their process-reported working directory synchronized with the shell's logical location. (#2879, thanks @Pimpmuckl) @@ -73,6 +73,11 @@ build: bench-render-scale: cargo test --release --locked --bin herdr render_scale_profile -- --ignored --nocapture --test-threads=1 +# ~3-5 minute CPU comparison; downloads stable unless HERDR_PERF_BASELINE_BIN is set +bench-release-smoke: + cargo build --release --locked + scripts/release_perf_smoke.sh "${CARGO_TARGET_DIR:-target}/release/herdr" + # Build the website and documentation website-build: cd website && bun install --frozen-lockfile && bun run build @@ -130,10 +135,11 @@ release-docs-check: just website-build cd website && bun run build:draft -# Validate release docs and review full-render scaling before release preparation +# Validate release docs, render scaling, and end-to-end CPU before release preparation pre-release-check: just release-docs-check just bench-render-scale + just bench-release-smoke @echo "release review required: investigate material render-scaling regressions before publishing." @echo "release review required: update skills/herdr/SKILL.md for this stable release so it matches the current CLI, IDs, agent lifecycle semantics, and safety guidance." @echo "release policy: do not update skills/herdr/SKILL.md between stable releases; preview builds keep the latest stable skill." diff --git a/scripts/release_perf_case.sh b/scripts/release_perf_case.sh new file mode 100755 index 00000000..47d72259 --- /dev/null +++ b/scripts/release_perf_case.sh @@ -0,0 +1,149 @@ +#!/usr/bin/env bash +set -euo pipefail + +if [[ $# -ne 8 ]]; then + echo "usage: $0 <binary> <variant> <scenario> <round> <seconds> <warmup> <output-root> <platform>" >&2 + exit 2 +fi + +bin_dir=$(cd "$(dirname "$1")" && pwd) +bin="$bin_dir/$(basename "$1")" +variant=$2 +scenario=$3 +round=$4 +seconds=$5 +warmup=$6 +out_root=$(cd "$7" && pwd) +platform=$8 +script_dir=$(cd "$(dirname "$0")" && pwd) +producer="$script_dir/release_perf_producer.pl" +cols=86 +rows=47 + +case "$scenario" in + visible30) scenario_tag=v3; total_panes=1; writers=visible; rate=30 ;; + hidden50) scenario_tag=h5; total_panes=50; writers=hidden; rate=60 ;; + *) echo "unknown scenario: $scenario" >&2; exit 2 ;; +esac +case "$platform" in linux) platform_tag=l ;; macos) platform_tag=m ;; *) exit 2 ;; esac +variant_tag=${variant:0:1} +name="rps${platform_tag}${variant_tag}${scenario_tag}r${round}x$$" +state="/var/tmp/herdr-release-perf-$name" +xdg="$state/xdg" +runtime="$state/run" +gate="$state/start-output" +out="$out_root/$variant/$scenario/r$round" +mkdir -p "$xdg" "$runtime" "$out" + +launch_env=(env -u HERDR_BIN_PATH -u HERDR_ENV -u HERDR_SOCKET_PATH -u HERDR_CLIENT_SOCKET_PATH -u HERDR_SESSION -u HERDR_STARTUP_CWD -u HERDR_WORKSPACE_ID -u HERDR_TAB_ID -u HERDR_PANE_ID XDG_CONFIG_HOME="$xdg" XDG_RUNTIME_DIR="$runtime" HERDR_DISABLE_SOUND=1 SHELL=/bin/sh) +control_env=(env -u HERDR_BIN_PATH -u HERDR_ENV -u HERDR_SOCKET_PATH -u HERDR_CLIENT_SOCKET_PATH -u HERDR_STARTUP_CWD -u HERDR_WORKSPACE_ID -u HERDR_TAB_ID -u HERDR_PANE_ID XDG_CONFIG_HOME="$xdg" XDG_RUNTIME_DIR="$runtime" HERDR_DISABLE_SOUND=1 SHELL=/bin/sh HERDR_SESSION="$name") + +cleaned=0 +cleanup() { + if [[ $cleaned -eq 1 ]]; then return; fi + cleaned=1 + "${control_env[@]}" "$bin" session stop "$name" >/dev/null 2>&1 || true + for _ in $(seq 1 50); do + if "${control_env[@]}" "$bin" session delete "$name" >/dev/null 2>&1; then break; fi + sleep 0.1 + done + tmux kill-session -t "$name" >/dev/null 2>&1 || true + rm -rf "$state" +} +trap cleanup EXIT +trap 'exit 130' INT +trap 'exit 143' TERM + +printf -v launch 'exec ' +printf -v quoted '%q ' "${launch_env[@]}" "$bin" --session "$name" +launch+=$quoted +tmux new-session -d -s "$name" -x "$cols" -y "$rows" "$launch" + +panes_json= +for _ in $(seq 1 150); do + if panes_json=$("${control_env[@]}" "$bin" pane list 2>/dev/null); then break; fi + sleep 0.1 +done +[[ -n "$panes_json" ]] || { echo "session API did not become ready" >&2; exit 1; } +root_pane=$(printf '%s\n' "$panes_json" | jq -r '.result.panes[0].pane_id') +workspace_id=$("${control_env[@]}" "$bin" workspace list | jq -r '.result.workspaces[0].workspace_id') +[[ -n "$root_pane" && "$root_pane" != null ]] || { echo "session did not report a root pane" >&2; exit 1; } +[[ -n "$workspace_id" && "$workspace_id" != null ]] || { echo "session did not report a workspace" >&2; exit 1; } +pane_file="$state/pane-ids.txt" +printf '%s\n' "$root_pane" > "$pane_file" + +for ((index = 2; index <= total_panes; index++)); do + created=$("${control_env[@]}" "$bin" tab create --workspace "$workspace_id" --label "bench-$index" --no-focus) + pane_id=$(printf '%s\n' "$created" | jq -r '.result.root_pane.pane_id') + [[ -n "$pane_id" && "$pane_id" != null ]] || { echo "tab $index did not return a pane" >&2; exit 1; } + printf '%s\n' "$pane_id" >> "$pane_file" +done + +index=0 +while IFS= read -r pane_id; do + index=$((index + 1)) + if [[ $writers == visible && $index -eq 1 ]] || [[ $writers == hidden && $index -gt 1 ]]; then + "${control_env[@]}" "$bin" pane run "$pane_id" "$producer" "$rate" "$gate" "p$index" >/dev/null + fi +done < "$pane_file" +touch "$gate" + +socket="$xdg/herdr/sessions/$name/herdr.sock" +server_pid= +for _ in $(seq 1 80); do + server_pid=$(lsof -t "$socket" 2>/dev/null | head -n1 || true) + [[ -n "$server_pid" ]] && break + sleep 0.1 +done +[[ -n "$server_pid" ]] || { echo "could not find server pid" >&2; exit 1; } +client_pid=$(tmux list-panes -s -t "$name" -F '#{pane_pid}') +[[ -n "$client_pid" ]] || { echo "could not find client pid" >&2; exit 1; } +all_pids=("$server_pid" "$client_pid") + +sleep "$warmup" +raw="$out/cpu-raw.txt" +if [[ $platform == linux ]]; then + pid_csv=$(IFS=,; echo "${all_pids[*]}") + LC_ALL=C pidstat -h -u -p "$pid_csv" 1 "$seconds" > "$raw" +else + top_args=(top -l $((seconds + 1)) -s 1 -stats pid,cpu,time -n 2) + for pid in "${all_pids[@]}"; do top_args+=(-pid "$pid"); done + LC_ALL=C "${top_args[@]}" > "$raw" +fi + +mean_linux() { + awk -v target="$2" ' + /^Linux/ || /^#/ || NF < 5 { next } + $3 == target && $(NF-2) ~ /^[0-9]+([.][0-9]+)?$/ { sum += $(NF-2); count++ } + END { if (!count) exit 1; printf "%.6f,%d", sum/count, count } + ' "$1" +} +mean_macos() { + awk -v target="$2" ' + $1 == target && $2 ~ /^[0-9]+([.][0-9]+)?%?$/ { + seen++; if (seen == 1) next; value=$2; gsub(/%/, "", value); sum += value; count++ } + END { if (!count) exit 1; printf "%.6f,%d", sum/count, count } + ' "$1" +} + +total=0 +for pid in "${all_pids[@]}"; do + if [[ $platform == linux ]]; then parsed=$(mean_linux "$raw" "$pid"); else parsed=$(mean_macos "$raw" "$pid"); fi + mean=${parsed%,*} + samples=${parsed#*,} + [[ $samples -eq $seconds ]] || { echo "expected $seconds samples for pid $pid, got $samples" >&2; exit 1; } + total=$(awk -v total="$total" -v mean="$mean" 'BEGIN { printf "%.6f", total + mean }') +done + +index=0 +while IFS= read -r pane_id; do + index=$((index + 1)) + if [[ $writers == visible && $index -eq 1 ]] || [[ $writers == hidden && $index -gt 1 ]]; then + read_file="$out/pane-$index.txt" + "${control_env[@]}" "$bin" pane read "$pane_id" --source visible --format text > "$read_file" + grep -q 'bench-output-' "$read_file" || { echo "writer pane $index produced no output" >&2; exit 1; } + fi +done < "$pane_file" + +printf '%s\n' "$total" > "$out/total-cpu.txt" +printf '%s,%s,%s,%s\n' "$variant" "$scenario" "$round" "$total" diff --git a/scripts/release_perf_producer.pl b/scripts/release_perf_producer.pl new file mode 100755 index 00000000..e99973cc --- /dev/null +++ b/scripts/release_perf_producer.pl @@ -0,0 +1,26 @@ +#!/usr/bin/env perl +use strict; +use warnings; +use Time::HiRes qw(clock_gettime sleep CLOCK_MONOTONIC); + +my ($rate, $gate, $label) = @ARGV; +die "usage: $0 <rate-hz> <gate-file> <label>\n" + unless defined $rate && defined $gate && defined $label + && length $gate && length $label; +die "rate must be a positive number\n" + unless $rate =~ /\A(?:[0-9]+(?:\.[0-9]*)?|\.[0-9]+)\z/ && $rate > 0; + +sleep 0.01 until -e $gate; +$| = 1; +my $period = 1 / $rate; +my $next = clock_gettime(CLOCK_MONOTONIC); +my $sequence = 0; +while (1) { + $sequence++; + printf "\rbench-output-%08d-%s", $sequence, $label; + $next += $period; + my $now = clock_gettime(CLOCK_MONOTONIC); + $next = $now + $period if $next < $now - $period; + my $remaining = $next - $now; + sleep $remaining if $remaining > 0; +} diff --git a/scripts/release_perf_smoke.sh b/scripts/release_perf_smoke.sh new file mode 100755 index 00000000..7ce1f736 --- /dev/null +++ b/scripts/release_perf_smoke.sh @@ -0,0 +1,92 @@ +#!/usr/bin/env bash +set -euo pipefail + +if [[ $# -ne 1 ]]; then + echo "usage: $0 <candidate-binary>" >&2 + exit 2 +fi + +candidate=$(cd "$(dirname "$1")" && pwd)/$(basename "$1") +[[ -x "$candidate" ]] || { echo "candidate binary is not executable: $candidate" >&2; exit 1; } +script_dir=$(cd "$(dirname "$0")" && pwd) +repo_root=$(cd "$script_dir/.." && pwd) +baseline=${HERDR_PERF_BASELINE_BIN:-} +for command in jq lsof perl tmux; do + command -v "$command" >/dev/null || { echo "required command not found: $command" >&2; exit 1; } +done +if [[ -z "$baseline" ]]; then + command -v curl >/dev/null || { echo "required command not found: curl" >&2; exit 1; } +fi + +case "$(uname -s)" in + Linux) platform=linux; command -v pidstat >/dev/null || { echo "required command not found: pidstat" >&2; exit 1; } ;; + Darwin) platform=macos ;; + *) echo "release performance smoke supports Linux and macOS" >&2; exit 1 ;; +esac +case "$(uname -m)" in + x86_64|amd64) arch=x86_64 ;; + arm64|aarch64) arch=aarch64 ;; + *) echo "unsupported architecture: $(uname -m)" >&2; exit 1 ;; +esac + +root=$(mktemp -d /var/tmp/herdr-release-perf-smoke.XXXXXX) +cleanup() { rm -rf "$root"; } +trap cleanup EXIT +trap 'exit 130' INT +trap 'exit 143' TERM +mkdir -p "$root/results" + +if [[ -z "$baseline" ]]; then + baseline_version=$(jq -er '.version' "$repo_root/website/latest.json") + baseline="$root/herdr-baseline" + curl -fL --retry 3 \ + "https://github.com/herdrdev/herdr/releases/download/v${baseline_version}/herdr-${platform}-${arch}" \ + -o "$baseline" + chmod +x "$baseline" +else + baseline=$(cd "$(dirname "$baseline")" && pwd)/$(basename "$baseline") +fi +[[ -x "$baseline" ]] || { echo "baseline binary is not executable: $baseline" >&2; exit 1; } + +case_script="$script_dir/release_perf_case.sh" +seconds=${HERDR_PERF_SAMPLE_SECONDS:-10} +warmup=${HERDR_PERF_WARMUP_SECONDS:-3} + +for round in 1 2; do + if [[ $round -eq 1 ]]; then variants="baseline candidate"; else variants="candidate baseline"; fi + for scenario in hidden50 visible30; do + for variant in $variants; do + if [[ $variant == baseline ]]; then binary=$baseline; else binary=$candidate; fi + "$case_script" "$binary" "$variant" "$scenario" "$round" "$seconds" "$warmup" "$root/results" "$platform" + done + done +done + +mean_total() { + awk '{ sum += $1; count++ } END { if (!count) exit 1; printf "%.3f", sum/count }' \ + "$root/results/$1/$2/r1/total-cpu.txt" \ + "$root/results/$1/$2/r2/total-cpu.txt" +} + +failed=0 +printf '\nrelease performance smoke (%s/%s, two rounds, %ss samples)\n' "$platform" "$arch" "$seconds" +printf '%-12s %12s %12s %12s\n' scenario baseline candidate change +for scenario in hidden50 visible30; do + baseline_total=$(mean_total baseline "$scenario") + candidate_total=$(mean_total candidate "$scenario") + if awk -v before="$baseline_total" -v after="$candidate_total" 'BEGIN { exit !(before <= 0 || after <= 0) }'; then + echo "error: $scenario measured no CPU usage; the benchmark did not exercise the binaries" >&2 + failed=1 + fi + change=$(awk -v before="$baseline_total" -v after="$candidate_total" 'BEGIN { if (before == 0) print "n/a"; else printf "%+.1f%%", (after-before)/before*100 }') + printf '%-12s %12s %12s %12s\n' "$scenario" "$baseline_total" "$candidate_total" "$change" + if awk -v before="$baseline_total" -v after="$candidate_total" 'BEGIN { exit !(after > before * 1.25 && after - before > 0.5) }'; then + echo "error: $scenario candidate CPU exceeds baseline by more than 25% and 0.5 CPU points" >&2 + failed=1 + fi +done + +if [[ $failed -ne 0 ]]; then + exit 1 +fi +echo "release performance smoke passed" diff --git a/scripts/test_ui_hot_path_architecture.py b/scripts/test_ui_hot_path_architecture.py index 44412197..be3d35f0 100644 --- a/scripts/test_ui_hot_path_architecture.py +++ b/scripts/test_ui_hot_path_architecture.py @@ -11,16 +11,21 @@ HOT_PATH_SOURCES = ( *sorted((PROJECT_ROOT / "src" / "ui").rglob("*.rs")), PROJECT_ROOT / "src" / "server" / "render_stream.rs", ) +APP_SERVER_SOURCES = ( + *sorted((PROJECT_ROOT / "src" / "app").rglob("*.rs")), + *sorted((PROJECT_ROOT / "src" / "server").rglob("*.rs")), +) TEST_MODULE = re.compile(r"(?m)^#\[cfg\(test\)\]\s*\nmod\s+\w+\s*\{") +INPUT_STATE_CALL = re.compile(r"(?:\.|::)input_state\b") +KEYBOARD_STATE_ANSI_CALL = re.compile( + r"(?:\.|::)(?:keyboard_state_ansi|kitty_keyboard_state_ansi)\b" +) +AGGREGATE_STATE_CALLS = ( + (INPUT_STATE_CALL, "aggregate terminal input state; add a narrow accessor"), + (KEYBOARD_STATE_ANSI_CALL, "formatted keyboard state"), +) FORBIDDEN_CALLS = ( - ( - re.compile(r"(?:\.|::)input_state\b"), - "aggregate terminal input state; add a narrow accessor", - ), - ( - re.compile(r"(?:\.|::)(?:keyboard_state_ansi|kitty_keyboard_state_ansi)\b"), - "formatted keyboard state", - ), + *AGGREGATE_STATE_CALLS, ( re.compile(r"(?:\.|::)screen_text_snapshot\b"), "formatted terminal screen snapshot", @@ -130,18 +135,21 @@ def production_code(source: str) -> str: return code +def find_violations(paths, rules) -> list[str]: + violations: list[str] = [] + for path in paths: + code = production_code(path.read_text(encoding="utf-8")) + for pattern, description in rules: + for match in pattern.finditer(code): + line = code.count("\n", 0, match.start()) + 1 + relative_path = path.relative_to(PROJECT_ROOT) + violations.append(f"{relative_path}:{line}: {description}") + return violations + + class UiHotPathArchitectureTests(unittest.TestCase): def test_render_hot_paths_avoid_known_expensive_runtime_queries(self) -> None: - violations: list[str] = [] - - for path in HOT_PATH_SOURCES: - source = path.read_text(encoding="utf-8") - code = production_code(source) - for pattern, description in FORBIDDEN_CALLS: - for match in pattern.finditer(code): - line = code.count("\n", 0, match.start()) + 1 - relative_path = path.relative_to(PROJECT_ROOT) - violations.append(f"{relative_path}:{line}: {description}") + violations = find_violations(HOT_PATH_SOURCES, FORBIDDEN_CALLS) self.assertEqual( violations, @@ -150,6 +158,17 @@ class UiHotPathArchitectureTests(unittest.TestCase): + "\n".join(violations), ) + def test_app_and_server_avoid_aggregate_terminal_state(self) -> None: + self.assertTrue(APP_SERVER_SOURCES, "No app/server Rust sources were discovered") + violations = find_violations(APP_SERVER_SOURCES, AGGREGATE_STATE_CALLS) + + self.assertEqual( + violations, + [], + "App/server code must use narrow terminal-state accessors:\n" + + "\n".join(violations), + ) + def test_scanner_ignores_non_production_references(self) -> None: source = ''' // runtime.input_state() @@ -175,6 +194,16 @@ fn render() { TerminalRuntime::input_state; } ''' self.assertRegex(production_code(source), FORBIDDEN_CALLS[0][0]) + def test_scanner_catches_each_aggregate_state_call(self) -> None: + cases = ( + ("fn render() { runtime.input_state(); }", INPUT_STATE_CALL), + ("fn render() { runtime.keyboard_state_ansi(); }", KEYBOARD_STATE_ANSI_CALL), + ("fn render() { runtime.kitty_keyboard_state_ansi(); }", KEYBOARD_STATE_ANSI_CALL), + ) + for source, pattern in cases: + with self.subTest(source=source): + self.assertRegex(production_code(source), pattern) + def test_scanner_catches_imported_process_query(self) -> None: source = "fn render() { foreground_job(pid); }" self.assertRegex(production_code(source), FORBIDDEN_CALLS[3][0]) diff --git a/src/app/actions.rs b/src/app/actions.rs index d6e264f5..a45687b4 100644 --- a/src/app/actions.rs +++ b/src/app/actions.rs @@ -2175,10 +2175,7 @@ impl AppState { else { return false; }; - if rt - .input_state() - .is_some_and(crate::pane::InputState::mouse_reporting_enabled) - { + if rt.mouse_reporting_enabled() { return false; } diff --git a/src/app/api_helpers.rs b/src/app/api_helpers.rs index 95224b9f..a224938d 100644 --- a/src/app/api_helpers.rs +++ b/src/app/api_helpers.rs @@ -23,10 +23,7 @@ fn normalize_api_key_alias(key: &str) -> &str { } pub(super) fn encode_api_text(runtime: &crate::terminal::TerminalRuntime, text: &str) -> Vec<u8> { - let bracketed = runtime - .input_state() - .map(|state| state.bracketed_paste) - .unwrap_or(false); + let bracketed = runtime.bracketed_paste_enabled(); if bracketed { format!("\x1b[200~{text}\x1b[201~").into_bytes() } else { diff --git a/src/app/input/mouse.rs b/src/app/input/mouse.rs index 3066c2a4..3c6c86f4 100644 --- a/src/app/input/mouse.rs +++ b/src/app/input/mouse.rs @@ -1761,9 +1761,7 @@ impl AppState { let Some(host) = self.host_mouse_pixels else { return Some(cell); }; - let wants_pixels = runtime.input_state().is_some_and(|state| { - state.mouse_protocol_encoding == crate::input::MouseProtocolEncoding::SgrPixels - }); + let wants_pixels = runtime.sgr_pixel_mouse_enabled(); if !wants_pixels { return Some(cell); } diff --git a/src/app/input/terminal.rs b/src/app/input/terminal.rs index e5b40a74..6b3d3537 100644 --- a/src/app/input/terminal.rs +++ b/src/app/input/terminal.rs @@ -154,8 +154,8 @@ impl App { if matches!(key_event.code, KeyCode::PageUp | KeyCode::PageDown) && key_event.modifiers.is_empty() { - if let Some(input_state) = rt.input_state() { - if input_state.plain_page_keys_use_host_scrollback() { + if let Some(host_scroll) = rt.plain_page_keys_use_host_scrollback() { + if host_scroll { if key_event.kind == crossterm::event::KeyEventKind::Release { return None; } @@ -282,14 +282,7 @@ impl App { None }; - runtime.is_some_and(|runtime| { - let protocol = runtime.keyboard_protocol(); - protocol.reports_all_keys() - || (protocol.reports_event_types() - && runtime - .input_state() - .is_some_and(|state| state.modify_other_keys)) - }) + runtime.is_some_and(crate::terminal::TerminalRuntime::keyboard_report_all_requested) } fn terminal_input_runtime( diff --git a/src/app/mod.rs b/src/app/mod.rs index 75568990..85ad4fa3 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -1065,6 +1065,8 @@ impl App { } let now = Instant::now(); + self.render_dirty + .set_immediate_pty_sources(self.state.app_surface_pane_ids()); self.sync_host_mouse_capture(&mut host_mouse_capture_active)?; self.sync_host_keyboard_report_all(&mut host_keyboard_report_all_active)?; diff --git a/src/app/state.rs b/src/app/state.rs index b8205451..d9dcfc9d 100644 --- a/src/app/state.rs +++ b/src/app/state.rs @@ -1614,6 +1614,26 @@ impl AppState { section == SettingsSection::Integrations && self.integration_updates_available() } + pub(crate) fn app_surface_pane_ids(&self) -> std::collections::HashSet<PaneId> { + let mut pane_ids = std::collections::HashSet::new(); + if let Some(popup) = &self.popup_pane { + pane_ids.insert(popup.pane_id); + } + let Some(tab) = self + .active + .and_then(|ws_idx| self.workspaces.get(ws_idx)) + .and_then(crate::workspace::Workspace::active_tab) + else { + return pane_ids; + }; + if tab.zoomed { + pane_ids.insert(tab.layout.focused()); + } else { + pane_ids.extend(tab.panes.keys().copied()); + } + pane_ids + } + pub(crate) fn focused_pane_requests_mouse_capture_from( &self, terminal_runtimes: &crate::terminal::TerminalRuntimeRegistry, @@ -1622,8 +1642,7 @@ impl AppState { && self .active .and_then(|idx| self.focused_runtime_in_workspace(terminal_runtimes, idx)) - .and_then(crate::terminal::TerminalRuntime::input_state) - .is_some_and(crate::pane::InputState::mouse_reporting_enabled) + .is_some_and(crate::terminal::TerminalRuntime::mouse_reporting_enabled) } pub(crate) fn should_capture_host_mouse_from( diff --git a/src/ghostty/bindings.rs b/src/ghostty/bindings.rs index af743eaa..bf39199e 100644 --- a/src/ghostty/bindings.rs +++ b/src/ghostty/bindings.rs @@ -2574,7 +2574,8 @@ pub const GhosttyTerminalData_GHOSTTY_TERMINAL_DATA_KITTY_GRAPHICS: GhosttyTermi pub const GhosttyTerminalData_GHOSTTY_TERMINAL_DATA_SELECTION: GhosttyTerminalData = 31; #[doc = " Whether the viewport is currently pinned to the active area.\n\n This is true when the viewport is following the active terminal area,\n and false when the user has scrolled into history.\n\n Output type: bool *"] pub const GhosttyTerminalData_GHOSTTY_TERMINAL_DATA_VIEWPORT_ACTIVE: GhosttyTerminalData = 32; -#[doc = " Whether the viewport is currently pinned to the active area.\n\n This is true when the viewport is following the active terminal area,\n and false when the user has scrolled into history.\n\n Output type: bool *"] +#[doc = " Whether xterm modifyOtherKeys mode 2 is enabled.\n\n Output type: bool *"] +pub const GhosttyTerminalData_GHOSTTY_TERMINAL_DATA_MODIFY_OTHER_KEYS: GhosttyTerminalData = 33; pub const GhosttyTerminalData_GHOSTTY_TERMINAL_DATA_MAX_VALUE: GhosttyTerminalData = 2147483647; #[doc = " Terminal data types.\n\n These values specify what type of data to extract from a terminal\n using `ghostty_terminal_get`.\n\n @ingroup terminal"] pub type GhosttyTerminalData = ::std::os::raw::c_uint; diff --git a/src/ghostty/mod.rs b/src/ghostty/mod.rs index cea7c283..c044d3b0 100644 --- a/src/ghostty/mod.rs +++ b/src/ghostty/mod.rs @@ -1034,6 +1034,10 @@ impl Terminal { self.get_bool(ffi::GhosttyTerminalData_GHOSTTY_TERMINAL_DATA_MOUSE_TRACKING) } + pub fn modify_other_keys_enabled(&self) -> Result<bool, Error> { + self.get_bool(ffi::GhosttyTerminalData_GHOSTTY_TERMINAL_DATA_MODIFY_OTHER_KEYS) + } + pub fn active_screen(&self) -> Result<ActiveScreen, Error> { let mut out = ffi::GhosttyTerminalScreen_GHOSTTY_TERMINAL_SCREEN_PRIMARY; unsafe { @@ -3636,6 +3640,17 @@ mod tests { } #[test] + fn modify_other_keys_query_tracks_mode_two() { + let mut terminal = Terminal::new(80, 24, 0).unwrap(); + + assert!(!terminal.modify_other_keys_enabled().unwrap()); + terminal.write(b"\x1b[>4;2m"); + assert!(terminal.modify_other_keys_enabled().unwrap()); + terminal.write(b"\x1b[>4;0m"); + assert!(!terminal.modify_other_keys_enabled().unwrap()); + } + + #[test] fn terminal_read_text_viewport_unwraps_soft_wrapped_selection() { let mut terminal = Terminal::new(5, 3, 0).unwrap(); terminal.write("1ABCD2EFGH3IJKL".as_bytes()); diff --git a/src/input/mod.rs b/src/input/mod.rs index 21893303..8108bf99 100644 --- a/src/input/mod.rs +++ b/src/input/mod.rs @@ -9,8 +9,10 @@ pub use encode::{ }; #[cfg(not(windows))] pub use model::ime_compatible_keyboard_enhancement_flags; +#[cfg(any(unix, test))] +pub use model::MouseProtocolMode; pub use model::{ - host_modify_other_keys_mode, KeyIdentity, KeyboardProtocol, MouseProtocolEncoding, - MouseProtocolMode, TerminalKey, TextCommit, WindowsKeyRecord, + host_modify_other_keys_mode, KeyIdentity, KeyboardProtocol, MouseProtocolEncoding, TerminalKey, + TextCommit, WindowsKeyRecord, }; pub use parse::parse_terminal_key_sequence; diff --git a/src/input/model.rs b/src/input/model.rs index 21252d2d..cada5506 100644 --- a/src/input/model.rs +++ b/src/input/model.rs @@ -295,6 +295,7 @@ impl KeyboardProtocol { } } +#[cfg(any(unix, test))] #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum MouseProtocolMode { @@ -305,7 +306,9 @@ pub enum MouseProtocolMode { AnyMotion, } +#[cfg(any(unix, test))] impl MouseProtocolMode { + #[cfg(test)] pub fn reporting_enabled(self) -> bool { self != Self::None } diff --git a/src/pane.rs b/src/pane.rs index 0b3edf7d..b2e1603d 100644 --- a/src/pane.rs +++ b/src/pane.rs @@ -40,6 +40,8 @@ use self::agent_detection::{ DetectionScreenReadInput, PendingIdleConfirmation, ScreenDetectionPublishInput, AGENT_PENDING_IDLE_RECHECK, AGENT_STARTUP_GRACE_WINDOW, }; +#[cfg(any(unix, test))] +pub use self::terminal::InputState; use self::terminal::{GhosttyPaneTerminal, PaneTerminal}; pub(crate) use self::terminal::{ TerminalDirtyPatch, TerminalDirtyPatchOutcome, TerminalReadSnapshot, TerminalTextMatch, @@ -47,7 +49,7 @@ pub(crate) use self::terminal::{ }; pub use self::{ state::PaneState, - terminal::{InputState, ScrollMetrics, TerminalCursorState}, + terminal::{ScrollMetrics, TerminalCursorState}, }; const RELEASE_REACQUIRE_SUPPRESSION: std::time::Duration = std::time::Duration::from_secs(1); @@ -1671,11 +1673,7 @@ impl PaneRuntime { #[cfg(unix)] pub fn handoff_history_ansi(&self) -> Option<String> { - if self - .terminal - .input_state() - .is_some_and(|input_state| input_state.alternate_screen) - { + if self.terminal.alternate_screen_active() { return None; } self.snapshot_history().map(|history| { @@ -2688,12 +2686,37 @@ impl PaneRuntime { self.terminal.word_motion_target(row, col, motion) } + #[cfg(any(unix, test))] pub fn input_state(&self) -> Option<InputState> { #[cfg(test)] AGGREGATE_INPUT_STATE_READS.set(AGGREGATE_INPUT_STATE_READS.get() + 1); self.terminal.input_state() } + pub fn keyboard_report_all_requested(&self) -> bool { + self.terminal.keyboard_report_all_requested() + } + + pub fn bracketed_paste_enabled(&self) -> bool { + self.terminal.bracketed_paste_enabled() + } + + pub fn focus_reporting_enabled(&self) -> bool { + self.terminal.focus_reporting_enabled() + } + + pub fn mouse_reporting_enabled(&self) -> bool { + self.terminal.mouse_reporting_enabled() + } + + pub fn sgr_pixel_mouse_enabled(&self) -> bool { + self.terminal.sgr_pixel_mouse_enabled() + } + + pub fn plain_page_keys_use_host_scrollback(&self) -> Option<bool> { + self.terminal.plain_page_keys_use_host_scrollback() + } + pub fn alternate_screen_active(&self) -> bool { self.terminal.alternate_screen_active() } @@ -2831,10 +2854,7 @@ impl PaneRuntime { } fn paste_payload(&self, text: String) -> Bytes { - let bracketed = self - .input_state() - .map(|state| state.bracketed_paste) - .unwrap_or(false); + let bracketed = self.bracketed_paste_enabled(); let payload = if bracketed { format!("\x1b[200~{text}\x1b[201~") } else { @@ -2844,11 +2864,7 @@ impl PaneRuntime { } pub fn try_send_focus_event(&self, event: crate::ghostty::FocusEvent) -> bool { - if !self - .input_state() - .map(|state| state.focus_reporting) - .unwrap_or(false) - { + if !self.focus_reporting_enabled() { return false; } @@ -2881,7 +2897,7 @@ impl PaneRuntime { position: crate::input::mouse::Position, modifiers: crossterm::event::KeyModifiers, ) -> Option<Vec<u8>> { - if !self.input_state()?.mouse_protocol_mode.reporting_enabled() { + if !self.mouse_reporting_enabled() { return None; } self.terminal.encode_mouse_button(kind, position, modifiers) @@ -2919,7 +2935,6 @@ impl PaneRuntime { &self, kind: crossterm::event::MouseEventKind, ) -> Option<Vec<u8>> { - self.input_state()?; if self.wheel_routing()? != WheelRouting::AlternateScroll { return None; } diff --git a/src/pane/terminal.rs b/src/pane/terminal.rs index b853f0b0..0b06302e 100644 --- a/src/pane/terminal.rs +++ b/src/pane/terminal.rs @@ -7,6 +7,7 @@ use std::time::{Duration, Instant}; use bytes::Bytes; use ratatui::style::{Color, Modifier, Style}; use ratatui::{layout::Rect, Frame}; +#[cfg(any(unix, test))] use serde::{Deserialize, Serialize}; use tokio::sync::mpsc; use tracing::{debug, error}; @@ -113,6 +114,7 @@ fn decscusr_cursor_shape(style: crate::ghostty::CursorVisualStyle, blinking: boo } } +#[cfg(any(unix, test))] #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] pub struct InputState { pub alternate_screen: bool, @@ -128,6 +130,7 @@ pub struct InputState { pub color_scheme_reporting: bool, } +#[cfg(test)] impl InputState { pub fn mouse_reporting_enabled(self) -> bool { self.mouse_protocol_mode.reporting_enabled() @@ -385,10 +388,35 @@ impl PaneTerminal { Some((RetainedTextBuffer::new_search(cols, rows, 0), active_screen)) } + #[cfg(any(unix, test))] pub fn input_state(&self) -> Option<InputState> { self.ghostty.input_state() } + pub fn keyboard_report_all_requested(&self) -> bool { + self.ghostty.keyboard_report_all_requested() + } + + pub fn bracketed_paste_enabled(&self) -> bool { + self.ghostty.bracketed_paste_enabled() + } + + pub fn focus_reporting_enabled(&self) -> bool { + self.ghostty.focus_reporting_enabled() + } + + pub fn mouse_reporting_enabled(&self) -> bool { + self.ghostty.mouse_reporting_enabled() + } + + pub fn sgr_pixel_mouse_enabled(&self) -> bool { + self.ghostty.sgr_pixel_mouse_enabled() + } + + pub fn plain_page_keys_use_host_scrollback(&self) -> Option<bool> { + self.ghostty.plain_page_keys_use_host_scrollback() + } + pub fn alternate_screen_active(&self) -> bool { self.ghostty.alternate_screen_active() } @@ -1668,14 +1696,66 @@ impl GhosttyPaneTerminal { core.kitty_keyboard.replay_ansi() } + pub fn keyboard_report_all_requested(&self) -> bool { + self.core.lock().is_ok_and(|core| { + let protocol = crate::input::KeyboardProtocol::from_kitty_flags( + core.terminal.kitty_keyboard_flags().unwrap_or(0) as u16, + ); + protocol.reports_all_keys() + || (protocol.reports_event_types() + && core.terminal.modify_other_keys_enabled().unwrap_or(false)) + }) + } + + pub fn bracketed_paste_enabled(&self) -> bool { + self.mode_enabled(crate::ghostty::MODE_BRACKETED_PASTE) + } + + pub fn focus_reporting_enabled(&self) -> bool { + self.mode_enabled(crate::ghostty::MODE_FOCUS_EVENT) + } + + pub fn mouse_reporting_enabled(&self) -> bool { + self.core + .lock() + .is_ok_and(|core| core.terminal.mouse_tracking_enabled().unwrap_or(false)) + } + + pub fn sgr_pixel_mouse_enabled(&self) -> bool { + self.mode_enabled(crate::ghostty::MODE_MOUSE_SGR_PIXELS) + } + + fn mode_enabled(&self, mode: u16) -> bool { + self.core + .lock() + .is_ok_and(|core| core.terminal.mode_get(mode).unwrap_or(false)) + } + + pub fn plain_page_keys_use_host_scrollback(&self) -> Option<bool> { + let core = self.core.lock().ok()?; + let alternate_screen = + core.terminal.active_screen().ok()? == crate::ghostty::ActiveScreen::Alternate; + let mouse_reporting = core.terminal.mouse_tracking_enabled().ok()?; + let application_cursor = core + .terminal + .mode_get(crate::ghostty::MODE_APPLICATION_CURSOR_KEYS) + .ok()?; + let bracketed_paste = core + .terminal + .mode_get(crate::ghostty::MODE_BRACKETED_PASTE) + .ok()?; + Some(!alternate_screen && !mouse_reporting && (!application_cursor || bracketed_paste)) + } + pub fn alternate_screen_active(&self) -> bool { self.core.lock().is_ok_and(|core| { core.terminal.active_screen().ok() == Some(crate::ghostty::ActiveScreen::Alternate) }) } - // This aggregate snapshot performs multiple terminal queries and may format - // keyboard state. Pane-scaled callers should add a narrow accessor instead. + // This aggregate snapshot performs multiple terminal queries. Pane-scaled + // callers should add a narrow accessor instead. + #[cfg(any(unix, test))] pub fn input_state(&self) -> Option<InputState> { let Ok(core) = self.core.lock() else { return None; @@ -1738,14 +1818,7 @@ impl GhosttyPaneTerminal { mouse_protocol_mode, mouse_protocol_encoding, mouse_alternate_scroll, - #[cfg(windows)] - modify_other_keys: core.kitty_keyboard.modify_other_keys_enabled(), - #[cfg(not(windows))] - modify_other_keys: core - .terminal - .keyboard_state_ansi() - .ok() - .is_some_and(|ansi| !ansi.is_empty()), + modify_other_keys: core.terminal.modify_other_keys_enabled().ok()?, color_scheme_reporting: core .terminal .mode_get(crate::ghostty::MODE_COLOR_SCHEME_REPORT) diff --git a/src/render_signal.rs b/src/render_signal.rs index f4b4391d..a6ebd839 100644 --- a/src/render_signal.rs +++ b/src/render_signal.rs @@ -16,7 +16,13 @@ pub(crate) struct RenderRequest { #[derive(Debug, Default)] pub(crate) struct RenderSignal { pending: AtomicBool, - request: Mutex<RenderRequest>, + state: Mutex<RenderSignalState>, +} + +#[derive(Debug, Default)] +struct RenderSignalState { + request: RenderRequest, + immediate_pty_sources: HashSet<PaneId>, } impl RenderSignal { @@ -29,78 +35,75 @@ impl RenderSignal { } pub(crate) fn request_generic(&self) { - let mut request = self - .request + let mut state = self + .state .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); - request.generic = true; + state.request.generic = true; self.pending.store(true, Ordering::Release); } - /// Returns true when the signal becomes pending or a new PTY source joins it. - /// - /// A new source may be visible even when the existing pending sources are - /// hidden, so the consumer must re-evaluate the coalesced request. + /// Returns true when the signal becomes pending or visible PTY work joins it. pub(crate) fn request_pty(&self, pane_id: PaneId) -> bool { - let mut request = self - .request + let mut state = self + .state .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); - let source_added = request.pty_sources.insert(pane_id); + let source_added = state.request.pty_sources.insert(pane_id); + let wake_for_source = source_added && state.immediate_pty_sources.contains(&pane_id); let became_pending = !self.pending.swap(true, Ordering::AcqRel); - became_pending || source_added + became_pending || wake_for_source } - pub(crate) fn has_generic_or_terminal_title(&self) -> bool { - let request = self - .request + pub(crate) fn set_immediate_pty_sources(&self, sources: HashSet<PaneId>) { + self.state .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - request.generic || !request.terminal_title_sources.is_empty() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .immediate_pty_sources = sources; } - /// Checks pending PTY origins without allocating a source snapshot. - /// Keep the predicate narrow because producers share this lock. - pub(crate) fn has_pty_source_matching( - &self, - mut predicate: impl FnMut(PaneId) -> bool, - ) -> bool { - self.request + pub(crate) fn has_immediate_work(&self) -> bool { + let state = self + .state .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .pty_sources - .iter() - .copied() - .any(&mut predicate) + .unwrap_or_else(std::sync::PoisonError::into_inner); + state.request.generic + || !state.request.terminal_title_sources.is_empty() + || state + .request + .pty_sources + .iter() + .any(|pane_id| state.immediate_pty_sources.contains(pane_id)) } /// Coalesces terminal-title changes separately from ordinary PTY damage so /// consumers can update metadata without inspecting every pane. pub(crate) fn request_terminal_title(&self, pane_id: PaneId) -> bool { - let mut request = self - .request + let mut state = self + .state .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); - let source_added = request.terminal_title_sources.insert(pane_id); + let source_added = state.request.terminal_title_sources.insert(pane_id); let became_pending = !self.pending.swap(true, Ordering::AcqRel); became_pending || source_added } pub(crate) fn pending_terminal_title_sources(&self) -> HashSet<PaneId> { - self.request + self.state .lock() .unwrap_or_else(std::sync::PoisonError::into_inner) + .request .terminal_title_sources .clone() } pub(crate) fn take(&self) -> RenderRequest { - let mut request = self - .request + let mut state = self + .state .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); self.pending.store(false, Ordering::Release); - std::mem::take(&mut *request) + std::mem::take(&mut state.request) } } @@ -116,7 +119,7 @@ mod tests { assert!(signal.request_pty(first)); assert!(!signal.request_pty(first)); - assert!(signal.request_pty(second)); + assert!(!signal.request_pty(second)); let request = signal.take(); assert!(!request.generic); @@ -126,6 +129,31 @@ mod tests { } #[test] + fn hidden_pty_sources_coalesce_to_one_wake() { + let signal = RenderSignal::new(); + signal.set_immediate_pty_sources(HashSet::from([PaneId::from_raw(100)])); + + let wakes = (1..=50) + .filter(|pane_id| signal.request_pty(PaneId::from_raw(*pane_id))) + .count(); + + assert_eq!(wakes, 1); + } + + #[test] + fn immediate_pty_source_wakes_pending_hidden_work() { + let signal = RenderSignal::new(); + let hidden = PaneId::from_raw(10); + let visible = PaneId::from_raw(20); + signal.set_immediate_pty_sources(HashSet::from([visible])); + + assert!(signal.request_pty(hidden)); + assert!(!signal.request_pty(PaneId::from_raw(30))); + assert!(signal.request_pty(visible)); + assert!(!signal.request_pty(visible)); + } + + #[test] fn terminal_title_source_wakes_pending_pty_work() { let signal = RenderSignal::new(); let pane_id = PaneId::from_raw(10); @@ -158,7 +186,7 @@ mod tests { let pane_id = PaneId::from_raw(10); signal.request_generic(); - assert!(signal.request_pty(pane_id)); + assert!(!signal.request_pty(pane_id)); let request = signal.take(); assert!(request.generic); diff --git a/src/server/headless.rs b/src/server/headless.rs index ae0d7b20..9161cd34 100644 --- a/src/server/headless.rs +++ b/src/server/headless.rs @@ -357,8 +357,8 @@ fn apply_terminal_attach_scroll( }; if let AttachScrollSource::PageKey { input } = source { let host_scroll = runtime - .input_state() - .is_some_and(crate::pane::InputState::plain_page_keys_use_host_scrollback); + .plain_page_keys_use_host_scrollback() + .unwrap_or(false); if host_scroll { match direction { AttachScrollDirection::Up => runtime.scroll_up(lines.max(1) as usize), @@ -695,6 +695,7 @@ impl HeadlessServer { } self.drain_client_config_reload_request(); + self.sync_immediate_pty_sources(); self.stream_host_mouse_capture_mode(); self.stream_host_keyboard_enhancement_flags(); @@ -3998,10 +3999,7 @@ impl HeadlessServer { ) }) }) - .and_then(crate::terminal::TerminalRuntime::input_state) - .is_some_and(|state| { - state.mouse_protocol_encoding == crate::input::MouseProtocolEncoding::SgrPixels - }); + .is_some_and(crate::terminal::TerminalRuntime::sgr_pixel_mouse_enabled); let mut broken_clients: Vec<u64> = Vec::new(); for (&client_id, client) in &mut self.clients { if !client.is_full_app_client() { @@ -4086,24 +4084,33 @@ impl HeadlessServer { needs_full_render: bool, needs_graphics_render: bool, ) -> bool { - if needs_full_render - || needs_graphics_render - || self.app.render_dirty.has_generic_or_terminal_title() - { - return true; - } + needs_full_render || needs_graphics_render || self.app.render_dirty.has_immediate_work() + } + fn sync_immediate_pty_sources(&self) { let (has_app_target, direct_terminal_targets) = self.pty_render_targets(); - if !has_app_target && direct_terminal_targets.is_empty() { - return false; + let mut pane_ids = if has_app_target { + self.app.state.app_surface_pane_ids() + } else { + HashSet::new() + }; + if !direct_terminal_targets.is_empty() { + for workspace in &self.app.state.workspaces { + for tab in &workspace.tabs { + pane_ids.extend(tab.panes.iter().filter_map(|(&pane_id, pane)| { + direct_terminal_targets + .contains(pane.attached_terminal_id.as_str()) + .then_some(pane_id) + })); + } + } + if let Some(popup) = &self.app.state.popup_pane { + if direct_terminal_targets.contains(popup.terminal_id.as_str()) { + pane_ids.insert(popup.pane_id); + } + } } - self.app.render_dirty.has_pty_source_matching(|pane_id| { - self.pty_source_visible_to_render_targets( - pane_id, - has_app_target, - &direct_terminal_targets, - ) - }) + self.app.render_dirty.set_immediate_pty_sources(pane_ids); } fn pty_render_targets(&self) -> (bool, HashSet<&str>) { @@ -9684,6 +9691,7 @@ next_tab = "" fn visible_source_wakes_pending_hidden_work() { let (server, background_pane) = hidden_pty_visibility_test_server(&[(120, 40)]); let visible_pane = server.app.state.workspaces[0].tabs[0].root_pane; + server.sync_immediate_pty_sources(); assert!(server.app.render_dirty.request_pty(background_pane)); assert!(!server.has_pending_presentation_work(false, false)); @@ -9776,15 +9784,17 @@ next_tab = "" } #[test] - fn direct_terminal_observer_keeps_hidden_pty_source_renderable() { - let (mut server, background_pane) = hidden_pty_visibility_test_server(&[]); + fn direct_terminal_observer_keeps_hidden_pty_source_renderable_with_app_client() { + let (mut server, background_pane) = hidden_pty_visibility_test_server(&[(120, 40)]); + assert!(!server.pty_sources_visible_to_any_render_target(&HashSet::from([background_pane]))); + let terminal_id = server.app.state.workspaces[0] .terminal_id(background_pane) .expect("background terminal id") .to_string(); let (client_tx, _client_control_rx, _client_rx) = test_client_writer(); server.clients.insert( - 1, + 2, ClientConnection::new_with_mode( ClientConnectionMode::TerminalObserve { terminal_id }, None, @@ -9792,7 +9802,7 @@ next_tab = "" crate::kitty_graphics::HostCellSize::default(), crate::terminal_theme::TerminalTheme::default(), None, - 1, + 2, RenderEncoding::SemanticFrame, false, Some(client_tx), @@ -9800,6 +9810,12 @@ next_tab = "" ); assert!(server.pty_sources_visible_to_any_render_target(&HashSet::from([background_pane]))); + + let hidden_pane = server.app.state.workspaces[0].tabs[0].root_pane; + server.sync_immediate_pty_sources(); + assert!(server.app.render_dirty.request_pty(background_pane)); + assert!(server.has_pending_presentation_work(false, false)); + assert!(server.app.render_dirty.request_pty(hidden_pane)); } #[tokio::test] diff --git a/src/server/terminal_attach.rs b/src/server/terminal_attach.rs index 01fc899f..e7a52be5 100644 --- a/src/server/terminal_attach.rs +++ b/src/server/terminal_attach.rs @@ -2,11 +2,7 @@ pub(crate) fn paste_payload_for_runtime( runtime: &crate::terminal::TerminalRuntime, text: &str, ) -> String { - if runtime - .input_state() - .map(|state| state.bracketed_paste) - .unwrap_or(false) - { + if runtime.bracketed_paste_enabled() { format!("\x1b[200~{text}\x1b[201~") } else { text.to_owned() diff --git a/src/terminal/runtime.rs b/src/terminal/runtime.rs index 619c98a4..5051a46b 100644 --- a/src/terminal/runtime.rs +++ b/src/terminal/runtime.rs @@ -313,13 +313,33 @@ impl TerminalRuntime { /// Collects the complete terminal input-mode snapshot. /// - /// This performs multiple terminal queries and may format keyboard state. - /// Keep it out of render/layout and pane-scaled loops; add a narrow accessor - /// when only one terminal fact is needed. + /// This performs multiple terminal queries. Keep it out of render/layout + /// and pane-scaled loops; add a narrow accessor when one fact is needed. + #[cfg(test)] pub fn input_state(&self) -> Option<crate::pane::InputState> { self.0.input_state() } + pub fn keyboard_report_all_requested(&self) -> bool { + self.0.keyboard_report_all_requested() + } + + pub fn bracketed_paste_enabled(&self) -> bool { + self.0.bracketed_paste_enabled() + } + + pub fn mouse_reporting_enabled(&self) -> bool { + self.0.mouse_reporting_enabled() + } + + pub fn sgr_pixel_mouse_enabled(&self) -> bool { + self.0.sgr_pixel_mouse_enabled() + } + + pub fn plain_page_keys_use_host_scrollback(&self) -> Option<bool> { + self.0.plain_page_keys_use_host_scrollback() + } + /// Reads only whether the alternate screen is active. pub fn alternate_screen_active(&self) -> bool { self.0.alternate_screen_active() diff --git a/vendor/libghostty-vt.patches.md b/vendor/libghostty-vt.patches.md index 04305960..dff98533 100644 --- a/vendor/libghostty-vt.patches.md +++ b/vendor/libghostty-vt.patches.md @@ -38,3 +38,40 @@ cargo nextest run --locked grapheme_cluster_mode_is_default_and_survives_full_re cargo nextest run --locked grapheme_cluster_mode_renders_flag_emoji_in_single_wide_cell cargo nextest run --locked grapheme_cluster_mode_renders_zwj_family_in_single_wide_cell ``` + +## 0002 expose modifyOtherKeys mode through terminal data + +status: active + +patch: `vendor/patches/libghostty-vt/0002-expose-modify-other-keys-mode.patch` + +herdr issue: none; fixes the performance regression exposed by +https://github.com/herdrdev/herdr/pull/2303 + +upstream discussion: not opened + +upstream pr: not opened + +vendored base: `c5a21edfcbc2d5b46540ad91b7980aca31f5f1f3` + +local files: + +- `vendor/libghostty-vt/include/ghostty/vt/terminal.h` +- `vendor/libghostty-vt/src/terminal/c/terminal.zig` + +reason: Herdr must know whether xterm modifyOtherKeys mode 2 is active to +request printable key releases from the outer terminal. The formatter API can +recover this fact only by formatting the active screen and scrollback. A typed +terminal-data query exposes the authoritative scalar without formatting or +allocation. + +remove when: the vendored source exposes an equivalent scalar query for +modifyOtherKeys mode 2 and Herdr can use it without this patch. + +verification: + +```sh +cargo nextest run --locked modify_other_keys_query_tracks_mode_two +cargo nextest run --locked host_report_all_supplies_printable_releases_for_event_type_only_panes +python3 -m unittest scripts.test_vendor_libghostty_vt scripts.test_ui_hot_path_architecture +``` diff --git a/vendor/libghostty-vt/include/ghostty/vt/terminal.h b/vendor/libghostty-vt/include/ghostty/vt/terminal.h index 26ee0c0b..9bb961d3 100644 --- a/vendor/libghostty-vt/include/ghostty/vt/terminal.h +++ b/vendor/libghostty-vt/include/ghostty/vt/terminal.h @@ -1197,6 +1197,13 @@ typedef enum GHOSTTY_ENUM_TYPED { * Output type: bool * */ GHOSTTY_TERMINAL_DATA_VIEWPORT_ACTIVE = 32, + + /** + * Whether xterm modifyOtherKeys mode 2 is enabled. + * + * Output type: bool * + */ + GHOSTTY_TERMINAL_DATA_MODIFY_OTHER_KEYS = 33, GHOSTTY_TERMINAL_DATA_MAX_VALUE = GHOSTTY_ENUM_MAX_VALUE, } GhosttyTerminalData; diff --git a/vendor/libghostty-vt/src/terminal/c/terminal.zig b/vendor/libghostty-vt/src/terminal/c/terminal.zig index a896c0fd..0f95fcf1 100644 --- a/vendor/libghostty-vt/src/terminal/c/terminal.zig +++ b/vendor/libghostty-vt/src/terminal/c/terminal.zig @@ -757,13 +757,19 @@ pub const TerminalData = enum(c_int) { kitty_graphics = 30, selection = 31, viewport_active = 32, + modify_other_keys = 33, /// Output type expected for querying the data of the given kind. pub fn OutType(comptime self: TerminalData) type { return switch (self) { .invalid => void, .cols, .rows, .cursor_x, .cursor_y => size.CellCountInt, - .cursor_pending_wrap, .cursor_visible, .mouse_tracking, .viewport_active => bool, + .cursor_pending_wrap, + .cursor_visible, + .mouse_tracking, + .viewport_active, + .modify_other_keys, + => bool, .active_screen => TerminalScreen, .kitty_keyboard_flags => u8, .scrollbar => TerminalScrollbar, @@ -899,6 +905,7 @@ fn getTyped( t.screens.active.selection orelse return .no_value, ), .viewport_active => out.* = t.screens.active.pages.viewport == .active, + .modify_other_keys => out.* = t.flags.modify_other_keys_2, } return .success; @@ -1654,6 +1661,32 @@ test "get kitty_keyboard_flags" { try testing.expectEqual(3, flags); } +test "get modify_other_keys" { + var t: Terminal = null; + try testing.expectEqual(Result.success, new( + &lib.alloc.test_allocator, + &t, + .{ + .cols = 80, + .rows = 24, + .max_scrollback = 0, + }, + )); + defer free(t); + + var enabled: bool = undefined; + try testing.expectEqual(Result.success, get(t, .modify_other_keys, @ptrCast(&enabled))); + try testing.expect(!enabled); + + vt_write(t, "\x1b[>4;2m", 7); + try testing.expectEqual(Result.success, get(t, .modify_other_keys, @ptrCast(&enabled))); + try testing.expect(enabled); + + vt_write(t, "\x1b[>4;0m", 7); + try testing.expectEqual(Result.success, get(t, .modify_other_keys, @ptrCast(&enabled))); + try testing.expect(!enabled); +} + test "get mouse_tracking" { var t: Terminal = null; try testing.expectEqual(Result.success, new( diff --git a/vendor/patches/libghostty-vt/0002-expose-modify-other-keys-mode.patch b/vendor/patches/libghostty-vt/0002-expose-modify-other-keys-mode.patch new file mode 100644 index 00000000..5d8540db --- /dev/null +++ b/vendor/patches/libghostty-vt/0002-expose-modify-other-keys-mode.patch @@ -0,0 +1,69 @@ +diff --git a/vendor/libghostty-vt/include/ghostty/vt/terminal.h b/vendor/libghostty-vt/include/ghostty/vt/terminal.h +index 26ee0c0b..9bb961d3 100644 +--- a/vendor/libghostty-vt/include/ghostty/vt/terminal.h ++++ b/vendor/libghostty-vt/include/ghostty/vt/terminal.h +@@ -1199,2 +1199,9 @@ typedef enum GHOSTTY_ENUM_TYPED { + GHOSTTY_TERMINAL_DATA_VIEWPORT_ACTIVE = 32, ++ ++ /** ++ * Whether xterm modifyOtherKeys mode 2 is enabled. ++ * ++ * Output type: bool * ++ */ ++ GHOSTTY_TERMINAL_DATA_MODIFY_OTHER_KEYS = 33, + GHOSTTY_TERMINAL_DATA_MAX_VALUE = GHOSTTY_ENUM_MAX_VALUE, +diff --git a/vendor/libghostty-vt/src/terminal/c/terminal.zig b/vendor/libghostty-vt/src/terminal/c/terminal.zig +index a896c0fd..0f95fcf1 100644 +--- a/vendor/libghostty-vt/src/terminal/c/terminal.zig ++++ b/vendor/libghostty-vt/src/terminal/c/terminal.zig +@@ -759,3 +759,4 @@ pub const TerminalData = enum(c_int) { + viewport_active = 32, ++ modify_other_keys = 33, +- ++ + /// Output type expected for querying the data of the given kind. +@@ -765,3 +766,8 @@ pub const TerminalData = enum(c_int) { + .cols, .rows, .cursor_x, .cursor_y => size.CellCountInt, +- .cursor_pending_wrap, .cursor_visible, .mouse_tracking, .viewport_active => bool, ++ .cursor_pending_wrap, ++ .cursor_visible, ++ .mouse_tracking, ++ .viewport_active, ++ .modify_other_keys, ++ => bool, + .active_screen => TerminalScreen, +@@ -901,2 +907,3 @@ fn getTyped( + .viewport_active => out.* = t.screens.active.pages.viewport == .active, ++ .modify_other_keys => out.* = t.flags.modify_other_keys_2, + } +@@ -1655,3 +1662,29 @@ test "get kitty_keyboard_flags" { + } +- ++ ++test "get modify_other_keys" { ++ var t: Terminal = null; ++ try testing.expectEqual(Result.success, new( ++ &lib.alloc.test_allocator, ++ &t, ++ .{ ++ .cols = 80, ++ .rows = 24, ++ .max_scrollback = 0, ++ }, ++ )); ++ defer free(t); ++ ++ var enabled: bool = undefined; ++ try testing.expectEqual(Result.success, get(t, .modify_other_keys, @ptrCast(&enabled))); ++ try testing.expect(!enabled); ++ ++ vt_write(t, "\x1b[>4;2m", 7); ++ try testing.expectEqual(Result.success, get(t, .modify_other_keys, @ptrCast(&enabled))); ++ try testing.expect(enabled); ++ ++ vt_write(t, "\x1b[>4;0m", 7); ++ try testing.expectEqual(Result.success, get(t, .modify_other_keys, @ptrCast(&enabled))); ++ try testing.expect(!enabled); ++} ++ + test "get mouse_tracking" { |
