Graphs: an added or duplicated top row sets the series' stored paint

appearance.addFill, appearance.addStroke and appearance.duplicateItem can
put a new topmost row on a series mark with the paint the mark already
shows. When another mark of the series had set a different stored paint,
the mark's top paint did not change, so the graph kept the other mark's
paint and drew it again. A new topmost visible row now stores its paint,
as an Eyedropper append already does.
This commit is contained in:
dotCooCoo
2026-11-08 12:01:39 +07:00
committed by bflatastic
parent 0ce16936b7
commit 90cd4797ea
2 changed files with 71 additions and 2 deletions
+27 -2
View File
@@ -551,12 +551,17 @@ fn add_item(s: &mut Session, p: &Value, fill: bool) -> Result<Value> {
// Where the new fill went in on the active item's object, when it went above the active row.
let mut above = None;
s.edit(if fill { "Add New Fill" } else { "Add New Stroke" }, |d, _| {
// The objects that got a new topmost row of the kind: the series stores its paint even when the object showed it.
let mut added = Vec::new();
for (id, row) in ids.iter().zip(&per) {
if let Some(n) = d.node_mut(*id) {
if let Some(i) = n.appearance.item_of_kind(*row, true)
&& let Some(paint) = n.appearance.paint_at(Some(i), true).cloned()
{
n.appearance.insert_item(i + 1, AppearanceItem::Fill(FillLayer::new(paint)));
if let Some(kind) = top_row_kind(&n.appearance, i + 1) {
added.push((*id, kind));
}
if active == Some(*id) {
above = Some(i + 1);
}
@@ -591,11 +596,14 @@ fn add_item(s: &mut Session, p: &Value, fill: bool) -> Result<Value> {
}
} else {
n.appearance.items.push(item);
added.push((*id, fill));
}
}
}
super::graph::capture_series_opacity(d, &top_opacity_changed(&before, d, &ids));
super::graph::capture_series_top_paints(d, &top_paint_changed(&before, d, &ids));
let mut rows = top_paint_changed(&before, d, &ids);
rows.extend(painted_rows(d, added));
super::graph::capture_series_top_paints(d, &rows);
Ok(())
})?;
// A fill added above the active row becomes the active row; otherwise the rows above the new
@@ -653,6 +661,12 @@ fn painted_rows(d: &vectorcraft_doc::Document, rows: impl IntoIterator<Item = (N
rows.into_iter().filter(|(id, fill)| d.node(*id).is_some_and(|n| !super::graph::shown_paint(n, *fill).0.is_none())).collect()
}
/// The kind of the row at `at` (fill `true`, stroke `false`) when it is the topmost visible row of that kind in `ap`.
fn top_row_kind(ap: &Appearance, at: usize) -> Option<bool> {
let fill = ap.items.get(at)?.is_fill();
(ap.items.iter().rposition(|it| it.is_fill() == fill && it.visible()) == Some(at)).then_some(fill)
}
/// Clear Appearance on `n`: no fill, no stroke, and the object's Opacity row back to Default. A
/// group keeps no fill or stroke rows of its own.
fn clear(n: &mut Node) {
@@ -898,6 +912,9 @@ fn duplicate_item(s: &mut Session, p: &Value) -> Result<Value> {
let per = items_per_target(&before, &ids, &idx, aligned(p, C)?);
s.edit("Duplicate Item", |d, _| {
let mut any = false;
// Copies that became the topmost visible row of their kind: the series stores their paint even when the object
// showed it.
let mut added = Vec::new();
for (id, idx) in ids.iter().zip(&per) {
let Some(n) = d.node_mut(*id) else { continue };
let has_contents = n.contents_label().is_some();
@@ -913,9 +930,11 @@ fn duplicate_item(s: &mut Session, p: &Value) -> Result<Value> {
if active == Some(*id) {
active_at = Some(at);
}
let count = copies.len();
for (k, item) in copies.into_iter().enumerate() {
ap.insert_item(at + k, item);
}
added.extend((at..at + count).filter_map(|k| top_row_kind(ap, k)).map(|kind| (*id, kind)));
if let Some(c) = contents.filter(|_| has_contents) {
ap.set_contents_at(c);
}
@@ -929,6 +948,10 @@ fn duplicate_item(s: &mut Session, p: &Value) -> Result<Value> {
if let Some(item) = ap.items.get(i).cloned() {
first_at.get_or_insert(i + 1);
ap.insert_item(i + 1, item);
// Copies below this one go in lower down, so a copy that is the topmost row of its kind stays it.
if let Some(kind) = top_row_kind(ap, i + 1) {
added.push((*id, kind));
}
any = true;
}
}
@@ -937,7 +960,9 @@ fn duplicate_item(s: &mut Session, p: &Value) -> Result<Value> {
return Err(bad(C, format!("no item at index {}", idx[0])));
}
super::graph::capture_series_opacity(d, &top_opacity_changed(&before, d, &ids));
super::graph::capture_series_top_paints(d, &top_paint_changed(&before, d, &ids));
let mut rows = top_paint_changed(&before, d, &ids);
rows.extend(painted_rows(d, added));
super::graph::capture_series_top_paints(d, &rows);
Ok(())
})?;
// Rows at or above each copy move up by one (by the block, for copies placed at `to`).
+44
View File
@@ -1211,6 +1211,50 @@ fn appending_a_visible_row_under_a_hidden_one_replaces_the_kept_stroke() {
assert_eq!(kept.stroke_width, Some(2.0));
}
/// A stroke added on top of a mark replaces the stroke another mark of the series left on the graph, even when the new
/// row shows the paint the mark already showed.
#[test]
fn adding_a_stroke_replaces_the_kept_stroke_with_the_marks_own() {
let (mut s, id, _, marks) = column_marks();
let stroke = top_row(&s, marks[0], false);
s.execute("appearance.setItem", &json!({"ids": [marks[1].0], "index": stroke, "color": "#ff0000", "weight": 2})).unwrap();
s.execute("appearance.setItem", &json!({"ids": [marks[0].0], "index": stroke, "color": "#0000ff", "weight": 5})).unwrap();
s.execute("appearance.addStroke", &json!({"ids": [marks[1].0]})).unwrap();
let kept = &spec(&s, id).series_paints[0];
assert_eq!(kept.stroke, Some(Paint::solid(vectorcraft_color::Color::rgb(1.0, 0.0, 0.0))));
assert_eq!(kept.stroke_width, Some(2.0));
}
/// A fill added right above a mark's active top fill replaces the fill another mark of the series left on the graph.
#[test]
fn adding_a_fill_above_the_active_top_fill_replaces_the_kept_fill() {
let (mut s, id, was, marks) = column_marks();
let fill = top_row(&s, marks[0], true);
s.execute("appearance.setItem", &json!({"ids": [marks[0].0], "index": fill, "color": "#0000ff"})).unwrap();
s.execute("select.set", &json!({"ids": [marks[1].0]})).unwrap();
s.execute("appearance.setActiveItem", &json!({"index": fill})).unwrap();
s.execute("appearance.addFill", &json!({})).unwrap();
assert_eq!(spec(&s, id).series_paints[0].fill, Some(was));
}
/// A copy of a mark's top stroke replaces the stroke another mark of the series left on the graph.
#[test]
fn duplicating_a_marks_top_stroke_replaces_the_kept_stroke() {
let (mut s, id, _, marks) = column_marks();
let stroke = top_row(&s, marks[0], false);
s.execute("appearance.setItem", &json!({"ids": [marks[1].0], "index": stroke, "color": "#ff0000", "weight": 2})).unwrap();
s.execute("appearance.setItem", &json!({"ids": [marks[0].0], "index": stroke, "color": "#0000ff", "weight": 5})).unwrap();
for to in [false, true] {
s.execute("appearance.setItem", &json!({"ids": [marks[0].0], "index": stroke, "color": "#0000ff", "weight": 5})).unwrap();
let top = top_row(&s, marks[1], false);
let p = if to { json!({"ids": [marks[1].0], "index": top, "to": top + 1}) } else { json!({"ids": [marks[1].0], "index": top}) };
s.execute("appearance.duplicateItem", &p).unwrap();
let kept = &spec(&s, id).series_paints[0];
assert_eq!(kept.stroke, Some(Paint::solid(vectorcraft_color::Color::rgb(1.0, 0.0, 0.0))), "{p}");
assert_eq!(kept.stroke_width, Some(2.0), "{p}");
}
}
/// A weight set on a stroke without paint (a line graph's marker) leaves the paint the series keeps for its line.
#[test]
fn a_weight_on_a_stroke_without_paint_leaves_the_kept_stroke() {