Merge pull request: land #580 (fix #522)

Land #580 (fix #522: SVG round trips keep hidden objects and data-* attributes)
This commit is contained in:
BFlat 2026-10-09 03:47:30 +07:00 committed by GitHub
commit d066e0ddc0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 218 additions and 31 deletions

File diff suppressed because one or more lines are too long

View File

@ -69,7 +69,8 @@ USAGE:
Open IN (any readable format) and export OUT in the format its extension picks (see Writable
formats). --artboard is 0-based, --range 1-based (\"1-3,5\"); a PDF gets every artboard
unless one of them is given, EPS the bounds of the art, the other formats the first artboard.
Live effects are kept; --outline-text writes SVG text as paths.
Live effects are kept, and hidden layers and objects (written hidden in SVG and PSD), with
SVG's data-* attributes; --outline-text writes SVG text as paths.
vectorcraft-cli info FILE
Print a JSON summary: the import warnings (what didn't come in as it was, such as an EPS
@ -177,12 +178,10 @@ fn convert(args: &[String]) -> Result<(), String> {
let [input, output] = <[String; 2]>::try_from(files).map_err(|_| "convert needs IN and OUT")?;
let mut h = Headless::new();
h.call("app.open", json!({"path": input})).map_err(|e| format!("open {input}: {e}"))?;
let r = h
.call(
"engine.execute",
json!({"command": "document.export", "params": {"path": output, "scale": scale, "artboard": artboard, "range": range, "outlineText": outline_text}}),
)
.map_err(|e| format!("export {output}: {e}"))?;
// A conversion keeps hidden layers and objects, written hidden, where the format can (SVG,
// PSD).
let params = json!({"path": output, "scale": scale, "artboard": artboard, "range": range, "outlineText": outline_text, "hiddenLayers": true});
let r = h.call("engine.execute", json!({"command": "document.export", "params": params})).map_err(|e| format!("export {output}: {e}"))?;
outln!("{r}");
Ok(())
}

View File

@ -1214,6 +1214,10 @@ pub struct ObjectAttributes {
/// The note shown in the Attributes panel.
#[serde(default, skip_serializing_if = "String::is_empty")]
pub note: String,
/// The object's own data, in order: SVG's `data-*` attributes (`data-pivot="100,180"` is
/// `("pivot", "100,180")`), kept from import to export (`object.setProps {data}`).
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub data: Vec<(String, String)>,
}
/// The Attributes panel's Image Map shapes.

View File

@ -70,7 +70,7 @@ pub const OPTIONS: &[FormatOption] = &[
name: "hiddenLayers",
ty: "boolean",
default: "false",
description: "keep hidden layers, not displayed (display:none); document.save sets it unless given",
description: "keep hidden layers and objects, not displayed (display=\"none\"); document.save sets it unless given",
},
FormatOption {
name: "encoding",

View File

@ -150,7 +150,7 @@ pub fn specs() -> Vec<CommandSpec> {
"Object Properties",
[],
None,
"{ids?|id?, name?, visible?, locked?, opacity?: 0..100, blend?: \"Multiply\"…, isolate?, knockout?: \"on\"|\"off\"|\"neutral\"|bool (true = on, false = neutral), knockoutShape?: bool}",
"{ids?|id?, name?, visible?, locked?, opacity?: 0..100, blend?: \"Multiply\"…, isolate?, knockout?: \"on\"|\"off\"|\"neutral\"|bool (true = on, false = neutral), knockoutShape?: bool, data?: {key: \"value\" | null (removes it)} (the object's own data, SVG data-* attributes: {pivot: \"100,180\"} is data-pivot; document.node → attrs.data)}",
has_doc,
set_props
),
@ -738,6 +738,32 @@ fn exit_isolation(s: &mut Session, _: &Value) -> Result<Value> {
ok()
}
/// The most data entries `object.setProps` takes at once.
const MAX_DATA: usize = 256;
/// `object.setProps`'s `data`: each key (a data-* attribute's name: letters, digits, `-`, `_`,
/// `.`) with its value, or `None` to remove it.
fn data_param(m: &serde_json::Map<String, Value>) -> Result<Vec<(String, Option<String>)>> {
const C: &str = "object.setProps";
if m.len() > MAX_DATA {
return Err(bad(C, format!("data takes at most {MAX_DATA} keys at once")));
}
m.iter()
.map(|(k, v)| {
let k = k.strip_prefix("data-").unwrap_or(k);
if k.is_empty() || !k.chars().all(|c| c.is_ascii_alphanumeric() || matches!(c, '-' | '_' | '.')) || k == "name" || k.starts_with("vc-") {
return Err(bad(C, format!("data key `{k}`: letters, digits, -, _ and . (not name or vc-…)")));
}
let v = match v {
Value::Null => None,
Value::String(s) => Some(s.clone()),
other => Some(other.to_string()),
};
Ok((k.to_string(), v))
})
.collect()
}
fn set_props(s: &mut Session, p: &Value) -> Result<Value> {
let ids = targets(s, p)?;
let opacity = p.get("opacity").and_then(Value::as_f64).map(percent);
@ -745,6 +771,11 @@ fn set_props(s: &mut Session, p: &Value) -> Result<Value> {
Some(b) => Some(BlendMode::parse(b).ok_or_else(|| bad("object.setProps", format!("unknown blend mode `{b}`")))?),
None => None,
};
let data = match p.get("data") {
None | Some(Value::Null) => None,
Some(Value::Object(m)) => Some(data_param(m)?),
Some(v) => return Err(bad("object.setProps", format!("data must be an object of key: value, not {v}"))),
};
let knockout = match p.get("knockout") {
Some(v) => Some(
Knockout::from_value(v)
@ -779,6 +810,24 @@ fn set_props(s: &mut Session, p: &Value) -> Result<Value> {
if let Some(v) = p.get("knockoutShape").and_then(Value::as_bool) {
n.knockout_shape = v;
}
if let Some(changes) = &data {
n.edit_attrs(|a| {
for (k, v) in changes {
match (a.data.iter().position(|(key, _)| key == k), v) {
(Some(i), Some(v)) => {
if let Some(e) = a.data.get_mut(i) {
e.1 = v.clone();
}
}
(Some(i), None) => {
a.data.remove(i);
}
(None, Some(v)) => a.data.push((k.clone(), v.clone())),
(None, None) => {}
}
}
});
}
}
Ok(())
})?;

View File

@ -190,3 +190,27 @@ fn svg_export_links_objects_and_import_reads_the_links_back() {
});
assert_eq!(urls, [Some("https://example.com/?a=1&b=2".to_string()), None], "the link wraps the path alone");
}
/// #522: an object's own data (SVG `data-*` attributes) is set with `object.setProps {data}`,
/// undone in one step, written to SVG and kept by the native format.
#[test]
fn object_data_is_set_saved_and_written_to_svg() {
let mut s = session();
let a = rect(&mut s, 10.0);
let data = |s: &Session| s.doc().unwrap().doc.node(a).unwrap().attrs.as_deref().map(|x| x.data.clone()).unwrap_or_default();
let pair = |k: &str, v: &str| (k.to_string(), v.to_string());
run(&mut s, "object.setProps", json!({"data": {"pivot": "60,100", "z": 3}}));
assert_eq!(data(&s), [pair("pivot", "60,100"), pair("z", "3")], "values are text");
run(&mut s, "object.setProps", json!({"data": {"data-pivot": "1,2", "z": null}}));
assert_eq!(data(&s), [pair("pivot", "1,2")], "a data- prefix is optional; null removes a key");
run(&mut s, "edit.undo", json!({}));
assert_eq!(data(&s).len(), 2, "one step");
for bad in [json!({"data": {"vc-x": "1"}}), json!({"data": {"name": "x"}}), json!({"data": {"a b": "1"}}), json!({"data": "pivot"})] {
assert!(s.execute("object.setProps", &bad).is_err(), "{bad}");
}
let svg = run(&mut s, "document.serialize", json!({"format": "svg"}))["text"].as_str().unwrap().to_string();
assert!(svg.contains("data-pivot=\"60,100\" data-z=\"3\""), "{svg}");
let native = vectorcraft_format::save(&s.doc().unwrap().doc, false);
let back = vectorcraft_format::load(&native).unwrap();
assert_eq!(back.node(a).unwrap().attrs, s.doc().unwrap().doc.node(a).unwrap().attrs);
}

View File

@ -50,9 +50,13 @@ pub(crate) fn blend_css(b: BlendMode) -> &'static str {
}
}
/// An object's opacity, blend mode and isolation.
/// An object's opacity, blend mode and isolation, and `display: none` when it is hidden (written
/// only when hidden objects are kept).
pub(crate) fn transparency(n: &Node) -> Props {
let mut p = Props::new();
if !n.visible {
p.push(("display", "none".into()));
}
if n.opacity < 1.0 {
p.push(("opacity", fmt_num(n.opacity as f64, 3)));
}

View File

@ -545,15 +545,25 @@ impl Writer<'_> {
}
/// ` id="…"` for a named object, plus ` data-name="…"` with the name itself when the id had to
/// differ from it (spaces, punctuation, duplicates, the unique prefix).
/// The id of `n`'s element (with its name when the id isn't it), then the object's own data
/// as `data-*` attributes.
fn id_attr(&self, n: &Node) -> String {
if self.anonymous {
return String::new();
}
let Some(id) = self.names.get(&n.id) else { return String::new() };
match n.name.as_deref() {
Some(name) if name != id => format!(" id=\"{}\" data-name=\"{}\"", xml_escape(id), xml_escape(name)),
_ => format!(" id=\"{}\"", xml_escape(id)),
let mut out = match (self.names.get(&n.id), n.name.as_deref()) {
(Some(id), Some(name)) if name != id => format!(" id=\"{}\" data-name=\"{}\"", xml_escape(id), xml_escape(name)),
(Some(id), _) => format!(" id=\"{}\"", xml_escape(id)),
(None, _) => String::new(),
};
for (k, v) in n.attrs.as_deref().map_or(&[][..], |a| a.data.as_slice()) {
// Names as XML takes them; ours (`name`, `vc-…`) are written by the export itself.
let valid = k.chars().all(|c| c.is_ascii_alphanumeric() || matches!(c, '-' | '_' | '.')) && k != "name" && !k.starts_with("vc-");
if valid && !k.is_empty() {
out.push_str(&format!(" data-{k}=\"{}\"", xml_escape(v)));
}
}
out
}
fn matrix(&self, m: Affine) -> String {
let c = m.as_coeffs();
@ -1130,15 +1140,12 @@ impl Writer<'_> {
std::mem::replace(&mut self.body, body)
}
/// Props of a group (or layer): a knockout group is isolated, a hidden layer not displayed.
/// Props of a group (or layer): a knockout group is isolated, a hidden one not displayed.
fn group_props(&self, n: &Node) -> Props {
let mut p = css::transparency(n);
if !n.isolate && n.knocks_out(self.knockout) {
p.push(("isolation", "isolate".into()));
}
if !n.visible {
p.push(("display", "none".into()));
}
p
}
@ -1221,9 +1228,9 @@ impl Writer<'_> {
}
fn node_body(&mut self, n: &Node) {
// Hidden objects are left out; hidden layers too, unless the options keep them (hidden).
let kept = self.opts.hidden_layers && matches!(n.kind, NodeKind::Layer { template: false, .. });
if !(n.visible || kept) {
// Hidden layers and objects are left out, unless the options keep them (hidden). Template
// layers never print.
if !(n.visible || self.opts.hidden_layers) {
return;
}
if has_raster(&n.appearance.effects) {

View File

@ -64,6 +64,7 @@ pub(crate) fn import(svg: &str, opts: &ImportOptions) -> Result<(Document, Vec<S
blends: fx::blends(&xml),
files,
non_scaling: found.non_scaling,
data: found.data,
screen: (kx * ky).sqrt(),
};
if im.files.nested_text() {
@ -124,6 +125,7 @@ pub(crate) fn import(svg: &str, opts: &ImportOptions) -> Result<(Document, Vec<S
}
im.add_unclipped(&mut l, 0, std::mem::take(&mut loose));
l.visible = !im.hidden.contains(g.id());
im.keep_data(g.id(), &mut l);
im.doc.layers.push(Arc::new(l));
}
} else {
@ -210,6 +212,8 @@ struct Importer {
files: files::Files,
/// Ids of the shapes whose strokes don't scale ([`vector_effect`]).
non_scaling: HashSet<String>,
/// The `data-*` attributes of elements, by id ([`user_data`]).
data: HashMap<String, Vec<(String, String)>>,
/// Points per screen pixel (the root's user unit before its `viewBox`): what a non-scaling
/// stroke's width is measured in.
screen: f64,
@ -320,6 +324,8 @@ struct Found {
files: files::Files,
/// Ids of the shapes whose strokes don't scale ([`vector_effect`]).
non_scaling: HashSet<String>,
/// The `data-*` attributes of elements, by id ([`user_data`]).
data: HashMap<String, Vec<(String, String)>>,
warnings: Vec<String>,
}
@ -390,7 +396,7 @@ impl Edits {
fn prepass<'s>(svg: &'s str, opts: &ImportOptions) -> (Cow<'s, str>, Found) {
let mut found = Found::default();
// (`:` for prefixed elements such as `<svg:image>`.)
if !["<a", "display", "<use", ":use", "<image", ":image", vector_effect::NON_SCALING].iter().any(|t| svg.contains(t)) {
if !["<a", "display", "<use", ":use", "<image", ":image", vector_effect::NON_SCALING, "data-"].iter().any(|t| svg.contains(t)) {
return (svg.into(), found);
}
let Ok(xml) = roxmltree::Document::parse_with_options(svg, roxmltree::ParsingOptions { allow_dtd: true, ..Default::default() }) else {
@ -403,6 +409,14 @@ fn prepass<'s>(svg: &'s str, opts: &ImportOptions) -> (Cow<'s, str>, Found) {
found.links.insert(edits.id(svg, a), url.to_string());
}
}
if svg.contains("data-") {
for e in elements().filter(|e| OBJECT_ELEMENTS.contains(&e.tag_name().name())) {
let data = user_data(e);
if !data.is_empty() {
found.data.insert(edits.id(svg, e), data);
}
}
}
let symbols: HashSet<&str> = elements().filter(|n| n.tag_name().name() == "symbol").filter_map(|n| n.attribute("id")).collect();
for u in elements().filter(|n| n.tag_name().name() == "use") {
if let Some(sym) = href(u).and_then(|h| h.strip_prefix('#')).filter(|s| symbols.contains(s)) {
@ -456,6 +470,24 @@ fn tag_name_end(svg: &str, n: XNode) -> usize {
svg.get(start + 1..).and_then(|s| s.find(|c: char| c.is_whitespace() || c == '/' || c == '>')).map_or(start + 2, |i| start + 1 + i)
}
/// The elements that become objects, whose `data-*` attributes are kept.
const OBJECT_ELEMENTS: &[&str] = &["g", "a", "use", "path", "rect", "circle", "ellipse", "line", "polyline", "polygon", "image"];
/// The most `data-*` attributes kept per element (the file is untrusted).
const MAX_DATA: usize = 64;
/// The `data-*` attributes of element `e`, in order, without the `data-` (ours, `data-name` and
/// `data-vc-*`, aside).
fn user_data(e: XNode) -> Vec<(String, String)> {
e.attributes()
.filter(|a| a.namespace().is_none())
.filter_map(|a| Some((a.name().strip_prefix("data-")?, a.value())))
.filter(|(k, _)| !k.is_empty() && *k != "name" && !k.starts_with("vc-"))
.take(MAX_DATA)
.map(|(k, v)| (k.to_string(), v.to_string()))
.collect()
}
/// Make `n` link to `url`, unless it links somewhere already (an inner link wins).
fn link(n: &mut Node, url: &str) {
if n.url().is_none() {
@ -592,9 +624,18 @@ impl Importer {
if self.hidden.contains(n.id()) {
out.visible = false;
}
self.keep_data(n.id(), &mut out);
Some(out)
}
/// Give `n` the `data-*` attributes of element `id`, unless it has data already (what an
/// inner element made of it keeps).
fn keep_data(&self, id: &str, n: &mut Node) {
if let Some(data) = self.data.get(id).filter(|_| n.attrs.as_ref().is_none_or(|a| a.data.is_empty())) {
n.edit_attrs(|a| a.data = data.clone());
}
}
/// A gradient's stops, midpoints restored.
fn stops(&self, g: &usvg::BaseGradient) -> Vec<GradientStop> {
gradient_stops(g, self.midpoints.get(g.id()).map(Vec::as_slice))
@ -1114,9 +1155,10 @@ impl Importer {
std::mem::take(&mut self.uses),
std::mem::take(&mut self.labels),
std::mem::take(&mut self.non_scaling),
std::mem::take(&mut self.data),
);
let art = self.group_node(tree.root(), acc);
(self.links, self.hidden, self.uses, self.labels, self.non_scaling) = outer;
(self.links, self.hidden, self.uses, self.labels, self.non_scaling, self.data) = outer;
art
}
}

View File

@ -208,7 +208,8 @@ pub struct ExportOptions {
/// One positioned `<tspan>` per line of type instead of one per style run, tab stop and
/// justified word (smaller; viewers then space the line with their own font metrics).
pub fewer_tspans: bool,
/// Keep hidden layers, written hidden (`display:none`), as Save does; exports leave them out.
/// Keep hidden layers and objects, written hidden (`display="none"`), as Save does; exports
/// leave them out unless asked.
pub hidden_layers: bool,
/// The file's character encoding (see [`Output::bytes`]).
pub encoding: Encoding,

View File

@ -1,6 +1,7 @@
//! Symbols export as one `<symbol>` def and a `<use>` per instance (instances the def can't stand
//! for get their own art), hidden layers are kept hidden when asked and come back hidden, clipped
//! layers come in as clipping layers, and the editing data notices edits made elsewhere.
//! for get their own art), hidden layers and objects are kept hidden when asked and come back
//! hidden with their `data-*` attributes, clipped layers come in as clipping layers, and the
//! editing data notices edits made elsewhere.
// Integration tests: unwrapping and panicking on failure is fine here, unlike in shipped code (AGENTS.md › Robustness).
#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)]
@ -205,6 +206,61 @@ fn hidden_layers_are_kept_hidden_when_asked() {
}
}
/// An animation rig (#522): hidden alternates (a group and a lone path) and rotation pivots kept as
/// `data-*` attributes.
const RIG: &str = r##"<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 200 200" width="200" height="200">
<g id="character" data-pivot="100,180">
<g id="arm" data-pivot="60,100" data-z-order="2"><path d="M40 100 L60 90 L80 110 Z" fill="#b5835a"/></g>
<g id="arm-raised" data-pivot="60,100" display="none"><path d="M40 100 L50 60 L70 70 Z" fill="#b5835a"/></g>
<path id="eye-closed" d="M90 80 L110 80 L100 95 Z" fill="#000" style="display:none" data-blink="1"/>
</g>
</svg>"##;
/// The `data-*` attributes of the object named `name`.
fn data_of(d: &Document, name: &str) -> Vec<(String, String)> {
let mut found = vec![];
d.walk(|n| {
if n.name.as_deref() == Some(name) {
found = n.attrs.as_deref().map(|a| a.data.clone()).unwrap_or_default();
}
});
found
}
#[test]
fn hidden_objects_and_data_attributes_round_trip() {
let d = import(RIG).unwrap();
let pivot = |v: &str| vec![("pivot".to_string(), v.to_string())];
assert_eq!(data_of(&d, "character"), pivot("100,180"));
assert_eq!(data_of(&d, "arm"), [("pivot".to_string(), "60,100".to_string()), ("z-order".to_string(), "2".to_string())]);
assert_eq!(data_of(&d, "arm-raised"), pivot("60,100"));
let hidden = |d: &Document| {
let mut v = vec![];
d.walk(|n| {
if !n.visible {
v.push(n.name.clone().unwrap_or_default());
}
});
v
};
assert_eq!(hidden(&d), ["arm-raised", "eye-closed"]);
// An export leaves the hidden objects out, their data with them; the others keep theirs.
let plain = export(&d, &ExportOptions::default());
assert!(!plain.contains("arm-raised") && !plain.contains("eye-closed") && plain.contains("data-pivot=\"60,100\""), "{plain}");
let styles = [vectorcraft_svg::Styling::PresentationAttributes, vectorcraft_svg::Styling::InlineStyle, vectorcraft_svg::Styling::InternalCss];
for styling in styles {
// Kept (Save, or asked), they are written hidden and come back hidden, with every attribute.
let kept = export(&d, &ExportOptions { hidden_layers: true, styling, ..Default::default() });
assert!(kept.contains("data-pivot=\"100,180\"") && kept.contains("data-z-order=\"2\"") && kept.contains("data-blink=\"1\""), "{kept}");
assert_eq!(resvg_render(&kept, 200, 200).pixel(100, 85), [255, 255, 255, 255], "hidden, not drawn: {kept}");
let back = import(&kept).unwrap();
assert_eq!(hidden(&back), ["arm-raised", "eye-closed"], "{kept}");
for name in ["character", "arm", "arm-raised", "eye-closed"] {
assert_eq!(data_of(&back, name), data_of(&d, name), "{name}: {kept}");
}
}
}
/// Each layer's art: clipping layer or not, and the kinds of its children.
fn layer_art(d: &Document) -> Vec<(String, bool, Vec<&'static str>)> {
d.layers.iter().map(|l| (l.name.clone().unwrap_or_default(), l.clips(), l.children().unwrap().iter().map(|c| c.kind_label()).collect())).collect()

View File

@ -266,9 +266,10 @@ place and paste). `run_command document.save {path: "x.svg", svg: {…}}` saves
Symbols export as one `<symbol>` with a `<use>` per instance. An instance the def can't stand for is written as its
own art: one stained by its fill; one scaled while the symbol has strokes (their weight doesn't scale), or rotated or
scaled while it has effects, brushes or live objects; and every instance of a symbol with pattern paints or unlinked
opacity masks (those stay on the page). `hiddenLayers: true` keeps hidden layers as groups that aren't displayed
(`display:none`); `document.save` keeps them unless told otherwise, exports leave them out, and they reopen as
hidden layers. A `preserveEditing` SVG carries the native document (CDATA in `<metadata>`) and a hash of the
opacity masks (those stay on the page). `hiddenLayers: true` keeps hidden layers and objects, not displayed
(`display="none"`); `document.save` and the CLI's `convert` keep them unless told otherwise, other exports leave
them out, and they reopen hidden. An object's own data (`object.setProps {data: {pivot: "100,180"}}`) is written as
`data-*` attributes (`data-pivot="100,180"`), and an SVG's `data-*` attributes come in as it. A `preserveEditing` SVG carries the native document (CDATA in `<metadata>`) and a hash of the
markup around it: if another app changed the SVG since, `document.open` reads it as plain SVG and says so in
`warnings`.