srdusr
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorsrdusr <[email protected]>2025-02-17 21:12:00 +0200
committersrdusr <[email protected]>2025-02-17 21:12:00 +0200
commitf7c6e9c607742ed373752aadf5761e99ec3eb64a (patch)
tree433e666d8082821ce70fd97a2851f261fed864b8
parent413daa7ba2ea0ebd1424c024fd0566423aaea3f8 (diff)
downloadsrdwm-f7c6e9c607742ed373752aadf5761e99ec3eb64a.tar.gz
srdwm-f7c6e9c607742ed373752aadf5761e99ec3eb64a.zip
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.
-rw-r--r--crates/config/src/engine/general.rs32
-rw-r--r--crates/wayland/src/udev/drm.rs1
-rw-r--r--crates/wayland/src/udev/mod.rs24
-rw-r--r--crates/wayland/src/udev/render.rs58
-rw-r--r--crates/wayland/src/udev/session.rs1
-rw-r--r--crates/wayland/src/winit/render.rs70
6 files changed, 163 insertions, 23 deletions
diff --git a/crates/config/src/engine/general.rs b/crates/config/src/engine/general.rs
index 98cfebe..a50890f 100644
--- a/crates/config/src/engine/general.rs
+++ b/crates/config/src/engine/general.rs
@@ -194,14 +194,40 @@ impl Engine {
})?)
}
+ /// `srd.load("keybindings")`/`"themes"`/`"rules"`/`"startup"`: each is
+ /// its own file, its own logical concern, and - deliberately, since
+ /// this function catches its own execution error rather than letting
+ /// `?` propagate one - its own failure domain. A `mlua::Error` from
+ /// one module used to unwind straight out through this function and
+ /// back into whatever was running `init.lua` itself, aborting every
+ /// statement after that `srd.load` call, including every *other*
+ /// `srd.load` - a typo in `rules.lua` silently took `startup.lua`
+ /// (autostart) down with it, and there was no way from the config
+ /// author's side to prevent that, short of never making a mistake.
+ /// Reproduced live in the worse but related case (the error was in
+ /// `init.lua` itself, above every `srd.load` call, which this
+ /// function alone can't isolate against): a session that started with
+ /// nothing but a bare cursor, zero autostart, zero keybindings, no
+ /// error visible anywhere except one `WARN` line in a multi-hundred-
+ /// megabyte log file. A module failing now still leaves the user
+ /// without whatever that module would have set up, logged clearly at
+ /// `error` level with the module name and the file path - but
+ /// everything *else* `init.lua` goes on to load still does.
pub(super) fn fn_load(&self) -> Result<mlua::Function<'_>> {
let state = self.state.clone();
Ok(self.lua.create_function(move |lua, module: String| {
let dir = state.borrow().config_dir.clone();
let path = dir.join(format!("{module}.lua"));
- let src = std::fs::read_to_string(&path)
- .map_err(|e| mlua::Error::RuntimeError(format!("srd.load('{module}'): {e} ({})", path.display())))?;
- lua.load(&src).set_name(path.to_string_lossy().as_ref()).exec()?;
+ let src = match std::fs::read_to_string(&path) {
+ Ok(src) => src,
+ Err(e) => {
+ log::error!("srd.load('{module}'): {e} ({}) - this module did not load, but the rest of init.lua still will", path.display());
+ return Ok(());
+ }
+ };
+ if let Err(e) = lua.load(&src).set_name(path.to_string_lossy().as_ref()).exec() {
+ log::error!("srd.load('{module}'): {e} - this module did not finish loading, but the rest of init.lua still will");
+ }
Ok(())
})?)
}
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<i32, Logical>,
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<Instant>,
}
/// 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();
}
diff --git a/crates/wayland/src/winit/render.rs b/crates/wayland/src/winit/render.rs
index ec4fd69..4d14649 100644
--- a/crates/wayland/src/winit/render.rs
+++ b/crates/wayland/src/winit/render.rs
@@ -19,7 +19,41 @@ impl WaylandPlatform {
layer_map_for_output(&self.output).arrange();
}
- let age = self.backend.buffer_age().unwrap_or(0);
+ // Always `0` ("contents undefined, damage everything"), not
+ // `self.backend.buffer_age()` - deliberately never trusted here,
+ // confirmed live to actually be the source of a real corruption
+ // bug, not just a missed optimization. `age` tells
+ // `damage_tracker.render_output` how many past frames' worth of
+ // damage history it can trust the *current* target buffer to
+ // already reflect, so it can skip repainting regions nothing has
+ // touched since. That guarantee depends on the platform's own
+ // buffer-age report being an honest account of this exact backing
+ // buffer's real history - which this backend cannot get from a
+ // nested `winit`/EGL surface hosted inside another Wayland
+ // session: reproduced live, stable, not a one-frame race --
+ // raising one floating window (Firefox) above another (a plain
+ // terminal) left a rectangular patch of the terminal's own old
+ // pixels sitting untouched at one edge of Firefox's new, larger,
+ // fully-opaque topmost window, persisting indefinitely across
+ // many further frames with no damage anywhere near it to explain
+ // clearing it. Forcing `age = 0` here (full redraw, every frame)
+ // made the corruption disappear completely and immediately, and
+ // nothing else about the scene changed - narrowing the cause to
+ // exactly this value being wrong, not a geometry, occlusion, or
+ // z-order bug elsewhere in this file (all independently verified
+ // correct against the same repro: the window's own border and
+ // content elements were logged as computed with the right full
+ // geometry and zero occluders on every single one of the frames
+ // that still rendered the stale patch). Real hardware's udev
+ // backend is not affected - it never queries a platform buffer
+ // age at all, only its own hand-tracked `UdevHead::ages`, advanced
+ // deterministically from this codebase's own strict two-buffer
+ // page-flip alternation, not borrowed trust in an outer
+ // compositor's EGL implementation. This backend exists for nested
+ // development/testing, not as the real compositor, so paying a
+ // full software-composited redraw every frame here is the safe
+ // trade against silently wrong pixels persisting on screen.
+ let age = 0;
let (renderer, mut framebuffer) = self.backend.bind().map_err(err)?;
// Locked: srdwm's own native lock UI, or an external locker's
@@ -147,15 +181,35 @@ impl WaylandPlatform {
// (reported live as the border "not flush" with the window
// during an animated maximize/fullscreen/open-slide transition).
let geom = self.state.window_anims.get(&id).map(crate::state::WindowAnim::current_rect).unwrap_or(w.geometry);
- // Same reasoning as udev/render.rs's matching push: positioned from
- // `geom`, not `w.geometry`, and not fragment-clipped against
- // `occluders` - see that comment.
+ // Same reasoning as udev/render.rs's matching push: positioned
+ // from `geom`, not `w.geometry`. Fragment-clipped against
+ // `occluders` now, same as the titlebar/border below - a
+ // shadow used to draw in full regardless of what was stacked
+ // in front of it (its own doc comment argued the low opacity
+ // made that read as "a soft edge, not the hard-line bleed-
+ // through that made the titlebar/border need it"), but that
+ // reasoning only holds along a shadow's straight edges: the
+ // corner regions use Chebyshev (square-ring), not radial,
+ // falloff (see `shadow_bitmap`'s own doc comment), so a
+ // shadow's corner is a hard-edged square block at up to
+ // `SHADOW_MAX_ALPHA` (~35%) opacity, not a soft radial
+ // 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.state.shadow_buffers.get(&id) {
let rect = decoration::shadow_rect(geom);
- let pos = (rect.x as f64, rect.y as f64);
- match MemoryRenderBufferRenderElement::from_buffer(renderer, pos, shadow, None, None, None, Kind::Unspecified) {
- Ok(elem) => custom_elements.push(crate::rounded_corners::WinitElement::Base(crate::elements::OverlayElement::Memory(elem))),
- Err(e) => log::warn!("failed to import shadow buffer for window {id}: {e}"),
+ for fragment in crate::elements::visible_border_fragments(rect, &occluders) {
+ let pos = (fragment.x as f64, fragment.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(renderer, pos, shadow, None, Some(src), None, Kind::Unspecified) {
+ Ok(elem) => custom_elements.push(crate::rounded_corners::WinitElement::Base(crate::elements::OverlayElement::Memory(elem))),
+ Err(e) => log::warn!("failed to import shadow buffer for window {id}: {e}"),
+ }
}
}
if let Some(deco) = self.state.decorations.get(&id) {