From c5e2e9210a7d0ce9a7acd33eeb495ad882c7b30e Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Fri, 13 Nov 2026 21:16:20 -0800 Subject: [PATCH] Sort Objects: an opacity sort that gives marks their own opacities changes nothing With apply "opacity", Sort Objects compared each object's opacity with the value it gets as a percentage, within 1e-6. An evenly stepped value can differ from a mark's percentage by more than that while it rounds to the same stored opacity: marks at 30%, 65% and 100% sorted by x with distribute on keep their opacities, yet the 65% mark counted as changed. The sort then made an undo step and replaced the opacity the graph stores for the series (30%) with 65%. The sort now compares each object's stored opacity with the opacity it is given, as stored, both to skip a run that changes nothing and to pick the marks whose opacity the graph keeps. --- crates/engine/src/cmd/addons/arrange.rs | 12 +++++++----- crates/engine/src/cmd/graph/tests.rs | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/cmd/addons/arrange.rs b/crates/engine/src/cmd/addons/arrange.rs index d7cae868ed..67cf475979 100644 --- a/crates/engine/src/cmd/addons/arrange.rs +++ b/crates/engine/src/cmd/addons/arrange.rs @@ -516,16 +516,18 @@ fn sort(s: &mut Session, p: &Value) -> Result { } let wanted: Vec<(&Sorted, f64)> = rank.iter().filter_map(|i| items.get(*i)).zip(values).collect(); if apply == "opacity" { - if wanted.iter().all(|(o, v)| (o.opacity - v).abs() < 1e-6) { + // The opacity each object is given, as stored; an object whose stored opacity it already is has not changed. + let given: Vec<(NodeId, f32)> = wanted.iter().map(|(o, v)| (o.id, (v / 100.0) as f32)).collect(); + let changed: Vec = given.iter().filter(|(id, a)| d.node(*id).is_some_and(|n| n.opacity != *a)).map(|(id, _)| *id).collect(); + if changed.is_empty() { return Ok(count); } s.edit("Sort Objects", |d, _| { - for (o, v) in &wanted { - if let Some(n) = d.node_mut(o.id) { - n.opacity = (v / 100.0) as f32; + for (id, a) in &given { + if let Some(n) = d.node_mut(*id) { + n.opacity = *a; } } - let changed: Vec = wanted.iter().filter(|(o, v)| (o.opacity - v).abs() >= 1e-6).map(|(o, _)| o.id).collect(); keep_series_opacity(d, &changed); Ok(()) })?; diff --git a/crates/engine/src/cmd/graph/tests.rs b/crates/engine/src/cmd/graph/tests.rs index c997e803d5..0ca3cf1d96 100644 --- a/crates/engine/src/cmd/graph/tests.rs +++ b/crates/engine/src/cmd/graph/tests.rs @@ -757,6 +757,24 @@ fn an_opacity_sort_objects_stores_the_frontmost_mark_it_changes() { assert_eq!(spec(&s, id).series_paints[0].opacity, Some(0.6)); } +/// A Sort Objects that gives series marks the opacities they already have changes nothing: the value the graph stores +/// stays and no undo step is made, even when the evenly stepped percentage differs from a mark's in the last digits. +#[test] +fn an_opacity_sort_objects_that_changes_no_mark_keeps_the_stored_opacity() { + let (mut s, id, _, marks) = column_marks(); + s.execute("object.setProps", &json!({"id": marks[1].0, "opacity": 65})).unwrap(); + s.execute("object.setProps", &json!({"id": marks[0].0, "opacity": 30})).unwrap(); + assert_eq!(spec(&s, id).series_paints[0].opacity, Some(0.3)); + // From left to right the columns have 30%, 65% and 100%, the values an even step from 30% to 100% gives them. + let undo = s.doc().unwrap().history.undo.len(); + let ids = [marks[0].0, marks[1].0, marks[2].0]; + s.execute("addon.arrange.sort", &json!({"ids": ids, "by": "x", "apply": "opacity", "distribute": true})).unwrap(); + let opacities: Vec = marks[..3].iter().map(|m| s.doc().unwrap().doc.node(*m).unwrap().opacity).collect(); + assert_eq!(opacities, [0.3, 0.65, 1.0]); + assert_eq!(spec(&s, id).series_paints[0].opacity, Some(0.3)); + assert_eq!(s.doc().unwrap().history.undo.len(), undo, "no undo step"); +} + /// An object opacity, blend mode, fill, stroke and weight that Global Edit gives a mark of another series from the mark /// that was edited are stored on the graph for that series in the same undo step, and every mark of the series has them /// when the graph is drawn again.