srdusr
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorsrdusr <[email protected]>2025-08-18 23:44:00 +0200
committersrdusr <[email protected]>2025-08-18 23:44:00 +0200
commitd2af06d00e1b91bf9953e8c86eb30ab3ad9ee829 (patch)
treebf6f73d4c89263f0be842429c2a597ff8e34fa0c
parentb8376bd631a72f7484c24663699a7d4c813f600e (diff)
downloadsrdwm-d2af06d00e1b91bf9953e8c86eb30ab3ad9ee829.tar.gz
srdwm-d2af06d00e1b91bf9953e8c86eb30ab3ad9ee829.zip
Fix layer-shell surfaces unclickable/unpainted on a fractionally-scaled output
Reported live, in stages: general input sluggishness, then specifically dock/bar buttons not responding on the secondary monitor. Root-caused jointly with a peer session (dotfiles-16), who independently instrumented AGS itself (both bar and dock report correct visible/realized/revealed state - the client is asking for the right thing) and srdwm's own layer_hit_test log (the dock received zero hits across ~40 minutes while the same output's wallpaper and bar took hundreds). Confirmed against smithay 0.7.0's own source (desktop/wayland/layer.rs:: arrange): LayerMap::arrange() divides the output's physical mode by its own scale before arranging layers, so LayerMap::layer_geometry() is logical, not physical. Two call sites used it as physical, this compositor's convention everywhere else: - input/layers.rs::layer_surface_under_layers compared the physical pointer position directly against logical layer geometry. On a sub-1.0 scale output, logical space is larger than physical, so a bottom-anchored dock's rect sat entirely past the pointer's reachable range - permanently unclickable. A top-anchored bar only lost its own right-hand end, which is what made this look like "the dock is broken" rather than a scale bug affecting every layer surface there. - elements.rs::output_layer_elements pushed the same logical position straight into the physical framebuffer - for the dock, past the bottom edge entirely, painting nothing. Both fixed the same way udev/platform.rs::monitors() and udev/outputs.rs already fix the identical unit mismatch for usable-area computation (existing precedent, not a new technique): multiply by output. current_scale().fractional_scale(), rounding to the nearest physical pixel, before use. Also removed a temporary per-pointer-motion-event diagnostic log in layer_hit_test, still live from an earlier debugging session and explicitly marked for removal but never removed - a real, measurable cost on the hot input path, likely the direct cause of the separately reported general slowness. Full workspace test suite and clippy clean.
-rw-r--r--crates/wayland/src/elements.rs19
-rw-r--r--crates/wayland/src/input/layers.rs67
-rw-r--r--docs/TODO.md15
3 files changed, 72 insertions, 29 deletions
diff --git a/crates/wayland/src/elements.rs b/crates/wayland/src/elements.rs
index de50ccf..b5be5a8 100644
--- a/crates/wayland/src/elements.rs
+++ b/crates/wayland/src/elements.rs
@@ -256,6 +256,22 @@ where
R: Renderer + ImportAll + ImportMem,
R::TextureId: Clone + Send + 'static,
{
+ // `LayerMap::layer_geometry` is logical (confirmed against smithay
+ // 0.7.0's own source, `desktop/wayland/layer.rs::arrange`: it divides
+ // the output's physical mode by its own scale before arranging), while
+ // every position this function's own caller pushes an element at is
+ // physical - this compositor's convention throughout. Left
+ // unconverted, a layer surface on any output with a non-1.0 scale
+ // renders at the wrong physical position; for a bottom/right-anchored
+ // one on a *sub*-1.0 scale (this output shrinks logical space *larger*
+ // than physical, so a position derived from it overshoots), that was
+ // enough to push it fully past the real framebuffer edge - confirmed
+ // live (independently measured by a peer session) as a bottom-anchored
+ // dock never painted at all on a 0.843-scale output, while a
+ // top-anchored bar on the same output still rendered (it starts at
+ // logical/physical `0` either way) but with its own far edge
+ // increasingly wrong the further right it drew.
+ let scale = output.current_scale().fractional_scale();
let map = layer_map_for_output(output);
let mut elements = Vec::new();
for layer in map.layers().rev() {
@@ -263,7 +279,8 @@ where
continue;
}
let Some(geo) = map.layer_geometry(layer) else { continue };
- elements.extend(surface_content_elements(renderer, layer.wl_surface(), (geo.loc.x, geo.loc.y), 1.0));
+ let pos = ((geo.loc.x as f64 * scale).round() as i32, (geo.loc.y as f64 * scale).round() as i32);
+ elements.extend(surface_content_elements(renderer, layer.wl_surface(), pos, 1.0));
}
elements
}
diff --git a/crates/wayland/src/input/layers.rs b/crates/wayland/src/input/layers.rs
index 718d83f..b429fbb 100644
--- a/crates/wayland/src/input/layers.rs
+++ b/crates/wayland/src/input/layers.rs
@@ -5,7 +5,6 @@
use smithay::desktop::{layer_map_for_output, WindowSurfaceType};
use smithay::output::Output;
use smithay::reexports::wayland_server::protocol::wl_surface::WlSurface;
-use smithay::reexports::wayland_server::Resource as _;
use smithay::utils::{Logical, Point};
use smithay::wayland::compositor::with_states;
use smithay::wayland::shell::wlr_layer::{Anchor, ExclusiveZone, Layer, LayerSurfaceCachedState};
@@ -41,7 +40,26 @@ use crate::state::CompState;
pub(super) fn layer_surface_under_layers(state: &CompState, pos: Point<f64, Logical>, layers: [Layer; 2]) -> Option<(WlSurface, Point<i32, Logical>)> {
let entry = state.output_at(pos)?;
let origin = entry.location;
- let local = pos - origin.to_f64();
+ // `pos`/`origin` are physical (this compositor's own convention
+ // throughout, including `OutputEntry::location`'s own `Logical`-typed-
+ // but-physical-valued field - see its own doc comment), but `LayerMap
+ // ::layer_geometry` below is not: confirmed against smithay 0.7.0's
+ // own source (`desktop/wayland/layer.rs::arrange`), it divides the
+ // output's physical mode by its own scale before arranging layers, so
+ // it - and `LayerSurface::surface_under`'s own coordinate space,
+ // which reads surface-local points in that same system - is genuinely
+ // logical. Comparing a physical point against that without converting
+ // silently clips off however much a non-1.0 scale shrinks the surface
+ // by: confirmed live (with a peer session's own independent
+ // measurement) on a 0.843-scale output, a bottom-anchored dock sits
+ // entirely past the physical pointer's own reachable range - always
+ // unclickable, not just at the edges - while a top-anchored bar on
+ // the same output only loses its own right-hand end, which is what
+ // made this look like "the dock is broken" rather than a scale bug
+ // affecting every layer surface on that output.
+ let scale = entry.output.current_scale().fractional_scale();
+ let local_physical = pos - origin.to_f64();
+ let local: Point<f64, Logical> = (local_physical.x / scale, local_physical.y / scale).into();
let map = layer_map_for_output(&entry.output);
for layer_kind in layers {
// Not `map.layer_under(layer_kind, local)` - that hands back only
@@ -66,33 +84,26 @@ pub(super) fn layer_surface_under_layers(state: &CompState, pos: Point<f64, Logi
if !geo.to_f64().contains(local) {
continue;
}
- // Temporary: verifying the `layer_surfaces_shown_once` fix
- // (state/layers.rs) actually stops a reused `wl_surface`'s
- // stale layer-shell entry from outliving its role destroy --
- // live-reproduced this session as a full-monitor click-catcher
- // popup whose hit-tested geometry came back wider than the
- // real output after several open/close cycles. Remove once a
- // restart confirms the geometry stays sane across repeated
- // popup toggles.
- let local_in_surface = local - geo.loc.to_f64();
- // `None` here means "no region ever committed" - per-protocol
- // that means the *whole* surface is input-sensitive, not that
- // nothing is, so it is its own distinct, meaningful answer from
- // `Some([])` (a region was committed and it is empty).
- let region_dump = with_states(layer.wl_surface(), |states| {
- states.cached_state.get::<smithay::wayland::compositor::SurfaceAttributes>().current().input_region.as_ref().map(|r| r.rects.clone())
- });
- log::info!(
- "layer_hit_test: layer={:?} namespace={:?} surface={:?} geo={:?} local_in_surface={:?} input_region={:?}",
- layer_kind,
- layer.namespace(),
- layer.wl_surface().id(),
- geo,
- local_in_surface,
- region_dump
- );
+ // The `layer_surfaces_shown_once` fix (state/layers.rs) stops a
+ // reused `wl_surface`'s stale layer-shell entry from outliving
+ // its role destroy - previously live-reproduced as a full-
+ // monitor click-catcher popup whose hit-tested geometry came
+ // back wider than the real output after several open/close
+ // cycles. A per-motion-event diagnostic log verifying this
+ // used to sit here; removed after it was found to be a real,
+ // significant cost on the hot pointer-motion path (querying
+ // compositor cached state and formatting/writing a log line
+ // on every single pixel of every mouse move), reported live as
+ // general input sluggishness, not just log noise.
if let Some((surface, surface_loc)) = layer.surface_under(local - geo.loc.to_f64(), WindowSurfaceType::ALL) {
- return Some((surface, origin + geo.loc + surface_loc));
+ // `geo.loc + surface_loc` is still logical (same space as
+ // `local` above) - scaled back to physical here so the
+ // returned point matches every caller's own expected space
+ // (`pos`'s own convention, and `origin`'s, already
+ // physical).
+ let logical = geo.loc + surface_loc;
+ let physical: Point<i32, Logical> = ((logical.x as f64 * scale).round() as i32, (logical.y as f64 * scale).round() as i32).into();
+ return Some((surface, origin + physical));
}
}
}
diff --git a/docs/TODO.md b/docs/TODO.md
index d2d8a49..257dd56 100644
--- a/docs/TODO.md
+++ b/docs/TODO.md
@@ -13,6 +13,21 @@ that has the full story. Keep this list current as items close or open;
update the source doc's own entry too, don't let this drift into a
second stale copy the way `PANEL_SUPPORT_TODO.md` did.
+## Real bug, root-caused and fixed, jointly with a peer session (dotfiles-16): layer-shell surfaces on a fractionally-scaled output were unclickable and, for a bottom/right-anchored one, never painted at all (2026-08-26)
+
+Reported live in stages: "AGS isn't even working fast/seems weird now on this monitor even", "dock and AGS buttons aren't working in the other monitor". The peer session independently instrumented AGS itself (both bar and dock report `win_visible=true realized=true reveal=true overlapped=false` on the affected output - the client is asking for the right thing) and srdwm's own `layer_hit_test` log, and found the dock received zero hits across ~40 minutes while the same output's wallpaper and bar took hundreds - full findings in their own `docs/PANEL_SUPPORT_TODO.md`/`SESSION_HANDOFF.md` in the `rust-rewrite` worktree.
+
+Root cause, confirmed against smithay 0.7.0's own source (`desktop/wayland/layer.rs::arrange`): `LayerMap::arrange()` divides the output's *physical* mode size by its own scale before arranging layers, so `LayerMap::layer_geometry()` - and `LayerSurface::surface_under`'s own coordinate space - is genuinely logical, not physical. Two call sites used it as if it were physical, this compositor's own convention everywhere else:
+
+- `input/layers.rs::layer_surface_under_layers` compared the physical pointer position directly against logical layer geometry with no conversion. On a sub-1.0 scale (this machine's `HDMI-A-1`, ~0.843), logical space is *larger* than physical, so a bottom-anchored dock's logical rect sat entirely past the physical pointer's own reachable range - permanently unclickable. A top-anchored bar on the same output only lost its own right-hand end (it starts at `0` in both spaces either way), which is what made this look like "the dock is broken" rather than a scale bug affecting every layer surface on that output.
+- `elements.rs::output_layer_elements` pushed each layer's render position straight from that same logical geometry into the physical framebuffer - for the dock, past the bottom edge entirely, painting nothing.
+
+Both fixed the same way `udev/platform.rs::monitors()` and `udev/outputs.rs`'s own `non_exclusive_zone()` handling already fix the identical unit mismatch for usable-area computation (confirmed working precedent already in this codebase, not a new technique): multiply the logical value by `output.current_scale().fractional_scale()`, rounding to the nearest physical pixel, before using it as a physical position.
+
+Separately, while investigating: a temporary per-pointer-motion-event diagnostic log in `layer_hit_test` (querying compositor cached state and formatting/writing a log line on every single pixel of every mouse move) was still live from an earlier debugging session, explicitly marked "Temporary... remove once a restart confirms" but never removed. Real, measurable cost on the hot input path - removed; likely the direct cause of the separately reported "seems weird/slow" on top of the click-dead dock.
+
+Built, full workspace test suite and clippy clean; installed, pending a live restart to confirm both the dock/bar's full click area and general input responsiveness on the scaled output.
+
## Feature, implemented, v2: real desktop icons plus proper right-click desktop/icon menus (2026-08-25/26)
Closes the "Right-click on bare desktop" item that used to sit under