pauljd/vectorcraft
1
0
Fork 0
mirror of https://github.com/storytold/vectorcraft.git synced 2026-11-08 13:00:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

Color management: commands reject a param of the wrong type

Browse Source 
The color management commands read their params with helpers that fell
back to the default for a value of the wrong type, so "bpc": "no" kept
the old setting, "on": "true" toggled the view and "convert": "no"
converted the colors. They now go through switch_or and text_param:
such a value is an error and nothing changes. Omitting a param or
giving null keeps its default.

- edit.colorSettings: rgb, cmyk, intent, bpc
- file.documentColorMode (and its alias): convert, intent, grays,
  checked before the early return for the current mode; PDF import's
  grays reads the same way
- color.convert: to, intent
- color.loadProfile: path; color.removeProfile: name
- view.proofSetup: target, intent, simulatePaper, proof
- view.proofColors and view.overprintPreview: on
- view.separationsPreview: on, only, toggle, and plates, which must be
  an array of plate names
- swatch.setSpot: name, spot
This commit is contained in:
dotCooCoo
2026-11-08 12:48:00 +07:00
committed by bflatastic
parent 96252edb74
commit 8901877e40
3 changed files with 135 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/engine/src/cmd/colormgmt.rs
+42 -25
View File
@@ -19,7 +19,7 @@ use vectorcraft_render::proof;
#[cfg(not(target_arch = "wasm32"))]
use super::EngineError;
use super::{CommandSpec, Result, Session, always, bad, bool_or, cmd, color_value, has_doc, str_param, targets};
use super::{CommandSpec, Result, Session, always, bad, cmd, color_value, has_doc, switch_or, targets, text_param};
/// `Document.unknown` key under which files before format v3 kept the assigned profiles (migrated
/// to `Document::color_profiles` on load).
@@ -100,7 +100,7 @@ pub fn specs() -> Vec<CommandSpec> {
proof_setup
),
cmd!(query "view.proofColors", "Proof Colors", ["View"], None, "{on?: bool (default: toggle)} → proof state", always, |s, p| {
toggle_view(s, p, |v, on| v.proof_colors = on.unwrap_or(!v.proof_colors))
toggle_view(s, p, "view.proofColors", |v, on| v.proof_colors = on.unwrap_or(!v.proof_colors))
}),
cmd!(
query "view.overprintPreview",
@@ -109,7 +109,7 @@ pub fn specs() -> Vec<CommandSpec> {
Some("Cmd+Alt+Shift+Y"),
"{on?: bool (default: toggle)} → proof state",
always,
|s, p| toggle_view(s, p, |v, on| v.overprint = on.unwrap_or(!v.overprint))
|s, p| toggle_view(s, p, "view.overprintPreview", |v, on| v.overprint = on.unwrap_or(!v.overprint))
),
cmd!(
query "view.separationsPreview",
@@ -143,12 +143,21 @@ pub fn specs() -> Vec<CommandSpec> {
}
fn intent_param(p: &Value, cmd: &str) -> Result<Option<Intent>> {
match str_param(p, "intent") {
match text_param(p, "intent", cmd)? {
None => Ok(None),
Some(i) => Intent::parse(i).map(Some).ok_or_else(|| bad(cmd, format!("unknown intent `{i}`"))),
}
}
/// The true or false `p` gives for `key`, or `None` when `p` leaves it out or gives null. Any other
/// value is an error, as [`switch_or`] reads it.
fn on_param(p: &Value, key: &str, cmd: &str) -> Result<Option<bool>> {
if p.get(key).is_none_or(Value::is_null) {
return Ok(None);
}
switch_or(p, key, false, cmd).map(Some)
}
fn settings_json(st: &ColorSettings) -> Value {
json!({
"rgb": st.rgb,
@@ -171,16 +180,16 @@ fn color_settings(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "edit.colorSettings";
let mut st = cms::active_settings();
let before = st.clone();
if let Some(v) = str_param(p, "rgb") {
if let Some(v) = text_param(p, "rgb", C)? {
st.rgb = cms::canonical_name(v).to_string();
}
if let Some(v) = str_param(p, "cmyk") {
if let Some(v) = text_param(p, "cmyk", C)? {
st.cmyk = cms::canonical_name(v).to_string();
}
if let Some(i) = intent_param(p, C)? {
st.intent = i;
}
st.bpc = bool_or(p, "bpc", st.bpc);
st.bpc = switch_or(p, "bpc", st.bpc, C)?;
if st != before {
cms::set_active(&st).map_err(|e| bad(C, e.to_string()))?;
touch_all(s);
@@ -191,7 +200,7 @@ fn color_settings(s: &mut Session, p: &Value) -> Result<Value> {
#[cfg(not(target_arch = "wasm32"))]
fn load_profile(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "color.loadProfile";
let path = str_param(p, "path").ok_or_else(|| bad(C, "missing `path`"))?;
let path = text_param(p, "path", C)?.ok_or_else(|| bad(C, "missing `path`"))?;
crate::file_access::check_read(path).map_err(EngineError::Other)?;
let bytes = crate::store::read_capped(std::path::Path::new(path), crate::store::MAX_PROFILE_FILE).map_err(|e| bad(C, format!("{path}: {e}")))?;
let info = cms::register_icc(&bytes, None).map_err(|e| bad(C, e.to_string()))?;
@@ -209,7 +218,7 @@ fn load_profile(s: &mut Session, p: &Value) -> Result<Value> {
/// Forget a profile `color.loadProfile` loaded, and the copy that is kept of it.
fn remove_profile(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "color.removeProfile";
let name = str_param(p, "name").ok_or_else(|| bad(C, "missing `name`"))?;
let name = text_param(p, "name", C)?.ok_or_else(|| bad(C, "missing `name`"))?;
let info = cms::profiles().into_iter().find(|i| i.name == name).ok_or_else(|| bad(C, format!("no profile named `{name}`")))?;
if info.builtin {
return Err(bad(C, format!("`{name}` is a built-in profile and stays")));
@@ -289,7 +298,7 @@ pub enum Grays {
impl Grays {
/// The `grays` param of `cmd`: `"profile"` (default) or `"black"`.
pub(crate) fn param(cmd: &str, p: &Value) -> Result<Self> {
match str_param(p, "grays").map(str::to_ascii_lowercase).as_deref() {
match text_param(p, "grays", cmd)?.map(str::to_ascii_lowercase).as_deref() {
None | Some("profile") => Ok(Grays::Profile),
Some("black") => Ok(Grays::Black),
Some(g) => Err(bad(cmd, format!("grays must be \"profile\" or \"black\", not `{g}`"))),
@@ -302,18 +311,18 @@ impl Grays {
/// colour through the colour settings.
pub(crate) fn convert_mode(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "file.documentColorMode";
let mode = match str_param(p, "mode").map(str::to_ascii_lowercase).as_deref() {
let mode = match p.get("mode").and_then(Value::as_str).map(str::to_ascii_lowercase).as_deref() {
Some("cmyk") => ColorMode::Cmyk,
Some("rgb") => ColorMode::Rgb,
_ => return Err(bad(C, "mode must be \"cmyk\" or \"rgb\"")),
};
let intent = intent_param(p, C)?;
let grays = Grays::param(C, p)?;
let convert = switch_or(p, "convert", true, C)?;
// Choosing the mode the document already has (the checked menu item) is not an edit.
if s.doc()?.doc.color_mode == mode {
return Ok(json!({ "changed": 0 }));
}
let convert = bool_or(p, "convert", true);
let mut changed = 0usize;
s.edit("Document Color Mode", |d, _| {
changed = set_color_mode(d, mode, convert, intent, grays);
@@ -654,7 +663,7 @@ fn convert_color(_: &mut Session, p: &Value) -> Result<Value> {
let c = cms::active();
let intent = intent_param(p, C)?.unwrap_or(c.settings().intent);
let lab = c.lab(&col);
let (model, vals) = match str_param(p, "to").unwrap_or("rgb") {
let (model, vals) = match text_param(p, "to", C)?.unwrap_or("rgb") {
"rgb" => ("rgb", values(&c.convert(&col, Model::Rgb, intent))),
"cmyk" => ("cmyk", values(&c.convert(&col, Model::Cmyk, intent))),
"gray" | "grayscale" => ("gray", values(&c.convert(&col, Model::Gray, intent))),
@@ -728,7 +737,7 @@ fn view_json(v: &proof::ProofView) -> Value {
fn proof_setup(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "view.proofSetup";
let mut v = proof::view();
if let Some(t) = str_param(p, "target") {
if let Some(t) = text_param(p, "target", C)? {
let t = ProofTarget::parse(t).ok_or_else(|| bad(C, format!("unknown proof target `{t}`")))?;
if let ProofTarget::Cmyk(name) = &t
&& !cms::profile(name).is_some_and(|k| k.kind == cms::ProfileKind::Cmyk)
@@ -740,8 +749,8 @@ fn proof_setup(s: &mut Session, p: &Value) -> Result<Value> {
if let Some(i) = intent_param(p, C)? {
v.setup.intent = i;
}
v.setup.simulate_paper = bool_or(p, "simulatePaper", v.setup.simulate_paper);
if let Some(on) = p.get("proof").and_then(Value::as_bool) {
v.setup.simulate_paper = switch_or(p, "simulatePaper", v.setup.simulate_paper, C)?;
if let Some(on) = on_param(p, "proof", C)? {
v.proof_colors = on;
}
proof::set_view(v.clone());
@@ -749,9 +758,10 @@ fn proof_setup(s: &mut Session, p: &Value) -> Result<Value> {
Ok(view_json(&v))
}
fn toggle_view(s: &mut Session, p: &Value, f: impl FnOnce(&mut proof::ProofView, Option<bool>)) -> Result<Value> {
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, p.get("on").and_then(Value::as_bool));
f(&mut v, on);
proof::set_view(v.clone());
touch_all(s);
Ok(view_json(&v))
@@ -779,16 +789,23 @@ fn separations(s: &mut Session, p: &Value) -> Result<Value> {
.map(|d| proof::plates(&d.doc).into_iter().map(|p| p.name).collect())
.unwrap_or_else(|| cms::PROCESS_PLATES.iter().map(|s| s.to_string()).collect());
let check = |n: &str| if all.iter().any(|a| a == n) { Ok(n.to_string()) } else { Err(bad(C, format!("unknown plate `{n}`"))) };
if let Some(on) = p.get("on").and_then(Value::as_bool) {
if let Some(on) = on_param(p, "on", C)? {
v.separations = if on { Some(v.separations.take().unwrap_or_else(|| all.clone())) } else { None };
}
if let Some(a) = p.get("plates").and_then(Value::as_array) {
v.separations = Some(a.iter().filter_map(Value::as_str).map(check).collect::<Result<Vec<_>>>()?);
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<_>>>()?);
}
Some(_) => {
let key = "plates";
return Err(bad(C, format!("`{key}` must be an array of these names: {}", all.join(", "))));
}
}
if let Some(n) = str_param(p, "only") {
if let Some(n) = text_param(p, "only", C)? {
v.separations = Some(vec![check(n)?]);
}
if let Some(n) = str_param(p, "toggle") {
if let Some(n) = text_param(p, "toggle", C)? {
let n = check(n)?;
let vis = v.separations.get_or_insert_with(|| all.clone());
if let Some(i) = vis.iter().position(|x| *x == n) {
@@ -804,8 +821,8 @@ fn separations(s: &mut Session, p: &Value) -> Result<Value> {
fn set_spot(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "swatch.setSpot";
let name = str_param(p, "name").ok_or_else(|| bad(C, "missing `name`"))?.to_string();
let spot = bool_or(p, "spot", true);
let name = text_param(p, "name", C)?.ok_or_else(|| bad(C, "missing `name`"))?.to_string();
let spot = switch_or(p, "spot", true, C)?;
let sw = s.doc()?.doc.swatch(&name).ok_or_else(|| bad(C, format!("no swatch `{name}`")))?;
if !matches!(sw.paint, Paint::Solid { .. }) {
return Err(bad(C, "only solid-colour swatches can be spot colours"));
crates/engine/src/tests/colormgmt.rs
+89
View File
@@ -542,3 +542,92 @@ fn intents_reach_the_document_conversion() {
assert_ne!(first(&a), first(&b));
let _ = Intent::Perceptual;
}
/// The error `cmd` gives for `params`: it must name `msg`.
fn refused(s: &mut Session, cmd: &str, params: Value, msg: &str) {
let e = match s.execute(cmd, &params) {
Ok(r) => panic!("{cmd} {params} gave {r}"),
Err(e) => e.to_string(),
};
assert!(e.contains(msg), "{cmd} {params}: {e}");
}
#[test]
fn color_settings_refuse_a_param_of_the_wrong_type() {
let _g = GLOBAL.lock().unwrap_or_else(|e| e.into_inner());
let mut s = session();
let before = cms::active_settings();
let rev = s.doc().unwrap().revision;
let c = "edit.colorSettings";
refused(&mut s, c, json!({"bpc": "no"}), "`bpc` must be true or false");
refused(&mut s, c, json!({"rgb": 5}), "`rgb` must be a string");
refused(&mut s, c, json!({"cmyk": true}), "`cmyk` must be a string");
refused(&mut s, c, json!({"intent": 2}), "`intent` must be a string");
assert_eq!(cms::active_settings(), before, "the settings are unchanged");
assert_eq!(s.doc().unwrap().revision, rev, "nothing redraws");
// Omitted and null params keep the settings.
s.execute(c, &json!({"bpc": null, "rgb": null})).unwrap();
assert_eq!(cms::active_settings(), before);
refused(&mut s, "color.convert", json!({"color": "#808080", "to": 1}), "`to` must be a string");
refused(&mut s, "color.convert", json!({"color": "#808080", "intent": ["relative"]}), "`intent` must be a string");
assert_eq!(s.execute("color.convert", &json!({"color": "#808080", "to": null})).unwrap()["model"], "rgb");
refused(&mut s, "color.loadProfile", json!({"path": 7}), "`path` must be a string");
refused(&mut s, "color.removeProfile", json!({"name": 7}), "`name` must be a string");
}
#[test]
fn document_color_mode_refuses_a_param_of_the_wrong_type() {
let _g = GLOBAL.lock().unwrap_or_else(|e| e.into_inner());
let mut s = session();
let a = rect(&mut s, 0.0, 0.0, 10.0, 10.0, json!("#ff0000"));
let fill = fill_of(&s, a);
let undo = s.doc().unwrap().history.undo.len();
for c in ["file.documentColorMode", "object.convertDocumentColorMode"] {
refused(&mut s, c, json!({"mode": "cmyk", "convert": "no"}), "`convert` must be true or false");
refused(&mut s, c, json!({"mode": "rgb", "convert": 0}), "`convert` must be true or false");
refused(&mut s, c, json!({"mode": "cmyk", "grays": 1}), "`grays` must be a string");
refused(&mut s, c, json!({"mode": "cmyk", "intent": false}), "`intent` must be a string");
}
let st = s.doc().unwrap();
assert_eq!(st.doc.color_mode, ColorMode::Rgb);
assert_eq!(st.history.undo.len(), undo, "no undo step");
assert_eq!(fill_of(&s, a), fill, "the colors are unchanged");
}
#[test]
fn proof_views_refuse_a_param_of_the_wrong_type() {
let _g = GLOBAL.lock().unwrap_or_else(|e| e.into_inner());
let mut s = session();
let view = vectorcraft_render::proof::view();
let rev = s.doc().unwrap().revision;
refused(&mut s, "view.proofSetup", json!({"simulatePaper": "yes"}), "`simulatePaper` must be true or false");
refused(&mut s, "view.proofSetup", json!({"proof": 1}), "`proof` must be true or false");
refused(&mut s, "view.proofSetup", json!({"target": 3}), "`target` must be a string");
refused(&mut s, "view.proofColors", json!({"on": "true"}), "`on` must be true or false");
refused(&mut s, "view.overprintPreview", json!({"on": 1}), "`on` must be true or false");
refused(&mut s, "view.separationsPreview", json!({"on": "false"}), "`on` must be true or false");
refused(&mut s, "view.separationsPreview", json!({"plates": "Cyan"}), "`plates` must be an array of these names: Cyan, Magenta");
refused(&mut s, "view.separationsPreview", json!({"plates": ["Cyan", 2]}), "`plates` must be an array of these names");
refused(&mut s, "view.separationsPreview", json!({"only": 0}), "`only` must be a string");
refused(&mut s, "view.separationsPreview", json!({"toggle": ["Cyan"]}), "`toggle` must be a string");
assert_eq!(vectorcraft_render::proof::view(), view, "the proof view is unchanged");
assert_eq!(s.doc().unwrap().revision, rev, "nothing redraws");
// A null `on` toggles, as an omitted one does.
let r = s.execute("view.proofColors", &json!({"on": null})).unwrap();
assert_eq!(r["proofColors"], !view.proof_colors);
s.execute("view.proofColors", &json!({"on": view.proof_colors})).unwrap();
}
#[test]
fn set_spot_refuses_a_param_of_the_wrong_type() {
let _g = GLOBAL.lock().unwrap_or_else(|e| e.into_inner());
let mut s = session();
s.execute("swatch.new", &json!({"name": "Ink", "color": {"c": 0, "m": 60, "y": 100, "k": 0}, "global": true})).unwrap();
let undo = s.doc().unwrap().history.undo.len();
refused(&mut s, "swatch.setSpot", json!({"name": "Ink", "spot": "true"}), "`spot` must be true or false");
refused(&mut s, "swatch.setSpot", json!({"name": 1}), "`name` must be a string");
assert!(!s.doc().unwrap().doc.swatch("Ink").unwrap().spot, "the swatch stays a process color");
assert_eq!(s.doc().unwrap().history.undo.len(), undo, "no undo step");
s.execute("swatch.setSpot", &json!({"name": "Ink", "spot": null})).unwrap();
assert!(s.doc().unwrap().doc.swatch("Ink").unwrap().spot, "a null `spot` is the default, true");
}
docs/mcp.md
+4
View File
@@ -662,6 +662,10 @@ them. `grays: "black"` puts RGB greys with R = G = B on the black plate instead,
Convert to Grayscale before the switch would give); other colors convert as usual. The reply's `changed` counts the
colors that changed in the art, symbols, graph designs and pattern tiles; it does not count the colors of graphic
styles and brushes.
`edit.colorSettings`, `file.documentColorMode`, `color.convert`, `color.loadProfile`, `color.removeProfile`,
`view.proofSetup`, `view.proofColors`, `view.overprintPreview`, `view.separationsPreview` and `swatch.setSpot` reject a
param of the wrong type, such as `"bpc": "no"`, `"on": 1` or `"to": 5`, and change nothing. Omitting a param or giving
null keeps its default.
SVG import keeps what the canvas edits live. `<pattern>` becomes a pattern swatch. A `<symbol>` with `<use>` becomes a
symbol (named after its `data-name`, else its id) with an instance per `<use>`; a `<use>` that shows it differently