fix(overlay): stale pixels and show/hide races on Wayland layer shell - #1909
Open
xilec wants to merge 12 commits into
Open
fix(overlay): stale pixels and show/hide races on Wayland layer shell#1909xilec wants to merge 12 commits into
xilec wants to merge 12 commits into
Conversation
…land WebKitGTK repaints only damaged regions, so when the overlay surface is unmapped mid-transition and re-shown at the same size, stale pixels from the previous session stay on screen (the "ghost card" glitch) — or no fresh frame is committed at all and the overlay never appears. Before hiding, shrink the GTK layer surface back to the compact size while it is still mapped: a real size change forces WebKit to repaint the reused buffer with fresh, correct pixels. The next show then maps a clean compact surface instead of resurrecting stale large-size content, which also removes the brief "big card flashes, then shrinks" flicker. Also add a show-generation guard so a delayed hide scheduled before a quick re-trigger aborts instead of hiding the freshly shown overlay.
Rapid re-triggers raced the delayed hide chain: a hide scheduled by the previous session could unmap a freshly shown overlay, or a new recording starting mid-chain was hidden as soon as the old chain finished, leaving the overlay invisible (or stuck in an "inverted" state) until the next trigger. OverlayController now tracks phase (Hidden / Visible / Hiding) and an operation id, and all show/hide requests go through pure decision functions: a stale hide for a superseded operation is ignored, a hide arriving mid-chain retargets the running chain, and a show requested while a hide chain is in flight is deferred to the chain's end instead of fighting it. Hides are tagged with the operation that started the recording so only same-generation chains can finish the hide. Transition coverage lives in a dedicated test module exercising the full decision matrix and the race scenarios (mid-chain reshow, stale hide, double stop).
…ethods The visibility scenario tests mirrored the mutation halves of show_overlay_state / hide_recording_overlay / the hide-chain end in a hand-written Sim, so a future change to the real paths could silently drift from what the tests exercise. The mutations now live on OverlayController itself (request_show / request_hide / settle_chain_end / reset_to_hidden, plus begin_operation); the production paths call them under the same locks, and the tests drive a real controller instance. No behavior change. Also fixes a needless-borrow clippy warning in the park block.
3 tasks
If run_on_main_thread scheduling of the chain-end closure fails (e.g. during app shutdown), the controller stayed in Hiding forever and every later show was deferred behind a chain that could never finish. Settle it to Hidden instead so future shows apply immediately.
On Wayland a re-trigger during a hide chain defers the overlay show to the chain's end (~200 ms of park repaints). A warm microphone can deliver its first samples inside that window, so recording-ready hit the frontend before the deferred show-overlay did; the frontend's show handler then reset its arming state right over it and the card stayed dimmed until the end of the session, with no further readiness event ever coming. Latch readiness onto the pending card itself: Desired::Visible now carries mic_ready, so an immediate show delivers as before while a deferred one replays the event immediately after its own show-overlay at the chain end. Bundling the flag with the card it belongs to makes cross-session leakage unrepresentable - every request_show starts a fresh arming state.
xilec
marked this pull request as draft
August 27, 2026 14:28
the webview unmount commits Two races left the previous session's card baked into the overlay: - a restart that lands before the previous session's hide (the new operation begins first, so the hide is correctly ignored by op id) transitions streaming->streaming at the same 400x120 surface size; with no resize WebKitGTK repaints incrementally and the old wide card blends under the new pill (ghost-card hybrid: stale text, spinner and a second cancel button stuck for the whole recording); - under main-thread load (paste burst, focus churn) the webview missed the blind 80ms beat before parking, so the parked frame kept the old card clipped to the compact viewport. The show path now converts a same-size show-while-visible into the park chain (unmount -> shrink -> re-show), and the chain waits for the frontend to acknowledge the unmount via the overlay_hidden_ack command (flushSync before sending; a command is delivered even while the GTK main thread is busy, unlike a JS-emitted event). Normal-path hides get faster: the ack arrives in ~10-30ms instead of the fixed 80ms beat. A dead webview falls back to the bounded blind park. Reproduced with the overlay integration harness (busy-main-thread and restart-without-hide scenarios): the full hybrid (stale text, spinner, two cancel buttons) before the fix, a clean compact card after it.
…ize show The park-chain start sequence — arm a fresh ack round, emit hide-overlay, spawn the chain thread, fall back to reset_to_hidden when the window is gone — existed twice: in hide_recording_overlay and in the ParkShow arm of show_overlay_state. Collapse them into one helper so the chain-start contract (store before emit before spawn; at most one chain in flight) lives in a single place.
spawn_park_chain nested four phases inside each other (thread spawn > layer-shell guard > ack poll loop > main-thread closures), which buried the order-of-operations contract. Extract the phases as named steps — wait_for_hidden_ack, park_surface_compact, apply_chain_end — so the chain reads as a linear script: wait for the unmount ack, park the surface at the compact size, settle whatever the controller wants. Also name the 130 ms commit settle (PARK_COMMIT_SETTLE_MS), enrich the ack-timeout warning with elapsed time and the pending want, document the payload-less-ack invariant (and its timeout-window limitation) at HIDDEN_ACK, note that the park is layer-shell-specific, and pin the frontend contract that no async may run between the flushSync unmount and the ack invoke. Debug log lines are preserved verbatim; behavior is unchanged.
show_overlay_state_on_main interleaved the layer-shell probe with the default size/position/show sequence and its per-step timings. Extract show_with_layer_shell (returns whether it handled the show) and apply_default_geometry so the main function reads: size, path, emit. The gtk_window() failure quirk is preserved verbatim: it logs an error but still shows the window and reports handled — routing a configured layer surface through the default set_size path would be wrong. Behavior is unchanged.
Contributor
Author
|
Note: this push also fixes the overlay ghost-card regression that appeared after merging the latest upstream changes and resolving conflicts, and includes a small refactoring for more linear and clear code. |
xilec
marked this pull request as ready for review
August 28, 2026 12:43
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before Submitting This PR
Human Written Description
Follow-up to #1700.
Two cooperating problems on Wayland (GTK layer shell), both around the reused, cached overlay window:
Show/hide races between sessions. Rapid re-triggers interleaved async show/hide calls, so a late hide from session N could unmap session N+1's overlay. Fix: an
OverlayControllerstate machine (Hidden / Visible / Hiding) serializes all visibility transitions, and every hide is tagged with the owning operation id (begin_operation()at transcription start), so a stale hide from a finished operation is dropped instead of unmapping a newer session's overlay.Out of scope here: the coordinator can itself desync on rapid external toggles (a press dropped while the pipeline is processing, or a sub-30 ms edge lost to debounce) and then legitimately show a recording overlay with the button already released. That is fixed separately in the follow-up PR: fix(shortcut): keep toggle parity when presses arrive mid-pipeline #1910.
Also removed the fade-out sleep in the hide path: the frontend returns null when the overlay is not visible, so no fade ever played — the delay just widened the race window.
All controller transitions live in pure methods on
OverlayController; scenario tests drive a real controller instance and cannot drift from production behavior.Related Issues/Discussions
Community Feedback
Bug fix; no prior discussion. Reproduced deterministically on niri (Wayland) — see Testing.
Testing
cargo test: 214 passed (full transition matrix for the controller: show/hide × phases × fresh/stale op-id, mid-chain re-trigger, bursts, reuse after cycle).cargo clippy: clean for the touched files.Known minor issue (for reviewers)
With the readiness indicator from 549cbde, a show deferred to the hide-chain end (~210 ms window on Linux layer shell) can deliver
recording-readybeforeshow-overlay, so the restarted session briefly shows the "arming" state. Cosmetic; the ordering predates this PR.Screenshots/Videos
AI Assistance
If AI was used: