diff options
| -rw-r--r-- | crates/wayland/src/decoration/border.rs | 14 | ||||
| -rw-r--r-- | crates/wayland/src/decoration/corners.rs | 160 | ||||
| -rw-r--r-- | crates/wayland/src/decoration/tests.rs | 90 | ||||
| -rw-r--r-- | crates/wayland/src/decoration/titlebar.rs | 8 | ||||
| -rw-r--r-- | crates/wayland/src/native_lock.rs | 4 |
5 files changed, 218 insertions, 58 deletions
diff --git a/crates/wayland/src/decoration/border.rs b/crates/wayland/src/decoration/border.rs index 472ef3b..9a2ed91 100644 --- a/crates/wayland/src/decoration/border.rs +++ b/crates/wayland/src/decoration/border.rs @@ -71,7 +71,14 @@ pub fn render_border_top(width: u32, thickness: u32, color: (u8, u8, u8), radius for px in buf.chunks_exact_mut(4) { px.copy_from_slice(&bg); } - round_top_corners(&mut buf, width, height, radius, radius as i32); + // `Some(radius - thickness)`: without this, the corner stayed a solid + // filled disk out to the centre column/row instead of a proper ring -- + // see `corners::carve_inner_corner_pixel`'s own doc comment for the + // full story (reported live as "squares on the inside corners"). + // `None` when `radius <= thickness` - no ring to carve, the corner is + // already exactly `thickness` px wide at most. + let inner_radius = (radius as usize > thickness).then(|| radius - thickness as u32); + round_top_corners(&mut buf, width, height, radius, radius as i32, radius as i32, inner_radius); clip_middle_beyond_thickness(&mut buf, width, radius as usize, thickness..height); buf } @@ -127,7 +134,10 @@ pub fn render_border_bottom(width: u32, thickness: u32, color: (u8, u8, u8), rad // real corner. `render_border_top` gets `radius` used unshifted here // for the same reason it does: this buffer's own outermost row is // genuinely the true tip of the shape. - round_bottom_corners(&mut buf, width, height, radius); + // See `render_border_top`'s own matching comment for why this needs an + // inner radius too. + let inner_radius = (radius as usize > thickness).then(|| radius - thickness as u32); + round_bottom_corners(&mut buf, width, height, radius, inner_radius); // Extra rows sit above the original `thickness`, not below - the // bottom strip's curve resolves going *up* into content, the mirror of // the top strip's resolving *down* into it. See diff --git a/crates/wayland/src/decoration/corners.rs b/crates/wayland/src/decoration/corners.rs index 10bad4d..aeac076 100644 --- a/crates/wayland/src/decoration/corners.rs +++ b/crates/wayland/src/decoration/corners.rs @@ -60,13 +60,52 @@ /// starting `thickness` rows *into* the circle instead of at its top, /// that needs to shift, by passing `radius as i32 - border_width as /// i32` (see `render_titlebar`'s call site). -pub(crate) fn round_top_corners(buf: &mut [u8], width: usize, height: usize, radius: u32, center_row: i32) { +/// +/// `center_col` is the exact same idea, horizontally: which *column* of +/// this buffer's own local coordinates the left corner's circle centre +/// sits on (the right corner mirrors it, `width - r` outward from the +/// right edge by the same amount `center_col` is inward from the left). +/// A border strip's own column 0 is the *true* left edge, so it passes +/// `radius as i32`, same as its unshifted `center_row`. A titlebar's own +/// column 0 sits `border_width` columns *inside* that same true edge -- +/// its buffer is only as wide as the content it sits above, not the +/// wider border strip around it - so it needs the identical `radius as +/// i32 - border_width as i32` shift horizontally too, or its own circle +/// centre ends up `border_width` columns to the right of the border +/// strip's, two different circles again despite `center_row` already +/// lining up the vertical one. Confirmed live at a real corner, zoomed: +/// the border's own curve covered most of the shared corner correctly, +/// but a `border_width`-wide sliver of the titlebar's own (wrongly +/// centred) curve poked through right where the two should have met +/// exactly, reading as a small square notch bitten out of an otherwise +/// smooth arc - reported as "squares on the inside corners of each +/// vertex/border corner." Every existing caller before this parameter +/// existed passed the unshifted, no-op case (`radius as i32`, same as +/// `center_row`'s own default), so this is additive, not a behaviour +/// change for border's own corner. +/// +/// `inner_radius`, when `Some`, also carves this corner into a proper ring +/// - see [`carve_inner_corner_pixel`]'s own doc comment for why that's +/// needed at all. Only `render_border_top` passes one (`radius - +/// border_width`, the ring's real visible thickness); every other caller +/// (a titlebar's own corner, the lock-screen box) passes `None` and keeps +/// today's solid-disk-past-the-nominal-edge behaviour, which is correct +/// for a single flat-coloured panel with nothing of a *different* colour +/// underneath it needing to show through. +pub(crate) fn round_top_corners(buf: &mut [u8], width: usize, height: usize, radius: u32, center_row: i32, center_col: i32, inner_radius: Option<u32>) { let r = (radius as usize).min(width / 2); if r == 0 { return; } let rf = r as f32; let cy = center_row as f32; + // How far `center_col` sits from the unshifted default (`radius`) -- + // the right corner's own centre needs shifting by the same amount, in + // the opposite direction (further *into* the buffer from the right + // edge, mirroring how the left corner shifts further *into* it from + // the left), since the buffer's own right edge is the mirror image of + // its left one, not an independent second true edge. + let col_inset = radius as i32 - center_col; // Only rows that could plausibly need blending at all: below // `center_row` (this buffer's slice of the circle, whatever portion // of it falls within `[0, height)`) is where the actual curve lives; @@ -82,9 +121,18 @@ pub(crate) fn round_top_corners(buf: &mut [u8], width: usize, height: usize, rad // generalised to an arbitrary `center_row`. let y_lo = (center_row - r as i32).max(0) as usize; let y_hi = (center_row.max(0) as usize).min(height); + // `> 0`, not just `.is_some()`: a `radius <= border_width` window + // (an unusually thick border relative to its corner radius) has no + // ring to carve at all - the whole disk out to `radius` already *is* + // the intended `border_width`-ish thickness, and an inner radius of + // zero or less would carve away the entire corner instead of nothing. + let inner_rf = inner_radius.filter(|&r| r > 0).map(|r| r as f32); for y in y_lo..y_hi { for x in 0..r { - blend_corner_pixel(buf, width, x, y, rf, cy, rf); + blend_corner_pixel(buf, width, x, y, center_col as f32, cy, rf); + if let Some(inner_rf) = inner_rf { + carve_inner_corner_pixel(buf, width, x, y, center_col as f32, cy, inner_rf); + } } for x in (width - r)..width { // `width - r`, not `width - r - 1` - see `blend_corner_pixel`'s @@ -93,7 +141,14 @@ pub(crate) fn round_top_corners(buf: &mut [u8], width: usize, height: usize, rad // right corner's centre column lines up with `rounded_corners_ // pixman.rs`'s `apply_corner_mask` (`px.clamp(radius, wf - // radius)`, which clamps to exactly `w - r` here) without it. - blend_corner_pixel(buf, width, x, y, (width - r) as f32, cy, rf); + // `+ col_inset`: the same horizontal shift `center_col` applies + // to the left corner, mirrored - see this function's own doc + // comment on `center_col`/`col_inset`. + let cx = (width - r) as f32 + col_inset as f32; + blend_corner_pixel(buf, width, x, y, cx, cy, rf); + if let Some(inner_rf) = inner_rf { + carve_inner_corner_pixel(buf, width, x, y, cx, cy, inner_rf); + } } } } @@ -121,23 +176,43 @@ pub(crate) fn round_top_corners(buf: &mut [u8], width: usize, height: usize, rad /// `clipped_corner_pixels_are_fully_premultiplied_zero_not_just_alpha` /// already established for the hard-cut case this replaces. fn blend_corner_pixel(buf: &mut [u8], width: usize, x: usize, y: usize, cx: f32, cy: f32, radius: f32) { - // Sampled at the pixel's own *center* (`+ 0.5`), not its raw integer - // coordinate - matching `rounded_corners_pixman.rs`'s `apply_corner_ - // mask` (`px = x as f32 + 0.5`) and `rounded_corners.rs`'s GLES shader, - // both of which already use this standard rasterization convention. - // This function didn't, a half-pixel systematic difference between - // this border-strip curve and the client-content curve it's supposed - // to trace exactly the same circle as - confirmed live at extreme - // zoom: a small but real right-angle step partway along an otherwise - // smooth arc, right where the two curves are supposed to meet - // (reported as "squares on the inside corners of each vertex"). - // `round_top_corners`/`round_bottom_corners`'s own right-edge/bottom - // `cx`/`cy` had a compensating `- 1` baked in against the *old* - // convention - removed alongside this, see their own doc comments. + let keep = corner_keep_mask(x, y, cx, cy, radius); + scale_pixel(buf, width, x, y, keep); +} + +/// `1.0` (fully kept) within `radius - 1` of `(cx, cy)`, `0.0` (fully cut) +/// beyond `radius + 1`, smoothstepped between - the shared falloff both +/// [`blend_corner_pixel`] (an *outer* cut: keep near the centre, cut far +/// from it) and [`carve_inner_corner_pixel`] (an *inner* cut: the same +/// falloff, inverted, so it cuts *near* the centre instead) are built from. +/// Sampled at the pixel's own *center* (`+ 0.5`), not its raw integer +/// coordinate - matching `rounded_corners_pixman.rs`'s `apply_corner_mask` +/// (`px = x as f32 + 0.5`) and `rounded_corners.rs`'s GLES shader, both of +/// which already use this standard rasterization convention. This function +/// didn't, once - a half-pixel systematic difference between this +/// border-strip curve and the client-content curve it's supposed to trace +/// exactly the same circle as, confirmed live at extreme zoom: a small but +/// real right-angle step partway along an otherwise smooth arc, right +/// where the two curves are supposed to meet. +fn corner_keep_mask(x: usize, y: usize, cx: f32, cy: f32, radius: f32) -> f32 { let (dx, dy) = (x as f32 + 0.5 - cx, y as f32 + 0.5 - cy); let dist = (dx * dx + dy * dy).sqrt(); let t = ((dist - (radius - 1.0)) / 2.0).clamp(0.0, 1.0); - let mask = 1.0 - (t * t * (3.0 - 2.0 * t)); + 1.0 - (t * t * (3.0 - 2.0 * t)) +} + +/// Multiplies every BGRA byte of the pixel at `(x, y)` by `mask` - `buf` is +/// premultiplied BGRA (`color::rgb_to_bgra`'s own convention), so scaling +/// all four bytes by the same factor is the correct way to reduce a +/// pixel's effective alpha (zeroing all four, not just alpha, for a fully +/// cut pixel - a genuinely transparent premultiplied pixel is `(0, 0, 0, +/// 0)` in every channel, not just alpha, since the stored colour already +/// carries the alpha multiplied in; leaving stale opaque RGB behind while +/// zeroing only alpha produced a byte pattern Pixman's own `OVER` +/// compositing does not actually treat as "nothing here" - confirmed +/// live, pixel-by-pixel, no visible transparency despite alpha already +/// being zero). +fn scale_pixel(buf: &mut [u8], width: usize, x: usize, y: usize, mask: f32) { if mask >= 1.0 { return; } @@ -151,11 +226,46 @@ fn blend_corner_pixel(buf: &mut [u8], width: usize, x: usize, y: usize, cx: f32, } } +/// The border strip's own missing half of a proper rounded-corner *ring*: +/// [`blend_corner_pixel`] already cuts everything *outside* `radius` of the +/// shared corner centre (the true rounded silhouette), but nothing used to +/// cut anything *inside* it - so the strip's own "extra" rows (past its +/// nominal `border_width`, present whenever `corner_radius > border_width` +/// - see `render_border_top`'s own doc comment) stayed a solid *filled* +/// quarter-disk out to the centre column/row, then hit `clip_middle_ +/// beyond_thickness`'s hard, unblended rectangular cut at exactly column/ +/// row `radius` - which is essentially the disk's own *most opaque* +/// point (dead centre, mask ~1.0), not somewhere the curve had already +/// faded out. The result: a solid wedge of border colour with two straight +/// inner edges meeting the titlebar/content at a right angle, not a +/// uniform-width curved ring - confirmed live, zoomed: a clean rectangular +/// step, not a blend, reported as "squares on the inside corners." +/// +/// The fix: cut *this* pixel wherever it falls within `border_width` of +/// the *same* shared centre `blend_corner_pixel` already cut around -- +/// i.e. within `radius - border_width` of it, same smoothstep falloff, +/// inverted. Combined with the existing outer cut, the strip's corner +/// becomes a genuine ring of ~`border_width` visible thickness tapering +/// smoothly to nothing by the point `clip_middle_beyond_thickness`'s own +/// (already-transparent-by-then) hard cut takes over, instead of jumping +/// from opaque to transparent in one pixel. Only ever called with `radius +/// > border_width` (`round_top_corners`/`round_bottom_corners` skip it +/// otherwise, since there is no ring to speak of - see their own call +/// sites) - `inner_radius` would otherwise be zero or negative, cutting +/// the entire disk including the visible outer sliver that's supposed to +/// remain `border_width` px thick. +fn carve_inner_corner_pixel(buf: &mut [u8], width: usize, x: usize, y: usize, cx: f32, cy: f32, inner_radius: f32) { + let keep = corner_keep_mask(x, y, cx, cy, inner_radius); + scale_pixel(buf, width, x, y, 1.0 - keep); +} + /// [`round_top_corners`]'s mirror for the bottom two corners - same /// construction, corner centres `r` *up* from the bottom instead of down /// from the top. Same anti-aliasing, same reason - see -/// [`blend_corner_pixel`]'s own doc comment. -pub(crate) fn round_bottom_corners(buf: &mut [u8], width: usize, height: usize, radius: u32) { +/// [`blend_corner_pixel`]'s own doc comment. `inner_radius` is the same +/// idea as `round_top_corners`' own parameter of the same name - see its +/// doc comment. +pub(crate) fn round_bottom_corners(buf: &mut [u8], width: usize, height: usize, radius: u32, inner_radius: Option<u32>) { let r = (radius as usize).min(width / 2); if r == 0 { return; @@ -178,14 +288,24 @@ pub(crate) fn round_bottom_corners(buf: &mut [u8], width: usize, height: usize, // pixman.rs`'s own bottom-box centre (`py.clamp(radius, hf - radius)`, // which clamps to exactly `h - r`) without it. let cy = height as f32 - rf; + // See `round_top_corners`'s matching line for why this is `.filter(|&r| + // r > 0)`, not just `.is_some()`. + let inner_rf = inner_radius.filter(|&r| r > 0).map(|r| r as f32); for y in (height - rows)..height { for x in 0..r { blend_corner_pixel(buf, width, x, y, rf, cy, rf); + if let Some(inner_rf) = inner_rf { + carve_inner_corner_pixel(buf, width, x, y, rf, cy, inner_rf); + } } for x in (width - r)..width { // See `round_top_corners`'s matching comment for why this is // `width - r`, not `width - r - 1`. - blend_corner_pixel(buf, width, x, y, (width - r) as f32, cy, rf); + let cx = (width - r) as f32; + blend_corner_pixel(buf, width, x, y, cx, cy, rf); + if let Some(inner_rf) = inner_rf { + carve_inner_corner_pixel(buf, width, x, y, cx, cy, inner_rf); + } } } } diff --git a/crates/wayland/src/decoration/tests.rs b/crates/wayland/src/decoration/tests.rs index e9ef02e..4507a05 100644 --- a/crates/wayland/src/decoration/tests.rs +++ b/crates/wayland/src/decoration/tests.rs @@ -503,36 +503,46 @@ fn border_top_and_titlebar_corners_meet_without_a_seam() { let border = render_border_top(width, thickness, color, radius); let titlebar = render_titlebar(width, 24, "", color, (0xff, 0xff, 0xff), true, radius, thickness, true, None, false, false, false, None, true, false); let border_alpha_at = |x: usize| border[((thickness as usize - 1) * width as usize + x) * 4 + 3]; - let titlebar_alpha_at = |x: usize| titlebar[x * 4 + 3]; - // Every column across the curve's actual reach, not just one - // sample point - a seam bug shows up as a jump at some columns - // and not others (the exact shape of the mismatch between two - // differently-sized circles), so checking only the corner pixel - // or only the centre could miss it entirely, the same way the - // original bug slipped past the test above it for months. Worked - // out by hand (see this fix's own commit) what the two designs - // actually produce at every column for radius=6/thickness=4: the - // old (`radius + thickness`-for-the-border) design jumped by as - // much as 187 out of 255, at *every* column past the first two; - // this design jumps by at most 70, confined to the two columns - // nearest the exact tip - an inherent limit of splitting one - // steep curve across two separately-rasterised bitmaps a single - // row apart, not a bug to chase further. `< 90` catches any - // regression back toward the old behaviour without demanding - // more precision than two 1px-apart raster buffers can give. - for x in 0..radius as usize + 2 { - let (b, t) = (border_alpha_at(x), titlebar_alpha_at(x)); + let titlebar_alpha_at = |xt: usize| titlebar[xt * 4 + 3]; + // `x` below is the shared *global* column - distance from the true + // left corner tip, in the border strip's own coordinate frame (its + // column 0 is that tip). The titlebar's own buffer starts `thickness` + // columns further in (its column 0 sits at global column `thickness`, + // not 0 - a border strip is wider than the titlebar it sits above by + // `thickness` on each side, same as `round_top_corners`'s own + // `center_col` doc comment already says for the vertical case), so + // the titlebar-local column that corresponds to a given global column + // `x` is `x - thickness`, not `x` itself. Comparing raw index `x` on + // both sides used to "pass" here for the wrong reason: both curves + // used the same (incorrectly) unshifted centre column, so indexing + // them identically happened to compare matching *formula* output + // without the two indices actually referring to the same real screen + // column - fixing that shared centre (`round_top_corners`'s own + // `center_col` parameter) is what surfaced this test needing the same + // correction. + // + // Every column across the curve's actual reach, not just one sample + // point - a seam bug shows up as a jump at some columns and not + // others, so checking only the corner pixel or only the centre could + // miss it entirely, the same way the original bug slipped past the + // test above it for months. `< 90` catches any regression back toward + // a visibly stepped seam without demanding more precision than two + // 1px-apart raster buffers a single row apart can give. + for x in thickness as usize..radius as usize + 2 { + let xt = x - thickness as usize; + let (b, t) = (border_alpha_at(x), titlebar_alpha_at(xt)); let jump = (b as i32 - t as i32).abs(); - assert!(jump < 90, "column {x}: border's last row (alpha={b}) and titlebar's first row (alpha={t}) must be close, not a sharp seam (jump={jump})"); + assert!(jump < 90, "global column {x} (titlebar column {xt}): border's last row (alpha={b}) and titlebar's first row (alpha={t}) must be close, not a sharp seam (jump={jump})"); } - // Past the tip's unavoidable steepness (columns 0-1 above), the - // curve should be genuinely, near-exactly continuous - both - // rows fully opaque by then for this radius/thickness, not just + // Past the tip's unavoidable steepness (the first two global columns + // above), the curve should be genuinely, near-exactly continuous -- + // both rows fully opaque by then for this radius/thickness, not just // "close enough". - for x in 2..radius as usize + 2 { - let (b, t) = (border_alpha_at(x), titlebar_alpha_at(x)); + for x in (thickness as usize + 2)..radius as usize + 2 { + let xt = x - thickness as usize; + let (b, t) = (border_alpha_at(x), titlebar_alpha_at(xt)); let jump = (b as i32 - t as i32).abs(); - assert!(jump <= 2, "column {x}: past the corner tip the seam should be essentially exact, not just under the looser tip tolerance (jump={jump})"); + assert!(jump <= 2, "global column {x} (titlebar column {xt}): past the corner tip the seam should be essentially exact, not just under the looser tip tolerance (jump={jump})"); } } @@ -621,17 +631,31 @@ fn border_top_curve_actually_closes_within_the_side_strips_own_width() { // `border_width` columns - so with `radius` meaningfully larger // than `border_width`, there was a real gap of bare background // between them, at exactly the column range a real vertical border - // strip occupies. This checks that gap is actually closed: by this - // buffer's own last row (where the curve should have fully - // resolved to flat, for a `radius` that fits within the strip's - // half-width), the column right at the edge of where a - // `border_width`-wide side strip would sit must already be opaque. + // strip occupies. `border_width` here matches `thickness` (as every + // real call site does - both come from the same `w.border_width`), + // not an arbitrary different value: `corners::carve_inner_corner_ + // pixel`'s own inner-ring cut is sized from `thickness`, so a test + // that fed it a different, unrelated "real side strip width" would + // no longer be testing this compositor's own real geometry at all. + // + // `> 180`, not `== 255`: the ring now has smooth antialiasing on + // *both* its outer and inner edges (see `carve_inner_corner_pixel`'s + // own doc comment for why the inner one is new) - the exact corner + // this samples (right at the ring's own width, on its very last row) + // sits close enough to both transition bands to be genuinely, by + // design, a little short of fully opaque, the same way the outer + // curve's own edge already was before this fix existed. `> 180` + // catches the real regression this test exists for - the curve + // never reaching this column at all (alpha near 0, a visible gap) -- + // without demanding more precision than two overlapping antialiased + // edges can give at their closest approach. let color = (0x40, 0x50, 0x60); - let (width, thickness, border_width, radius) = (60u32, 2u32, 3u32, 6u32); + let (width, thickness, radius) = (60u32, 3u32, 6u32); let buf = render_border_top(width, thickness, color, radius); let height = (thickness as usize).max(radius as usize); let alpha_at = |x: usize, y: usize| buf[(y * width as usize + x) * 4 + 3]; - assert_eq!(alpha_at(border_width as usize - 1, height - 1), 255, "the side strip's own rightmost column must be fully covered by the curve at the buffer's last row, not left as a gap"); + let alpha = alpha_at(thickness as usize - 1, height - 1); + assert!(alpha > 180, "the side strip's own rightmost column must be visibly covered by the curve at the buffer's last row, not left as a gap (alpha={alpha})"); } #[test] diff --git a/crates/wayland/src/decoration/titlebar.rs b/crates/wayland/src/decoration/titlebar.rs index 49f9de5..03a614a 100644 --- a/crates/wayland/src/decoration/titlebar.rs +++ b/crates/wayland/src/decoration/titlebar.rs @@ -288,7 +288,13 @@ pub fn render_titlebar( } } if round_corners { - round_top_corners(&mut buf, width, height, radius, radius as i32 - border_width as i32); + // Same shift both ways - see `round_top_corners`'s own doc comment + // on `center_col`: this titlebar's buffer starts `border_width` + // columns inside the true left/right edges the same way it starts + // `border_width` rows inside the true top, so both centres need + // the identical correction, not just the row one. + let shift = radius as i32 - border_width as i32; + round_top_corners(&mut buf, width, height, radius, shift, shift, None); } buf } diff --git a/crates/wayland/src/native_lock.rs b/crates/wayland/src/native_lock.rs index 31c4fc3..3f090bb 100644 --- a/crates/wayland/src/native_lock.rs +++ b/crates/wayland/src/native_lock.rs @@ -434,8 +434,8 @@ fn render_ui_box(native: &NativeLock, theme: &srdwm_core::LockConfig) -> (Vec<u8 // value. Clipping after the border fill above means the corner pixels // of that border get cut along with the background, the same "cut, // don't stroke" treatment `render_titlebar`'s own corners get. - round_top_corners(&mut buf, WIDTH, HEIGHT, theme.corner_radius, theme.corner_radius as i32); - round_bottom_corners(&mut buf, WIDTH, HEIGHT, theme.corner_radius); + round_top_corners(&mut buf, WIDTH, HEIGHT, theme.corner_radius, theme.corner_radius as i32, theme.corner_radius as i32, None); + round_bottom_corners(&mut buf, WIDTH, HEIGHT, theme.corner_radius, None); (buf, (WIDTH as i32, HEIGHT as i32)) } |