diff --git a/crates/geom/src/lib.rs b/crates/geom/src/lib.rs index 191b30419..c415f3b37 100644 --- a/crates/geom/src/lib.rs +++ b/crates/geom/src/lib.rs @@ -474,15 +474,29 @@ pub fn ellipse(width: f64, height: f64, start: f64, sweep: f64, inner: f64) -> B let mut path = BezPath::new(); // A whole turn as an arc with these radii: kurbo's `Ellipse` recovers its radii from an // affine map, which overflows to infinity for radii near f64's limit (and infinite cubics). - let whole = |rx: f64, ry: f64| kurbo::Arc::new(c, (rx, ry), 0.0, TAU, 0.0); + // Four cubics through the right, bottom, left and top extremes (clockwise from 3 o'clock), + // the same four anchors an ellipse has when edited as a vector; an arc subdivided by + // tolerance would give five or six. + let whole = |rx: f64, ry: f64| { + const K: f64 = 0.552_284_749_830_793_4; + let (kx, ky) = (rx * K, ry * K); + let mut p = BezPath::new(); + p.move_to(c + Vec2::new(rx, 0.0)); + p.curve_to(c + Vec2::new(rx, ky), c + Vec2::new(kx, ry), c + Vec2::new(0.0, ry)); + p.curve_to(c + Vec2::new(-kx, ry), c + Vec2::new(-rx, ky), c + Vec2::new(-rx, 0.0)); + p.curve_to(c + Vec2::new(-rx, -ky), c + Vec2::new(-kx, -ry), c + Vec2::new(0.0, -ry)); + p.curve_to(c + Vec2::new(kx, -ry), c + Vec2::new(rx, -ky), c + Vec2::new(rx, 0.0)); + p.close_path(); + p + }; if full && inner <= 0.0 { - return whole(rx, ry).to_path(tol); + return whole(rx, ry); } if full { // A ring: outer clockwise, inner counter-clockwise. - path.extend(whole(rx, ry).path_elements(tol)); - let inner_path = whole(rx * inner, ry * inner).to_path(tol); - path.extend(inner_path.reverse_subpaths()); + path.extend(whole(rx, ry).elements().iter().copied()); + let inner_path = whole(rx * inner, ry * inner); + path.extend(inner_path.reverse_subpaths().elements().iter().copied()); return path; } let outer = kurbo::Arc::new(c, (rx, ry), start, sweep, 0.0); diff --git a/crates/geom/src/tests.rs b/crates/geom/src/tests.rs index 786bf1935..019f22b83 100644 --- a/crates/geom/src/tests.rs +++ b/crates/geom/src/tests.rs @@ -263,7 +263,7 @@ fn smoothed_polygon_smooths_every_corner() { #[test] fn ellipse_ring_area() { let full = area(&ellipse(100.0, 100.0, 0.0, TAU, 0.0)); - assert!((full - PI * 2500.0).abs() < 2.0); + assert!((full - PI * 2500.0).abs() < 5.0, "{full}"); let ring = ellipse(100.0, 100.0, 0.0, TAU, 0.5); // Winding area of a ring with a reversed inner contour: outer - inner. let a = ring.area().abs(); @@ -321,6 +321,46 @@ fn reverse_subpaths_preserves_open_and_closed() { assert!(matches!(r.elements().last(), Some(kurbo::PathEl::ClosePath)), "{:?}", r.elements()); } +fn cubics(path: &BezPath) -> Vec { + path.segments() + .filter_map(|s| match s { + kurbo::PathSeg::Cubic(c) => Some(c), + _ => None, + }) + .collect() +} + +#[test] +fn a_whole_ellipse_is_four_cubics_through_the_extremes() { + for (w, h) in [(100.0, 100.0), (200.0, 60.0), (3.0, 900.0)] { + let segs = cubics(&ellipse(w, h, 0.0, TAU, 0.0)); + assert_eq!(segs.len(), 4, "{w}x{h}"); + let (rx, ry) = (w / 2.0, h / 2.0); + let want = [Point::new(rx, h), Point::new(0.0, ry), Point::new(rx, 0.0), Point::new(w, ry)]; + for (i, s) in segs.iter().enumerate() { + assert!((s.p3 - want[i]).hypot() < 1e-9, "{w}x{h}: anchor {i} is {:?}", s.p3); + // Handle length is 0.5523 of the radius along the tangent. + let k = 0.552_284_749_830_793_4; + let r_along = if i % 2 == 0 { ry } else { rx }; + assert!(((s.p1 - s.p0).hypot() - k * r_along).abs() < 1e-9, "{w}x{h}: handle {i}"); + } + } +} + +#[test] +fn a_whole_ring_is_four_cubics_per_contour() { + let segs = cubics(&ellipse(100.0, 100.0, 0.0, TAU, 0.5)); + assert_eq!(segs.len(), 8); +} + +#[test] +fn a_sliced_ellipse_still_builds_its_arc() { + let pie = ellipse(100.0, 100.0, 0.0, PI, 0.0); + assert!(!cubics(&pie).is_empty()); + assert!(matches!(pie.elements().last(), Some(kurbo::PathEl::ClosePath))); + assert!((area(&pie) - PI * 1250.0).abs() < 2.0); +} + #[test] fn a_huge_ellipse_arc_is_a_few_cubics() { let big = ellipse(1e300, 1e300, 0.3, 2.0, 0.5); diff --git a/crates/render/src/band_tests.rs b/crates/render/src/band_tests.rs index b9a7bfbcc..6cdf2cd88 100644 --- a/crates/render/src/band_tests.rs +++ b/crates/render/src/band_tests.rs @@ -56,20 +56,61 @@ fn view(scale: f64) -> View { View { origin: (-10.0, -10.0), scale, width: (440.0 * scale) as u32, height: (240.0 * scale) as u32 } } -/// Per row: the mean channel difference and how many pixels differ by more than 2. A band artifact -/// differs along a whole row; edge antialiasing may differ in a few pixels per edge (a curve -/// clipped at a band's edge rasterises a hair differently, most where it runs nearly level). +/// A pixel whose largest channel differs by at most this much between two renders is edge +/// antialiasing noise (a curve clipped at a band's edge rasterises a hair differently, most where +/// it runs level, as at the top of an ellipse), not a band artifact. +const NOISE: u8 = 24; + +/// One row of two renders: the channel differences of the pixels above noise (see `NOISE`) summed +/// and divided by the whole row's channel count, so noise pixels add nothing but still count in +/// the denominator; and how many pixels differ by more than 2 at all. A band artifact differs along +/// a whole row, by more than noise. +fn row_stats(x: &[u8], y: &[u8]) -> (f64, usize) { + let mut sum = 0.0; + let mut off = 0; + for (p, q) in x.chunks(4).zip(y.chunks(4)) { + let worst = p.iter().zip(q).map(|(c, d)| c.abs_diff(*d)).max().unwrap_or(0); + if worst > 2 { + off += 1; + } + if worst > NOISE { + sum += p.iter().zip(q).map(|(c, d)| f64::from(c.abs_diff(*d))).sum::(); + } + } + (sum / x.len().max(1) as f64, off) +} + fn row_diffs(a: &Pixmap, b: &Pixmap) -> Vec<(f64, usize)> { let row = a.width() as usize * 4; - a.data() - .chunks(row) - .zip(b.data().chunks(row)) - .map(|(x, y)| { - let sum: f64 = x.iter().zip(y).map(|(p, q)| f64::from(p.abs_diff(*q))).sum(); - let off = x.chunks(4).zip(y.chunks(4)).filter(|(p, q)| p.iter().zip(*q).any(|(c, d)| c.abs_diff(*d) > 2)).count(); - (sum / row as f64, off) - }) - .collect() + a.data().chunks(row).zip(b.data().chunks(row)).map(|(x, y)| row_stats(x, y)).collect() +} + +/// Whether a row passes: the original 0.25 limit on the mean from `row_stats`, and at most a +/// twentieth of the row differing at all. +fn row_ok(mean: f64, off: usize, width: usize) -> bool { + mean < 0.25 && off * 20 <= width +} + +#[test] +fn a_real_band_artifact_fails_but_edge_noise_passes() { + let width = 880; // scale 2 + let flat = vec![100u8; width * 4]; + // 44 pixels whose colour channels differ by 60 (alpha unchanged): the mean is + // 44 * 3 * 60 / (880 * 4) = 2.25, and the count, 44, is within a twentieth of the row. A real + // artifact, so it fails on the mean. + let mut artifact = flat.clone(); + for px in artifact.chunks_mut(4).take(44) { + px.iter_mut().take(3).for_each(|c| *c += 60); + } + let (mean, off) = row_stats(&flat, &artifact); + assert!(!row_ok(mean, off, width), "{mean} {off}"); + // 8 pixels differing by 16 (the ellipse top's antialiasing): forgiven. + let mut noise = flat.clone(); + for px in noise.chunks_mut(4).take(8) { + px.iter_mut().for_each(|c| *c += 16); + } + let (mean, off) = row_stats(&flat, &noise); + assert!(row_ok(mean, off, width), "{mean} {off}"); } #[test] @@ -82,10 +123,7 @@ fn bands_match_one_piece_rendering() { let banded = render_bands(&d, p, &v, &Options::default(), bands).unwrap(); assert_eq!((banded.width(), banded.height()), (one.width(), one.height())); for (y, (mean, off)) in row_diffs(&one, &banded).into_iter().enumerate() { - assert!( - mean < 0.25 && off * 20 <= v.width as usize, - "scale {scale}, {bands} bands: row {y} differs ({off} pixels, {mean:.2} on average)" - ); + assert!(row_ok(mean, off, v.width as usize), "scale {scale}, {bands} bands: row {y} differs ({off} pixels, {mean:.2} on average)"); } } }