diff --git a/crates/engine/src/cmd/colormgmt.rs b/crates/engine/src/cmd/colormgmt.rs index 9ec21b6541..1046c4a948 100644 --- a/crates/engine/src/cmd/colormgmt.rs +++ b/crates/engine/src/cmd/colormgmt.rs @@ -210,8 +210,12 @@ fn load_profile(s: &mut Session, p: &Value) -> Result { let remembered = s.store_profile(&info.name, &bytes); // A new file of a working space: the active settings still hold the old one. let working = cms::active_settings(); - if working.rgb == info.name || working.cmyk == info.name { + let is_working = working.rgb == info.name || working.cmyk == info.name; + if is_working { cms::set_active(&working).map_err(|e| bad(C, e.to_string()))?; + } + // A new file of a working space, or of the CMYK profile View → Proof Setup targets: redraw. + if is_working || proof_targets(&info.name) { touch_all(s); } Ok(json!({"name": info.name, "kind": info.kind, "remembered": remembered})) @@ -233,10 +237,17 @@ fn remove_profile(s: &mut Session, p: &Value) -> Result { return Err(bad(C, format!("no profile named `{name}`"))); } s.unstore_profile(&info.name); - touch_all(s); + if proof_targets(&info.name) { + touch_all(s); + } Ok(json!({"removed": info.name})) } +/// Whether View → Proof Setup targets the CMYK profile `name`. +fn proof_targets(name: &str) -> bool { + matches!(proof::view().setup.target, ProofTarget::Cmyk(t) if t == name) +} + #[cfg(target_arch = "wasm32")] fn load_profile(_: &mut Session, _: &Value) -> Result { Err(bad("color.loadProfile", "loading profiles from disk isn't available in the browser")) @@ -763,17 +774,33 @@ fn proof_setup(s: &mut Session, p: &Value) -> Result { if let Some(on) = on_param(p, "proof", C)? { v.proof_colors = on; } - proof::set_view(v.clone()); - touch_all(s); + store_view(s, v.clone()); Ok(view_json(&v)) } +/// Make `v` the proof view and redraw every document. When `v` equals the current view, the view +/// and the revisions stay as they are; Separations Preview's visible plates are compared as a set. +fn store_view(s: &mut Session, v: proof::ProofView) { + let shown = |v: &proof::ProofView| proof::ProofView { + separations: v.separations.as_ref().map(|vis| { + let mut vis = vis.clone(); + vis.sort(); + vis.dedup(); + vis + }), + ..v.clone() + }; + if shown(&v) != shown(&proof::view()) { + proof::set_view(v); + touch_all(s); + } +} + fn toggle_view(s: &mut Session, p: &Value, cmd: &str, f: impl FnOnce(&mut proof::ProofView, Option)) -> Result { let on = on_param(p, "on", cmd)?; let mut v = proof::view(); f(&mut v, on); - proof::set_view(v.clone()); - touch_all(s); + store_view(s, v.clone()); Ok(view_json(&v)) } @@ -805,7 +832,15 @@ fn separations(s: &mut Session, p: &Value) -> Result { match p.get("plates") { None | Some(Value::Null) => {} Some(Value::Array(a)) if a.iter().all(Value::is_string) => { - v.separations = Some(a.iter().filter_map(Value::as_str).map(check).collect::>>()?); + let mut vis: Vec = Vec::new(); + for n in a.iter().filter_map(Value::as_str) { + let n = check(n)?; + // A plate named twice is shown once. + if !vis.contains(&n) { + vis.push(n); + } + } + v.separations = Some(vis); } Some(_) => { let key = "plates"; @@ -824,8 +859,7 @@ fn separations(s: &mut Session, p: &Value) -> Result { vis.push(n); } } - proof::set_view(v); - touch_all(s); + store_view(s, v); Ok(plates_json(s)) } diff --git a/crates/engine/src/command_sets.rs b/crates/engine/src/command_sets.rs index 95b44776f0..c9de286514 100644 --- a/crates/engine/src/command_sets.rs +++ b/crates/engine/src/command_sets.rs @@ -84,7 +84,7 @@ pub(crate) fn strip_replay_only(id: &str, p: &mut Value) { /// Queries that apply the edit the active tool is computing before they run, as edits do /// ([`Session::finish_tool_edit`]): a batch runs edits, Find Objects with `select`, Select All /// Instances and Lock Guides change the selection, and the proof views, the preferences and added -/// fonts give the documents new revisions (they draw again, and type lays out again). +/// fonts can give the documents new revisions (they draw again, and type lays out again). const WAIT_FOR_TOOL_EDIT: &[&str] = &[ "script.run", "command.batch", diff --git a/crates/engine/src/tests/build.rs b/crates/engine/src/tests/build.rs index 0fc2bcb476..49a806b258 100644 --- a/crates/engine/src/tests/build.rs +++ b/crates/engine/src/tests/build.rs @@ -793,8 +793,9 @@ fn shape_builder_applies_a_merge_before_what_comes_next() { let labels: Vec<&str> = s.doc().unwrap().history.undo.iter().rev().take(2).map(|e| e.label.as_str()).collect(); assert_eq!(labels, ["Move", "Shape Builder"]); }), - // The proof views redraw every document (a new revision); they are left off, as the - // other tests of the process-wide proof view expect. + // The proof views apply the merge first by command id (`WAIT_FOR_TOOL_EDIT`), also when + // the view does not change and no document gets a new revision. They are left off, as + // the other tests of the process-wide proof view expect. ("Separations Preview", |s, _, _| { s.execute("view.separationsPreview", &json!({"on": false})).unwrap(); }), @@ -807,6 +808,13 @@ fn shape_builder_applies_a_merge_before_what_comes_next() { ("Proof Setup", |s, _, _| { s.execute("view.proofSetup", &json!({})).unwrap(); }), + ("a proof view that changes", |s, _, _| { + let _v = crate::tests_colormgmt::DefaultView::new(); + s.execute("view.proofColors", &json!({"on": true})).unwrap(); + let on = s.doc().unwrap().revision; + s.execute("view.proofColors", &json!({"on": false})).unwrap(); + assert!(s.doc().unwrap().revision > on, "the view changes and the documents get new revisions"); + }), ("a preference", |s, _, _| { let every = s.prefs.gridline_every; s.execute("prefs.set", &json!({"key": "gridlineEvery", "value": every + 10.0})).unwrap(); diff --git a/crates/engine/src/tests/colormgmt.rs b/crates/engine/src/tests/colormgmt.rs index ae8ac0c4f0..493f569805 100644 --- a/crates/engine/src/tests/colormgmt.rs +++ b/crates/engine/src/tests/colormgmt.rs @@ -455,6 +455,7 @@ fn overprint_black_multiplies_in_preview() { #[test] fn view_toggles_redraw_without_dirtying() { let _g = GLOBAL.lock().unwrap_or_else(|e| e.into_inner()); + let _v = DefaultView::new(); let mut s = session(); let rev = s.doc().unwrap().revision; let r = s.execute("view.proofColors", &json!({"on": true})).unwrap(); @@ -475,6 +476,101 @@ fn view_toggles_redraw_without_dirtying() { s.execute("view.overprintPreview", &json!({"on": false})).unwrap(); } +/// Sets the default proof view when made and again when dropped, also after a failed assertion, +/// so the next test starts from the default view. +pub(crate) struct DefaultView; + +impl DefaultView { + pub(crate) fn new() -> Self { + vectorcraft_render::proof::set_view(Default::default()); + DefaultView + } +} + +impl Drop for DefaultView { + fn drop(&mut self) { + vectorcraft_render::proof::set_view(Default::default()); + } +} + +/// Runs `cmd` with `p` twice from the default proof view: the first run changes the view and +/// redraws, and the second asks for the view that is already showing and leaves the revision alone. +fn repeat_keeps_revision(cmd: &str, p: Value) { + let _g = GLOBAL.lock().unwrap_or_else(|e| e.into_inner()); + let _v = DefaultView::new(); + let mut s = session(); + let rev = s.doc().unwrap().revision; + s.execute(cmd, &p).unwrap(); + let changed = s.doc().unwrap().revision; + assert!(changed > rev, "{cmd} {p}: the first run changes the view"); + s.execute(cmd, &p).unwrap(); + assert_eq!(s.doc().unwrap().revision, changed, "{cmd} {p}: the same view again"); +} + +/// Runs `cmd` with `p` once from the default proof view, which `p` leaves as it is. +fn default_view_keeps_revision(cmd: &str, p: Value) { + let _g = GLOBAL.lock().unwrap_or_else(|e| e.into_inner()); + let _v = DefaultView::new(); + let mut s = session(); + let rev = s.doc().unwrap().revision; + s.execute(cmd, &p).unwrap(); + assert_eq!(s.doc().unwrap().revision, rev, "{cmd} {p}"); + assert_eq!(vectorcraft_render::proof::view(), Default::default(), "{cmd} {p}"); +} + +#[test] +fn proof_setup_with_nothing_changed_keeps_the_revision() { + repeat_keeps_revision("view.proofSetup", json!({"target": "deuteranopia"})); + repeat_keeps_revision("view.proofSetup", json!({"simulatePaper": true, "proof": true})); + default_view_keeps_revision("view.proofSetup", json!({})); + default_view_keeps_revision("view.proofSetup", json!({"target": "workingCmyk", "proof": false})); +} + +#[test] +fn proof_colors_with_nothing_changed_keeps_the_revision() { + repeat_keeps_revision("view.proofColors", json!({"on": true})); + default_view_keeps_revision("view.proofColors", json!({"on": false})); +} + +#[test] +fn overprint_preview_with_nothing_changed_keeps_the_revision() { + repeat_keeps_revision("view.overprintPreview", json!({"on": true})); + default_view_keeps_revision("view.overprintPreview", json!({"on": false})); +} + +#[test] +fn separations_preview_with_nothing_changed_keeps_the_revision() { + repeat_keeps_revision("view.separationsPreview", json!({"on": true})); + repeat_keeps_revision("view.separationsPreview", json!({"only": "Cyan"})); + repeat_keeps_revision("view.separationsPreview", json!({"plates": ["Magenta", "Black"]})); + default_view_keeps_revision("view.separationsPreview", json!({"on": false})); +} + +/// Separations Preview's visible plates are a set: the same plates in another order, or with one +/// named twice, leave the revision alone, and a plate named twice hides with one `toggle`. +#[test] +fn separations_preview_compares_the_visible_plates_as_a_set() { + let _g = GLOBAL.lock().unwrap_or_else(|e| e.into_inner()); + let _v = DefaultView::new(); + let mut s = session(); + let sep = |s: &mut Session, p: Value| { + s.execute("view.separationsPreview", &p).unwrap(); + s.doc().unwrap().revision + }; + let shown = sep(&mut s, json!({"plates": ["Magenta", "Black"]})); + assert_eq!(sep(&mut s, json!({"plates": ["Black", "Magenta"]})), shown, "another order"); + assert_eq!(sep(&mut s, json!({"plates": ["Magenta", "Black", "Black"]})), shown, "a plate named twice"); + // Cyan hidden and shown again goes to the end of the list; every plate is shown. + sep(&mut s, json!({"on": false})); + sep(&mut s, json!({"on": true})); + sep(&mut s, json!({"toggle": "Cyan"})); + let all = sep(&mut s, json!({"toggle": "Cyan"})); + assert_eq!(sep(&mut s, json!({"plates": ["Cyan", "Magenta", "Yellow", "Black"]})), all, "every plate in plate order"); + sep(&mut s, json!({"plates": ["Cyan", "Cyan"]})); + let r = s.execute("view.separationsPreview", &json!({"toggle": "Cyan"})).unwrap(); + assert!(r["plates"].as_array().unwrap().iter().all(|p| p["visible"] == false), "{r}"); +} + #[test] fn proof_setup_simulates_tritanopia() { let _g = GLOBAL.lock().unwrap_or_else(|e| e.into_inner()); diff --git a/crates/engine/tests/color_settings.rs b/crates/engine/tests/color_settings.rs index c84921f6de..c3d6df771b 100644 --- a/crates/engine/tests/color_settings.rs +++ b/crates/engine/tests/color_settings.rs @@ -446,6 +446,60 @@ fn loading_the_working_rgb_profile_again_takes_effect_at_once() { assert!(redrawn, "the canvas redraws"); } +/// Loading a new file of the CMYK profile View → Proof Setup targets, or removing that profile, +/// gives the documents new revisions, as a working space's new file does; loading or removing +/// another profile does not. +#[test] +fn loading_or_removing_the_proof_profile_redraws() { + let _g = settings(); + // A built-in CMYK profile's file renamed `name`: padded with spaces, which names drop. + let renamed = |builtin: &str, name: &str| { + let utf16 = |s: &str| s.encode_utf16().flat_map(u16::to_be_bytes).collect::>(); + let mut bytes = cms::icc_bytes(builtin).unwrap().to_vec(); + let (from, to) = (utf16(builtin), utf16(&format!("{name: revision, "the proof profile's file"); + exec(&mut s, "color.removeProfile", json!({"name": other})); + assert_eq!(s.doc().unwrap().revision, reloaded, "another profile removed"); + exec(&mut s, "color.removeProfile", json!({"name": proofed})); + assert!(s.doc().unwrap().revision > reloaded, "the proof profile removed"); +} + +/// Sets the default proof view and unregisters the profiles when dropped, also after a failed +/// assertion. +struct ProofReset([&'static str; 2]); + +impl Drop for ProofReset { + fn drop(&mut self) { + vectorcraft_render::proof::set_view(Default::default()); + for name in self.0 { + cms::unregister_icc(name); + } + } +} + /// A folder of its own for the test `tag`, empty. fn fresh_dir(tag: &str) -> std::path::PathBuf { let dir = vectorcraft_testkit::temp_dir(&format!("color-settings-{tag}")); diff --git a/crates/ui-egui/src/control/tests.rs b/crates/ui-egui/src/control/tests.rs index 5797b0ddb9..04116d738d 100644 --- a/crates/ui-egui/src/control/tests.rs +++ b/crates/ui-egui/src/control/tests.rs @@ -1073,8 +1073,8 @@ fn menu_dialogs_open_after_a_merge_on_another_thread() { } /// The Separations panel's checkboxes and a preference set while a Shape Builder merge is -/// computed on another thread give the documents new revisions: the merge is applied first, -/// and the document keeps it. +/// computed on another thread apply the merge first, and the document keeps it. The checkboxes +/// leave the view off, so they apply it by command id without giving the documents new revisions. #[test] fn panels_that_redraw_apply_a_merge_on_another_thread_first() { let cases = [