Skip to content

fix(linux): repair X11 overlay after display changes#2354

Open
KeroZelvin wants to merge 4 commits into
trycua:mainfrom
KeroZelvin:fix/linux-x11-overlay-hotplug
Open

fix(linux): repair X11 overlay after display changes#2354
KeroZelvin wants to merge 4 commits into
trycua:mainfrom
KeroZelvin:fix/linux-x11-overlay-hotplug

Conversation

@KeroZelvin

@KeroZelvin KeroZelvin commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Summary

  • subscribe the Linux/X11 overlay to RandR screen, CRTC, and output changes
  • service X11 events on a 50 ms quiescent heartbeat without authorizing idle repaint work
  • on a display change, fail closed by clearing input and bounding shapes before resizing the overlay to the live root geometry
  • upload replacement pixels before exposing the checked cursor-local XShape against the new bounds
  • treat unsafe asynchronous X11 server errors as fatal while recovering the stale pinned-sibling BadWindow race
  • query initial root geometry after RandR subscription to close the startup race

Fixes #2350.
Follow-up to #2331.

Why this is separate from #2331

#2331 fixed initial XShape safety and removed the continuous full-root render loop. Its X11 overlay still snapshots the root dimensions once, discards X11 events, and blocks indefinitely on the command channel when all cursors are quiescent.

That leaves a long-lived overlay stale after monitor hotplug or root resize. On the reported GNOME/Xorg system, the stale root-sized overlay was strongly correlated with an opaque black surface after both displays reconnected.

This patch repairs the deterministic geometry/lifecycle gap. The exact GNOME/Xorg shape-exposure mechanism remains field-only and is not claimed as deterministically reproduced.

Safety behavior

Before ConfigureWindow, the repair path sets both XShape input and bounding regions to empty and checks each request. It uploads replacement pixels while the overlay remains hidden, then installs the checked cursor-local shape last.

A pinned target can disappear after its liveness probe but before the unchecked z-order ConfigureWindow reaches X11. The event path recovers only a core ConfigureWindow BadWindow naming a non-overlay resource, then lets the next z-order tick fall back to no sibling. An overlay-window BadWindow, BadMatch, PutImage failure, or any other asynchronous server error remains fatal: the owner thread exits, drops the X11 connection/window, releases active animation waiters, and marks the X11 renderer unavailable instead of leaving an unsafe overlay or a hung caller.

After resize, normal tile rendering restores either:

  • an empty bounding shape for hidden or off-screen cursors, or
  • cursor-local shape rectangles for visible cursors.

No full-root pixel buffer or full-root paint path is restored.

Validation

Automated:

  • cargo fmt --all -- --check
  • cargo test -p platform-linux --lib --locked — 151 passed
  • cargo test -p cursor-overlay --locked — 12 passed
  • cargo test --workspace --no-run --locked
  • cargo build -p cua-driver --locked
  • git diff --check

Isolated A/B harness (never used the live display):

  • Xvfb :109 hosted resizable Xephyr :110
  • exact installed fix(cua-driver/linux): bound X11 overlay render work #2331 merge build (428d7aa, SHA-256 6b33fb6798d14e20e32fbc8e2bb91965e81ef39ae333bbe2d0748999cee9907a): root changed 1280×1024 → 1024×768, but overlay remained stale at 1280×1024
  • final candidate: overlay tracked 1280×1024 → 1024×768 → 1280×1024
  • hidden shape remained 0×0 throughout
  • resting visible cursor shape remained cursor-local at 23×23 throughout
  • cursor outside the shrunken root became 0×0 and restored only its 23×23 local shape when the root grew
  • hidden candidate consumed 1 CPU tick over a 5.001 second sample (0.2%) despite the 50 ms event-service heartbeat
  • terminating the isolated X server during a configured 5-second glide released the in-flight call; a subsequent move returned in 0.01 seconds instead of hanging
  • independent Codex re-review after lifecycle/protocol fixes: No actionable findings.
  • Codex P2 stale-target follow-up adds explicit recovery/fatal-boundary regression tests; independent exact-diff re-review: No actionable findings.
  • collaborator cleanup preserves the public send_command_for(CursorKey, OverlayCommand) -> () contract with a compile-time regression test, removes the unused geometry-update result, renames the event/error classifier and drain path, and refreshes render-loop documentation; independent exact-diff re-review: No actionable findings.
  • lifecycle teardown regression transfers receiver ownership to the cleanup guard, disconnects command delivery before disabling render state and releasing waiters, and deterministically registers in the teardown interval to prove both ordering boundaries; independent strengthened exact-diff re-review: No actionable findings.

Xephyr preserved #2331's empty shape and therefore did not reproduce the opaque-black GNOME/Xorg symptom; it did deterministically reproduce the stale-geometry defect fixed here.

@KeroZelvin
KeroZelvin marked this pull request as ready for review July 19, 2026 15:52

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9f222afbdc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread libs/cua-driver/rust/crates/platform-linux/src/overlay.rs
@injaneity

injaneity commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator

the fix appears to address the hotplug defect, but some fixes needed to tighten the diff before merge:

preserve the existing send_command_for() public signature, remove or use the ignored update_render_map_geometry() return value, rename the event classifier to reflect that it also handles protocol errors, and update the now-stale render-loop documentation. we want to keep the safety-critical path self-documenting

@KeroZelvin

Copy link
Copy Markdown
Contributor Author

@injaneity Addressed all four cleanup requests in 7b5f6234da1d8c0626e6121648d677480dbae33c:

  • restored the public send_command_for(CursorKey, OverlayCommand) -> () signature and added a compile-time signature regression test; the X11 enqueue result is now private to the arrival-waiting path, so failed delivery still cancels the waiter instead of hanging
  • removed the unused update_render_map_geometry() boolean result and updated its state-based tests
  • renamed the mixed classifier/drain helpers to classify_x11_overlay_event() / drain_x11_overlay_events() and documented the Ok(true) / Ok(false) / Err safety contract
  • refreshed the render-loop, quiescent maintenance, tile-rendering, and z-order comments to match the actual 50 ms event-service and 80 ms stacking behavior

Validation: rustfmt; platform-linux 150/150; cursor-overlay 12/12; workspace no-run compile; cua-driver build; git diff --check. Independent exact-diff review returned No actionable findings.

@injaneity

Copy link
Copy Markdown
Collaborator

one remaining lifecycle race before merge:

X11OverlayThreadCleanup::drop() releases arrival waiters while rx is still alive. because the cleanup guard is a local and rx is a function parameter, rust drops the guard first. a concurrent animate_cursor_to_for() can pass the render-state check, register after release_all_arrivals(), successfully enqueue into the still-connected channel, then wait forever once rx drops.

please make shutdown disconnect the receiver before releasing waiters and add a deterministic regression covering registration during teardown.

@KeroZelvin
KeroZelvin force-pushed the fix/linux-x11-overlay-hotplug branch from 7b5f623 to 2b86978 Compare July 19, 2026 22:08
@KeroZelvin

Copy link
Copy Markdown
Contributor Author

@injaneity Addressed the lifecycle race in rebased head 2b869788263cbcef0f25cf98c76e1004dd57db2e.

The branch is now rebased onto current upstream/main@15a3738d5cd363e9219b20615224e8fa31eba73d. The upstream-only commits did not touch platform-linux/src/overlay.rs and the rebase was clean.

Teardown now transfers ownership of the command receiver into X11OverlayThreadCleanup. Its shared teardown method:

  1. drops/disconnects the receiver,
  2. exposes a deterministic interleaving point,
  3. marks X11 render state unavailable,
  4. releases all arrival waiters.

This closes both sides of the race: registrations before waiter release are swept, while registrations afterward cannot enqueue and cancel their own waiter. The regression inserts registration exactly after disconnect and before release; inside that hook it first proves try_send already returns Disconnected, then verifies the newly registered oneshot is closed by the subsequent release.

Validation on the rebased tree: rustfmt; platform-linux 151/151; cursor-overlay 12/12; workspace no-run compile; cua-driver build; git diff --check. Independent review found and prompted the stronger in-hook disconnect assertion; the strengthened exact diff was re-reviewed with No actionable findings.

@injaneity

injaneity commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

thanks for your effort! one remaining follow-up from my end:

please gate the periodic z-order reassertion on next_z_order_tick_needed. the new 50 ms x11 event heartbeat wakes hidden overlays indefinitely, but the unconditional last_ztick.elapsed() >= z_order_interval branch still calls configure_window and flushes roughly every 100 ms even when no cursor is visible. that contradicts the adjacent comment and adds permanent idle x11 traffic.

using next_z_order_tick_needed && last_ztick.elapsed() >= z_order_interval preserves command, display-change, and pin-change reassertions while preventing hidden maintenance wakes from raising the overlay. please add a regression covering a hidden heartbeat that services events without requesting a z-order tick.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Linux/X11 overlay can become full-root black after display hotplug despite #2331

2 participants