Canvas redraw: what the renderer leaves out measured as nothing, alike

Review of #6290.

- An edit is measured as the renderer draws: an object it leaves out
  (hidden, a template layer or guide the view hides, drawn hidden, the
  instance a symbol is edited from) is measured as nothing, by one
  predicate the renderer and the measure share (node_checks::left_out,
  before culling).
- An Image Trace object's members are compared one by one again, its
  hidden source image, which some views draw, counted as shown: moving
  the source redraws where it was and is, and recoloring a traced shape
  redraws only the shape.
- The renderer checks the brush library between frames only while it
  keeps bounds measured with brushed strokes in 3D art.
- Tests: the sweep of 3D art's bounds counted while the art is held,
  and Fit to Artwork Bounds' menu answer kept for a revision.

crates/render/src/changed.rs

@@ -82,19 +82,19 @@ fn add_placed(symbols: &SymbolBoxes, n: &Node, files: &HashSet<String>, area: &m
 /// art also draws in the copies round it). Objects that an edit doesn't touch
 /// keep their identity, so this costs what changed, not the size of the document.
 pub fn changed(old: &Document, new: &Document) -> Changed {
-    changed_with(old, new, None, &[])
+    changed_with(old, new, None, &crate::RenderOptions::default())
 }
 
 impl crate::Renderer {
     /// [`changed`], measured through this renderer (`Renderer::bounds_of`): what it drew
     /// of `old` it measured to draw it, and what it measures of `new`, a 3D effect's art included,
-    /// it holds to draw that with `opts`; the objects `opts` draws hidden count as hidden objects
-    /// do, drawing nothing. The view that drew `old` with this renderer and `opts` asks this. With
+    /// it holds to draw that with `opts`; the objects `opts` leaves out (hidden, templates and
+    /// guides the view hides, the instance a symbol is edited from) draw nothing. The view that drew `old` with this renderer and `opts` asks this. With
     /// Overprint Preview or Separations Preview the renderer draws copies of the objects
     /// (`proof::prepare`), so they are measured afresh and nothing is held.
     pub fn changed(&mut self, old: &Document, new: &Document, opts: &crate::RenderOptions) -> Changed {
         let renderer = (!crate::proof::overprints(opts)).then_some(self);
-        changed_with(old, new, renderer, &opts.hidden)
+        changed_with(old, new, renderer, opts)
     }
 
     /// Let go of the art made to measure objects ([`crate::Renderer::changed`]) and held for this
@@ -114,11 +114,11 @@ impl crate::Renderer {
     }
 }
 
-fn changed_with(old: &Document, new: &Document, renderer: Option<&mut crate::Renderer>, hidden: &[NodeId]) -> Changed {
+fn changed_with(old: &Document, new: &Document, renderer: Option<&mut crate::Renderer>, opts: &crate::RenderOptions) -> Changed {
     if new.pattern_edit.is_some() || !old.same_outside_layers(new) {
         return Changed::Everything;
     }
-    let mut diff = Diff { old: old.symbol_boxes(), new: new.symbol_boxes(), area: None, renderer, hidden };
+    let mut diff = Diff { old: old.symbol_boxes(), new: new.symbol_boxes(), area: None, renderer, opts, source: None };
     diff.children(&old.layers, &new.layers);
     diff.area.map_or(Changed::Nothing, Changed::Area)
 }
@@ -136,13 +136,15 @@ pub fn view_pixels(area: Rect, view: Affine, w: u32, h: u32) -> Option<[u32; 4]>
 }
 
 /// [`changed`] in progress: the symbols' boxes of both versions, the area so far, the renderer
-/// that measures, if any (else each object is measured afresh), and the objects it draws hidden.
+/// that measures, if any (else each object is measured afresh), the options of the frames it
+/// measures for, and the hidden source image of the Image Trace object whose members it compares.
 struct Diff<'a, 'r> {
     old: SymbolBoxes<'a>,
     new: SymbolBoxes<'a>,
     area: Option<Rect>,
     renderer: Option<&'r mut crate::Renderer>,
-    hidden: &'r [NodeId],
+    opts: &'r crate::RenderOptions,
+    source: Option<NodeId>,
 }
 
 impl Diff<'_, '_> {
@@ -171,15 +173,17 @@ impl Diff<'_, '_> {
     /// only had anchors moved changed around them ([`moved_anchors`]). Any other object that
     /// changed counts whole, before and after.
     fn node(&mut self, old: &Arc<Node>, new: &Arc<Node>) {
-        if Arc::ptr_eq(old, new) || self.unseen(old) && self.unseen(new) {
+        if Arc::ptr_eq(old, new) || self.unseen(Side::Old, old) && self.unseen(Side::New, new) {
             return;
         }
-        // An Image Trace object draws its hidden source image in some views: it changed whole.
         if let (Some(a), Some(b)) = (members(old), members(new))
             && same_but_members(old, new)
-            && old.extras().trace.is_none()
         {
+            // An Image Trace object draws its hidden source image, its first member, in some views.
+            let source = old.extras().trace.is_some().then(|| a.first().map(|s| s.id)).flatten();
+            let outer = std::mem::replace(&mut self.source, source);
             self.children(a, b);
+            self.source = outer;
         } else if let Some((area, moved, anchors)) = moved_anchors(old, new) {
             if moved <= FEW_MOVED && anchors >= MANY_ANCHORS {
                 self.add(area);
@@ -195,16 +199,21 @@ impl Diff<'_, '_> {
         }
     }
 
-    /// Does `n` draw nothing: hidden, or drawn hidden? The members of a hidden group or layer are
-    /// never compared ([`Self::node`]).
-    fn unseen(&self, n: &Node) -> bool {
-        !n.visible || self.hidden.contains(&n.id)
+    /// Does `n` of the `side` version draw nothing, as the renderer leaves it out
+    /// (`node_checks::left_out`)? The source image of an Image Trace object draws in some views.
+    /// The members of a group or layer left out are never compared ([`Self::node`]).
+    fn unseen(&self, side: Side, n: &Node) -> bool {
+        let doc = match side {
+            Side::Old => self.old.document(),
+            Side::New => self.new.document(),
+        };
+        self.source != Some(n.id) && crate::node_checks::left_out(self.opts, doc, n)
     }
 
     /// The cull bounds of `n` of the `side` version, through the renderer when there is one;
     /// `None` when it draws nothing ([`Self::unseen`]), as nothing is measured to draw it.
     fn bounds(&mut self, side: Side, n: &Arc<Node>) -> Option<Rect> {
-        if self.unseen(n) {
+        if self.unseen(side, n) {
             return None;
         }
         let Diff { old, new, renderer, .. } = self;

crates/render/src/fx/renderer.rs

@@ -621,6 +621,10 @@ impl Renderer {
             if self.three_d_bounds.len() >= (2 * self.three_d_kept).max(1024) {
                 self.three_d_bounds.retain(|_, e| e.0.strong_count() > 0);
                 self.three_d_kept = self.three_d_bounds.len();
+                #[cfg(test)]
+                {
+                    self.three_d_sweeps += 1;
+                }
             }
             self.three_d_bounds.insert(Arc::as_ptr(&art) as usize, (Arc::downgrade(&art), None));
         }

crates/render/src/fx/tests.rs

@@ -819,6 +819,43 @@ fn a_measuring_pass_keeps_the_bounds_of_held_art_only() {
     assert!(r.three_d_bounds.len() <= 1024, "{} kept", r.three_d_bounds.len());
 }
 
+/// While its 3D art is held, a pass that only measures sweeps the bounds kept for 3D art each time
+/// they double, not on every new art past a size.
+#[test]
+fn a_measuring_pass_sweeps_the_bounds_of_held_art_as_they_double() {
+    let mut d = Document::new(100.0, 100.0);
+    let l = d.layers[0].id;
+    for i in 0..3000 {
+        let x = f64::from(i) * 30.0;
+        let mut n = Node::path(
+            d.alloc_id(),
+            shapes::rectangle(Rect::new(x, 30.0, x + 20.0, 50.0)),
+            Appearance::basic(Paint::solid(Color::BLACK), Paint::None, 0.0),
+        );
+        n.appearance.effects = vec![Effect { id: "threeD.extrude".into(), params: json!({"depth": 2.0}), visible: true }];
+        d.insert(Some(l), 0, n).unwrap();
+    }
+    let mut r = Renderer::new();
+    // Held here, as the art kept to draw is.
+    let held: Vec<_> = d.layers[0].children().unwrap().iter().map(|n| r.measured_fx_art(&d, n)).collect();
+    assert_eq!((held.len(), r.three_d_bounds.len()), (3000, 3000));
+    assert!(r.three_d_sweeps <= 2, "{} sweeps", r.three_d_sweeps);
+}
+
+/// A renderer checks the brush library between frames only while it keeps bounds measured with
+/// brushed strokes in 3D art.
+#[test]
+fn the_brush_library_is_checked_only_with_3d_bounds_kept() {
+    let mut d = doc_with(Rect::new(30.0, 30.0, 60.0, 60.0), Color::BLACK, vec![]);
+    let mut r = Renderer::new();
+    r.render(&d, 100, 100, Affine::IDENTITY, &RenderOptions::default());
+    assert_eq!(r.brush_library, None);
+    let id = d.layers[0].children().unwrap()[0].id;
+    d.node_mut(id).unwrap().appearance.effects = vec![Effect { id: "threeD.extrude".into(), params: json!({"depth": 20.0}), visible: true }];
+    r.render(&d, 100, 100, Affine::IDENTITY, &RenderOptions::default());
+    assert!(r.brush_library.is_some());
+}
+
 /// An object whose 3D effect builds nothing (Revolve on a group) is measured by the renderer as the
 /// free measure does it: as the renderer draws it, by its members.
 #[test]

crates/render/src/lib.rs

@@ -85,8 +85,9 @@ pub struct Renderer {
     /// last checked against (see [`Self::check_plugins`]).
     plugin_revision: u64,
     /// The key of the brush library ([`brush_fx::BrushCache::library_key`]) that `geom`'s bounds
-    /// of 3D art measured brushed strokes by (see [`Self::check_symbols`]).
-    brush_library: u64,
+    /// of 3D art measured brushed strokes by (see [`Self::check_symbols`]), while `geom` may hold
+    /// such bounds (`Some`).
+    brush_library: Option<u64>,
     /// Is a live-effect plug-in that paints its pieces installed (found when `plugin_revision`
     /// changes)?
     painting_plugins: bool,
@@ -178,6 +179,9 @@ pub struct Renderer {
     /// How many entries `three_d_bounds` kept when it last let go of the art no longer held: it
     /// does so again when it holds twice as many.
     three_d_kept: usize,
+    /// How many times `three_d_bounds` was swept for art no longer held between frames.
+    #[cfg(test)]
+    three_d_sweeps: usize,
     /// Symbol art as instances and inline graphics draw it (see [`Self::placed_art`]).
     placed_arts: FrameCache<PlacedKey, PlacedEntry>,
     /// The bitmap clock of [`placed_document`] when this frame first drew a placed document from

crates/render/src/node_checks.rs

@@ -107,6 +107,32 @@ pub(crate) fn holds_instance(n: &Node) -> bool {
     holds
 }
 
+/// Is `a` left out of a frame `doc` draws with `opts`, before any culling: hidden, a template or
+/// guide the view hides, drawn hidden, or the instance its symbol is being edited from?
+pub(crate) fn left_out(opts: &crate::RenderOptions, doc: &vectorcraft_doc::Document, a: &Node) -> bool {
+    let view_item = (opts.skip_templates || opts.hide_templates) && matches!(a.kind, NodeKind::Layer { template: true, .. })
+        || (opts.skip_templates || opts.hide_guides) && matches!(a.kind, NodeKind::Path { guide: true, .. });
+    !a.visible || view_item || opts.hidden.contains(&a.id) || edited_instance(opts, doc, a.id)
+}
+
+/// Is `id` the symbol instance that `doc`'s symbol is edited from in isolation (drawn without it)?
+pub(crate) fn edited_instance(opts: &crate::RenderOptions, doc: &vectorcraft_doc::Document, id: vectorcraft_doc::NodeId) -> bool {
+    opts.isolated.is_some() && doc.symbol_edit.as_ref().is_some_and(|e| e.instance == Some(id))
+}
+
+/// Does [`crate::Renderer::bounds_of`] measure `n` by the 3D art the renderer draws and keeps for
+/// it: a Revolve, Extrude & Bevel, Inflate or Rotate without Crop Marks or Vector Halftone, which
+/// draw other art?
+pub(crate) fn measures_three_d(n: &Node) -> bool {
+    (effects::has_revolve(n) || effects::has_solid(n)) && !effects::has_crop_marks(n) && !effects::addons::halftone::has_halftone(n)
+}
+
+/// Does `n` hold an object that [`crate::Renderer::bounds_of`] measures by its 3D effect's art
+/// ([`measures_three_d`])?
+pub(crate) fn holds_measured_three_d(n: &Node) -> bool {
+    measures_three_d(n) || n.children().is_some_and(|c| c.iter().any(|c| holds_measured_three_d(c)))
+}
+
 /// Does `n` hold an object with a visible plug-in effect ([`effects::has_plugin_effect`]), in it or
 /// in the opacity masks of the objects in it?
 pub(crate) fn holds_plugin_effect(n: &Node) -> bool {

crates/render/src/renderer.rs

@@ -45,7 +45,7 @@ impl Renderer {
             kept: PtrMap::default(),
             symbols: vec![],
             plugin_revision: 0,
-            brush_library: 0,
+            brush_library: None,
             painting_plugins: false,
             paint_names: 0,
             strokes: PtrMap::default(),
@@ -77,6 +77,8 @@ impl Renderer {
             pass_bounds: PtrMap::default(),
             three_d_bounds: PtrMap::default(),
             three_d_kept: 0,
+            #[cfg(test)]
+            three_d_sweeps: 0,
             placed_arts: FrameCache::new(lru::PLACED_ART_BUDGET, 256),
             placed_frame: None,
             nested: 0,

crates/render/src/renderer/nodes.rs

@@ -5,20 +5,7 @@ use super::{
     shape_masked_image, single_plain_fill, text_geom_snapped, white_shape_mask,
 };
 use crate::kept_paths::{self, KeptPath};
-
-/// Does [`Renderer::bounds_of`] measure `n` by the 3D art this renderer draws and keeps for it: a
-/// Revolve, Extrude & Bevel, Inflate or Rotate without Crop Marks or Vector Halftone, which draw
-/// other art?
-fn measures_three_d(n: &Node) -> bool {
-    (effects::has_revolve(n) || effects::has_solid(n)) && !effects::has_crop_marks(n) && !effects::addons::halftone::has_halftone(n)
-}
-
-/// Does `n` hold an object that [`Renderer::bounds_of`] measures by its 3D effect's art?
-fn holds_measured_three_d(n: &Node) -> bool {
-    let mut holds = false;
-    n.walk(&mut |c| holds |= measures_three_d(c));
-    holds
-}
+use crate::node_checks::{holds_measured_three_d, measures_three_d};
 
 impl Renderer {
     /// Cached cull bounds of a node (containers union their cached children), symbol instances
@@ -55,6 +42,9 @@ impl Renderer {
             {
                 // Culling leaves room for brush art as for any object ([`brush_fx::cull_bounds`]);
                 // the measure is made again when the brush library changes ([`Self::check_symbols`]).
+                if self.brush_library.is_none() {
+                    self.brush_library = Some(self.brushes.library_key_now(symbols.document(), self.stamp));
+                }
                 let pad = brush_fx::pad(symbols, a);
                 b.map(|b| fx::with_own_raster_reach(symbols, a, b).inflate(pad, pad))
             }
@@ -119,11 +109,13 @@ impl Renderer {
             self.geom.retain(|_, e| !holds_instance(&e.node));
             self.symbols = doc.symbols.iter().map(|s| (s.name.clone(), Arc::downgrade(&s.art))).collect();
         }
-        // That art is measured with the art its brushed strokes draw ([`Self::bounds_of`]).
-        let brushes = self.brushes.library_key_now(doc, self.stamp);
-        if brushes != self.brush_library {
-            self.brush_library = brushes;
+        // That art is measured with the art its brushed strokes draw ([`Self::bounds_of`]). Without
+        // such bounds, nothing is checked.
+        if let Some(was) = self.brush_library
+            && self.brushes.library_key_now(doc, self.stamp) != was
+        {
             self.geom.retain(|_, e| !holds_measured_three_d(&e.node));
+            self.brush_library = None;
         }
     }
 
@@ -228,15 +220,13 @@ impl Renderer {
     /// drawn (with its masks and effects), since the editing layer holds a copy of its art in its
     /// place.
     fn edited_instance(f: &Frame, id: NodeId) -> bool {
-        f.opts.isolated.is_some() && f.doc.symbol_edit.as_ref().is_some_and(|e| e.instance == Some(id))
+        crate::node_checks::edited_instance(f.opts, f.doc, id)
     }
 
     /// Whether `a` is left out of this frame: hidden, a skipped template or guide, the instance a
     /// symbol is edited from, or culled.
     fn skipped(&mut self, f: &Frame, a: &Arc<Node>) -> bool {
-        let skipped_view_item = (f.opts.skip_templates || f.opts.hide_templates) && matches!(a.kind, NodeKind::Layer { template: true, .. })
-            || (f.opts.skip_templates || f.opts.hide_guides) && matches!(a.kind, NodeKind::Path { guide: true, .. });
-        if !a.visible || skipped_view_item || f.opts.hidden.contains(&a.id) || Self::edited_instance(f, a.id) {
+        if crate::node_checks::left_out(f.opts, f.doc, a) {
             return true;
         }
         match self.bounds_of(f.symbols, a) {

crates/render/src/tests/changed.rs

@@ -539,3 +539,67 @@ fn a_renderer_measures_an_edit_with_the_3d_art_it_draws() {
     assert_eq!(built, 0, "drawn with the art made to measure it");
     assert_eq!(apart, 1, "changed keeps nothing for the renderer");
 }
+
+/// An Image Trace object shown with its source image (`view`): its hidden source, at the left, and
+/// two traced shapes, at the right. The ids of the source and of the second shape.
+fn traced(view: &str) -> (Document, NodeId, NodeId) {
+    let mut d = Document::new(W as f64, H as f64);
+    let mut source = rect(&mut d, Rect::new(10.0, 10.0, 40.0, 40.0), Color::rgb(0.5, 0.5, 0.5));
+    source.visible = false;
+    let a = rect(&mut d, Rect::new(60.0, 10.0, 70.0, 20.0), Color::rgb(0.0, 0.5, 0.0));
+    let b = rect(&mut d, Rect::new(80.0, 10.0, 90.0, 20.0), Color::rgb(1.0, 0.5, 0.0));
+    let ids = (source.id, b.id);
+    let mut g = Node::group(d.alloc_id(), vec![Arc::new(source), Arc::new(a), Arc::new(b)]);
+    g.extras_mut().trace = Some(Box::new(serde_json::json!({ "view": view })));
+    let l = d.layers[0].id;
+    d.insert(Some(l), 0, g).unwrap();
+    (d, ids.0, ids.1)
+}
+
+#[test]
+fn moving_a_trace_objects_source_changes_where_it_shows() {
+    for view in ["sourceImage", "outlinesWithSourceImage"] {
+        let (old, source, _) = traced(view);
+        let mut new = old.clone();
+        new.node_mut(source).unwrap().transform(Affine::translate((5.0, 30.0)), false);
+        let a = area(redraw(&old, &new));
+        assert!(a.contains(Point::new(12.0, 12.0)) && a.contains(Point::new(44.0, 69.0)), "{view}: before and after: {a:?}");
+    }
+}
+
+#[test]
+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));
+    assert!(a.x0 > 72.0, "not the source or the other shape: {a:?}");
+}
+
+/// Templates and guides that the view hides draw nothing: an edit to them changes nothing.
+#[test]
+fn an_edit_to_what_the_view_hides_changes_nothing() {
+    let mut old = Document::new(W as f64, H as f64);
+    let template = Node::layer(old.alloc_id(), "Template", vectorcraft_doc::LayerColor::Preset(1));
+    let template = old.insert(None, usize::MAX, template).unwrap();
+    if let Some(NodeKind::Layer { template: t, .. }) = old.node_mut(template).map(|n| &mut n.kind) {
+        *t = true;
+    }
+    let traced = rect(&mut old, Rect::new(10.0, 10.0, 40.0, 40.0), Color::rgb(0.5, 0.5, 0.5));
+    let traced = old.insert(Some(template), 0, traced).unwrap();
+    let mut guide = rect(&mut old, Rect::new(60.0, 10.0, 90.0, 40.0), Color::BLACK);
+    if let NodeKind::Path { guide: g, .. } = &mut guide.kind {
+        *g = true;
+    }
+    let l = old.layers[0].id;
+    let guide = old.insert(Some(l), 0, guide).unwrap();
+    let mut new = old.clone();
+    for id in [traced, guide] {
+        new.node_mut(id).unwrap().transform(Affine::translate((5.0, 5.0)), false);
+    }
+    let opts = RenderOptions { hide_templates: true, hide_guides: true, ..Default::default() };
+    let mut r = Renderer::new();
+    r.render(&old, W, H, view(), &opts);
+    assert_eq!(r.changed(&old, &new, &opts), Changed::Nothing);
+    assert!(matches!(changed(&old, &new), Changed::Area(_)), "shown, they change");
+}

crates/ui-egui/src/panels/artboards/tests.rs

@@ -1656,6 +1656,29 @@ fn the_open_menu_reads_what_the_right_click_measured() {
     assert!(labels.contains(&"Delete Empty Artboards") && !labels.contains(&"Fit to Artwork Bounds"), "{labels:?}");
 }
 
+/// Whether Fit to Artwork Bounds has art to fit is measured once for a revision of the document:
+/// the canvas menu asks on every frame it is open.
+#[test]
+fn fit_to_art_is_measured_once_a_revision() {
+    let mut app = menu_app(2);
+    assert!(!super::fit_has_art(&app, &[0, 1]));
+    // Art on the first artboard, the revision left as it was: the answer kept stands.
+    let st = app.session.active_mut().unwrap();
+    let mut doc = (*st.doc).clone();
+    let l = doc.layers[0].id;
+    let id = doc.alloc_id();
+    let art = vectorcraft_doc::Node::path(
+        id,
+        vectorcraft_geom::shapes::rectangle(vectorcraft_geom::Rect::new(10.0, 10.0, 20.0, 20.0)),
+        vectorcraft_doc::Appearance::basic(vectorcraft_color::Paint::solid(vectorcraft_color::Color::BLACK), vectorcraft_color::Paint::None, 0.0),
+    );
+    doc.insert(Some(l), 0, art).unwrap();
+    st.doc = std::sync::Arc::new(doc);
+    assert!(!super::fit_has_art(&app, &[0, 1]), "kept for the revision");
+    app.session.active_mut().unwrap().revision += 1;
+    assert!(super::fit_has_art(&app, &[0, 1]), "measured again");
+}
+
 /// `artboard.selectAll` chooses the Artboard tool first when another tool is in use, and fails
 /// without a document.
 #[test]