From bd9f6a04ef365d7cac513982ad6f714efdc73fa6 Mon Sep 17 00:00:00 2001 From: Drew Dibble Date: Mon, 9 Nov 2026 14:18:05 -0700 Subject: [PATCH] Canvas redraw: review follow-ups to the strengthened tests - A 3D effect that builds nothing: af754bcb2 said the object draws exactly as without it under a Drop Shadow or Gaussian Blur. That holds only for a stroked object. With only a fill, the raster effect draws from the art made for the object, where without the 3D effect it is clipped to the path, so Roughen's spikes differ: the earlier wording was right. The test keeps the stroked cases exact and restores fill-only cases with a Drop Shadow and a Gaussian Blur, which differ in under 4% of the pixels and nowhere inside the path. Its reach check compares with the object drawn without its raster effect, since Roughen alone reaches past the path. - The Vector Halftone test's bounds are a consistency check, and only holding no art tests the branch: its doc no longer says no solid is built (the free measure's halftone arm builds one). The overprint test's area check is labeled likewise. - Recoloring a traced shape bounds the area the renderer that drew the trace object measures too, inflated by half the stroke and a pixel. - The let-go test pins the nothing-to-draw branch: no frame, whole or part, began since the measure. - The parity doc's feature row names the stack overflow fix and its test; a group test's doc says what its fixture holds. --- crates/render/src/fx/tests.rs | 58 +++++++++++++++++++++--------- crates/render/src/tests/changed.rs | 19 ++++++---- crates/ui-egui/src/canvas/tests.rs | 8 +++++ docs/target-app-parity.md | 2 +- 4 files changed, 64 insertions(+), 23 deletions(-) diff --git a/crates/render/src/fx/tests.rs b/crates/render/src/fx/tests.rs index a754a25a5..7bdd8c3c6 100644 --- a/crates/render/src/fx/tests.rs +++ b/crates/render/src/fx/tests.rs @@ -674,8 +674,8 @@ fn the_renderer_measures_3d_art_with_crop_marks_as_drawn() { assert_eq!(Renderer::new().bounds_of(&symbols, &n), Some(free)); } -/// Vector Halftone draws dots in place of a 3D effect's solid: the renderer measures the object as -/// the free measure does, building no solid and holding no art to draw it with. +/// Vector Halftone draws dots in place of a 3D effect's solid: the renderer doesn't measure the +/// object by art it holds to draw it with, and measures it as the free measure does. #[test] fn the_renderer_measures_3d_art_with_vector_halftone_as_drawn() { let fx = vec![("threeD.extrude", json!({"depth": 30.0, "position": "isometricTop"})), (crate::effects::addons::halftone::HALFTONE, json!({}))]; @@ -684,9 +684,9 @@ fn the_renderer_measures_3d_art_with_vector_halftone_as_drawn() { let symbols = d.symbol_boxes(); let free = super::cull_bounds(&symbols, &n).unwrap(); let mut r = Renderer::new(); + // A consistency check: measured from the dots' art, the bounds come out the same. assert_eq!(r.bounds_of(&symbols, &n), Some(free)); - assert_eq!(r.stats.three_d_built, 0); - assert_eq!(r.measured_bytes(), 0); + assert_eq!(r.measured_bytes(), 0, "no art held"); } /// A renderer measures the art over each artboard as [`crate::encode::art_bounds_on`] does, and @@ -911,7 +911,7 @@ fn a_3d_effect_that_builds_nothing_is_measured_alike() { assert_eq!(Renderer::new().bounds_of(&symbols, &n), crate::brush_fx::cull_bounds(&symbols, &n)); } -/// A group with effects of its own that holds 3D type is measured once for every artboard of a +/// A group with effects of its own that holds an extruded path is measured once for every artboard of a /// pass: its bounds are kept after the first. #[test] fn a_group_holding_3d_art_is_measured_once_a_pass() { @@ -1748,9 +1748,12 @@ fn an_image_inner_glow_follows_its_visible_pixels() { /// A 3D effect on a path's fill or stroke that builds nothing (Revolve after Roughen) draws the /// item as without it: the art made for the object is drawn without it, not as an object with -/// effects again, and that art again, without end. The object draws exactly as without the 3D -/// effect, with a raster effect of its own (Drop Shadow, Gaussian Blur) too, which reaches past -/// the path. +/// effects again, and that art again, without end. A stroked object draws exactly as without the +/// 3D effect, with a raster effect of its own (Drop Shadow, Gaussian Blur) too. An object with +/// only a fill and a raster effect draws from that art, where without the 3D effect it draws as a +/// path, whose raster effects are clipped to the path's own geometry: there Roughen's spikes are +/// cut off, so the two differ in a few pixels at the edge and match inside the path. Either way +/// the raster effect reaches past what the object draws without it. #[test] fn a_3d_effect_that_builds_nothing_draws_the_item_as_without_it() { let rough = @@ -1758,15 +1761,25 @@ fn a_3d_effect_that_builds_nothing_draws_the_item_as_without_it() { let revolve = Effect { id: "threeD.revolve".into(), params: json!({}), visible: true }; let shadow = Effect { id: "stylize.dropShadow".into(), params: json!({}), visible: true }; let blur = Effect { id: "blur.gaussian".into(), params: json!({"radius": 2}), visible: true }; - for (object, on_fill) in [(vec![], true), (vec![], false), (vec![shadow], true), (vec![blur], false)] { - let draw = |three: bool| { + let cases = [ + (vec![], true, true), + (vec![], false, true), + (vec![shadow.clone()], true, true), + (vec![blur.clone()], false, true), + (vec![], true, false), + (vec![shadow], true, false), + (vec![blur], true, false), + ]; + for (object, on_fill, stroked) in cases { + let draw = |object: &[Effect], three: bool| { let mut d = Document::new(100.0, 100.0); - let ap = Appearance::basic(Paint::solid(Color::BLACK), Paint::solid(Color::rgb(0.0, 0.0, 1.0)), 2.0); + let stroke = if stroked { Paint::solid(Color::rgb(0.0, 0.0, 1.0)) } else { Paint::None }; + let ap = Appearance::basic(Paint::solid(Color::BLACK), stroke, 2.0); let n = Node::path(d.alloc_id(), shapes::rectangle(Rect::new(30.0, 30.0, 60.0, 60.0)), ap); let l = d.layers[0].id; let id = d.insert(Some(l), 0, n).unwrap(); let n = d.node_mut(id).unwrap(); - n.appearance.effects = object.clone(); + n.appearance.effects = object.to_vec(); let item = if on_fill { n.appearance.fill_mut().unwrap().effects = vec![]; &mut n.appearance.items[0] @@ -1777,11 +1790,24 @@ fn a_3d_effect_that_builds_nothing_draws_the_item_as_without_it() { assert!(crate::effects::revolve_art(d.node(id).unwrap()).is_none(), "builds nothing"); Renderer::new().render(&d, 100, 100, Affine::IDENTITY, &RenderOptions::default()).pixels }; - let (with, without) = (draw(true), draw(false)); - assert!(with == without, "{object:?} on the fill {on_fill}: as without it"); + let what = format!("{object:?} on the fill {on_fill}, stroked {stroked}"); + let (with, without) = (draw(&object, true), draw(&object, false)); + if stroked || object.is_empty() { + assert!(with == without, "{what}: as without it"); + } else { + // Under 4% of the pixels, all at the edge, where Roughen's spikes are: inside the path + // they match. + let differ: Vec<(usize, usize)> = + (0..100 * 100).filter(|i| with[i * 4..i * 4 + 4] != without[i * 4..i * 4 + 4]).map(|i| (i % 100, i / 100)).collect(); + assert!(differ.len() * 25 < 100 * 100, "{what}: {} pixels differ", differ.len()); + let inside = differ.iter().find(|(x, y)| (33..57).contains(x) && (33..57).contains(y)); + assert!(inside.is_none(), "{what}: differs inside the path at {inside:?}"); + } if !object.is_empty() { - let past = (0..100usize).flat_map(|y| (0..100usize).map(move |x| (x, y))).filter(|(x, y)| *x > 62 || *y > 62); - assert!(past.into_iter().any(|(x, y)| with[(y * 100 + x) * 4 + 3] > 0), "{object:?}: drawn past the path"); + // Ink where the object without its raster effect draws none. + let plain = draw(&[], true); + let past = (0..100 * 100).filter(|i| plain[i * 4 + 3] == 0 && with[i * 4 + 3] > 0).count(); + assert!(past > 0, "{what}: drawn past the object"); } } } diff --git a/crates/render/src/tests/changed.rs b/crates/render/src/tests/changed.rs index b7be9f715..67c476ee3 100644 --- a/crates/render/src/tests/changed.rs +++ b/crates/render/src/tests/changed.rs @@ -620,10 +620,16 @@ fn recolouring_a_traced_shape_changes_only_the_shape() { let (old, _, _, shape) = traced("outlinesWithSourceImage"); let mut new = old.clone(); new.node_mut(shape).unwrap().appearance.fill_mut().unwrap().paint = Paint::solid(Color::rgb(0.0, 0.0, 1.0)); - let a = area(redraw(&old, &new)); - // The shape, its 2 pt stroke and a pixel's anti-aliasing, at most. - let shape = Rect::new(80.0, 10.0, 90.0, 20.0).inflate(3.0, 3.0); - assert!(a.union(shape) == shape, "only the shape: {a:?}"); + // The shape, half its 2 pt stroke and a pixel's anti-aliasing, at most: measured by the + // renderer that drew the trace object in its view, and by the free measure. + let opts = RenderOptions { trace_views: true, ..Default::default() }; + let mut r = Renderer::new(); + r.render(&old, W, H, view(), &opts); + let measured = area(r.changed(&old, &new, &opts)); + let shape = Rect::new(80.0, 10.0, 90.0, 20.0).inflate(2.0, 2.0); + for (what, a) in [("measured", measured), ("free", area(redraw(&old, &new)))] { + assert!(a.union(shape) == shape, "{what}: only the shape: {a:?}"); + } } /// Templates and guides that the view hides draw nothing: an edit to them changes nothing. @@ -655,8 +661,8 @@ fn an_edit_to_what_the_view_hides_changes_nothing() { } /// Overprint Preview and separations draw objects through the inks they print, not through the -/// art a renderer keeps for them: a renderer measures an edit there as [`changed`] does and holds -/// none of the 3D art it made to measure it. +/// art a renderer keeps for them: a renderer measures an edit there holding none of the 3D art it +/// made to measure it, and as [`changed`] does. #[test] fn an_edit_measured_under_overprint_holds_no_art() { use vectorcraft_doc::{CharStyle, TextObject}; @@ -685,6 +691,7 @@ fn an_edit_measured_under_overprint_holds_no_art() { assert!(on_screen > 0, "the screen view holds the art it measured with"); let (measured, overprinted) = held(&RenderOptions { overprint_preview: true, ..Default::default() }); assert_eq!(overprinted, 0); + // A consistency check: the area itself doesn't tell the branch taken. assert_eq!(measured, area(changed(&old, &new)), "as changed measures it"); } diff --git a/crates/ui-egui/src/canvas/tests.rs b/crates/ui-egui/src/canvas/tests.rs index a74659893..ab196f3cf 100644 --- a/crates/ui-egui/src/canvas/tests.rs +++ b/crates/ui-egui/src/canvas/tests.rs @@ -10707,7 +10707,15 @@ fn art_measured_for_an_edit_not_drawn_here_is_let_go() { let now = app.session.active().unwrap().doc.clone(); app.canvas.renderer.changed(&shown, &now, &opts); assert!(app.canvas.renderer.measured_bytes() > 0, "worker {worker}: art held"); + if !worker { + app.canvas.last_ms = 7.25; + } shapes(&mut app, &ctx); + if !worker { + // Nothing in view to draw: no frame, whole or part, began since the measure. + assert_eq!(app.canvas.last_ms, 7.25, "not drawn whole"); + assert!(app.canvas.renderer.stats.three_d_built > 0, "no part drawn"); + } if let Some((rx, _)) = &jobs { assert!(rx.try_recv().is_ok(), "the worker draws it"); } diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 8022707cb..49f7d0c9b 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -41,7 +41,7 @@ Artboard Options forwards the active Artboard tool's Scale Artwork with Artboard | Date | Change | Summary | |---|---|---| -| 2026-11-09 | minor | Canvas: a 3D effect (Extrude & Bevel, Inflate, Rotate, Revolve) changed on type or a group draws again where its art was and is, measured by that art; the canvas measures through its renderer and draws the part with the art it made to measure it. Exports and Object › Rasterize take the solid of 3D type. Fit to Artwork Bounds and per-artboard SVG export measure every artboard through one renderer, building each solid once. Tests `tests::changed::a_3d_effect_changed_on_type_or_a_group_changes_where_its_solid_was`, `fx::tests::a_renderer_measures_the_art_on_each_artboard_building_each_solid_once`, `canvas::tests::an_edit_to_a_3d_effect_redraws_with_the_art_measured`. Scores and estimates unchanged. | +| 2026-11-09 | minor | Canvas: a 3D effect (Extrude & Bevel, Inflate, Rotate, Revolve) changed on type or a group draws again where its art was and is, measured by that art; the canvas measures through its renderer and draws the part with the art it made to measure it. Exports and Object › Rasterize take the solid of 3D type. Fit to Artwork Bounds and per-artboard SVG export measure every artboard through one renderer, building each solid once. A 3D effect on a fill or stroke that builds nothing (Revolve after Roughen), under an effect of the object's own, no longer overflows the stack on the canvas and in Object › Rasterize: the fill or stroke draws as without it. Tests `tests::changed::a_3d_effect_changed_on_type_or_a_group_changes_where_its_solid_was`, `fx::tests::a_renderer_measures_the_art_on_each_artboard_building_each_solid_once`, `canvas::tests::an_edit_to_a_3d_effect_redraws_with_the_art_measured`, `fx::tests::a_3d_effect_that_builds_nothing_draws_the_item_as_without_it`. Scores and estimates unchanged. | | 2026-11-07 | minor | File formats and Type core (#6147): `.ai`/EPS type keeps its paragraphs' tab stops (positions and alignments; leaders aren't read; at most 1,024 a paragraph and 2^20 a story), and right-aligned area type sets the text after a line's last tab against the right edge, the text before it at its stops; centred and justified lines are aligned as a whole as before; evidence added. Scores and estimates unchanged. | | 2026-11-07 | minor | File formats and Type core (#6137): `.ai`/EPS type from the text document keeps auto leading per line, its hyphenation options, may show more of the last lines the file sets in an area frame of its own, when growing the frame by its lines' deepest descent plus 0.05 pt shows them, and a line a soft return ends is justified; evidence added. Limits: frames grow only for area type of one frame, horizontal, by its lines' deepest descent plus 0.05 pt; type in a font that isn't installed, and hyphenation in a language that isn't imported, still lay out differently. Scores and estimates unchanged. | | 2026-11-08 | minor | Style panels: a double-click on a user character style's row opens its Character Style Options (`ui.charStyleOptions`), and on a paragraph style's row its Paragraph Style Options (rename); the Character Style Options dialog keeps a fixed width and scrolls its sections in a shorter window, so OK and Cancel stay on screen. Tests `panels::text_styles::tests::double_clicking_a_character_style_row_opens_its_options` and `dialogs::char_style_options::tests::the_sections_scroll_in_a_window_shorter_than_the_general_list`. Scores and estimates unchanged. |