A Sketch Scale window, replacing a toolbar button that only raised an engine error (#733)

Sketch Scale now opens a tool window with the entities, an optional base point and a typed factor, instead of running the command with no parameters and raising an engine error.
This commit is contained in:
William Dunn authored and GitHub committed 2026-10-13 11:46:00 -04:00
1 parent 1d30e4e1d2
commit 9b5e39844a
9 files changed
+334 -6

No files matched your search

+13 -6
View File
@@ -248,6 +248,9 @@ pub enum Kind {
/// Edit Text: a sketch text's content and placement (`sketch_text`).
Text(crate::sketch_text::Tx),
TextMove(crate::sketch_text_move::TextMove),
/// Sketch-space modify tools: scale, and later offset, fillet, chamfer, the patterns,
/// mirror and move (`sketch_modify_ui`).
SketchModify(crate::sketch_modify_ui::Mod),
}
/// The Single hole placement's input: places on planar faces. A pick on or near a sketch point
@@ -325,11 +328,10 @@ fn fits(a: Accept, s: &Sel) -> bool {
Sel::SketchCurve { .. } => a & (AXES | CURVES) != 0,
Sel::Vertex { .. } => a & selection::VERTICES != 0,
Sel::Feature { .. } => a & selection::FEATURES != 0,
Sel::SketchPoint { .. }
| Sel::SketchConstraint { .. }
| Sel::Occurrence { .. }
| Sel::ExternalSketchCurve { .. }
| Sel::ExternalSketchPoint { .. } => false,
// Sketch points go into a sketch command's own inputs (Entities, a base point), so a
// box selection and a pre-selection reach them the way a click already does.
Sel::SketchPoint { .. } => a & selection::POINTS != 0,
Sel::SketchConstraint { .. } | Sel::Occurrence { .. } | Sel::ExternalSketchCurve { .. } | Sel::ExternalSketchPoint { .. } => false,
}
}
@@ -528,7 +530,8 @@ impl Dialog {
.or_else(|| crate::dialogs_motion::start(app, id))
.or_else(|| crate::dialogs_parts::start(app, id))
.or_else(|| crate::sketch_text::start(app, id))
.or_else(|| crate::sketch_text_move::start(app, id))?;
.or_else(|| crate::sketch_text_move::start(app, id))
.or_else(|| crate::sketch_modify_ui::start(app, id))?;
Dialog::new(kind, inputs)
}
};
@@ -705,6 +708,7 @@ impl Dialog {
Kind::Sheet(k) => return k.primary(&self.inputs),
Kind::Surface(k) => return k.primary(),
Kind::Plastic(k) => return k.primary(),
Kind::SketchModify(k) => return k.primary(),
_ => return None,
})
}
@@ -997,6 +1001,7 @@ fn title(k: &Kind) -> &'static str {
Kind::Part(_) => "INSERT PART",
Kind::Text(_) => "EDIT TEXT",
Kind::TextMove(_) => "MOVE/COPY TEXT",
Kind::SketchModify(k) => k.title(),
}
}
@@ -2132,6 +2137,7 @@ pub fn show(app: &mut SolveApp, ctx: &egui::Context) {
Kind::Assembly(k) => enter |= crate::dialogs_assembly::rows(app, ui, k, &mut d.inputs, &mut d.active),
Kind::Sheet(k) => enter |= crate::dialogs_sheet::rows(app, ui, k, &mut d.inputs),
Kind::Surface(k) => enter |= crate::dialogs_surface::rows(ui, k),
Kind::SketchModify(k) => enter |= crate::sketch_modify_ui::rows(ui, k),
Kind::Plastic(k) => enter |= crate::dialogs_plastic::rows(app, ui, k, &d.inputs),
Kind::Appearance(k) => enter |= crate::dialogs_appearance::rows(app, ui, k, &mut d.inputs),
Kind::Motion(k) => enter |= crate::dialogs_motion::rows(app, ui, k, &mut d.inputs),
@@ -3165,6 +3171,7 @@ fn dialog_commands(app: &SolveApp, d: &Dialog) -> Result<Vec<(String, Value)>, S
Kind::Assembly(k) => return crate::dialogs_assembly::commands(app, k, &d.inputs),
Kind::Sheet(k) => return crate::dialogs_sheet::commands(app, k, &d.inputs, &d.extra),
Kind::Surface(k) => return crate::dialogs_surface::commands(app, k, &d.inputs, &d.extra),
Kind::SketchModify(k) => return crate::sketch_modify_ui::commands(app, k, &d.inputs, &d.extra),
Kind::Plastic(k) => return crate::dialogs_plastic::commands(app, k, &d.inputs, &d.extra),
Kind::Appearance(k) => return Ok(crate::dialogs_appearance::commands(k, &d.inputs)),
Kind::Motion(k) => return crate::dialogs_motion::commands(app, k, &d.inputs),
+3
View File
@@ -105,6 +105,9 @@ mod sketch_drag_ui_tests;
#[cfg(test)]
mod sketch_edit_tests;
mod sketch_families;
pub mod sketch_modify_ui;
#[cfg(test)]
mod sketch_modify_ui_tests;
pub mod sketch_palette;
pub(crate) mod sketch_text;
mod sketch_text_move;
+4
View File
@@ -39,6 +39,7 @@
"ui.autosave_every": "Autospeichern alle",
"ui.axes": "Achsen",
"ui.axis": "Achse",
"ui.base_point": "Basispunkt",
"ui.bodies": "Körper",
"ui.box": "Block",
"ui.break": "Unterbrechen",
@@ -95,12 +96,14 @@
"ui.edit": "Bearbeiten",
"ui.ellipse": "Ellipse",
"ui.end_radius": "Endradius",
"ui.entities": "Elemente",
"ui.equal": "Gleich",
"ui.expand": "Aufklappen",
"ui.export_ellipsis": "Exportieren...",
"ui.extend": "Verlängern",
"ui.extent_type": "Ausdehnungstyp",
"ui.extrude": "Extrudieren",
"ui.factor": "Faktor",
"ui.feature_options": "Funktionsoptionen",
"ui.features": "Funktionen",
"ui.file": "Datei",
@@ -157,6 +160,7 @@
"ui.heading_shell": "HÜLLE",
"ui.heading_sketch": "SKIZZE",
"ui.heading_sketch_palette": "SKIZZENPALETTE",
"ui.heading_sketch_scale": "SKIZZE SKALIEREN",
"ui.heading_solid": "KÖRPER",
"ui.heading_sphere": "SPHÄRE",
"ui.heading_split_body": "KÖRPER TRENNEN",
+4
View File
@@ -39,6 +39,7 @@
"ui.autosave_every": "Autosave every",
"ui.axes": "Axes",
"ui.axis": "Axis",
"ui.base_point": "Base point",
"ui.bodies": "Bodies",
"ui.box": "Box",
"ui.break": "Break",
@@ -95,12 +96,14 @@
"ui.edit": "Edit",
"ui.ellipse": "Ellipse",
"ui.end_radius": "End Radius",
"ui.entities": "Entities",
"ui.equal": "Equal",
"ui.expand": "Expand",
"ui.export_ellipsis": "Export...",
"ui.extend": "Extend",
"ui.extent_type": "Extent Type",
"ui.extrude": "Extrude",
"ui.factor": "Factor",
"ui.feature_options": "Feature Options",
"ui.features": "Features",
"ui.file": "File",
@@ -157,6 +160,7 @@
"ui.heading_shell": "SHELL",
"ui.heading_sketch": "SKETCH",
"ui.heading_sketch_palette": "SKETCH PALETTE",
"ui.heading_sketch_scale": "SKETCH SCALE",
"ui.heading_solid": "SOLID",
"ui.heading_sphere": "SPHERE",
"ui.heading_split_body": "SPLIT BODY",
+4
View File
@@ -39,6 +39,7 @@
"ui.autosave_every": "自動保存の間隔",
"ui.axes": "軸",
"ui.axis": "軸",
"ui.base_point": "基点",
"ui.bodies": "ボディ",
"ui.box": "直方体",
"ui.break": "分割",
@@ -95,12 +96,14 @@
"ui.edit": "編集",
"ui.ellipse": "楕円",
"ui.end_radius": "終了半径",
"ui.entities": "エンティティ",
"ui.equal": "等しい",
"ui.expand": "展開",
"ui.export_ellipsis": "エクスポート...",
"ui.extend": "延長",
"ui.extent_type": "範囲タイプ",
"ui.extrude": "押し出し",
"ui.factor": "倍率",
"ui.feature_options": "フィーチャーオプション",
"ui.features": "フィーチャー",
"ui.file": "ファイル",
@@ -157,6 +160,7 @@
"ui.heading_shell": "シェル",
"ui.heading_sketch": "スケッチ",
"ui.heading_sketch_palette": "スケッチ パレット",
"ui.heading_sketch_scale": "スケッチの尺度",
"ui.heading_solid": "ソリッド",
"ui.heading_sphere": "球",
"ui.heading_split_body": "ボディを分割",
+4
View File
@@ -39,6 +39,7 @@
"ui.autosave_every": "자동 저장 간격",
"ui.axes": "축",
"ui.axis": "축",
"ui.base_point": "기준점",
"ui.bodies": "바디",
"ui.box": "박스",
"ui.break": "끊기",
@@ -95,12 +96,14 @@
"ui.edit": "편집",
"ui.ellipse": "타원",
"ui.end_radius": "끝 반지름",
"ui.entities": "요소",
"ui.equal": "동일",
"ui.expand": "펼치기",
"ui.export_ellipsis": "내보내기...",
"ui.extend": "연장",
"ui.extent_type": "범위 유형",
"ui.extrude": "돌출",
"ui.factor": "배율",
"ui.feature_options": "피처 옵션",
"ui.features": "피처",
"ui.file": "파일",
@@ -157,6 +160,7 @@
"ui.heading_shell": "쉘",
"ui.heading_sketch": "스케치",
"ui.heading_sketch_palette": "스케치 팔레트",
"ui.heading_sketch_scale": "스케치 배율",
"ui.heading_solid": "솔리드",
"ui.heading_sphere": "구",
"ui.heading_split_body": "바디 분할",
+132
View File
@@ -0,0 +1,132 @@
//! Tool windows for sketch-space modify commands, on the `dialogs_surface` pattern (#19): one
//! `Mod` enum behind one `Kind::SketchModify` variant, grown by one variant and one arm per
//! command rather than by a new module each time. The first variant here is Scale, whose
//! toolbar button reached no tool and no dialog at all, so a press ran `sketch.scale` with no
//! parameters and the engine answered "`factor` must be a positive number".
//! `sketch.offset`, `sketch.fillet`, the three `sketch.chamfer.*` commands, the sketch
//! patterns, `sketch.mirror` and `sketch.move` are the same gap and join this module later,
//! each its own change.
use serde_json::{Map, Value, json};
use solidcraft_engine::Sel;
use solidcraft_engine::doc::expr::Kind as ValueKind;
use crate::SolveApp;
use crate::dialogs::{Kind, field, row_label};
use crate::selection::{CURVES, POINTS, SelInput};
/// A sketch modify tool window's values.
#[derive(Clone, Debug, PartialEq)]
pub enum Mod {
/// `sketch.scale`: the factor the picked entities scale by. The base point is the second
/// input; with none picked the command scales about the sketch origin, as its own default
/// already does.
Scale { factor: String },
}
impl Mod {
pub fn title(&self) -> &'static str {
match self {
Mod::Scale { .. } => "SKETCH SCALE",
}
}
/// The value on the canvas.
pub fn primary(&mut self) -> Option<(&'static str, ValueKind, &mut String)> {
Some(match self {
Mod::Scale { factor } => ("Factor", ValueKind::Unitless, factor),
})
}
}
fn inputs(k: &Mod) -> Vec<SelInput> {
match k {
Mod::Scale { .. } => vec![SelInput::new("Entities", CURVES | POINTS, true), SelInput::new("Base point", POINTS, false)],
}
}
/// The dialog for a sketch modify command.
pub fn start(_app: &SolveApp, id: &str) -> Option<(Kind, Vec<SelInput>)> {
let k = match id {
"sketch.scale" => Mod::Scale { factor: "2".into() },
_ => return None,
};
let i = inputs(&k);
Some((Kind::SketchModify(k), i))
}
/// The rows after the inputs; true when Enter was pressed in a field.
pub fn rows(ui: &mut egui::Ui, k: &mut Mod) -> bool {
let mut enter = false;
match k {
Mod::Scale { factor } => {
row_label(ui, "Factor");
enter |= field(ui, factor);
ui.end_row();
}
}
enter
}
/// What input `i` holds.
fn items(inputs: &[SelInput], i: usize) -> &[Sel] {
inputs.get(i).map(|x| x.items.as_slice()).unwrap_or(&[])
}
/// A picked sketch point as the id its own sketch knows it by. Picking prefixes the sketch
/// ("`<sketch>:<point>`") so points of any visible sketch stay distinct; a sketch command
/// names the point alone.
fn point_id(sel: &Sel) -> Option<&str> {
match sel {
Sel::SketchPoint { id } => Some(id.split_once(':').map(|(_, p)| p).unwrap_or(id)),
_ => None,
}
}
/// The curves and lone points picked in input `i`, as the engine's `entities` list.
fn entity_ids(inputs: &[SelInput], i: usize) -> Vec<String> {
items(inputs, i)
.iter()
.filter_map(|x| match x {
Sel::SketchCurve { id } => Some(id.clone()),
_ => point_id(x).map(str::to_string),
})
.collect()
}
/// A typed scale factor as the command wants it: the field evaluated against the document's
/// parameters, in the range the command accepts. Checking the range here lets the window say
/// so in its own error line instead of the status bar.
fn factor(app: &SolveApp, text: &str) -> Result<f64, String> {
let v = app.session.doc.eval(text.trim(), ValueKind::Unitless).map_err(|e| format!("Factor: {e}"))?;
if !(v.is_finite() && v > 1e-9 && v < 1e6) {
return Err("Factor must be a positive number".into());
}
Ok(v)
}
/// The command a sketch modify dialog makes. `sketch.scale` takes a plain evaluated number for
/// its factor, not an expression string: it has no model parameter to carry one, unlike
/// `sketch.fillet`'s radius.
pub fn commands(app: &SolveApp, k: &Mod, inputs: &[SelInput], extra: &Map<String, Value>) -> Result<Vec<(String, Value)>, String> {
let (cmd, mut params) = match k {
Mod::Scale { factor: text } => {
let entities = entity_ids(inputs, 0);
if entities.is_empty() {
return Err("Select the sketch curves or points to scale first".into());
}
let f = factor(app, text)?;
let mut p = json!({"entities": entities, "factor": f});
if let Some(base) = items(inputs, 1).first().and_then(point_id) {
p["base"] = json!(base);
}
("sketch.scale", p)
}
};
if let Value::Object(m) = &mut params {
for (key, value) in extra {
m.entry(key.clone()).or_insert_with(|| value.clone());
}
}
Ok(vec![(cmd.to_string(), params)])
}
@@ -0,0 +1,126 @@
//! The Sketch Scale tool window: the toolbar button used to reach no tool and no dialog, so a
//! press ran `sketch.scale` with no parameters and the engine refused it.
use serde_json::json;
use solidcraft_engine::Sel;
use crate::dialogs::Kind;
use crate::scenario::Harness;
use crate::selection::SelInput;
use crate::sketch_modify_ui::{Mod, commands};
/// A sketch on XY holding one 20 by 10 rectangle at the origin.
fn rectangle() -> Harness {
let mut h = Harness::new();
h.app.run("sketch.create", json!({"plane": "XY"})).unwrap();
h.app.run("sketch.rectangle.two_point", json!({"p0": [0, 0], "p1": [20, 10]})).unwrap();
h.settle();
h
}
fn curve_ids(h: &Harness) -> Vec<String> {
let sid = h.app.session.active_sketch.unwrap();
let st = h.app.session.model.state();
st.sketch(sid).unwrap().sketch.curves.iter().map(|c| c.id.clone()).collect()
}
#[test]
fn the_toolbar_button_opens_a_window_instead_of_reporting_an_engine_error() {
let mut h = rectangle();
let before = curve_ids(&h).len();
h.step(&json!({"start": "sketch.scale"})).unwrap();
let d = h.app.dialog.as_ref().expect("sketch.scale opens a tool window");
assert!(matches!(d.kind, Kind::SketchModify(Mod::Scale { .. })), "{:?}", d.kind);
assert_eq!(d.inputs.iter().map(|i| i.label).collect::<Vec<_>>(), vec!["Entities", "Base point"]);
// Opening the window commits nothing, and the old dead button's error is gone.
assert_eq!(curve_ids(&h).len(), before);
assert!(!h.app.status.as_ref().is_some_and(|(_, _, error)| *error), "{:?}", h.app.status);
}
#[test]
fn the_typed_factor_reaches_the_command_unchanged() {
let h = rectangle();
let ids = curve_ids(&h);
let mut inputs = vec![SelInput::new("Entities", crate::selection::CURVES, true), SelInput::new("Base point", crate::selection::POINTS, false)];
inputs[0].items = ids.iter().map(|id| Sel::SketchCurve { id: id.clone() }).collect();
let k = Mod::Scale { factor: "2.5".into() };
let out = commands(&h.app, &k, &inputs, &serde_json::Map::new()).unwrap();
assert_eq!(out.len(), 1);
assert_eq!(out[0].0, "sketch.scale");
assert_eq!(out[0].1["factor"], json!(2.5));
assert_eq!(out[0].1["entities"], json!(ids));
// No base point picked: the command's own default, the sketch origin, stands.
assert!(out[0].1.get("base").is_none(), "{}", out[0].1);
}
#[test]
fn an_expression_in_the_factor_field_is_evaluated() {
let mut h = rectangle();
h.app.run("parameters.add", json!({"name": "growth", "expression": "3", "unit": ""})).unwrap();
let ids = curve_ids(&h);
let mut inputs = vec![SelInput::new("Entities", crate::selection::CURVES, true)];
inputs[0].items = ids.iter().map(|id| Sel::SketchCurve { id: id.clone() }).collect();
let out = commands(&h.app, &Mod::Scale { factor: "growth * 2".into() }, &inputs, &serde_json::Map::new()).unwrap();
assert_eq!(out[0].1["factor"], json!(6.0));
}
#[test]
fn a_picked_base_point_loses_the_sketch_prefix_that_picking_adds() {
let h = rectangle();
let sid = h.app.session.active_sketch.unwrap();
let ids = curve_ids(&h);
let mut inputs = vec![SelInput::new("Entities", crate::selection::CURVES, true), SelInput::new("Base point", crate::selection::POINTS, false)];
inputs[0].items = vec![Sel::SketchCurve { id: ids[0].clone() }];
let point = {
let st = h.app.session.model.state();
st.sketch(sid).unwrap().sketch.points.first().unwrap().id.clone()
};
inputs[1].items = vec![Sel::SketchPoint { id: format!("{sid}:{point}") }];
let out = commands(&h.app, &Mod::Scale { factor: "2".into() }, &inputs, &serde_json::Map::new()).unwrap();
assert_eq!(out[0].1["base"], json!(point));
}
#[test]
fn an_empty_selection_or_a_bad_factor_is_an_error_the_window_shows() {
let h = rectangle();
let ids = curve_ids(&h);
let empty = vec![SelInput::new("Entities", crate::selection::CURVES, true)];
assert!(commands(&h.app, &Mod::Scale { factor: "2".into() }, &empty, &serde_json::Map::new()).is_err());
let mut filled = empty.clone();
filled[0].items = vec![Sel::SketchCurve { id: ids[0].clone() }];
for bad in ["0", "-2", "", "nonsense"] {
let r = commands(&h.app, &Mod::Scale { factor: bad.into() }, &filled, &serde_json::Map::new());
assert!(r.is_err(), "factor `{bad}` should not reach the command: {r:?}");
}
}
#[test]
fn the_window_scales_the_picked_geometry_and_leaves_the_rest_alone() {
let mut h = rectangle();
h.app.run("sketch.circle.center", json!({"center": [60, 0], "radius": 5})).unwrap();
h.settle();
let sid = h.app.session.active_sketch.unwrap();
let ids = curve_ids(&h);
let rect: Vec<String> = ids.iter().take(4).cloned().collect();
h.step(&json!({"start": "sketch.scale"})).unwrap();
{
let d = h.app.dialog.as_mut().unwrap();
d.inputs[0].items = rect.iter().map(|id| Sel::SketchCurve { id: id.clone() }).collect();
d.kind = Kind::SketchModify(Mod::Scale { factor: "2".into() });
}
// A frame with the picks in place: the canvas value box takes focus on the frame after a
// pick, and Enter only applies once it has had it.
h.frames(2);
h.step(&json!({"key": "Enter"})).unwrap();
h.settle();
assert!(h.app.dialog.is_none(), "Enter applies and closes the window");
let st = h.app.session.model.state();
let sk = &st.sketch(sid).unwrap().sketch;
let xs: Vec<f64> = sk.points.iter().map(|p| p.pos.x).collect();
let ys: Vec<f64> = sk.points.iter().map(|p| p.pos.y).collect();
// The rectangle doubled about the sketch origin; the circle at x = 60 did not move.
assert!(xs.iter().any(|x| (x - 40.0).abs() < 1e-6), "{xs:?}");
assert!(ys.iter().any(|y| (y - 20.0).abs() < 1e-6), "{ys:?}");
assert!(xs.iter().any(|x| (x - 60.0).abs() < 1e-6), "{xs:?}");
assert_eq!(sk.curves.len(), 5);
}
@@ -0,0 +1,44 @@
[
{"note": "sketch.scale drew a toolbar button that reached no tool and no dialog, so a press ran the command with no parameters at all and the engine answered \"`factor` must be a positive number\". It now opens a tool window carrying Entities, a Base point and a Factor."},
{"start": "sketch.create"},
{"click": {"plane": "XY"}},
{"expect": {"sketching": true}},
{"start": "sketch.rectangle.two_point"},
{"click": {"sketch": [0, 0]}},
{"click": {"sketch": [20, 10]}},
{"key": "Escape"},
{"expect": {"sketch_curves": {"own": 4}, "errors": 0}},
{"expect": {"query": {"command": "sketch.inspect", "path": ["profiles", 0, "area_mm2"], "value": 200}}},
{"note": "The dead-button regression: the press opens a window, commits nothing and reports nothing. A pick fills the input and still commits nothing."},
{"start": "sketch.scale"},
{"expect": {"dialog": "SketchModify(Scale", "sketch_curves": {"own": 4}, "errors": 0}},
{"click": {"sketch": [10, 0]}},
{"expect": {"dialog": "SketchModify(Scale", "sketch_curves": {"own": 4}, "errors": 0}},
{"expect": {"query": {"command": "sketch.inspect", "path": ["profiles", 0, "area_mm2"], "value": 200}}},
{"note": "Escape leaves the sketch exactly as it was."},
{"key": "Escape"},
{"expect": {"dialog": null, "errors": 0, "sketch_curves": {"own": 4}}},
{"expect": {"query": {"command": "sketch.inspect", "path": ["profiles", 0, "area_mm2"], "value": 200}}},
{"note": "The typed factor reaches the command unchanged: all four sides picked and 2 typed doubles the rectangle about the sketch origin, so its area goes from 200 to 800 mm2 and its far corner from (20, 10) to (40, 20)."},
{"start": "sketch.scale"},
{"click": {"sketch": [10, 0]}},
{"click": {"sketch": [20, 5]}},
{"click": {"sketch": [10, 10]}},
{"click": {"sketch": [0, 5]}},
{"click": {"handle": "dialog:Factor"}},
{"key": "A", "cmd": true},
{"text": "2"},
{"key": "Enter"},
{"expect": {"dialog": null, "errors": 0, "sketch_curves": {"own": 4}}},
{"expect": {"query": {"command": "sketch.inspect", "path": ["profiles", 0, "area_mm2"], "value": 800}}},
{"shot": "sketch_scale_window"},
{"note": "Starting the command again and pressing Escape leaves the document and the undo history untouched."},
{"start": "sketch.scale"},
{"expect": {"dialog": "SketchModify(Scale"}},
{"key": "Escape"},
{"expect": {"dialog": null, "errors": 0}}
]