diff options
| author | srdusr <[email protected]> | 2025-08-09 00:57:00 +0200 |
|---|---|---|
| committer | srdusr <[email protected]> | 2025-08-09 00:57:00 +0200 |
| commit | 61d01bb79c6882c950f45a1176ba1f30fbbd4824 (patch) | |
| tree | eb2a24c7f301f74f987f954b62d768c9cb789fef /docs | |
| parent | 9e811d2f2f1037ba4ec08ab0e05b350e6cf69e66 (diff) | |
| download | srdwm-61d01bb79c6882c950f45a1176ba1f30fbbd4824.tar.gz srdwm-61d01bb79c6882c950f45a1176ba1f30fbbd4824.zip | |
Fix interactive-resize border/shadow lag without the OOB risk that sank the first attempt
effective_frame_of now returns the live drag target while a window is
being interactively resized (same change as the reverted first attempt),
but two things make it safe this time instead of reintroducing the
out-of-bounds texture sample that reversion was for:
- Every src crop rect built from a window's frame width in udev/render.rs
and winit/render.rs (titlebar, top border strip, bottom border strip)
is now clamped against DecorationSignature's own recorded width/
border_width - the bitmap's actual last-built size - before reaching
MemoryRenderBufferRenderElement::from_buffer, which does not itself
validate src against the real texture size. This is a structural floor
independent of timing, not a repeat of the previous unsafe approach.
- handle_pointer_position now calls redraw_decoration_buffer once per
resize motion event (throttled to 60Hz via a new
CompState::resize_redraw_at), closing the lag at its source instead of
only catching up on the next real client commit. This also fixes the
shadow bitmap's identical commit-vs-live-position gap for free, since
redraw_decoration_buffer rebuilds all three bitmaps together.
Updates the TODO.md entry for this bug with the full before/after.
Diffstat (limited to 'docs')
| -rw-r--r-- | docs/TODO.md | 12 |
1 files changed, 9 insertions, 3 deletions
diff --git a/docs/TODO.md b/docs/TODO.md index 139aa2a..06b3885 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -19,15 +19,21 @@ Found by a full-pipeline audit requested directly ("please do a deep dive into o Currently latent, not reachable today: nothing in `srd set`'s current command list can change any of the three live, so this was a landmine for whenever live theme reload or a new `srd set` grows to cover them, not a bug with a live repro today. Fixed anyway, matching the existing fields' own precedent, rather than left for a future session to rediscover the same gap. Test suite run pending; built. -## Real bug, root-caused, first fix attempt reverted as unsafe - still open: a decorated/undecorated window's border, shadow, and resize hit-test all lag a stale, previous-frame size for the whole duration of an interactive resize (2026-08-24) +## Real bug, root-caused, first fix attempt reverted as unsafe, now fixed properly (2026-08-25) Reported live: "notice how in firefox window the borders are misaligned, especially when i resize the window." Root cause in `state/geometry.rs`'s `effective_frame_of` - the function every border/shadow/occlusion/resize-hit-test call site uses to correct a window's drawn rect against what the client *actually* committed, rather than what this compositor merely requested (built earlier this session to fix a real, different bug: a terminal's own cell-quantized size leaving a gap between its content and the border). That correction is applied *unconditionally*, including while the window is being actively, interactively resized - but during a live drag, `geom` (the compositor's own live target) updates on every pointer-motion event, while the client's last real commit is however far behind that a full relayout pass takes. For a heavy client (Firefox, concretely - a plain terminal reflows near-instantly and never showed this visibly) that lag is real and continuous, so the border/shadow keeps drawing at a stale size for the *entire* drag, worst at exactly the edge/corner being dragged, while the content underneath (never routed through this function at all) renders whatever the client has actually gotten around to committing. **First fix attempt (skip the correction entirely while `WindowManager::resizing_window() == Some(id)`) was reverted in the same session it landed, before ever being confirmed live.** A full-pipeline audit (prompted by the user directly asking for one after this fix still didn't resolve their report) traced the actual consequence: the titlebar bitmap and the top/bottom border strip's own rounded-corner bitmap (both built by `redraw_decoration_buffer`, itself only called on a real client *commit*, not on every resize step) are sampled in `udev/render.rs`/`winit/render.rs` via a `src` crop rectangle sized from this same function's return value. Making this function return the *live* drag target while the underlying bitmap was still sized for the *last commit* means that crop can exceed the bitmap's real stored dimensions - `MemoryRenderBufferRenderElement::from_buffer` (smithay) does not validate `src` against the texture's real size, so an oversized crop is an out-of-bounds texture sample (stretched/repeated/garbage pixels, not a clean error), not just a stale-lag cosmetic issue. Almost shipped a worse bug in place of the one being fixed; caught by tracing the actual downstream consumers before installing, not by testing. -`effective_frame_of` is back to its original, unconditional behavior (safe, internally consistent, but still one-or-more-frames-stale during a fast resize). The audit's own recommendation, not yet done: fix this at the *source* - have `redraw_decoration_buffer` rebuild on every `update_resize` step (not just on commit) so the bitmap itself never falls behind what the live frame asks for - rather than patching the render-time read again. Real, bounded scope (titlebar re-rasterization involves font glyph rendering per character, so calling this on every pointer-motion tick during a fast drag needs its own cost check, possibly a throttle), not attempted this session given the demonstrated risk of a hasty fix in this exact area. +**Second fix, three parts, addressing the audit's own recommendation ("fix at the source") plus the specific failure mode that sank the first attempt:** -Same-audit related finding, lower priority: the shadow bitmap has the identical commit-vs-live-position gap (`shadow_rect(frame)`'s position is live, the shadow bitmap's own size is commit-gated) - but shadow is pushed with `src: None`, which smithay's own `from_buffer` resolves to the buffer's *real* native size rather than a crop, so this one is NOT at risk of out-of-bounds sampling, only the same soft cosmetic detachment the border/titlebar bug had before the corruption risk was found. Same eventual fix (rebuild on every resize step) would close this too. +1. `effective_frame_of` returns the live drag target directly, but *only* while `WindowManager::resizing_window() == Some(id)` - the same change the first attempt made. On its own this still carries the first attempt's out-of-bounds risk; parts 2 and 3 are what make it safe. +2. Every `src` crop rectangle built from a window's frame width in `udev/render.rs` and `winit/render.rs` (titlebar, top border strip, bottom border strip - six call sites total, three per backend) is now clamped against `DecorationSignature`'s own recorded `width`/`border_width`, the exact size the sampled buffer was actually last built at, before being handed to `from_buffer`. This is a structural floor independent of timing: even if the bitmap rebuild below is ever late, skipped, or throttled away, the crop can no longer exceed what the buffer actually contains, so the out-of-bounds sample that killed the first attempt is no longer reachable from this path at all. +3. `redraw_decoration_buffer` now also runs from `input/pointer.rs`'s `handle_pointer_position`, once per pointer-motion event that finds a resize in progress - closing the gap at its source, same as the audit recommended, instead of only on the next real client commit. Throttled to `RESIZE_REDRAW_INTERVAL` (60Hz) against a new `CompState::resize_redraw_at` timestamp, since font-glyph rasterization for the titlebar text is real per-character cost that a fast mouse/touchpad can otherwise ask for far more often than any visible difference would justify. + +Built, `cargo test --workspace` (106 wayland-crate tests plus the rest of the suite) and `cargo clippy --workspace --all-targets` all clean. Part 2's clamp is the actual safety net; parts 1 and 3 are what make the fix visible (live geometry, live redraw) rather than a no-op. Installed and pending a live restart to confirm the border/shadow no longer visibly lags a fast Firefox resize. + +Same-audit related finding, lower priority, closed by the same fix: the shadow bitmap had the identical commit-vs-live-position gap (`shadow_rect(frame)`'s position is live, the shadow bitmap's own size is commit-gated) - but shadow is pushed with `src: None`, which smithay's own `from_buffer` resolves to the buffer's *real* native size rather than a crop, so it was never at risk of the out-of-bounds sampling the border/titlebar bug was, only the same soft cosmetic detachment. `redraw_decoration_buffer` rebuilds the shadow bitmap in the same call as the titlebar and border strips, so part 3 of the fix above (calling it from every resize motion event) closes this gap too, with no separate change needed. ## Reported by a peer session (aegis), not reproducible as of 2026-08-25 - likely already fixed: `srd clients`'s own `focused` field going stale after a `zwlr_foreign_toplevel_handle_v1.activate`-driven focus change (2026-08-24) |