From f4979c7e5481c907b49c9152283a96530b6f0a41 Mon Sep 17 00:00:00 2001 From: Don Omondi <282319807+mockydevs@users.noreply.github.com> Date: Sun, 1 Nov 2026 22:46:12 +0300 Subject: [PATCH 1/3] Fix #5152: placed-document bitmap test no longer races its worker a_ready_bitmap_draws_again_where_its_file_shows assumed the first frame shows placeholders, but the background worker can make the tiny test bitmaps between scheduling and the frame's second lookup, so the frame already showed them ("the bitmaps show" failed on FreeBSD CI). The test's loader now waits until that frame is drawn. --- crates/render/src/tests/placed_document.rs | 33 +++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/crates/render/src/tests/placed_document.rs b/crates/render/src/tests/placed_document.rs index 8ce2953cd..00af69a2c 100644 --- a/crates/render/src/tests/placed_document.rs +++ b/crates/render/src/tests/placed_document.rs @@ -53,6 +53,34 @@ fn placed_node(id: u64, key: &str, width: f64, height: f64, xf: Affine) -> Node /// Held by the tests that draw placed documents from bitmaps, which share the bitmaps, their /// generation and the jobs being made: one test's bitmaps don't push out another's, and /// [`settle`] waits for its own. +/// Held shut, [`gated_blue_page`] makes the background worker wait before reading a file, so a +/// frame drawn meanwhile shows the placeholders whatever the scheduler does (#5152: a fast worker +/// could finish between scheduling a bitmap and the frame looking it up again). +static GATE_SHUT: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false); +/// The thread drawing while the gate is shut: it never waits (a file read there is drawn exactly). +static GATE_HOLDER: std::sync::Mutex> = std::sync::Mutex::new(None); + +/// [`blue_page`] once the gate opens (at most 10 s), except on the thread that shut it. +fn gated_blue_page(p: &PlacedDocument, bytes: &[u8]) -> Option<(Document, Node, Rect)> { + let here = std::thread::current().id(); + let holder = *GATE_HOLDER.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + let t = std::time::Instant::now(); + while holder != Some(here) && GATE_SHUT.load(std::sync::atomic::Ordering::SeqCst) && t.elapsed().as_secs() < 10 { + std::thread::sleep(std::time::Duration::from_millis(1)); + } + blue_page(p, bytes) +} + +/// Shut the gate for the calling thread's frames. +fn shut_gate() { + *GATE_HOLDER.lock().unwrap_or_else(std::sync::PoisonError::into_inner) = Some(std::thread::current().id()); + GATE_SHUT.store(true, std::sync::atomic::Ordering::SeqCst); +} + +fn open_gate() { + GATE_SHUT.store(false, std::sync::atomic::Ordering::SeqCst); +} + fn serial() -> std::sync::MutexGuard<'static, ()> { static SERIAL: std::sync::Mutex<()> = std::sync::Mutex::new(()); SERIAL.lock().unwrap_or_else(std::sync::PoisonError::into_inner) @@ -231,7 +259,7 @@ fn a_bitmap_that_cant_be_made_is_not_asked_for_again() { #[test] fn a_ready_bitmap_draws_again_where_its_file_shows() { let _serial = serial(); - vectorcraft_doc::placed_document::set_loader(blue_page); + vectorcraft_doc::placed_document::set_loader(gated_blue_page); let mut d = Document::new(100.0, 100.0); for key in ["area-a", "area-b"] { d.images.insert(key.into(), ImageBlob::new("application/json", format!("{{\"doc\": \"{key}\"}}").into_bytes())); @@ -251,7 +279,10 @@ fn a_ready_bitmap_draws_again_where_its_file_shows() { let opts = RenderOptions { background: Some([255, 255, 255, 255]), progressive_placed: true, ..Default::default() }; let mut r = Renderer::new(); let generation = placed_document::generation(); + // The first frame shows the placeholders: the worker waits until it is drawn. + shut_gate(); let before = r.render(&d, 100, 100, Affine::IDENTITY, &opts); + open_gate(); settle(); let made = placed_document::made_between(generation, placed_document::generation()).expect("known"); let files = |keys: &[&str]| keys.iter().map(|k| k.to_string()).collect::>(); From b903e2e7e78284f057bdb2e5116066ccdbc9e2d5 Mon Sep 17 00:00:00 2001 From: Don Omondi <282319807+mockydevs@users.noreply.github.com> Date: Sun, 1 Nov 2026 23:55:44 +0300 Subject: [PATCH 2/3] Placed-document tests: register the gated loader everywhere set_loader keeps the first registration, so a test registering blue_page first left the gate out. Every test in the file now registers gated_blue_page (blue_page while the gate is open). --- crates/render/src/tests/placed_document.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/render/src/tests/placed_document.rs b/crates/render/src/tests/placed_document.rs index 00af69a2c..32b36b77b 100644 --- a/crates/render/src/tests/placed_document.rs +++ b/crates/render/src/tests/placed_document.rs @@ -27,7 +27,7 @@ fn blue_page(p: &PlacedDocument, bytes: &[u8]) -> Option<(Document, Node, Rect)> /// A 100×100 document showing one placed file (`bytes` under `key`) in the box (10, 10)–(90, 50). fn doc_with(key: &str, bytes: &[u8]) -> Document { - vectorcraft_doc::placed_document::set_loader(blue_page); + vectorcraft_doc::placed_document::set_loader(gated_blue_page); let mut d = Document::new(100.0, 100.0); d.images.insert(key.into(), ImageBlob::new("application/json", bytes.to_vec())); let l = d.layers[0].id; @@ -60,7 +60,9 @@ static GATE_SHUT: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool: /// The thread drawing while the gate is shut: it never waits (a file read there is drawn exactly). static GATE_HOLDER: std::sync::Mutex> = std::sync::Mutex::new(None); -/// [`blue_page`] once the gate opens (at most 10 s), except on the thread that shut it. +/// [`blue_page`] once the gate opens (at most 10 s), except on the thread that shut it. Every test +/// here registers it: the first registration stays, so all of them must agree (open, it is +/// `blue_page`). fn gated_blue_page(p: &PlacedDocument, bytes: &[u8]) -> Option<(Document, Node, Rect)> { let here = std::thread::current().id(); let holder = *GATE_HOLDER.lock().unwrap_or_else(std::sync::PoisonError::into_inner); @@ -201,7 +203,7 @@ fn copies_of_one_placed_document_share_its_bitmap() { #[test] fn nine_large_bitmaps_on_screen_are_not_made_again() { let _serial = serial(); - vectorcraft_doc::placed_document::set_loader(blue_page); + vectorcraft_doc::placed_document::set_loader(gated_blue_page); let mut d = Document::new(100.0, 100.0); let l = d.layers[0].id; let keys: Vec = (0..9).map(|i| format!("nine-{i}")).collect(); From 59992ce155163762c6d33ec68cd75438fc4c3f3e Mon Sep 17 00:00:00 2001 From: bflatastic Date: Mon, 2 Nov 2026 13:45:22 +0700 Subject: [PATCH 3/3] Tests: serial()'s doc comment goes back above it --- crates/render/src/tests/placed_document.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/render/src/tests/placed_document.rs b/crates/render/src/tests/placed_document.rs index 32b36b77b..400fc23e8 100644 --- a/crates/render/src/tests/placed_document.rs +++ b/crates/render/src/tests/placed_document.rs @@ -50,9 +50,6 @@ fn placed_node(id: u64, key: &str, width: f64, height: f64, xf: Affine) -> Node Node::new(NodeId(id), NodeKind::PlacedDocument(Box::new(p))) } -/// Held by the tests that draw placed documents from bitmaps, which share the bitmaps, their -/// generation and the jobs being made: one test's bitmaps don't push out another's, and -/// [`settle`] waits for its own. /// Held shut, [`gated_blue_page`] makes the background worker wait before reading a file, so a /// frame drawn meanwhile shows the placeholders whatever the scheduler does (#5152: a fast worker /// could finish between scheduling a bitmap and the frame looking it up again). @@ -83,6 +80,9 @@ fn open_gate() { GATE_SHUT.store(false, std::sync::atomic::Ordering::SeqCst); } +/// Held by the tests that draw placed documents from bitmaps, which share the bitmaps, their +/// generation and the jobs being made: one test's bitmaps don't push out another's, and +/// [`settle`] waits for its own. fn serial() -> std::sync::MutexGuard<'static, ()> { static SERIAL: std::sync::Mutex<()> = std::sync::Mutex::new(()); SERIAL.lock().unwrap_or_else(std::sync::PoisonError::into_inner)