From 04abef9e32e43d38202f7d1ec07583830fdd92dc Mon Sep 17 00:00:00 2001 From: FredApps <153173854+FredApps@users.noreply.github.com> Date: Mon, 12 Oct 2026 17:48:12 +0200 Subject: [PATCH 1/2] Keep Spiral decay settings consistent with drawn geometry --- crates/engine/src/tests_toolsettings.rs | 61 +++++++++++++++++++++++++ crates/tools/src/draw2/family.rs | 51 ++++++++++++++++++++- 2 files changed, 111 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/tests_toolsettings.rs b/crates/engine/src/tests_toolsettings.rs index 49b930af46..6d999f2cd8 100644 --- a/crates/engine/src/tests_toolsettings.rs +++ b/crates/engine/src/tests_toolsettings.rs @@ -96,3 +96,64 @@ fn options_are_saved_with_the_preferences() { t.execute("prefs.reset", &json!({})).unwrap(); assert_eq!(t.prefs.tool_settings["polygon"]["sides"], json!(9)); } + +#[test] +fn spiral_decay_is_normalized_for_active_and_inactive_tools_and_preferences() { + let mut s = session(); + for active in [false, true] { + select(&mut s, if active { "spiral" } else { "selection" }); + let options = s.set_tool_option_cmd(&json!({"tool": "spiral", "values": {"decay": 150, "segments": 4, "clockwise": false}})).unwrap(); + assert_eq!(options["decay"], json!(99.99)); + assert_eq!(s.tool_options_of("spiral")["decay"], json!(99.99)); + assert_eq!(s.prefs.tool_settings["spiral"]["decay"], json!(99.99)); + assert_eq!(s.tool_id(), if active { "spiral" } else { "selection" }); + select(&mut s, "selection"); + select(&mut s, "spiral"); + assert_eq!(s.tool_options()["decay"], json!(99.99)); + } + let saved = serde_json::to_string(&s.prefs).unwrap(); + let mut restored = session(); + restored.apply_prefs(serde_json::from_str(&saved).unwrap()); + select(&mut restored, "spiral"); + assert_eq!(restored.tool_options(), json!({"decay": 99.99, "segments": 4, "clockwise": false})); +} + +#[test] +fn legacy_spiral_decay_preferences_report_the_ratio_drawn_after_reopening() { + use vectorcraft_geom::Point; + use vectorcraft_tools::{PointerEvent, PointerKind}; + + // Older versions saved 100–150, although their geometry already capped the ratio at 0.9999. + for clockwise in [false, true] { + let mut s = session(); + let mut prefs = Prefs::default(); + prefs.tool_settings.insert("spiral".into(), json!({"decay": 150, "segments": 4, "clockwise": clockwise}).as_object().unwrap().clone()); + s.apply_prefs(serde_json::from_str(&serde_json::to_string(&prefs).unwrap()).unwrap()); + let view = ViewInfo { smart_guides: false, ..Default::default() }; + s.select_tool("spiral", view).unwrap(); + let decay = s.tool_options()["decay"].as_f64().unwrap(); + assert_eq!(decay, 99.99); + let before = s.doc().unwrap().history.undo.len(); + for (kind, x) in [(PointerKind::Down, 200.0), (PointerKind::Drag, 300.0), (PointerKind::Up, 300.0)] { + s.pointer(&PointerEvent::new(kind, x, 200.0), view).unwrap(); + } + let d = s.doc().unwrap(); + assert_eq!(d.history.undo.len(), before + 1); + assert_eq!(d.selection.objects.len(), 1); + let id = d.selection.objects[0]; + let path = d.doc.node(id).unwrap().path_data().unwrap().clone(); + let anchors = &path.subpaths[0].anchors; + assert_eq!(anchors.len(), 5); + assert!(!path.is_closed()); + let center = Point::new(200.0, 200.0); + for pair in anchors.windows(2) { + let ratio = pair[0].p.distance(center) / pair[1].p.distance(center); + assert!((100.0 * ratio - decay).abs() < 1e-10); + } + assert_eq!(s.prefs.tool_settings["spiral"]["decay"], json!(99.99)); + s.execute("edit.undo", &json!({})).unwrap(); + assert!(s.doc().unwrap().doc.node(id).is_none()); + s.execute("edit.redo", &json!({})).unwrap(); + assert_eq!(s.doc().unwrap().doc.node(id).unwrap().path_data().unwrap(), &path); + } +} diff --git a/crates/tools/src/draw2/family.rs b/crates/tools/src/draw2/family.rs index 9b068bc054..f16e01db05 100644 --- a/crates/tools/src/draw2/family.rs +++ b/crates/tools/src/draw2/family.rs @@ -225,7 +225,8 @@ impl Tool for FamilyTool { fn set_option(&mut self, key: &str, v: &Value) { match key { "closed" => self.closed = v.as_bool().unwrap_or(self.closed), - "decay" => self.decay = v.as_f64().unwrap_or(self.decay).clamp(5.0, 150.0), + // Report and persist the decay that shapes::spiral can actually draw. + "decay" => self.decay = v.as_f64().unwrap_or(self.decay).clamp(5.0, 99.99), "segments" => self.segments = v.as_u64().unwrap_or(10).clamp(2, 1000) as u32, "clockwise" => self.clockwise = v.as_bool().unwrap_or(self.clockwise), "rows" | "concentric" => self.rows = v.as_u64().unwrap_or(5).min(999) as u32, @@ -252,6 +253,54 @@ mod tests { use crate::testutil::*; use vectorcraft_doc::Selection; + #[test] + fn spiral_reports_the_decay_its_command_draws() { + let mut t = FamilyTool::new("spiral"); + assert_eq!(t.options()["decay"], json!(80.0)); + for (requested, expected) in [(-10.0, 5.0), (5.0, 5.0), (80.0, 80.0), (99.99, 99.99), (100.0, 99.99), (150.0, 99.99), (200.0, 99.99)] { + t.set_option("decay", &json!(requested)); + assert_eq!(t.options()["decay"], json!(expected)); + let (id, params) = t.command(Point::ZERO, Point::new(100.0, 0.0), Mods::default()); + assert_eq!(id, "shape.spiral"); + assert_eq!(params["decay"], json!(expected)); + // Inspect the actual adjacent radii, not a second copy of the tool's clamp. + let path = vectorcraft_geom::shapes::spiral(Point::ZERO, 100.0, params["decay"].as_f64().unwrap(), 2, true); + let anchors = &path.subpaths[0].anchors; + let outer = anchors[2].p.distance(Point::ZERO); + let next = anchors[1].p.distance(Point::ZERO); + assert!((100.0 * next / outer - expected).abs() < 1e-10); + } + for invalid in [Value::Null, json!("80"), json!(false), json!({})] { + t.set_option("decay", &invalid); + assert_eq!(t.options()["decay"], json!(99.99)); + } + } + + #[test] + fn normalized_spiral_decay_preserves_existing_capped_paths_within_roundoff() { + for requested in [100.0, 150.0] { + let mut t = FamilyTool::new("spiral"); + t.set_option("decay", &json!(requested)); + for clockwise in [false, true] { + for segments in [2, 10, 1000] { + for radius in [1.0, 100.0, 1e6] { + let previous = vectorcraft_geom::shapes::spiral(Point::ZERO, radius, requested, segments, clockwise); + let normalized = vectorcraft_geom::shapes::spiral(Point::ZERO, radius, t.decay, segments, clockwise); + let old = &previous.subpaths[0].anchors; + let new = &normalized.subpaths[0].anchors; + assert_eq!(old.len(), new.len()); + for (a, b) in old.iter().zip(new) { + assert_eq!(a.kind, b.kind); + for (p, q) in [(a.p, b.p), (a.h_in, b.h_in), (a.h_out, b.h_out)] { + assert!(p.distance(q) <= 1e-10 * (1.0 + radius), "{requested}% / {segments} / {radius}: {p:?} != {q:?}"); + } + } + } + } + } + } + } + #[test] fn grid_drag_and_arrows() { let (d, _) = doc_with_rect(); From 4552915aaec77248c380a0a62efb427bbf1c54cc Mon Sep 17 00:00:00 2001 From: FredApps Date: Sat, 14 Nov 2026 11:35:04 +0100 Subject: [PATCH 2/2] Tests: spiral decay is per full wind on main, so four segments give the decay --- crates/engine/src/tests/toolsettings.rs | 4 +++- crates/tools/src/draw2/family.rs | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/tests/toolsettings.rs b/crates/engine/src/tests/toolsettings.rs index 194f3ab26b..2ec7667bde 100644 --- a/crates/engine/src/tests/toolsettings.rs +++ b/crates/engine/src/tests/toolsettings.rs @@ -290,9 +290,11 @@ fn legacy_spiral_decay_preferences_report_the_ratio_drawn_after_reopening() { assert_eq!(anchors.len(), 5); assert!(!path.is_closed()); let center = Point::new(200.0, 200.0); + // The path runs from the centre end outwards, and decay is per full wind: four quarter-turn + // segments give the decay. for pair in anchors.windows(2) { let ratio = pair[0].p.distance(center) / pair[1].p.distance(center); - assert!((100.0 * ratio - decay).abs() < 1e-10); + assert!((100.0 * ratio.powi(4) - decay).abs() < 1e-9); } assert_eq!(s.prefs.tool_settings["spiral"]["decay"], json!(99.99)); s.execute("edit.undo", &json!({})).unwrap(); diff --git a/crates/tools/src/draw2/family.rs b/crates/tools/src/draw2/family.rs index 97193ec431..2e93888d02 100644 --- a/crates/tools/src/draw2/family.rs +++ b/crates/tools/src/draw2/family.rs @@ -545,12 +545,13 @@ mod tests { let (id, params) = t.command(Point::ZERO, Point::new(100.0, 0.0), Mods::default()); assert_eq!(id, "shape.spiral"); assert_eq!(params["decay"], json!(expected)); - // Inspect the actual adjacent radii, not a second copy of the tool's clamp. + // Inspect the actual adjacent radii, not a second copy of the tool's clamp. Decay is per full + // wind and a segment is a quarter turn, so four segments give the decay. let path = vectorcraft_geom::shapes::spiral(Point::ZERO, 100.0, params["decay"].as_f64().unwrap(), 2, true); let anchors = &path.subpaths[0].anchors; let outer = anchors[2].p.distance(Point::ZERO); let next = anchors[1].p.distance(Point::ZERO); - assert!((100.0 * next / outer - expected).abs() < 1e-10); + assert!((100.0 * (next / outer).powi(4) - expected).abs() < 1e-9); } for invalid in [Value::Null, json!("80"), json!(false), json!({})] { t.set_option("decay", &invalid);