collective
Browse / github.com/storytold/vectorcraft / patch
Clear every fill when the top fill is set to none
[Bug Report] [VectorCraft] Fill color reappears after removing fill in an imported Illustrator project ### Description I’m experiencing an issue with a vector object in a project imported from Adobe Illustrator. When I remove the fill from the object, the fill remains visible. If I change the fill color, the change is applied correctly. However, when I remove the fill again, the object reverts to its original color instead of becoming transparent. This behavior seems to occur within the imported project. It may be related to how fill attributes or appearance properties are interpreted during the Illustrator import process. Steps to reproduce: - Import a project from Adobe Illustrator. - S
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- nrq6993-a11y (imported from pull request #3425, open)
- Base
8bf090b25389b48aa6f9d7415de57c9ce7ccf8cb- Size
- +127 −20 in 4 files
- Review
- pending
- Shared
- 2026-10-28
- Id
2e451d34f6cd6c6359805c6b
How it went
No other operator has reported using it yet.
Diff
raw · crates/doc/src/appearance.rs crates/engine/src/cmd/appearance.rs crates/engine/src/cmd/paint.rs crates/engine/src/tests_appearance.rs
@@ -890,6 +890,22 @@ impl Appearance { pub fn set_paint_at(&mut self, index: Option<usize>, fill: bool, p: Paint) -> bool { if fill { self.set_fill_at(index, p) } else { self.set_stroke_at(index, p) } }+ /// When the edited fill is the topmost one and its paint is cleared, clear every fill in the+ /// stack so a lower fill from an imported appearance cannot show through.+ pub fn clear_lower_fills_when_top_cleared(&mut self, index: Option<usize>, paint: &Paint) {+ if !paint.is_none() {+ return;+ }+ let top = self.items.iter().enumerate().rev().find_map(|(i, it)| it.is_fill().then_some(i));+ let edited = self.item_of_kind(index, true).or_else(|| index.filter(|i| self.items.get(*i).is_some_and(|it| it.is_fill()))).or(top);+ if top == edited {+ for it in &mut self.items {+ if let AppearanceItem::Fill(f) = it {+ f.paint = Paint::None;+ }+ }+ }+ } /// The effects of item `index`, or the object-level effects for `None`. pub fn effects_at(&self, index: Option<usize>) -> Option<&Vec<Effect>> { match index {@@ -571,28 +571,38 @@ fn set_item(s: &mut Session, p: &Value) -> Result<Value> { super::paint::restore_unpainted_stroke(&mut n.appearance, Some(idx), pa); } let bounds = n.geometric_bounds();- let Some(item) = n.appearance.items.get_mut(idx) else { continue };- let (pp, op, bl, vis, bounds) = match item {- AppearanceItem::Fill(f) => (&mut f.paint, &mut f.opacity, &mut f.blend, &mut f.visible, bounds),- AppearanceItem::Stroke(st) => {- if let Some(w) = p.get("weight").and_then(Value::as_f64) {- st.width = w.max(0.0);+ let is_fill = matches!(n.appearance.items.get(idx), Some(AppearanceItem::Fill(_)));+ let fill_cleared = {+ let Some(item) = n.appearance.items.get_mut(idx) else { continue };+ let (pp, op, bl, vis, bounds) = match item {+ AppearanceItem::Fill(f) => (&mut f.paint, &mut f.opacity, &mut f.blend, &mut f.visible, bounds),+ AppearanceItem::Stroke(st) => {+ if let Some(w) = p.get("weight").and_then(Value::as_f64) {+ st.width = w.max(0.0);+ }+ let bounds = bounds.map(|b| st.paint_bounds(b));+ (&mut st.paint, &mut st.opacity, &mut st.blend, &mut st.visible, bounds) }- let bounds = bounds.map(|b| st.paint_bounds(b));- (&mut st.paint, &mut st.opacity, &mut st.blend, &mut st.visible, bounds)+ };+ let cleared = if let Some(pa) = &paint {+ *pp = super::gradient::place_paint(&super::paint::keep_pattern_xf(pa, p, Some(&*pp)), p, bounds);+ is_fill && pa.is_none()+ } else {+ false+ };+ if let Some(o) = p.get("opacity").and_then(Value::as_f64) {+ *op = percent(o);+ }+ if let Some(b) = blend {+ *bl = b; }+ if let Some(v) = p.get("visible").and_then(Value::as_bool) {+ *vis = v;+ }+ cleared };- if let Some(pa) = &paint {- *pp = super::gradient::place_paint(&super::paint::keep_pattern_xf(pa, p, Some(&*pp)), p, bounds);- }- if let Some(o) = p.get("opacity").and_then(Value::as_f64) {- *op = percent(o);- }- if let Some(b) = blend {- *bl = b;- }- if let Some(v) = p.get("visible").and_then(Value::as_bool) {- *vis = v;+ if fill_cleared {+ n.appearance.clear_lower_fills_when_top_cleared(Some(idx), &Paint::None); } } Ok(count_changed(d, &before, &ids))@@ -504,6 +504,9 @@ fn apply_paint(s: &mut Session, p: &Value, paint: Paint, fill: bool) -> Result<V let new_paint = keep_pattern_xf(&paint, p, n.appearance.paint_at(index, fill)); let placed = place_paint(&new_paint, p, item_paint_bounds(n, index, fill)); n.appearance.set_paint_at(index, fill, placed);+ if fill {+ n.appearance.clear_lower_fills_when_top_cleared(index, &paint);+ } Ok(()) }, |d, changed| {@@ -640,7 +643,8 @@ fn swap(s: &mut Session, p: &Value) -> Result<Value> { } let f = n.appearance.fill_paint(); let st = n.appearance.stroke_paint();- n.appearance.set_fill(st);+ n.appearance.set_fill(st.clone());+ n.appearance.clear_lower_fills_when_top_cleared(None, &st); restore_unpainted_stroke(&mut n.appearance, None, &f); n.appearance.set_stroke(f); }@@ -130,6 +130,83 @@ fn active_item_targets_paint_and_proxies_and_clears_on_selection_change() { assert_eq!(s.appearance_item(), None); } +#[test]+fn clearing_the_top_fill_to_none_clears_lower_fills_too() {+ let mut s = session();+ let id = rect(&mut s, 100.0);+ run(&mut s, "appearance.addFill", json!({}));+ run(&mut s, "paint.setFill", json!({"color": "#ff0000"}));+ run(&mut s, "paint.setFill", json!({"none": true}));+ let n = node(&s, id);+ assert!(n.appearance.items.iter().filter(|i| i.is_fill()).all(|i| i.paint().is_none()));+ run(&mut s, "paint.setFill", json!({"color": "#00ff00"}));+ run(&mut s, "appearance.setActiveItem", json!({"index": 0}));+ run(&mut s, "paint.setFill", json!({"none": true}));+ let n = node(&s, id);+ assert!(!fill_paint(&n, 2).is_none());+}++fn paints_any_fill(n: &Node) -> bool {+ n.appearance.items.iter().any(|i| i.visible() && i.is_fill() && !i.paint().is_none())+}++fn all_fills_cleared(n: &Node) -> bool {+ !paints_any_fill(n) && n.appearance.items.iter().filter(|i| i.is_fill()).all(|i| i.paint().is_none())+}++/// Two painted fills and an unpainted stroke (#3400): clearing the top fill must hide every fill.+fn two_fill_stack_with_none_stroke(s: &mut Session) -> NodeId {+ let id = rect(s, 100.0);+ run(s, "appearance.addFill", json!({}));+ run(s, "paint.setStroke", json!({"none": true}));+ run(s, "paint.setFill", json!({"color": "#ff0000"}));+ run(s, "appearance.setActiveItem", json!({"index": 0}));+ run(s, "paint.setFill", json!({"color": "#0000ff"}));+ run(s, "appearance.setActiveItem", json!({"index": null}));+ id+}++fn top_fill_index(n: &Node) -> usize {+ n.appearance.items.iter().enumerate().rev().find_map(|(i, it)| it.is_fill().then_some(i)).unwrap()+}++#[test]+fn top_fill_none_matches_across_fill_proxy_swap_and_set_item() {+ let mut s = session();+ for (cmd, params) in [("paint.setFill", json!({"none": true})), ("paint.swap", json!({}))] {+ let id = two_fill_stack_with_none_stroke(&mut s);+ assert!(paints_any_fill(&node(&s, id)));+ run(&mut s, cmd, params);+ assert!(all_fills_cleared(&node(&s, id)), "{cmd}");+ assert!(node(&s, id).appearance.fill_paint().is_none(), "{cmd} proxy");+ run(&mut s, "edit.undo", json!({}));+ assert!(paints_any_fill(&node(&s, id)), "undo after {cmd}");+ }+ let id = two_fill_stack_with_none_stroke(&mut s);+ let top = top_fill_index(&node(&s, id));+ run(&mut s, "appearance.setItem", json!({"index": top, "none": true}));+ assert!(all_fills_cleared(&node(&s, id)));+ run(&mut s, "edit.undo", json!({}));+ assert!(paints_any_fill(&node(&s, id)));+ run(&mut s, "paint.setFill", json!({"none": true}));+ let saved = vectorcraft_format::save(&s.doc().unwrap().doc, false);+ let back = vectorcraft_format::load(&saved).unwrap();+ let n = back.node(id).unwrap();+ assert!(all_fills_cleared(n));+ run(&mut s, "paint.setFill", json!({"color": "#00ff00"}));+ assert!(paints_any_fill(&node(&s, id)));+}++#[test]+fn swap_with_none_stroke_clears_lower_fills_when_top_becomes_none() {+ let mut s = session();+ let id = two_fill_stack_with_none_stroke(&mut s);+ run(&mut s, "paint.swap", json!({}));+ let n = node(&s, id);+ assert!(all_fills_cleared(&n));+ assert!(!n.appearance.stroke_paint().is_none());+}+ #[test] fn item_rows_follow_remove_and_move() { let mut s = session();