Tests: make timing-dependent render and UI tests deterministic (#2812)

Seven tests failed at random on slow or busy machines, main included
(FreeBSD run 38834256439 at 5fdd820b). They now count the work or wait
for the state they check; no assertion is loosened and nothing retries.

- spot_colour_images_run_their_tint_transform_once_per_sample_value
  counts tint-transform runs (exactly 256, not one per pixel) through
  vendored hayro-interpret patch (34), color::tint_transform_runs, a
  thread-local counter, instead of a 10 s deadline.
- shadings_with_large_sampled_functions_paint_quickly: no counter tells
  the flat table from the hash map (both read every sample per build),
  so the build each paint does is timed against reading its samples on
  the same thread, fastest of 8 rounds each, and must stay under 10x
  (flat table 1.1x, hash map 109x), instead of a 10 s deadline.
- page_contents_and_nested_forms_share_one_budget: the budget is
  checked by its warnings; the deadline is only a hang guard, now 600 s.
- The pen-stroke tests hold finished renders back from the view
  (DocView::hold_rasters, for tests) while they look at the settling
  stroke, then wait by state for its raster.
- files_and_quit_from_the_operating_system waits for the tab before
  deleting the file the open worker reads.
- image_pixels_follow_the_frame_while_a_corner_is_dragged waits for the
  page raster on screen before each drag.

Product fix (#1858): a released drawing stayed painted only until any
raster of its page arrived. In full screen its edit is applied a frame
later, so a raster already under way could end the preview first. A
settling drawing is now marked committed when its AddAnnotation lands,
only rasters after that end its preview, and a failed edit drops it.

crates/render/src/raster.rs

@@ -4586,7 +4586,11 @@ trailer << /Root 1 0 R >>
             let mut r = PageRenderer::new(Arc::new(pdf), RenderConfig::default());
             let _ = tx.send(r.render(RenderRequest { page: 0, kind: RequestKind::Pixels, tile: None, scale: 1.0, tag: 0, ..Default::default() }));
         });
-        let page = rx.recv_timeout(std::time::Duration::from_secs(60)).expect("a self-painting form must not stall the renderer");
+        // Only a guard against a hang: the shared budget is checked by the warnings below, which
+        // come out the same on every machine. The render decodes and reads 512 MiB of content,
+        // a third of a second on a desktop, and a busy FreeBSD VM in CI ran past the minute this
+        // used to allow.
+        let page = rx.recv_timeout(std::time::Duration::from_secs(600)).expect("a self-painting form must not stall the renderer");
         assert!(page.error.is_none(), "{:?}", page.error);
         assert!(page.warnings.contains(&RenderWarning::ContentTruncated), "the skipped nested content is observable");
         assert_eq!(
@@ -4657,30 +4661,73 @@ trailer << /Root 1 0 R >>
     /// pdf.js corpus spent most of its 1.5 s hashing them. The samples are now one flat array
     /// indexed by strides: this page paints a 2,097,152-sample ramp (the most a table may hold)
     /// 30 times, which took 38 seconds through the hash map.
+    ///
+    /// The build each paint does is timed against reading the function's samples, the part of it
+    /// no table can skip, not against a deadline: through the hash map it took about a hundred
+    /// times as long as the read, through the flat array about as long. A deadline can't tell
+    /// them apart on every machine: the hash map's 30 paints took 14 s on one desktop, the flat
+    /// array's over 10 s on a busy FreeBSD VM in CI. No counter does either (both read every
+    /// sample on every build), so the test compares two times taken on this thread, the fastest
+    /// of several rounds of each: a stall from other work (other tests, swapping) would have to
+    /// slow every build and spare a read to fail it.
     #[test]
     fn shadings_with_large_sampled_functions_paint_quickly() {
+        use hayro::hayro_interpret::Function;
+        use hayro_syntax::bit_reader::BitReader;
+        use hayro_syntax::object::{FromBytes, Object};
+        use std::time::{Duration, Instant};
+
         const SAMPLES: usize = 1 << 21;
         let samples = zlib_zeros(SAMPLES);
+        let function = format!(
+            "<< /FunctionType 0 /Domain [0 1] /Range [0 1] /Size [{SAMPLES}] /BitsPerSample 8 /Filter /FlateDecode /Length {} >>",
+            samples.len()
+        );
         // Black ramp on the left half, 30 times over; a red square after it.
         let content = "q 0 0 20 40 re W n /Sh0 sh Q\n".repeat(30) + "1 0 0 rg 36 36 4 4 re f";
         let mut pdf = b"%PDF-1.7\n1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj\n2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj\n3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 40 40] /Contents 4 0 R /Resources << /Shading << /Sh0 5 0 R >> >> >> endobj\n".to_vec();
         pdf.extend_from_slice(format!("4 0 obj << /Length {} >> stream\n{content}\nendstream endobj\n", content.len()).as_bytes());
         pdf.extend_from_slice(b"5 0 obj << /ShadingType 2 /ColorSpace /DeviceGray /Coords [0 0 40 0] /Function 6 0 R >> endobj\n");
-        pdf.extend_from_slice(
-            format!("6 0 obj << /FunctionType 0 /Domain [0 1] /Range [0 1] /Size [{SAMPLES}] /BitsPerSample 8 /Filter /FlateDecode /Length {} >> stream\n", samples.len())
-                .as_bytes(),
-        );
+        pdf.extend_from_slice(format!("6 0 obj {function} stream\n").as_bytes());
         pdf.extend_from_slice(&samples);
         pdf.extend_from_slice(b"\nendstream endobj\ntrailer << /Root 1 0 R >>\n%%EOF\n");
-        let (tx, rx) = std::sync::mpsc::channel();
-        std::thread::spawn(move || {
-            let _ = tx.send(render_first_page(pdf));
-        });
-        let page =
-            rx.recv_timeout(std::time::Duration::from_secs(10)).expect("painting a large sampled function must not rebuild a hash map each time");
+        let page = render_first_page(pdf);
         assert_eq!(rgb_at(&page.rgba, 40, 5, 20), [0, 0, 0], "the ramp is drawn");
         assert_eq!(rgb_at(&page.rgba, 40, 30, 20), [255, 255, 255], "outside the clip");
         assert_eq!(rgb_at(&page.rgba, 40, 38, 2), [255, 0, 0], "the rest of the page draws");
+
+        // What each paint does: build the function from its stream. Timed against reading the
+        // stream's samples, the part of the build no table can skip.
+        let mut bytes = format!("{function} stream\n").into_bytes();
+        bytes.extend_from_slice(&samples);
+        bytes.extend_from_slice(b"\nendstream");
+        let object = Object::from_bytes(&bytes).expect("a stream object");
+        let Object::Stream(stream) = &object else { panic!("the function is a stream") };
+        let (mut build, mut read) = (Duration::MAX, Duration::MAX);
+        for _ in 0..8 {
+            let started = Instant::now();
+            let function = Function::new(&object);
+            assert!(function.is_some(), "a sampled function");
+            drop(function);
+            build = build.min(started.elapsed());
+
+            let started = Instant::now();
+            let decoded = stream.decoded().expect("the samples decode");
+            let mut reader = BitReader::new(&decoded);
+            let mut values = Vec::new();
+            while values.len() < SAMPLES
+                && let Some(value) = reader.read(8)
+            {
+                values.push(value);
+            }
+            assert_eq!(std::hint::black_box(&values).len(), SAMPLES);
+            drop(values);
+            read = read.min(started.elapsed());
+        }
+        assert!(
+            build < read * 10,
+            "building a {SAMPLES}-sample function took {build:?}, over ten times the {read:?} reading its samples takes (a hash map took a hundred times)"
+        );
     }
 
     /// Patch 31's flat table gives every sampled function the values the hash map gave: points
@@ -6186,6 +6233,8 @@ endstream endobj
     /// each of a 3304 × 4105 background's pixels, every page took four seconds and 430 MB to
     /// draw, again after each field was filled. This image (2048 × 2048, a 9,000-operator
     /// program) took about 40 seconds that way; from a 256-entry table it takes milliseconds.
+    /// The runs are counted (patch 34) rather than timed: a deadline also failed when a slow,
+    /// busy CI machine took longer than it allowed, and the count is the same on every machine.
     #[test]
     fn spot_colour_images_run_their_tint_transform_once_per_sample_value() {
         const SIDE: usize = 2048;
@@ -6202,11 +6251,11 @@ endstream endobj
         let function = format!("<< /FunctionType 4 /Domain [0 1] /Range [0 1 0 1 0 1] /Length {} >>", program.len());
         let pdf =
             image_cache_pdf("q 130 0 0 70 0 0 cm /A Do Q", "<< /XObject << /A 5 0 R >> >>", &[(&image, &data), (&function, program.as_bytes())]);
-        let (tx, rx) = std::sync::mpsc::channel();
-        std::thread::spawn(move || {
-            let _ = tx.send(render_first_page(pdf));
-        });
-        let out = rx.recv_timeout(std::time::Duration::from_secs(10)).expect("a spot-colour image must not run its tint transform for every pixel");
+        // The count is per thread, and this render runs on this one.
+        let before = hayro::hayro_interpret::color::tint_transform_runs();
+        let out = render_first_page(pdf);
+        let runs = hayro::hayro_interpret::color::tint_transform_runs() - before;
+        assert_eq!(runs, 256, "the tint transform runs once per possible sample value, not once for each of the {} pixels", SIDE * SIDE);
         let w = out.width as usize;
         assert_eq!(rgb_at(&out.rgba, w, 10, 35), [255, 0, 0], "tint 0");
         assert_eq!(rgb_at(&out.rgba, w, 120, 35), [0, 255, 0], "tint 1");

crates/ui-egui/src/canvas.rs

@@ -452,6 +452,9 @@ pub struct DocView {
     last_press_time: f64,
     last_press_pos: Option<Pos2>,
     last_queue: Vec<RenderRequest>,
+    /// Finished renders wait in the pool instead of reaching the view (tests): what the view
+    /// shows before a raster arrives can be looked at however fast the renderer is.
+    pub hold_rasters: bool,
     viewport_w: f32,
     viewport_h: f32,
     page_count: usize,
@@ -729,6 +732,7 @@ impl DocView {
             last_press_time: 0.0,
             last_press_pos: None,
             last_queue: Vec::new(),
+            hold_rasters: false,
             viewport_w: 800.0,
             viewport_h: 600.0,
             page_count: info.pages.len(),
@@ -1733,7 +1737,9 @@ impl DocView {
             pool.set_queue(queue.clone());
             self.last_queue = queue;
         }
-        self.receive(ctx, pool);
+        if !self.hold_rasters {
+            self.receive(ctx, pool);
+        }
         if !self.last_queue.is_empty() {
             ctx.request_repaint_after(std::time::Duration::from_millis(30));
         }
@@ -5580,6 +5586,30 @@ trailer << /Root 1 0 R >>
         assert_eq!(v.pages.get(&0).unwrap().tag, STALE_TAG);
     }
 
+    /// #1858: a released drawing stays painted from its points until a raster that has it
+    /// arrives. A page raster that lands before the drawing's edit is applied (in full screen the
+    /// edit waits for the next frame, and a render already under way can finish first) is of the
+    /// document without it, and must not end the preview.
+    #[test]
+    fn a_raster_from_before_a_drawing_is_committed_keeps_it_drawn() {
+        let ctx = egui::Context::default();
+        let info = thumbnail_info(1);
+        let mut v = DocView::new(DocId(1), &info, ViewDefaults::default());
+        let req = RenderRequest { scale: 1.0, tag: 1000, ..Default::default() };
+        v.comments.settle(0.0, 0, crate::comments::CommentTool::Ink, &[[10.0, 10.0], [40.0, 30.0]]);
+        v.last_queue = vec![req];
+        assert!(v.receive_result(&ctx, solid_result(req, 10, 10)) > 0, "the raster is taken");
+        assert_eq!(v.comments.settling_len(), 1, "a raster from before the commit doesn't have the stroke");
+        v.comments.drawings_committed();
+        v.last_queue = vec![req];
+        assert!(v.receive_result(&ctx, solid_result(req, 10, 10)) > 0);
+        assert_eq!(v.comments.settling_len(), 0, "a raster after the commit draws it");
+        // An edit that fails leaves nothing to wait for.
+        v.comments.settle(0.0, 0, crate::comments::CommentTool::Ink, &[[10.0, 10.0], [40.0, 30.0]]);
+        v.comments.drawings_refused();
+        assert_eq!(v.comments.settling_len(), 0, "a refused drawing isn't left painted");
+    }
+
     #[test]
     fn page_and_tile_admission_accounts_for_bytes_before_count_limits() {
         let mut info = thumbnail_info(40);

crates/ui-egui/src/comments.rs

@@ -420,6 +420,10 @@ pub(crate) struct Settling {
     pub points: Vec<[f64; 2]>,
     /// Frame time (seconds) at release; a raster that never arrives must not leave a ghost.
     pub since: f64,
+    /// Its annotation is in the document, so a raster of the page from now on draws it. Until
+    /// then a raster is of the document without it (in full screen the edit is applied the frame
+    /// after the release, and a raster already on its way can arrive in between).
+    pub committed: bool,
 }
 
 /// Longest a released drawing stays painted without its page raster, and how many are kept.
@@ -428,7 +432,7 @@ const SETTLE_MAX: usize = 16;
 
 impl CommentView {
     /// Remember a released drag drawing so it stays on screen until its page is re-rendered.
-    fn settle(&mut self, now: f64, page: usize, tool: CommentTool, points: &[[f64; 2]]) {
+    pub(crate) fn settle(&mut self, now: f64, page: usize, tool: CommentTool, points: &[[f64; 2]]) {
         if !matches!(tool, CommentTool::Ink | CommentTool::Line | CommentTool::Arrow | CommentTool::Rectangle | CommentTool::Oval) {
             return;
         }
@@ -436,7 +440,7 @@ impl CommentView {
         if self.settling.len() >= SETTLE_MAX {
             self.settling.remove(0);
         }
-        self.settling.push(Settling { page, tool, points: points.to_vec(), since: now });
+        self.settling.push(Settling { page, tool, points: points.to_vec(), since: now, committed: false });
     }
 
     /// How many released drawings are still painted from their points (waiting for a raster).
@@ -444,9 +448,22 @@ impl CommentView {
         self.settling.len()
     }
 
-    /// A fresh raster of `page` is on screen: it draws the committed annotation itself.
+    /// A fresh raster of `page` is on screen: it draws the committed annotations itself. One
+    /// still waiting for its edit isn't in that raster, and stays drawn from its points.
     pub(crate) fn page_rendered(&mut self, page: usize) {
-        self.settling.retain(|s| s.page != page);
+        self.settling.retain(|s| s.page != page || !s.committed);
+    }
+
+    /// The edit adding the released drawings was applied: rasters from now on draw them.
+    pub(crate) fn drawings_committed(&mut self) {
+        for s in &mut self.settling {
+            s.committed = true;
+        }
+    }
+
+    /// The edit adding the released drawings failed: nothing will draw them.
+    pub(crate) fn drawings_refused(&mut self) {
+        self.settling.retain(|s| s.committed);
     }
 }
 

crates/ui-egui/src/editing.rs

@@ -52,7 +52,10 @@ impl PdfCraftApp {
                 let Some(doc) = self.session.get(id) else { return true };
                 let info = &doc.info;
                 let view = &mut self.views[i];
-                if !matches!(edit, Edit::AddAnnotation(_)) {
+                if matches!(edit, Edit::AddAnnotation(_)) {
+                    // A released drawing's raster can come now: it will have the annotation.
+                    view.comments.drawings_committed();
+                } else {
                     // Only a new drawing waits on its raster; any other edit may remove or move it.
                     view.comments.settling.clear();
                 }
@@ -163,6 +166,10 @@ impl PdfCraftApp {
                 true
             }
             Err(e) => {
+                if let Some(view) = self.views.get_mut(i) {
+                    // A drawing whose annotation wasn't added must not stay painted.
+                    view.comments.drawings_refused();
+                }
                 let message = if matches!(edit, Edit::MoveObjects { .. }) { crate::object_ui::move_error(&e) } else { e.to_string() };
                 self.notify_fmt("{label} failed: {e}", &[("label", &crate::i18n::edit_label(&label)), ("e", &message)]);
                 false

crates/ui-egui/tests/comments.rs

@@ -7,6 +7,8 @@ use egui_kittest::kittest::Queryable;
 use pdfcraft_render::Annotation;
 use pdfcraft_ui_egui::{PdfCraftApp, QuickTool};
 
+mod support;
+
 /// Two 300×200 pages of Helvetica text.
 const TEXT_FIXTURE: &[u8] = b"%PDF-1.7
 1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj
@@ -406,8 +408,11 @@ fn shapes_and_ink_are_drawn_by_dragging() {
     assert!((r[0] - 40.0).abs() < 3.0 && (r[1] - 40.0).abs() < 3.0 && (r[2] - 140.0).abs() < 3.0 && (r[3] - 100.0).abs() < 3.0, "{r:?}");
 }
 
-/// Press, drag and release a stroke, running only one frame after the release.
+/// Press, drag and release a stroke, running only one frame after the release. Finished
+/// renders are held back from the view (`hold_rasters`), so the raster the commit asks for can't
+/// arrive before the test looks, however fast it renders; a test lets it through by clearing it.
 fn stroke_without_settling(h: &mut Harness<'static, PdfCraftApp>, a: (f32, f32), b: (f32, f32)) {
+    h.state_mut().views[0].hold_rasters = true;
     let (from, to) = (at(h, a.0, a.1), at(h, b.0, b.1));
     h.hover_at(from);
     h.run_steps(1);
@@ -429,7 +434,10 @@ fn a_released_pen_stroke_stays_drawn_until_the_page_raster_has_it() {
     stroke_without_settling(&mut h, (40.0, 100.0), (140.0, 60.0));
     assert_eq!(comments(&h).len(), 1, "the stroke is committed");
     assert_eq!(h.state().views[0].comments.settling_len(), 1, "still drawn from its points");
-    settle(&mut h);
+    h.run_steps(5);
+    assert_eq!(h.state().views[0].comments.settling_len(), 1, "drawn from its points for as long as its raster takes");
+    h.state_mut().views[0].hold_rasters = false;
+    support::wait_until(&mut h, "the page raster with the stroke", |app| app.views[0].comments.settling_len() == 0);
     assert_eq!(h.state().views[0].comments.settling_len(), 0, "the raster draws it now");
 }
 

crates/ui-egui/tests/editing.rs

@@ -4225,9 +4225,25 @@ fn image_pixels_follow_the_frame_while_a_corner_is_dragged() {
             .filter(|b| b.width() < page.width() - 1.0)
             .collect()
     };
+    // The dragged picture is cut from the page raster, which a render thread delivers after
+    // however many frames it takes (more on a slow runner): wait for it on screen, by state.
+    let raster_shown = |h: &Harness<'static, PdfCraftApp>| {
+        h.output().shapes.iter().any(|s| {
+            matches!(&s.shape, egui::Shape::Mesh(m) if m.texture_id != egui::TextureId::default() && m.calc_bounds().width() >= page.width() - 1.0)
+        })
+    };
+    let wait_for_raster = |h: &mut Harness<'static, PdfCraftApp>| {
+        let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30);
+        while h.state().render_pending() || !raster_shown(h) {
+            assert!(std::time::Instant::now() < deadline, "timed out waiting for the page raster");
+            h.step();
+            std::thread::sleep(std::time::Duration::from_millis(5));
+        }
+    };
     let start = frame(&h).expect("the selected image has a frame");
     let (grip, fixed) = (start.right_top(), start.left_bottom());
     for factor in [0.6, 1.3] {
+        wait_for_raster(&mut h);
         let to = fixed + (grip - fixed) * factor;
         h.hover_at(grip);
         h.run_steps(1);

crates/ui-egui/tests/ui.rs

@@ -839,6 +839,9 @@ fn files_and_quit_from_the_operating_system() {
     let queue = std::rc::Rc::new(std::cell::RefCell::new(vec![OsEvent::Open(vec![path.to_string_lossy().into_owned()])]));
     let q = queue.clone();
     let mut h = harness(move |app| app.os_events = Some(Box::new(move || q.borrow_mut().drain(..).collect())));
+    // The file is read on a worker thread, after however many frames that takes: it must still
+    // be there until the tab has opened.
+    support::wait_until(&mut h, "the file the operating system opened", |app| !app.views.is_empty());
     std::fs::remove_file(&path).ok();
     assert_eq!(h.state().views.len(), 1, "the file opens in a tab");
     h.get_by_label_contains(&name);

vendor/hayro-interpret/src/color.rs

@@ -22,6 +22,25 @@ use std::sync::{Arc, LazyLock, OnceLock};
 /// A storage for the components of colors.
 pub type ColorComponents = SmallVec<[f32; 4]>;
 
+std::thread_local! {
+    // PdfCraft patch (34): see `tint_transform_runs`.
+    static TINT_TRANSFORM_RUNS: core::cell::Cell<u64> = const { core::cell::Cell::new(0) };
+}
+
+/// PdfCraft patch (34): how many colours this thread has run through a Separation or DeviceN
+/// tint transform, for diagnostics and tests. An image converted through patch (28)'s lookup
+/// table runs it once per possible sample value, not once per pixel. Per thread, so a caller
+/// counting its own render is not disturbed by renders on other threads.
+pub fn tint_transform_runs() -> u64 {
+    TINT_TRANSFORM_RUNS.with(core::cell::Cell::get)
+}
+
+/// PdfCraft patch (34): count `colours` runs of a tint transform (called once per batch).
+fn count_tint_transform_runs(colours: usize) {
+    let colours = u64::try_from(colours).unwrap_or(u64::MAX);
+    TINT_TRANSFORM_RUNS.with(|runs| runs.set(runs.get().saturating_add(colours)));
+}
+
 /// An RGB color with an alpha channel.
 #[derive(Debug, Copy, Clone)]
 pub struct AlphaColor {
@@ -857,6 +876,7 @@ impl Separation {
 
 impl ToRgb for Separation {
     fn convert_f32(&self, input: &[f32], output: &mut [u8], _: bool) -> Option<()> {
+        count_tint_transform_runs(input.len());
         let evaluated = input
             .iter()
             .flat_map(|n| {
@@ -911,6 +931,8 @@ impl DeviceN {
 
 impl ToRgb for DeviceN {
     fn convert_f32(&self, input: &[f32], output: &mut [u8], _: bool) -> Option<()> {
+        let colours = input.len().checked_div(usize::from(self.num_components));
+        count_tint_transform_runs(colours.unwrap_or(0));
         let evaluated = input
             .chunks_exact(self.num_components as usize)
             .flat_map(|n| {