From 36aecff81d0f5a6f62529f9970d8115c800548c1 Mon Sep 17 00:00:00 2001 From: MortenHusted Date: Sun, 18 Oct 2026 18:17:27 +0200 Subject: [PATCH] Select: select.color reads compound paths and empty type as document.inspect does A compound path holds its members, so select.color passed over it and looked at the members instead, whose paint inspect doesn't report: a black compound path (art with holes, as PDFs bring in) never matched "#000000". It now matches by its own fill and stroke, and its members never do. Type without characters reads as the default character fill, as inspect reports it (CodeRabbit on #2939). --- crates/engine/src/cmd/select.rs | 9 ++++++++- crates/engine/src/cmd/wand.rs | 5 +++-- crates/engine/src/tests_menucmds.rs | 17 +++++++++++++++++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/select.rs b/crates/engine/src/cmd/select.rs index d169b3903..a6fb02140 100644 --- a/crates/engine/src/cmd/select.rs +++ b/crates/engine/src/cmd/select.rs @@ -550,7 +550,14 @@ fn select_color(s: &mut Session, p: &Value) -> Result { return Err(bad(C, "give `fill`, `stroke` or both")); } let reads_as = |paint: Paint, hex: Option<&str>| hex.is_none_or(|h| paint.color().is_some_and(|c| c.to_hex() == h)); - select_where(s, C, p, |_, n| !n.is_container() && reads_as(fill_of(n), fill.as_deref()) && reads_as(stroke_of(n).0, stroke.as_deref())) + select_where(s, C, p, |d, n| paint_reported(d, n) && reads_as(fill_of(n), fill.as_deref()) && reads_as(stroke_of(n).0, stroke.as_deref())) +} + +/// Does `document.inspect` report a fill and stroke for `n`? A compound path's are its own, though +/// it holds its members, and its members report none. +fn paint_reported(d: &Document, n: &Node) -> bool { + let compound = |n: &Node| matches!(n.kind, NodeKind::Compound { .. }); + compound(n) || (!n.is_container() && !d.parent_of(n.id).and_then(|p| d.node(p)).is_some_and(compound)) } fn same(s: &mut Session, cmd: &str, eq: impl Fn(&Node, &Node) -> bool) -> Result { diff --git a/crates/engine/src/cmd/wand.rs b/crates/engine/src/cmd/wand.rs index d73ef9222..23589e6b3 100644 --- a/crates/engine/src/cmd/wand.rs +++ b/crates/engine/src/cmd/wand.rs @@ -87,10 +87,11 @@ fn set(s: &mut Session, p: &Value) -> Result { Ok(json!(s.menu.wand)) } -/// The fill an object shows: type's comes from its characters (the first run speaks for all). +/// The fill an object shows: type's comes from its characters (the first run speaks for all; type +/// without any, the default character fill, as `document.inspect` reports it). pub(crate) fn fill_of(n: &Node) -> Paint { match &n.kind { - NodeKind::Text(t) => t.runs.first().map(|r| r.style.fill.clone()).unwrap_or_default(), + NodeKind::Text(t) => t.runs.first().map(|r| r.style.fill.clone()).unwrap_or_else(|| vectorcraft_doc::CharStyle::default().fill), _ => n.appearance.fill_paint(), } } diff --git a/crates/engine/src/tests_menucmds.rs b/crates/engine/src/tests_menucmds.rs index 17bae9209..637a4dfa7 100644 --- a/crates/engine/src/tests_menucmds.rs +++ b/crates/engine/src/tests_menucmds.rs @@ -1054,6 +1054,23 @@ fn select_color_matches_fill_and_stroke_without_a_selection() { s.execute("select.none", &json!({})).unwrap(); s.execute("select.reselect", &json!({})).unwrap(); assert_eq!(selected(&s), vec![a]); + // A compound path reads as its own paint, never its members'. + let outer = rect(&mut s, 100.0, 0.0, 50.0, 50.0); + let inner = rect(&mut s, 110.0, 10.0, 20.0, 20.0); + fill(&mut s, outer, "#000000"); + fill(&mut s, inner, "#000000"); + sel(&mut s, &[outer, inner]); + let compound = id_of(&s.execute("object.compoundPath.make", &json!({})).unwrap()); + assert_eq!(pick(&mut s, json!({"fill": "#000000"})), vec![a, t, compound]); + fill(&mut s, compound, "#ff0000"); + assert_eq!(pick(&mut s, json!({"fill": "#000000"})), vec![a, t]); + assert_eq!(pick(&mut s, json!({"fill": "#ff0000"})), vec![c, compound]); + // Type without characters reads as the default character fill, black. + let empty = text(&mut s, "x"); + if let Some(NodeKind::Text(t)) = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().doc).node_mut(empty).map(|n| &mut n.kind) { + t.runs.clear(); + } + assert_eq!(pick(&mut s, json!({"fill": "#000000"})), vec![a, t, empty]); assert!(s.execute("select.color", &json!({})).is_err()); assert!(s.execute("select.color", &json!({"fill": "not a colour"})).is_err()); }