From 5d1930c32394dd0b70067362a54e150fe15a6df3 Mon Sep 17 00:00:00 2001 From: Shinyaigeek Date: Tue, 15 Sep 2026 23:34:42 +0900 Subject: [PATCH] Fix 1px borders vanishing when one side is zero-width or at fractional scales Fixes #837. Two independent bugs each erased `border: 1px solid; border-top-width: 0; border-radius: 8px` entirely: blitz-paint: `start_angle()` splits a corner arc between its two adjacent sides from the ratio of their widths. A zero-width side made that `inf / inf = NaN`, and since all same-coloured edges are filled as one path the NaN corner dropped the whole outline. Return the limiting angle for a zero-width side instead (the arc belongs entirely to the other side). blitz-dom: `taffy::round_layout` snaps edges to whole CSS pixels, but stylo already snaps lengths to device pixels, so at a fractional scale a `1px` border is `1 / scale` CSS px wide and its two edges could round to the same value, leaving a zero-width border to paint. Replace it with a rounding pass on the device pixel grid (`round(v * scale) / scale`), identical to taffy's at scale 1. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SCkGi9MbYao9rnHMLiGq1D --- packages/blitz-dom/src/layout/mod.rs | 74 ++++++++++ packages/blitz-dom/src/resolve.rs | 2 +- packages/blitz-paint/src/kurbo_css/css_box.rs | 27 +++- .../tests/thin_border_one_side_zero.rs | 139 ++++++++++++++++++ 4 files changed, 240 insertions(+), 2 deletions(-) create mode 100644 tests/blitz-tests/tests/thin_border_one_side_zero.rs diff --git a/packages/blitz-dom/src/layout/mod.rs b/packages/blitz-dom/src/layout/mod.rs index 4b8e659641..b4a34f36f9 100644 --- a/packages/blitz-dom/src/layout/mod.rs +++ b/packages/blitz-dom/src/layout/mod.rs @@ -559,6 +559,80 @@ impl RoundTree for BaseDocument { } } +/// Round the computed layout to the device pixel grid, writing the result to +/// each node's `final_layout`. +/// +/// This is [`taffy::round_layout`] with the grid scaled by `scale` (the +/// device pixel ratio): every edge is snapped to a multiple of `1 / scale` +/// CSS pixels rather than to a whole CSS pixel. Stylo already snaps lengths +/// to whole device pixels, so at a fractional scale a `1px` border arrives +/// here as `1 / scale` CSS px (0.8 at 1.25x); rounding its two edges to whole +/// CSS pixels independently can land them on the same value and collapse the +/// border to nothing (DioxusLabs/blitz#837). Rounding to the device grid keeps +/// it exactly one device pixel wide. +/// +/// As in taffy, edges are rounded from their cumulative (viewport-relative) +/// position, and widths/heights are the difference of two rounded edges, so +/// that no gaps open up between adjacent boxes. +pub(crate) fn round_layout(tree: &mut impl RoundTree, root: NodeId, scale: f32) { + fn round_inner( + tree: &mut impl RoundTree, + node_id: NodeId, + cumulative_x: f32, + cumulative_y: f32, + round: &impl Fn(f32) -> f32, + ) { + let u = tree.get_unrounded_layout(node_id); + let mut layout = u; + + let cumulative_x = cumulative_x + u.location.x; + let cumulative_y = cumulative_y + u.location.y; + + layout.location.x = round(u.location.x); + layout.location.y = round(u.location.y); + layout.size.width = round(cumulative_x + u.size.width) - round(cumulative_x); + layout.size.height = round(cumulative_y + u.size.height) - round(cumulative_y); + layout.scrollbar_size.width = round(u.scrollbar_size.width); + layout.scrollbar_size.height = round(u.scrollbar_size.height); + layout.border.left = round(cumulative_x + u.border.left) - round(cumulative_x); + layout.border.right = round(cumulative_x + u.size.width) + - round(cumulative_x + u.size.width - u.border.right); + layout.border.top = round(cumulative_y + u.border.top) - round(cumulative_y); + layout.border.bottom = round(cumulative_y + u.size.height) + - round(cumulative_y + u.size.height - u.border.bottom); + layout.padding.left = round(cumulative_x + u.padding.left) - round(cumulative_x); + layout.padding.right = round(cumulative_x + u.size.width) + - round(cumulative_x + u.size.width - u.padding.right); + layout.padding.top = round(cumulative_y + u.padding.top) - round(cumulative_y); + layout.padding.bottom = round(cumulative_y + u.size.height) + - round(cumulative_y + u.size.height - u.padding.bottom); + layout.scrollable_overflow_rect.left = + round(cumulative_x + u.scrollable_overflow_rect.left) - round(cumulative_x); + layout.scrollable_overflow_rect.right = + round(cumulative_x + u.scrollable_overflow_rect.right) - round(cumulative_x); + layout.scrollable_overflow_rect.top = + round(cumulative_y + u.scrollable_overflow_rect.top) - round(cumulative_y); + layout.scrollable_overflow_rect.bottom = + round(cumulative_y + u.scrollable_overflow_rect.bottom) - round(cumulative_y); + + tree.set_final_layout(node_id, &layout); + + for index in 0..tree.child_count(node_id) { + let child = tree.get_child_id(node_id, index); + round_inner(tree, child, cumulative_x, cumulative_y, round); + } + } + + // Snap to whole device pixels. A scale of 1 is exactly taffy's rounding. + let scale = if scale.is_finite() && scale > 0.0 { + scale + } else { + 1.0 + }; + let round = move |v: f32| (v * scale).round() / scale; + round_inner(tree, root, 0.0, 0.0, &round); +} + impl PrintTree for BaseDocument { fn get_debug_label(&self, node_id: NodeId) -> &'static str { let node = &self.node_from_id(node_id); diff --git a/packages/blitz-dom/src/resolve.rs b/packages/blitz-dom/src/resolve.rs index c89b73609c..f6e459d6b0 100644 --- a/packages/blitz-dom/src/resolve.rs +++ b/packages/blitz-dom/src/resolve.rs @@ -391,7 +391,7 @@ impl BaseDocument { // println!("\n\nRESOLVE LAYOUT\n===========\n"); taffy::compute_root_layout(self, root_element_id, available_space); - taffy::round_layout(self, root_element_id); + crate::layout::round_layout(self, root_element_id, self.viewport.scale()); // println!("\n\n"); // taffy::print_tree(self, root_node_id) diff --git a/packages/blitz-paint/src/kurbo_css/css_box.rs b/packages/blitz-paint/src/kurbo_css/css_box.rs index 321077fe00..2e112fcdca 100644 --- a/packages/blitz-paint/src/kurbo_css/css_box.rs +++ b/packages/blitz-paint/src/kurbo_css/css_box.rs @@ -1,5 +1,5 @@ use kurbo::{Arc, BezPath, Insets, PathEl, Point, Rect, Shape as _, Vec2}; -use std::{f64::consts::FRAC_PI_2, f64::consts::PI}; +use std::f64::consts::{FRAC_PI_2, FRAC_PI_4, PI}; use super::non_uniform_radii::NonUniformRoundedRectRadii; use super::{Corner, CssBoxKind, Direction, Edge, add_insets, get_corner_insets}; @@ -614,6 +614,19 @@ impl BuildBezpath for BezPath { /// Get the start angle of the arc based on the border width and the radii fn start_angle(bt_width: f64, br_width: f64, radii: Vec2) -> f64 { + // A side with no width contributes nothing to the corner, so the split + // sits at the limit: the whole arc belongs to the other side. Handling + // this up front keeps the formula below from evaluating `inf / inf` + // (`bt_width == 0`) or `0 / 0` (both zero) and returning `NaN`, which would + // poison the corner arc and, since all four edges of a box are filled as + // one path, erase the entire border. + match (bt_width == 0.0, br_width == 0.0) { + (true, true) => return FRAC_PI_4, + (true, false) => return FRAC_PI_2, + (false, true) => return 0.0, + (false, false) => {} + } + // slope of the border intersection split let w = bt_width / br_width; let x = radii.y / (w * radii.x); @@ -747,6 +760,18 @@ mod tests { } } + /// Regression test for DioxusLabs/blitz#837: a zero-width side (e.g. + /// `border: 1px solid; border-top-width: 0` with a `border-radius`) made + /// the closed form evaluate `inf / inf` and return `NaN`. The whole corner + /// arc then belongs to the side that does have a width. + #[test] + fn handles_zero_width_sides() { + let radii = Vec2 { x: 8.0, y: 8.0 }; + assert_eq!(start_angle(0.0, 1.0, radii), FRAC_PI_2); + assert_eq!(start_angle(1.0, 0.0, radii), 0.0); + assert!(start_angle(0.0, 0.0, radii).is_finite()); + } + #[test] fn should_solve_properly() { // 0.643501 diff --git a/tests/blitz-tests/tests/thin_border_one_side_zero.rs b/tests/blitz-tests/tests/thin_border_one_side_zero.rs new file mode 100644 index 0000000000..27c5bc6aca --- /dev/null +++ b/tests/blitz-tests/tests/thin_border_one_side_zero.rs @@ -0,0 +1,139 @@ +//! Regression tests for DioxusLabs/blitz#837: `1px` borders that vanished. +//! +//! `border: 1px solid red; border-top-width: 0; border-radius: 8px` must draw +//! the left, right and bottom sides. Two separate bugs each erased them: +//! +//! - the corner arc split angle was computed from the ratio of the two +//! adjacent border widths, so a zero-width side produced `NaN` and, since +//! all four edges are filled as one path, the whole outline was dropped; +//! - at a fractional device pixel ratio a one-device-pixel border is less than +//! a CSS pixel wide, and rounding its two edges to whole CSS pixels could +//! land both on the same value, leaving a zero-width border to paint. + +use anyrender::render_to_buffer; +use anyrender_vello_cpu::VelloCpuImageRenderer; +use blitz_dom::DocumentConfig; +use blitz_html::{HtmlDocument, HtmlProvider}; +use blitz_paint::paint_scene; +use blitz_test_harness::{Harness, HarnessOptions}; +use blitz_traits::shell::{ColorScheme, Viewport}; +use std::sync::Arc; + +const CSS_W: u32 = 300; +const CSS_H: u32 = 120; + +/// Render a 260x80 panel at (20, 20) styled by `style`, at `scale`, and +/// count the red pixels in each edge band of the panel. +/// +/// Returns `[top, right, bottom, left]`. +fn red_edge_pixels(style: &str, scale: f64) -> [usize; 4] { + let html = format!( + "\ +
\ + " + ); + let w = (CSS_W as f64 * scale) as u32; + let h = (CSS_H as f64 * scale) as u32; + let mut doc = HtmlDocument::from_html( + &html, + DocumentConfig { + viewport: Some(Viewport::new(w, h, scale as f32, ColorScheme::Light)), + html_parser_provider: Some(Arc::new(HtmlProvider) as _), + ..Default::default() + }, + ); + doc.resolve(0.0); + let buffer = render_to_buffer::( + |scene| paint_scene(scene, &mut doc, scale, w, h, 0, 0), + w, + h, + ); + + let (w, h) = (w as usize, h as usize); + let mut counts = [0; 4]; + for y in 0..h { + for x in 0..w { + let i = (y * w + x) * 4; + let is_red = buffer[i] > 150 && buffer[i + 1] < 120 && buffer[i + 2] < 120; + if !is_red { + continue; + } + // Away from the corners, classify by the nearest edge of the panel. + let (fx, fy) = (x as f64 / w as f64, y as f64 / h as f64); + if (0.2..0.8).contains(&fx) { + counts[if fy < 0.5 { 0 } else { 2 }] += 1; + } else if (0.3..0.7).contains(&fy) { + counts[if fx < 0.5 { 3 } else { 1 }] += 1; + } + } + } + counts +} + +const ISSUE_STYLE: &str = "border: 1px solid #ff0000; border-top-width: 0; border-radius: 8px; \ + background: #212121; overflow: hidden;"; + +#[test] +fn a_rounded_border_with_one_zero_width_side_draws_the_other_three() { + let [top, right, bottom, left] = red_edge_pixels(ISSUE_STYLE, 1.0); + assert_eq!(top, 0, "the top side has zero width and must not be drawn"); + assert!(right > 0, "the right side vanished"); + assert!(bottom > 0, "the bottom side vanished"); + assert!(left > 0, "the left side vanished"); +} + +#[test] +fn one_device_pixel_borders_survive_fractional_scaling() { + for scale in [1.25, 1.5, 1.75] { + for style in [ + ISSUE_STYLE, + "border: 1px solid #ff0000; border-top-width: 0;", + "border: 1px solid #ff0000;", + ] { + let [top, right, bottom, left] = red_edge_pixels(style, scale); + let expect_top = !style.contains("border-top-width: 0"); + assert_eq!( + top > 0, + expect_top, + "top side wrong at scale {scale} for `{style}`" + ); + assert!( + right > 0 && bottom > 0 && left > 0, + "sides vanished at scale {scale} for `{style}`: \ + right={right} bottom={bottom} left={left}" + ); + } + } +} + +/// The layout a `1px` border is painted from must stay a whole, non-zero +/// number of device pixels wide after rounding, whatever the device pixel +/// ratio. +#[test] +fn rounded_layout_keeps_one_device_pixel_borders() { + for scale in [1.0f32, 1.25, 1.5, 1.75, 2.0] { + let harness = Harness::from_html_with( + "
panel
", + HarnessOptions { + scale, + ..Default::default() + }, + ); + let id = harness.node("#p"); + let base = harness.base(); + let border = base.get_node(id).unwrap().final_layout().border; + for (side, width) in [ + ("left", border.left), + ("right", border.right), + ("bottom", border.bottom), + ] { + let device_px = width * scale; + assert!( + device_px >= 1.0 - 1e-3 && (device_px - device_px.round()).abs() < 1e-3, + "{side} border is {width} CSS px ({device_px} device px) at scale {scale}" + ); + } + assert_eq!(border.top, 0.0, "top border at scale {scale}"); + } +}