aleleba/vectorcraft
1
0
Fork 0
mirror of https://github.com/storytold/vectorcraft.git synced 2026-10-26 10:47:11 -06:00
Code Issues Packages Projects Releases Wiki Activity

Paint: fill, stroke, gradient and Appearance panel commands that change nothing record no undo step

Browse Source 
These commands edited the document even when nothing changed, so the
call recorded an undo step, emptied Redo and marked the document
modified:

- Fill, Stroke, None, Apply Last Color and Apply Last Gradient with the
  paint the objects have already, also on characters selected with the
  Type tool.
- stroke.set and stroke.setAdvanced with the options the strokes have
  already.
- paint.editGradient, paint.setGradientGeom and the freeform point
  commands when the gradient comes out as it was.
- Invert and Complement on the Appearance panel's selected row when no
  color changes.
- Swap Fill and Stroke and Default Fill and Stroke when every object
  comes out as it was.
- appearance.setItem with the values the item has or an index no
  object has, and transparency.set with an item and the values that
  item has; appearance.removeItem with indices past every stack;
  appearance.moveItem, Reduce to Basic Appearance, Clear Appearance,
  the Eyedropper (appearance.copyFrom) and appearance.transfer when
  every object comes out as it was.

These commands now return without editing the document. The replies
have the same form as before. On a graph's series or labels, a fill or
stroke that the art has already still records one undo step when the
graph does not keep that paint for them (for example after the
Eyedropper painted the art), and the graph then keeps it.

The Eyedropper test whose second copy changed nothing gives its target
a green fill first, which the copy turns red again.

One line in the evidence list of docs/target-app-parity.md names these
commands, apart from Invert and Complement.
This commit is contained in:
dotCooCoo
2026-10-12 09:43:15 +07:00
committed by bflatastic
parent d0bb09c16f
commit 9cbab1383b
8 changed files with 290 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/engine/src/cmd/appearance.rs
+58 -26
View File
@@ -318,7 +318,8 @@ fn require_targets(s: &Session, p: &Value, cmd: &str) -> Result<Vec<NodeId>> {
/// One undo step (`label`) running `f` on each of `ids` with the item a fill (`fill`) or stroke
/// edit aimed at `item` changes there (`None`: the topmost one; text then edits its characters).
/// Nothing targeted is not an edit.
/// Nothing targeted is not an edit, and when `f` leaves every target as it was, the document and
/// its undo history stay as they are.
pub(crate) fn edit_items(
s: &mut Session,
ids: &[NodeId],
@@ -328,10 +329,12 @@ pub(crate) fn edit_items(
fill: bool,
f: impl FnMut(&mut Node, Option<usize>) -> Result<()>,
) -> Result<()> {
edit_items_then(s, ids, item, cmd, label, fill, f, |_, _| Ok(()))
edit_items_then(s, ids, item, cmd, label, fill, f, |_, _| Ok(false))
}
/// [`edit_items`], then `after` on the ids `f` changed, inside the same undo step.
/// [`edit_items`], then `after` on the ids `f` ran on, inside the same undo step. `after` returns
/// whether it changed the document; when it did not and `f` left every target as it was, the
/// document and its undo history stay as they are.
#[allow(clippy::too_many_arguments)]
pub(crate) fn edit_items_then(
s: &mut Session,
@@ -341,21 +344,29 @@ pub(crate) fn edit_items_then(
label: &str,
fill: bool,
mut f: impl FnMut(&mut Node, Option<usize>) -> Result<()>,
mut after: impl FnMut(&mut vectorcraft_doc::Document, &[NodeId]) -> Result<()>,
mut after: impl FnMut(&mut vectorcraft_doc::Document, &[NodeId]) -> Result<bool>,
) -> Result<()> {
if ids.is_empty() {
return Ok(());
}
s.edit(label, |d, _| {
let mut changed = Vec::new();
let before = s.doc()?.doc.clone();
super::overprint::edit_counted(s, label, |d| {
let mut edited = Vec::new();
for id in ids {
let Some(n) = d.node_mut(*id) else { continue };
let index = item.resolve(&n.appearance, fill, cmd)?;
f(n, index)?;
changed.push(*id);
edited.push(*id);
}
after(d, &changed)
})
let more = after(d, &edited)?;
Ok(count_changed(d, &before, &edited) + usize::from(more))
})?;
Ok(())
}
/// How many of `ids` differ in `d` from `before`.
pub(crate) fn count_changed(d: &vectorcraft_doc::Document, before: &vectorcraft_doc::Document, ids: &[NodeId]) -> usize {
ids.iter().filter(|id| d.node(**id) != before.node(**id)).count()
}
/// Whether an edit aimed at `item` changes a stroke: the `stroke` param, else the kind of the
@@ -428,13 +439,15 @@ fn add_item(s: &mut Session, p: &Value, fill: bool) -> Result<Value> {
fn clear_appearance(s: &mut Session, p: &Value) -> Result<Value> {
let ids = require_targets(s, p, "appearance.clear")?;
s.edit("Clear Appearance", |d, _| {
// When every object is cleared already, the document and its undo history stay as they are.
let before = s.doc()?.doc.clone();
super::overprint::edit_counted(s, "Clear Appearance", |d| {
for id in &ids {
if let Some(n) = d.node_mut(*id) {
clear(n);
}
}
Ok(())
Ok(count_changed(d, &before, &ids))
})?;
s.active_appearance_item = None;
ok()
@@ -456,7 +469,9 @@ fn transfer(s: &mut Session, p: &Value) -> Result<Value> {
return Err(bad(C, "`source` and `target` are the same object"));
}
let copy = bool_or(p, "copy", false);
s.edit(if copy { "Copy Appearance" } else { "Move Appearance" }, |d, _| {
// When both objects come out as they were, the document and its undo history stay as they are.
let before = s.doc()?.doc.clone();
super::overprint::edit_counted(s, if copy { "Copy Appearance" } else { "Move Appearance" }, |d| {
let src = d.node(source).cloned().ok_or(EngineError::NoNode(source))?;
let to = d.node_mut(target).ok_or(EngineError::NoNode(target))?;
let mut ap = src.appearance.clone();
@@ -473,7 +488,7 @@ fn transfer(s: &mut Session, p: &Value) -> Result<Value> {
clear(n);
(n.isolate, n.knockout, n.knockout_shape) = (false, Default::default(), false);
}
Ok(())
Ok(count_changed(d, &before, &[source, target]))
})?;
s.active_appearance_item = None;
Ok(json!({ "source": source.0, "target": target.0 }))
@@ -486,7 +501,9 @@ fn is_group(n: &Node) -> bool {
fn reduce_basic(s: &mut Session, p: &Value) -> Result<Value> {
let ids = require_targets(s, p, "appearance.reduceToBasic")?;
s.edit("Reduce to Basic Appearance", |d, _| {
// When every object comes out as it was, the document and its undo history stay as they are.
let before = s.doc()?.doc.clone();
super::overprint::edit_counted(s, "Reduce to Basic Appearance", |d| {
for id in &ids {
if let Some(n) = d.node_mut(*id) {
// The topmost visible fill and stroke stay, without their own transparency or effects.
@@ -505,7 +522,7 @@ fn reduce_basic(s: &mut Session, p: &Value) -> Result<Value> {
}
}
}
Ok(())
Ok(count_changed(d, &before, &ids))
})?;
s.active_appearance_item = None;
ok()
@@ -519,7 +536,10 @@ fn set_item(s: &mut Session, p: &Value) -> Result<Value> {
return Err(bad("appearance.setItem", "select objects or give ids"));
}
let blend = str_param(p, "blend").and_then(BlendMode::parse);
s.edit("Appearance", |d, _| {
// When every object comes out as it was (for example, when none has item `index`), the document
// and its undo history stay as they are.
let before = s.doc()?.doc.clone();
super::overprint::edit_counted(s, "Appearance", |d| {
for id in &ids {
let Some(n) = d.node_mut(*id) else { continue };
if p.get("weight").is_none()
@@ -552,7 +572,7 @@ fn set_item(s: &mut Session, p: &Value) -> Result<Value> {
*vis = v;
}
}
Ok(())
Ok(count_changed(d, &before, &ids))
})?;
if let Some(pa) = &paint {
s.remember_paint(pa);
@@ -579,7 +599,10 @@ fn item_indices(p: &Value, cmd: &str) -> Result<Vec<usize>> {
fn remove_item(s: &mut Session, p: &Value) -> Result<Value> {
let idx = item_indices(p, "appearance.removeItem")?;
let ids = require_targets(s, p, "appearance.removeItem")?;
s.edit("Remove Item", |d, _| {
// When no object has an item at any of the indices, the document and its undo history stay as
// they are.
let before = s.doc()?.doc.clone();
super::overprint::edit_counted(s, "Remove Item", |d| {
for id in &ids {
if let Some(n) = d.node_mut(*id) {
// From the top down, so the lower indices stay valid.
@@ -588,7 +611,7 @@ fn remove_item(s: &mut Session, p: &Value) -> Result<Value> {
}
}
}
Ok(())
Ok(count_changed(d, &before, &ids))
})?;
s.remap_appearance_item(&ids, |a| (!idx.contains(&a)).then(|| a - idx.iter().filter(|i| **i < a).count()));
ok()
@@ -653,7 +676,10 @@ fn move_item(s: &mut Session, p: &Value) -> Result<Value> {
let from = index_param(p, "from", C)?;
let contents = p.get("contents").map(|_| index_param(p, "contents", C)).transpose()?;
let mut landed = to;
s.edit("Reorder Appearance", |d, _| {
// When every object's stack comes out as it was, the document and its undo history stay as they
// are.
let before = s.doc()?.doc.clone();
super::overprint::edit_counted(s, "Reorder Appearance", |d| {
for id in &ids {
let Some(n) = d.node_mut(*id) else { continue };
landed = n.appearance.move_item(from, to).ok_or_else(|| bad(C, format!("no item at index {from}")))?;
@@ -661,7 +687,7 @@ fn move_item(s: &mut Session, p: &Value) -> Result<Value> {
n.appearance.set_contents_at(k);
}
}
Ok(())
Ok(count_changed(d, &before, &ids))
})?;
// The moved row stays the active one; the rows it passed shift by one.
s.remap_appearance_item(&ids, |a| {
@@ -681,14 +707,18 @@ fn move_item(s: &mut Session, p: &Value) -> Result<Value> {
/// `appearance.moveItem {from: "contents"}`: put the Contents (Characters) row of each target that
/// has one above its bottom `to` items.
fn move_contents(s: &mut Session, ids: &[NodeId], to: usize) -> Result<Value> {
let slot = s.edit("Reorder Appearance", |d, _| {
let mut slot = None;
// When every Contents row is above the bottom `to` items already, the document and its undo
// history stay as they are.
let before = s.doc()?.doc.clone();
let mut slot = None;
super::overprint::edit_counted(s, "Reorder Appearance", |d| {
for id in ids {
let Some(n) = d.node_mut(*id).filter(|n| n.contents_label().is_some()) else { continue };
n.appearance.set_contents_at(to);
slot.get_or_insert(n.appearance.contents_at());
}
slot.ok_or_else(|| bad("appearance.moveItem", "only groups, layers and type have a Contents (Characters) row"))
slot.ok_or_else(|| bad("appearance.moveItem", "only groups, layers and type have a Contents (Characters) row"))?;
Ok(count_changed(d, &before, ids))
})?;
Ok(json!({ "contents": slot }))
}
@@ -733,13 +763,15 @@ fn copy_from(s: &mut Session, p: &Value) -> Result<Value> {
if targets.is_empty() || attrs.is_empty() {
return Ok(json!({ "ids": [] }));
}
s.edit("Eyedropper", |d, _| {
// When every target comes out as it was, the document and its undo history stay as they are.
let before = s.doc()?.doc.clone();
super::overprint::edit_counted(s, "Eyedropper", |d| {
for id in &targets {
if let Some(n) = d.node_mut(*id) {
picked.apply(n, &attrs, append);
}
}
Ok(())
Ok(count_changed(d, &before, &targets))
})?;
Ok(json!({ "ids": targets.iter().map(|i| i.0).collect::<Vec<_>>() }))
}
crates/engine/src/cmd/graph.rs
+48 -6
View File
@@ -1269,10 +1269,11 @@ pub(crate) fn paint_targets(doc: &Document, ids: &[NodeId]) -> Vec<NodeId> {
/// Copy the paint just applied to generated series art into that series' fill or stroke. Called
/// inside the same document edit, so the visible paint and future regeneration undo together.
/// Gradients, patterns and swatch links are kept as [`Paint`], not reduced to a solid colour.
pub(crate) fn capture_series_paints(doc: &mut Document, ids: &[NodeId], fill: bool) {
/// Returns whether a series' captured fill or stroke changed.
pub(crate) fn capture_series_paints(doc: &mut Document, ids: &[NodeId], fill: bool) -> bool {
let members = series_members(doc);
if members.is_empty() {
return;
return false;
}
let captures: Vec<_> = ids
.iter()
@@ -1285,8 +1286,10 @@ pub(crate) fn capture_series_paints(doc: &mut Document, ids: &[NodeId], fill: bo
Some((r.graph, r.index, paint, width))
})
.collect();
let mut changed = false;
for (graph, index, paint, width) in captures {
let Some(spec) = doc.node_mut(graph).and_then(|n| n.graph.as_deref_mut()) else { continue };
let was = spec.series_paints.clone();
// `index` is below the graph's series count (`series_members`).
if spec.series_paints.len() <= index {
spec.series_paints.resize(index + 1, vectorcraft_doc::SeriesPaint::default());
@@ -1298,7 +1301,9 @@ pub(crate) fn capture_series_paints(doc: &mut Document, ids: &[NodeId], fill: bo
slot.stroke = Some(paint);
slot.stroke_width = width;
}
changed |= spec.series_paints != was;
}
changed
}
/// Graph Data › Transpose row/column: categories become series and series categories.
@@ -1370,18 +1375,19 @@ fn graph_labels(doc: &Document) -> HashMap<NodeId, (NodeId, Role)> {
/// Keep a formatting change made to graph labels `ids` (Character or Appearance, on the whole graph or on labels
/// picked with Group Selection) for every label of their kind: `change` is applied to the kind's kept style (or
/// the default one), so the labels look the same when the graph is drawn again. Called inside the same edit, so
/// they undo together.
pub(crate) fn capture_label_styles(doc: &mut Document, ids: &[NodeId], change: &dyn Fn(&mut CharStyle)) {
/// they undo together. Returns whether a kept style changed.
pub(crate) fn capture_label_styles(doc: &mut Document, ids: &[NodeId], change: &dyn Fn(&mut CharStyle)) -> bool {
if ids.is_empty() {
return;
return false;
}
let labels = graph_labels(doc);
if labels.is_empty() {
return;
return false;
}
let mut kinds: Vec<(NodeId, Role)> = ids.iter().filter_map(|id| labels.get(id).copied()).collect();
kinds.sort_by_key(|(g, r)| (g.0, *r as u8));
kinds.dedup();
let mut changed = false;
for (graph, role) in kinds {
let Some(spec) = doc.node_mut(graph).and_then(|n| n.graph.as_deref_mut()) else { continue };
let mut style = label_style(&spec.label_styles, role);
@@ -1391,8 +1397,10 @@ pub(crate) fn capture_label_styles(doc: &mut Document, ids: &[NodeId], change: &
Role::Category => &mut spec.label_styles.categories,
Role::Legend => &mut spec.label_styles.legend,
};
changed |= slot.as_ref() != Some(&style);
*slot = Some(style);
}
changed
}
fn set_data(s: &mut Session, p: &Value) -> Result<Value> {
@@ -2100,6 +2108,40 @@ mod tests {
);
}
/// A fill that a series' marks or a value axis number have already is one undo step when the graph
/// keeps no fill for them (as after a command that paints only the art): the art stays as it was
/// and the graph keeps the fill. Given again, the fill records no undo step.
#[test]
fn a_fill_the_art_has_already_is_kept_by_the_graph_in_one_step() {
let mut s = Session::new();
s.execute("file.new", &json!({"width": 800, "height": 800})).unwrap();
let id = graph(&mut s, "column");
let series_id = group(s.doc().unwrap().doc.node(id).unwrap(), "2024").id;
let label = text_id(&s, id, "Axes", "4");
for target in [series_id, label] {
let fill = json!({"ids": [target.0], "color": "#e8573f"});
s.execute("paint.setFill", &fill).unwrap();
// The graph keeps nothing, as after a command that paints only the art.
s.edit("Forget", |d, _| {
let g = d.node_mut(id).unwrap().graph.as_deref_mut().unwrap();
g.series_paints.clear();
g.label_styles = vectorcraft_doc::LabelStyles::default();
Ok(())
})
.unwrap();
let art = s.doc().unwrap().doc.node(target).unwrap().clone();
let steps = s.doc().unwrap().history.undo.len();
s.execute("paint.setFill", &fill).unwrap();
assert_eq!(s.doc().unwrap().history.undo.len(), steps + 1, "{target:?}");
assert_eq!(s.doc().unwrap().doc.node(target).unwrap(), &art, "the art is as it was");
let g = spec(&s, id);
let kept = if target == series_id { g.series_paints[0].fill.clone() } else { g.label_styles.values.map(|st| st.fill) };
assert_eq!(kept.and_then(|p| p.color()).map(|c| c.to_hex()), Some("#e8573f".into()), "{target:?}");
s.execute("paint.setFill", &fill).unwrap();
assert_eq!(s.doc().unwrap().history.undo.len(), steps + 1, "nothing left to keep: {target:?}");
}
}
#[test]
fn painting_one_bar_sets_the_colour_used_by_the_whole_series_on_rebuild() {
let mut s = Session::new();
crates/engine/src/cmd/paint.rs
+15 -9
View File
@@ -381,7 +381,9 @@ fn apply_paint(s: &mut Session, p: &Value, paint: Paint, fill: bool) -> Result<V
&& let Some((id, a, b)) = super::textedit::editing_range(s)?
&& a != b
{
s.edit(if fill { "Fill Color" } else { "Stroke Color" }, |d, _| {
// When the type comes out as it was, the document and its undo history stay as they are.
let before = s.doc()?.doc.clone();
super::overprint::edit_counted(s, if fill { "Fill Color" } else { "Stroke Color" }, |d| {
let Some(NodeKind::Text(t)) = d.node_mut(id).map(|n| &mut n.kind) else { return Err(EngineError::NoNode(id)) };
let (xf, lb) = (t.xf, t.local_bounds());
vectorcraft_text::edit::style_range(&mut t.runs, a, b, |st| {
@@ -395,7 +397,7 @@ fn apply_paint(s: &mut Session, p: &Value, paint: Paint, fill: bool) -> Result<V
}
});
super::typecmd::refresh_bounds(t);
Ok(())
Ok(super::appearance::count_changed(d, &before, &[id]))
})?;
if bool_or(p, "focus", true) {
s.fill_active = fill;
@@ -454,8 +456,8 @@ fn apply_paint(s: &mut Session, p: &Value, paint: Paint, fill: bool) -> Result<V
Ok(())
},
|d, changed| {
super::graph::capture_series_paints(d, changed, fill);
super::graph::capture_label_styles(d, changed, &|st| {
let series = super::graph::capture_series_paints(d, changed, fill);
let labels = super::graph::capture_label_styles(d, changed, &|st| {
if fill {
st.fill = paint.clone();
} else {
@@ -465,7 +467,7 @@ fn apply_paint(s: &mut Session, p: &Value, paint: Paint, fill: bool) -> Result<V
st.stroke = paint.clone();
}
});
Ok(())
Ok(series || labels)
},
)?;
// Rejected edits leave the next shape's appearance and active proxy untouched.
@@ -515,7 +517,9 @@ pub(super) fn restore_unpainted_stroke(ap: &mut Appearance, index: Option<usize>
fn swap(s: &mut Session, p: &Value) -> Result<Value> {
let ids = paint_targets(s, p)?;
if !ids.is_empty() {
s.edit("Swap Fill and Stroke", |d, _| {
// When every object comes out as it was, the document and its undo history stay as they are.
let before = s.doc()?.doc.clone();
super::overprint::edit_counted(s, "Swap Fill and Stroke", |d| {
for id in &ids {
let Some(n) = d.node_mut(*id) else { continue };
if let NodeKind::Text(t) = &mut n.kind {
@@ -532,7 +536,7 @@ fn swap(s: &mut Session, p: &Value) -> Result<Value> {
restore_unpainted_stroke(&mut n.appearance, None, &f);
n.appearance.set_stroke(f);
}
Ok(())
Ok(super::appearance::count_changed(d, &before, &ids))
})?;
}
if s.paint.stroke.is_none() && !s.paint.fill.is_none() && s.paint.stroke_width == 0.0 {
@@ -545,7 +549,9 @@ fn swap(s: &mut Session, p: &Value) -> Result<Value> {
fn default_paint(s: &mut Session, p: &Value) -> Result<Value> {
let ids = paint_targets(s, p)?;
if !ids.is_empty() {
s.edit("Default Fill and Stroke", |d, _| {
// When every object comes out as it was, the document and its undo history stay as they are.
let before = s.doc()?.doc.clone();
super::overprint::edit_counted(s, "Default Fill and Stroke", |d| {
for id in &ids {
let Some(n) = d.node_mut(*id) else { continue };
match &mut n.kind {
@@ -558,7 +564,7 @@ fn default_paint(s: &mut Session, p: &Value) -> Result<Value> {
_ => n.appearance = Appearance::default_art().into(),
}
}
Ok(())
Ok(super::appearance::count_changed(d, &before, &ids))
})?;
}
super::newart::reset(s);