Merge pull request #4452 from dotCooCoo/fix/culling-tests-bound-work-not-time

Tests: the nested-instance culling tests count the symbol art a frame measures

crates/doc/src/symbols.rs

@@ -71,6 +71,8 @@ pub struct SymbolBoxes<'a> {
     /// The effect reach of each symbol's art with no transform, measured past [`MAX_REACHES`]
     /// ([`Self::unplaced_reach`]).
     unplaced: RefCell<HashMap<String, f64>>,
+    /// How many times [`Self::measure_art`] has run a measure on symbol art ([`Self::art_measured`]).
+    runs: Cell<usize>,
 }
 
 impl Document {
@@ -87,6 +89,7 @@ impl Document {
             holders: Cell::new(0),
             farthest: RefCell::default(),
             unplaced: RefCell::default(),
+            runs: Cell::new(0),
         }
     }
 }
@@ -146,9 +149,16 @@ impl<'a> SymbolBoxes<'a> {
     pub fn measure_art<R>(&self, name: &str, measure: impl FnOnce(&Arc<Node>) -> R) -> Option<R> {
         self.depth(name)?;
         let sym = self.doc.symbols.iter().find(|s| s.name == name)?;
+        self.runs.set(self.runs.get() + 1);
         Some(measure(&sym.art))
     }
 
+    /// How many times [`Self::measure_art`] has run a measure on symbol art for these boxes, the
+    /// measures behind the cached art boxes and placed visual boxes included.
+    pub fn art_measured(&self) -> usize {
+        self.runs.get()
+    }
+
     /// The geometric box of the art of the symbol named `name` in symbol space, with the instances
     /// in the art measured by their own art boxes. `None` when [`Self::art`] is `None`: instances of
     /// the symbol are then measured by [`SYMBOL_SQUARE`].
@@ -460,6 +470,26 @@ mod tests {
         assert_eq!(pair.geometric_bounds_with(&d.symbol_boxes()), Some(Rect::new(100.0, 100.0, 154.0, 154.0)));
     }
 
+    #[test]
+    fn each_symbols_art_box_is_measured_once() {
+        // `Pair` holds two instances of `Dot`; three instances of `Pair` are measured twice each.
+        let mut d = Document::new(200.0, 200.0);
+        let dot = rect(&mut d, Rect::new(0.0, 0.0, 4.0, 4.0), 0.0);
+        symbol(&mut d, "Dot", vec![dot]);
+        let right = instance(&mut d, "Dot", Affine::translate((50.0, 0.0)));
+        let below = instance(&mut d, "Dot", Affine::translate((0.0, 50.0)));
+        symbol(&mut d, "Pair", vec![right, below]);
+        let pairs: Vec<Node> = (0..3).map(|i| instance(&mut d, "Pair", Affine::translate((f64::from(i) * 10.0, 0.0)))).collect();
+        let s = d.symbol_boxes().quick();
+        assert_eq!(s.art_measured(), 0);
+        for _ in 0..2 {
+            for p in &pairs {
+                p.geometric_bounds_with(&s);
+            }
+        }
+        assert_eq!(s.art_measured(), 2, "the art of Pair and of Dot");
+    }
+
     #[test]
     fn symbols_nested_32_deep_measure_as_the_square_whatever_was_measured_first() {
         // A chain of 40 symbols, each holding a 1 × 1 square and an instance of the next one 1 pt to

crates/render/src/frame.rs

@@ -95,6 +95,10 @@ pub(crate) static MISSING_INLINE_LOGGED: std::sync::atomic::AtomicBool = std::sy
 pub struct FrameStats {
     pub drawn: usize,
     pub culled: usize,
+    /// How many times the frame's symbol boxes measured symbol art ([`SymbolBoxes::art_measured`]):
+    /// the art boxes of symbols and how far the effects in their art paint past it. Pattern tiles
+    /// and placed documents measure with symbol boxes of their own, which are not counted.
+    pub symbol_art: usize,
     /// The drop shadows, outer glows and filtered contents the frame drew from rasters made before
     /// their object moved, or made over such rasters ([`RenderOptions::interacting`]), and those it
     /// drew at a drag's lower resolution ([`Self::previewed`]).

crates/render/src/renderer.rs

@@ -121,6 +121,7 @@ impl Renderer {
         if let Some(m) = opts.mask_view.and_then(|id| doc.node(id)).and_then(|n| n.extras().mask.as_deref()) {
             let pixels = self.mask_values(&frame, m, w, h).into_iter().flat_map(|v| [v, v, v, 255]).collect();
             self.end_frame();
+            self.stats.symbol_art = symbols.art_measured();
             self.stats.micros = now().saturating_sub(start);
             return Rendered { width: w as u32, height: h as u32, pixels };
         }
@@ -144,6 +145,7 @@ impl Renderer {
         if !inks {
             proof::post(&mut pixels, opts);
         }
+        self.stats.symbol_art = symbols.art_measured();
         self.stats.micros = now().saturating_sub(start);
         Rendered { width: w as u32, height: h as u32, pixels }
     }

crates/render/src/tests.rs

@@ -306,7 +306,10 @@ fn past_the_measured_placements_an_instance_is_culled_by_an_estimate_of_its_art_
 /// Symbols 0 to 29 each hold two instances of the next, sheared two ways, so that the 2^30
 /// instances drawn from the first are all placed differently. Symbol 30 holds a square with a drop
 /// shadow. An instance of symbol 0 lies far left of the view. With `effect`, every instance has
-/// that effect of its own. The renderer must cull the instance of symbol 0 within 30 s.
+/// that effect of its own. The renderer must cull the instance of symbol 0 measuring symbol art at
+/// most 20,000 times ([`FrameStats::symbol_art`]); measuring the art for every placement would take
+/// 2^30 measurements or more. The frame runs on a thread of its own, and the test fails when it
+/// has not finished after 300 s.
 fn nested_instances_are_culled_in_bounded_time(effect: Option<vectorcraft_doc::Effect>) {
     let mut d = Document::new(100.0, 100.0);
     let mut art = rect_node(&mut d, Rect::new(0.0, 0.0, 1.0, 1.0), Paint::solid(Color::BLACK), Paint::None, 0.0);
@@ -334,11 +337,13 @@ fn nested_instances_are_culled_in_bounded_time(effect: Option<vectorcraft_doc::E
     std::thread::spawn(move || {
         let mut r = Renderer::new();
         let img = r.render(&d, 100, 100, Affine::IDENTITY, &RenderOptions::default());
-        tx.send((img.pixel(50, 50), r.stats.culled)).ok();
+        tx.send((img.pixel(50, 50), r.stats.culled, r.stats.symbol_art)).ok();
     });
-    let (pixel, culled) = rx.recv_timeout(std::time::Duration::from_secs(30)).expect("the frame within 30 s");
+    let (pixel, culled, measured) = rx.recv_timeout(std::time::Duration::from_secs(300)).expect("the frame within 300 s");
     assert_eq!(pixel, [0, 0, 0, 0]);
     assert!(culled > 0);
+    assert!(measured > 0, "the frame stats report no symbol art measures");
+    assert!(measured <= 20_000, "symbol art measured {measured} times");
 }
 
 #[test]