Color management: proof view commands redraw only when the view changes

view.proofSetup, view.proofColors, view.overprintPreview and
view.separationsPreview stored the proof view and raised every open
document's revision on each run. They did this also when the request
asked for the view that was already showing, for example Proof Colors on
while it is on, or Proof Setup with no params. Each such run redrew every
canvas. When the active document's revision changes, the canvas also
sends a tool that asks for it a new hover event, although the pointer has
not moved.

- The four commands compare the new proof view with the current one. When
  the two are equal, the commands leave the view and the revisions as
  they are. Separations Preview's visible plates are compared as a set,
  so the same plates in another order do not redraw.
- view.separationsPreview with `plates` naming a plate twice shows it
  once, so one `toggle` of that plate hides it.
- color.loadProfile with a new file of the CMYK profile that Proof Setup
  targets gives the documents new revisions, as it does for a working
  space. Before, running Proof Setup again redrew the documents through
  the new file; it now leaves an unchanged view alone.
- color.removeProfile gives the documents new revisions only when Proof
  Setup targets the removed profile.

crates/engine/src/cmd/colormgmt.rs

@@ -210,8 +210,12 @@ fn load_profile(s: &mut Session, p: &Value) -> Result<Value> {
     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<Value> {
         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<Value> {
     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<Value> {
     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<bool>)) -> Result<Value> {
     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<Value> {
     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::<Result<Vec<_>>>()?);
+            let mut vis: Vec<String> = 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<Value> {
             vis.push(n);
         }
     }
-    proof::set_view(v);
-    touch_all(s);
+    store_view(s, v);
     Ok(plates_json(s))
 }
 

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",

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();

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());

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::<Vec<u8>>();
+        let mut bytes = cms::icc_bytes(builtin).unwrap().to_vec();
+        let (from, to) = (utf16(builtin), utf16(&format!("{name:<width$}", width = builtin.len())));
+        let at = bytes.windows(from.len()).position(|w| w == from).unwrap();
+        bytes[at..at + to.len()].copy_from_slice(&to);
+        bytes
+    };
+    let dir = fresh_dir("reload-proof-cmyk");
+    std::fs::create_dir_all(&dir).unwrap();
+    let _reset = ColorSettingsReset { profile: None, paths: vec![dir.clone()] };
+    let load = |s: &mut Session, name: &str| {
+        let path = dir.join(format!("{}.icc", name.len()));
+        std::fs::write(&path, renamed(cms::GENERIC_CMYK, name)).unwrap();
+        let r = s.execute("color.loadProfile", &json!({"path": path.to_str().unwrap()})).unwrap();
+        assert_eq!(r["name"], name);
+        s.doc().unwrap().revision
+    };
+    let (proofed, other) = ("Reload Proof CMYK", "Reload Other CMYK 2");
+    let _view = ProofReset([proofed, other]);
+    let mut s = session();
+    load(&mut s, proofed);
+    load(&mut s, other);
+    exec(&mut s, "view.proofSetup", json!({"target": format!("cmyk:{proofed}"), "proof": true}));
+    let revision = s.doc().unwrap().revision;
+    assert_eq!(load(&mut s, other), revision, "another profile's file");
+    let reloaded = load(&mut s, proofed);
+    assert!(reloaded > 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}"));

crates/ui-egui/src/control/tests.rs

@@ -679,8 +679,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 = [