From ec350d64aedd019afc5eda290cc32909bc9bab7d Mon Sep 17 00:00:00 2001 From: shmuelbiller1 Date: Fri, 9 Oct 2026 02:24:33 -0400 Subject: [PATCH] fix(engine): finish background job bookkeeping on its document (#1572) * fix(engine): finish background job bookkeeping on its document * test(engine): cover Fade state after switching tabs during background job * style(test): format background job regression assertions * style(engine): follow rustfmt for background-job regression test --- crates/engine/src/jobs.rs | 11 +++++-- crates/engine/src/jobs_tests.rs | 52 +++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/jobs.rs b/crates/engine/src/jobs.rs index fb8e3494..f32689c9 100644 --- a/crates/engine/src/jobs.rs +++ b/crates/engine/src/jobs.rs @@ -704,13 +704,18 @@ impl Session { }; self.coalesce_request = None; self.color_restrict = None; + // Finish successful command bookkeeping on the document the job edited. + // Restoring the viewed document first would attribute Fade and slice updates + // to that document instead, when the user switched tabs mid-job. + if r.is_ok() { + self.after_command(command, params.clone(), *journal); + } // Keep the user's active document unless the job opened a new one. + // This also restores the selection when the apply step returned an error. if target.is_some() && self.active == target { self.active = prev_active.filter(|i| *i < self.docs.len()).or(self.active); } - let v = r?; - self.after_command(command, params.clone(), *journal); - Ok(v) + r } fn end_job(&mut self, job: &Running, outcome: JobOutcome) { diff --git a/crates/engine/src/jobs_tests.rs b/crates/engine/src/jobs_tests.rs index 0e352053..e9a49346 100644 --- a/crates/engine/src/jobs_tests.rs +++ b/crates/engine/src/jobs_tests.rs @@ -220,6 +220,58 @@ fn a_running_job_locks_its_document_but_not_others() { assert_eq!(s.documents()[0].history.past_len(), 3); } +#[test] +fn background_job_finishes_bookkeeping_on_edited_document_after_tab_switch() { + let mut s = session(64, 64); + let edited_doc = s.active().unwrap().doc.id; + let gate = Arc::new(std::sync::atomic::AtomicBool::new(false)); + let worker_gate = Arc::clone(&gate); + // Hold a fadeable filter job until another document is the active tab. + // A deterministic gate avoids relying on the filter being sufficiently slow. + let id = job(s + .start_job( + "filter.blur.gaussianBlur", + json!({"radius": 2}), + "Deferred Blur", + true, + move |ctx| { + while !worker_gate.load(std::sync::atomic::Ordering::Relaxed) { + ctx.check()?; + std::thread::sleep(Duration::from_millis(1)); + } + Ok(()) + }, + |s, ()| { + s.edit("Deferred Blur", |doc, active| { + let layer = active.ok_or_else(|| EngineError::Other("no active layer".into()))?; + let surface = doc.layer_mut(layer).ok_or(EngineError::NoLayer(layer))?.surface_mut().ok_or(EngineError::NoLayer(layer))?; + surface.fill_rect(photocraft_geom::Rect::new(0, 0, 1, 1), &[0.4, 0.3, 0.2, 1.0]); + Ok(()) + })?; + Ok(json!({"filter": {"radius": 2}})) + }, + ) + .unwrap()); + + s.execute("file.new", json!({"width": 8, "height": 8})).unwrap(); + s.execute("layer.new.layer", json!({})).unwrap(); + let viewed_doc = s.active().unwrap().doc.id; + let viewed_revision = s.active().unwrap().revision; + assert_eq!(s.active_index(), Some(1)); + + gate.store(true, std::sync::atomic::Ordering::Relaxed); + let event = wait_event(&mut s, id); + assert!(matches!(event.outcome, JobOutcome::Done(_)), "{event:?}"); + assert_eq!(s.active_index(), Some(1), "the user stays on the selected document"); + assert_eq!(s.documents()[1].revision, viewed_revision, "the viewed document is unchanged"); + assert_eq!(s.documents()[1].doc.id, viewed_doc); + assert_eq!(s.edit_state.fade.as_ref().map(|fade| fade.doc), Some(edited_doc), "Fade belongs to the edited document"); + assert!(!s.is_enabled("edit.fade"), "Fade must not target the viewed document"); + + assert!(s.set_active(0)); + assert!(s.is_enabled("edit.fade"), "Fade must be available on the document that was edited"); +} + #[test] fn closing_the_document_cancels_its_job() { let mut s = session(1600, 1200);