From f7c6e9c607742ed373752aadf5761e99ec3eb64a Mon Sep 17 00:00:00 2001 From: srdusr <99972264+srdusr@users.noreply.github.com> Date: Mon, 17 Feb 2025 21:12:00 +0200 Subject: Checkpoint: today's fixes before reconciling with the rust-rewrite worktree Fixes a live-reproduced VT-switch busy loop (failed page_flip retried with no backoff), shadow rendering bleeding onto occluding windows unclipped, and a winit-backend buffer-age correctness bug that left stale cross-window pixels on screen. Committing before merging in the much larger uncommitted rust-rewrite worktree, which independently touches several of the same files - this is the pre-merge baseline to diff against, not a claim that these are the final versions of these fixes. --- crates/wayland/src/udev/drm.rs | 1 + crates/wayland/src/udev/mod.rs | 24 ++++++++++++++++ crates/wayland/src/udev/render.rs | 58 ++++++++++++++++++++++++++++++-------- crates/wayland/src/udev/session.rs | 1 + 4 files changed, 72 insertions(+), 12 deletions(-) (limited to 'crates/wayland/src/udev') diff --git a/crates/wayland/src/udev/drm.rs b/crates/wayland/src/udev/drm.rs index fd06619..667747b 100644 --- a/crates/wayland/src/udev/drm.rs +++ b/crates/wayland/src/udev/drm.rs @@ -59,6 +59,7 @@ pub(crate) fn bring_up_head( ages: [0, 0], location, size: (width, height), + flip_retry_after: None, }; Ok((head, crate::state::OutputEntry { output, location })) } diff --git a/crates/wayland/src/udev/mod.rs b/crates/wayland/src/udev/mod.rs index 501e3fc..9a6bf49 100644 --- a/crates/wayland/src/udev/mod.rs +++ b/crates/wayland/src/udev/mod.rs @@ -149,6 +149,30 @@ pub(crate) struct UdevHead { /// Origin of this head in the global coordinate space. pub(crate) location: Point, pub(crate) size: (i32, i32), + /// Set when [`UdevHead::copy_and_flip`] fails; no new flip is attempted + /// for this head again until this deadline passes. + /// + /// Without this, a failed `page_flip` (real and reproduced live: the + /// kernel returns `EBUSY`/"device or resource busy" for a brief window + /// right after a VT-switch resume's `set_crtc` reasserts the mode, + /// before that commit has actually settled) left `flip_pending` still + /// `false` - `copy_and_flip`'s early-return `?` on the failing + /// `page_flip` call skips the line just after it that would have set + /// `flip_pending = true`, so nothing ever marked this head "busy". The + /// next call to `render_udev_frame` (every ~16ms, or sooner -- + /// `event_loop.dispatch`'s timeout is only an upper bound) saw the + /// exact same head still "ready" and every prior damage still pending, + /// tried the exact same flip again, failed the exact same way, forever + /// - a true busy loop with no backoff at all, not merely a missed + /// optimization. Confirmed live from a real session log: tens of + /// thousands of consecutive `page flip failed: Device or resource + /// busy` lines a few *microseconds* apart, the compositor's one thread + /// spinning flat out on nothing else, which is what actually explains + /// the user's report of losing pointer input and the ability to + /// switch VTs at all after switching away and back once - not a + /// separate input bug, this loop simply never yielded the CPU back to + /// anything else, libinput's own event processing included. + pub(crate) flip_retry_after: Option, } /// Everything the DRM/udev backend needs that the nested winit backend diff --git a/crates/wayland/src/udev/render.rs b/crates/wayland/src/udev/render.rs index 6c9c7d2..0647e26 100644 --- a/crates/wayland/src/udev/render.rs +++ b/crates/wayland/src/udev/render.rs @@ -66,11 +66,12 @@ impl CompState { self.screencopy_pending.extend(captures); return; } + let now = Instant::now(); let ready: Vec<(usize, Output)> = udev .heads .iter() .enumerate() - .filter(|(_, h)| !h.flip_pending) + .filter(|(_, h)| !h.flip_pending && h.flip_retry_after.is_none_or(|t| now >= t)) .map(|(i, h)| (i, h.output.clone())) .collect(); // Kept separately from `presented` below: layer-shell surfaces @@ -208,18 +209,35 @@ impl CompState { // else here - not `w.geometry` - for the identical // reason: a shadow that stayed at the pre-tween rect // while the window slid past it would look exactly as - // detached as the border did before that fix. Not - // fragment-clipped against `occluders` like the titlebar/ - // border below: at `SHADOW_MAX_ALPHA`'s low opacity, a - // shadow bleeding slightly onto a window stacked in front - // of this one reads as a soft edge, not the hard-line - // bleed-through that made the titlebar/border need it. + // detached as the border did before that fix. + // + // Fragment-clipped against `occluders` now, same as the + // titlebar/border below - this used to skip that on the + // reasoning that `SHADOW_MAX_ALPHA`'s low opacity would + // read as a soft edge, not the hard-line bleed-through + // that made the titlebar/border need it. True along a + // shadow's straight edges, false at its corners: + // `shadow_bitmap` falls off by Chebyshev (square-ring) + // distance, not radial, so each corner is a hard-edged + // square block at up to ~35% opacity, not a soft + // vignette - reported live as a small dark rectangular + // patch sitting on top of whatever window a floating/ + // cascaded window's own corner happened to overlap, + // most visible exactly where two windows' corners + // nearly meet, which this compositor's default cascade + // placement does constantly. if let Some(shadow) = self.shadow_buffers.get(&id) { let rect = decoration::shadow_rect(geom); - let pos = ((rect.x - origin.x) as f64, (rect.y - origin.y) as f64); - match MemoryRenderBufferRenderElement::from_buffer(&mut udev.renderer, pos, shadow, None, None, None, Kind::Unspecified) { - Ok(elem) => custom_elements.push(crate::elements::OverlayElement::Memory(elem)), - Err(e) => log::warn!("udev: failed to import shadow buffer: {e}"), + for fragment in crate::elements::visible_border_fragments(rect, &occluders) { + let pos = ((fragment.x - origin.x) as f64, (fragment.y - origin.y) as f64); + let src = Rectangle::new( + Point::from(((fragment.x - rect.x) as f64, (fragment.y - rect.y) as f64)), + Size::from((fragment.width as f64, fragment.height as f64)), + ); + match MemoryRenderBufferRenderElement::from_buffer(&mut udev.renderer, pos, shadow, None, Some(src), None, Kind::Unspecified) { + Ok(elem) => custom_elements.push(crate::elements::OverlayElement::Memory(elem)), + Err(e) => log::warn!("udev: failed to import shadow buffer: {e}"), + } } } if let Some(deco) = self.decorations.get(&id) { @@ -500,9 +518,25 @@ impl CompState { if has_damage { let head = &mut udev.heads[index]; if let Err(e) = head.copy_and_flip(&udev.card, back) { - log::error!("udev: page flip failed: {e}"); + // Backed off, not retried on the very next poll tick -- + // see `UdevHead::flip_retry_after`'s own doc comment for + // the real, live-reproduced incident this prevents: a + // failing flip (confirmed live as `EBUSY` right after a + // VT-switch resume, while the kernel's own `set_crtc` + // commit was still settling) used to be retried + // immediately, forever, since nothing else gated + // `ready` on anything but `flip_pending` - which a + // failed `page_flip` call never sets. A fixed, short + // cooldown is enough to ride out that kind of transient + // kernel-side race without needing to distinguish it + // from a real, permanent failure - either way, hammering + // the same doomed `page_flip` call in a tight loop with + // no backoff at all was never the right response. + head.flip_retry_after = Some(Instant::now() + Duration::from_millis(200)); + log::error!("udev: page flip failed: {e} - retrying in 200ms"); continue; } + head.flip_retry_after = None; // This buffer is now fully up to date. It won't be rendered // into again until the *other* slot has also been presented // once (strict two-buffer alternation), so by then it will diff --git a/crates/wayland/src/udev/session.rs b/crates/wayland/src/udev/session.rs index 4dd6db0..42805b5 100644 --- a/crates/wayland/src/udev/session.rs +++ b/crates/wayland/src/udev/session.rs @@ -99,6 +99,7 @@ pub(crate) fn register_session_notifier(handle: &LoopHandle<'static, CompState>, // scanned out something else entirely in between). head.flip_pending = false; head.ages = [0, 0]; + head.flip_retry_after = None; } data.render_udev_frame(); } -- cgit v1.2.3