From 8caa0e0e59c1268237198af9f5a0e0df39ff1542 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Thu, 5 Nov 2026 06:15:14 -0800 Subject: [PATCH] Layers: closing a document drops its running thumbnail job's image A thumbnail job that was running when its document closed still sent its image back, and the image stayed with the worker until a panel next collected results. ThumbWorker::forget now drops the jobs of the picked slots, queued or running, and the Layers panel uses it when a document closes, keeping the same worker. The close test now goes through File > Close with no panel drawn afterward. --- crates/ui-egui/src/panels/layers/tests.rs | 7 ++- crates/ui-egui/src/panels/layers/thumbs.rs | 48 +++++++++++++++++- crates/ui-egui/src/thumb_worker.rs | 59 +++++++++++++++++++--- 3 files changed, 104 insertions(+), 10 deletions(-) diff --git a/crates/ui-egui/src/panels/layers/tests.rs b/crates/ui-egui/src/panels/layers/tests.rs index 703d34553..f138b6cbf 100644 --- a/crates/ui-egui/src/panels/layers/tests.rs +++ b/crates/ui-egui/src/panels/layers/tests.rs @@ -260,8 +260,11 @@ fn closing_a_document_releases_only_its_layer_thumbnails() { frame(&mut app, &ctx, vec![], false); assert!(THUMBS.with(|c| c.borrow().cached(second))); - app.session.execute("file.close", &json!({"index": 0})).unwrap(); - app.sync_views(); + // Close the first document the way File › Close does (Don't Save), with no panel drawn after. + assert_eq!(app.run("file.close", json!({"index": 0})).unwrap(), json!({"pending": crate::unsaved::KIND})); + app.ui.dialog.as_mut().unwrap().fields.insert("discard".into(), json!(true)); + crate::dialogs::confirm(&mut app).unwrap(); + assert!(app.session.documents().iter().all(|d| d.uid != first), "the first document closed"); THUMBS.with(|c| { let thumbs = c.borrow(); assert!(!thumbs.cached(first), "closed document textures are released"); diff --git a/crates/ui-egui/src/panels/layers/thumbs.rs b/crates/ui-egui/src/panels/layers/thumbs.rs index c8b37eae7..49ce4b61b 100644 --- a/crates/ui-egui/src/panels/layers/thumbs.rs +++ b/crates/ui-egui/src/panels/layers/thumbs.rs @@ -258,18 +258,32 @@ impl Thumbs { } /// Release thumbnails and queued jobs for documents that have closed, keeping the cache for - /// other open documents even when the panels are hidden or another document is active. + /// other open documents even when the panels are hidden or another document is active. When a + /// document closes, the worker drops its jobs, the one running included, and the images it + /// finished that the panels have not collected yet. fn retain_open(&mut self, uids: &[u64]) { if self.open_docs.as_ref().is_some_and(|open| open.len() == uids.len() && uids.iter().all(|uid| open.contains(uid))) { return; } let open: HashSet = uids.iter().copied().collect(); + let closed = self.open_docs.as_ref().is_some_and(|before| before.iter().any(|uid| !open.contains(uid))); let stale = self.entries.keys().chain(self.pending.keys()).any(|key| !open.contains(&key.0)) || self.showing.is_some_and(|uid| !open.contains(&uid)); self.entries.retain(|key, _| open.contains(&key.0)); self.pending.retain(|key, _| open.contains(&key.0)); self.showing = self.showing.filter(|uid| open.contains(uid)); - if stale + if closed { + // Images finished before now are dropped with the rest of the queue: the open + // documents' rows are asked for again. + if let Some(w) = &self.worker { + let open = open.clone(); + w.forget(move |key| !open.contains(&key.0)); + if w.poll().is_none() { + self.worker = None; + } + } + self.pending.clear(); + } else if stale && let Some(w) = &self.worker && !w.discard({ let open = open.clone(); @@ -492,6 +506,36 @@ mod tests { assert!(thumbs.entries.contains_key(&(2, NodeId(2))), "an open document's result is kept"); } + #[test] + fn closing_a_document_drops_the_image_its_running_job_finishes_while_the_panels_are_hidden() { + let ctx = egui::Context::default(); + let Some(worker) = ThumbWorker::start(&ctx, "test") else { return }; + let mut thumbs = Thumbs { worker: Some(worker), worker_started: true, ..Default::default() }; + thumbs.retain_open(&[1, 2]); + // A job of document 1 is running, so discarding the queue cannot drop it. + let (started, running) = std::sync::mpsc::channel::<()>(); + let (release, held) = std::sync::mpsc::channel::<()>(); + let key = (1, NodeId(1)); + let job: crate::thumb_worker::JobFn = Box::new(move |_| { + started.send(()).ok(); + held.recv().ok(); + None + }); + assert!(thumbs.worker.as_ref().is_some_and(|w| w.submit(key, (key, Weak::new(), 1), job))); + running.recv().expect("the job starts"); + // Another row of document 1 waits behind it. + let queued = (1, NodeId(3)); + assert!(thumbs.worker.as_ref().is_some_and(|w| w.submit(queued, (queued, Weak::new(), 1), Box::new(|_| None)))); + // Document 1 closes while no panel collects results, then its running job finishes. + thumbs.retain_open(&[2]); + release.send(()).ok(); + // A job of document 2 queued after them comes back first: document 1's images were dropped. + let next = (2, NodeId(2)); + assert!(thumbs.worker.as_ref().is_some_and(|w| w.submit(next, (next, Weak::new(), 1), Box::new(|_| None)))); + let done = thumbs.worker.as_ref().and_then(ThumbWorker::wait).expect("the same worker finishes document 2's job"); + assert_eq!(done.key.0, next, "no worker keeps the closed document's image"); + } + /// An open zigzag of `anchors` anchors. fn line(anchors: usize) -> PathData { let pts: Vec = (0..anchors).map(|i| Point::new(i as f64 * 0.01, (i % 2) as f64 * 50.0)).collect(); diff --git a/crates/ui-egui/src/thumb_worker.rs b/crates/ui-egui/src/thumb_worker.rs index c754cd786..7a04484dc 100644 --- a/crates/ui-egui/src/thumb_worker.rs +++ b/crates/ui-egui/src/thumb_worker.rs @@ -9,6 +9,7 @@ use std::collections::VecDeque; use std::sync::mpsc::{Receiver, Sender, TryRecvError, channel}; +use std::sync::{Arc, Mutex}; use vectorcraft_render::{Rendered, Renderer}; @@ -32,9 +33,12 @@ struct Job { run: JobFn, } -/// Which slots' jobs [`ThumbWorker::discard`] drops. +/// Which slots' jobs [`ThumbWorker::discard`] or [`ThumbWorker::forget`] drops. type SlotTest = Box bool + Send>; +/// The slots [`ThumbWorker::forget`] picked since the worker was last idle. +type Forgotten = Arc>>>; + /// What a panel sends its worker: a job, or the slots whose jobs not started yet are dropped. enum Msg { Job(Job), @@ -47,6 +51,7 @@ enum Msg { pub struct ThumbWorker { tx: Sender>, rx: Receiver>, + forgotten: Forgotten, } impl ThumbWorker { @@ -64,6 +69,8 @@ impl ThumbWorker { pub(crate) fn start(ctx: &egui::Context, name: &str) -> Option { let (tx, jobs) = channel::>(); let (done_tx, rx) = channel::>(); + let forgotten: Forgotten = Arc::default(); + let gone = forgotten.clone(); let ctx = ctx.clone(); std::thread::Builder::new() .name(format!("vectorcraft-{name}")) @@ -79,24 +86,55 @@ impl ThumbWorker { }; loop { if queue.is_empty() { - // The panel is gone (its sender dropped): end the thread. - let Ok(job) = jobs.recv() else { break }; - add(&mut queue, job); + // With no job here or on its way (checked under the lock, since a panel + // sends a job before it forgets that job's slot), what `forget` picked + // has no job left to drop. + let waiting = match gone.lock() { + Ok(mut f) => match jobs.try_recv() { + Ok(msg) => Some(msg), + Err(_) => { + f.clear(); + None + } + }, + Err(_) => break, + }; + let msg = match waiting { + Some(msg) => msg, + // The panel is gone (its sender dropped): end the thread. + None => match jobs.recv() { + Ok(msg) => msg, + Err(_) => break, + }, + }; + add(&mut queue, msg); } while let Ok(job) = jobs.try_recv() { add(&mut queue, job); } let Some(job) = queue.pop_front() else { continue }; + let Ok(skip) = gone.lock().map(|f| f.iter().any(|which| which(&job.slot))) else { break }; + if skip { + continue; + } let t0 = crate::now_ms(); let img = (job.run)(&mut r); - if done_tx.send(Done { key: job.key, img, ms: crate::now_ms() - t0 }).is_err() { + let done = Done { key: job.key, img, ms: crate::now_ms() - t0 }; + // Sent under the lock, so a `forget` either drops it here or comes after it + // and finds it in the next `poll`. + let Ok(f) = gone.lock() else { break }; + if f.iter().any(|which| which(&job.slot)) { + continue; + } + if done_tx.send(done).is_err() { break; } + drop(f); ctx.request_repaint(); } }) .ok()?; - Some(Self { tx, rx }) + Some(Self { tx, rx, forgotten }) } /// Queue `run` for `slot` (replacing a job for the slot that hasn't started). False when the @@ -114,6 +152,15 @@ impl ThumbWorker { self.tx.send(Msg::Discard(Box::new(which))).is_ok() } + /// Drop the jobs whose slot `which` picks, queued or running: their images never come back + /// from [`ThumbWorker::poll`], except those finished before this call, which the next poll + /// returns. The panel must not submit jobs for those slots again. + pub fn forget(&self, which: impl Fn(&S) -> bool + Send + 'static) { + if let Ok(mut f) = self.forgotten.lock() { + f.push(Box::new(which)); + } + } + /// The jobs finished since the last call, oldest first; None when the thread is gone (the /// jobs it had will never finish). pub fn poll(&self) -> Option>> {