diff --git a/crates/ui-egui/src/dim_view.rs b/crates/ui-egui/src/dim_view.rs index 04d27893..5af1355f 100644 --- a/crates/ui-egui/src/dim_view.rs +++ b/crates/ui-egui/src/dim_view.rs @@ -63,6 +63,9 @@ struct DimensionDraft { preview_doc: Arc, preview: Arc, error: Option, + /// A dimension that would over-constrain the sketch, placed as a driven (reference) one: + /// it has no parameter and no value to edit (#320). + driven: bool, } impl DimensionDraft { @@ -102,8 +105,15 @@ pub fn cancel() { /// Infer the measured value through the same command as scripts, without creating a real /// parameter or an undo entry. Only accepting the editor executes the command on the design. -pub fn begin_dimension(app: &SolveApp, entities: Vec, at: Vec2) -> Result<(), String> { +/// A driven dimension has no value to accept, so it is added at once. +pub fn begin_dimension(app: &mut SolveApp, entities: Vec, at: Vec2) -> Result<(), String> { let (param, expr, draft) = dimension_draft(app, entities, at)?; + if draft.driven { + PLACEMENT.with(|preview| *preview.borrow_mut() = None); + app.run("sketch.dimension", json!({"entities": draft.entities, "text_at": [at.x, at.y], "driven": true}))?; + app.set_status("That dimension would over-constrain the sketch, so it was added as a driven (reference) dimension", false); + return Ok(()); + } EDIT.with(|edit| *edit.borrow_mut() = Some(Edit { param, expr, at: None, fresh: FRESH, wait: 10, draft: Some(draft) })); PLACEMENT.with(|preview| *preview.borrow_mut() = None); Ok(()) @@ -114,22 +124,41 @@ fn dimension_draft(app: &SolveApp, entities: Vec, at: Vec2) -> Result<(S if entities.is_empty() || entities.len() > 2 || !at.x.is_finite() || !at.y.is_finite() { return Err("Pick one edge or two endpoints, then place the dimension".into()); } + // A dimension the sketch can't take (it would over-constrain it) becomes a driven one, + // which only measures; when that fails too, the first refusal is the one to report. + draft_of(app, sketch, &entities, at, false).or_else(|error| draft_of(app, sketch, &entities, at, true).map_err(|_| error)) +} + +/// The draft of one dimension: its parameter (for a driven one, its constraint id, since it +/// has no parameter), the parameter's expression, and the scratch session it was added in. +fn draft_of(app: &SolveApp, sketch: u64, entities: &[String], at: Vec2, driven: bool) -> Result<(String, String, DimensionDraft), String> { let mut scratch = app.session.scratch(); - let result = scratch.execute("sketch.dimension", &json!({"entities": entities, "text_at": [at.x, at.y]})).map_err(|e| e.to_string())?; - let param = result.get("param").and_then(serde_json::Value::as_str).ok_or("This dimension has no editable value")?.to_string(); - let expr = scratch.doc.param(¶m).ok_or("The dimension value is unavailable")?.expr.clone(); + let result = + scratch.execute("sketch.dimension", &json!({"entities": entities, "text_at": [at.x, at.y], "driven": driven})).map_err(|e| e.to_string())?; + let (param, expr) = if driven { + let before = + app.session.doc.sketch(sketch).ok().map(|sk| sk.constraints.iter().map(|c| c.id.clone()).collect::>()).unwrap_or_default(); + let added = + scratch.doc.sketch(sketch).ok().and_then(|sk| sk.constraints.iter().find(|c| c.driven && !before.contains(&c.id)).map(|c| c.id.clone())); + (added.ok_or("The driven dimension is unavailable")?, String::new()) + } else { + let param = result.get("param").and_then(serde_json::Value::as_str).ok_or("This dimension has no editable value")?.to_string(); + let expr = scratch.doc.param(¶m).ok_or("The dimension value is unavailable")?.expr.clone(); + (param, expr) + }; let draft = DimensionDraft { baseline: app.session.doc.clone(), sketch, component: app.session.active_component, path: app.session.active_occurrence_path.clone(), pending: app.session.pending_moves.clone(), - entities, + entities: entities.to_vec(), at, checked_expr: expr.clone(), preview: scratch.world_state(), preview_doc: scratch.doc.clone(), error: None, + driven, }; if !draft.preview.sketch(sketch).is_some_and(|solved| solved.report.ok()) { return Err("That dimension conflicts with the sketch constraints".into()); @@ -368,7 +397,9 @@ pub fn show(app: &mut SolveApp, ui: &egui::Ui, painter: &egui::Painter, proj: &P let draft_centres = draft_sketch.map(|solved| chain_centres(&solved.sketch)); let mut constraints = sk.constraints.iter().map(|c| (ss, c, false)).collect::>(); if let (Some(source), Some((param, _))) = (draft_sketch, draft.as_ref()) { - constraints.extend(source.sketch.constraints.iter().filter(|c| c.param.as_deref() == Some(param.as_str())).map(|c| (source, c, true))); + // A driven draft is keyed by its constraint id (it has no parameter). + let ours = |c: &&solvecraft_engine::sketch::Constraint| c.param.as_deref() == Some(param.as_str()) || (c.driven && c.id == *param); + constraints.extend(source.sketch.constraints.iter().filter(ours).map(|c| (source, c, true))); } for (source, c, is_draft) in constraints { if ss.sketch.view.hide_dimensions && !is_draft { @@ -605,6 +636,35 @@ mod tests { cancel(); } + /// Placing a dimension the sketch can't take previews it as driven: in the draft it is a + /// driven constraint with no parameter, labelled in parentheses (#320). + #[test] + fn an_over_constraining_placement_previews_a_driven_dimension() { + cancel(); + let mut h = crate::scenario::Harness::new(); + h.app.run("sketch.create", json!({"plane":"XY"})).unwrap(); + h.app.run("sketch.line", json!({"points":[[10,10],[30,10]],"infer":false})).unwrap(); + h.app.run("sketch.dimension", json!({"entities":["l1"],"value":"20 mm"})).unwrap(); + h.call("ui.view", json!({"view":"top"})); + let baseline = h.app.session.doc.to_json(); + h.step(&json!({"start":"sketch.dimension"})).unwrap(); + h.step(&json!({"click":{"sketch":[20,10]}})).unwrap(); + h.step(&json!({"move":{"sketch":[20,20]}})).unwrap(); + h.settle(); + let (id, draft) = placement_preview(&h.app).expect("an over-constraining dimension still previews"); + assert!(draft.driven); + let sk = &draft.preview.sketch(draft.sketch).unwrap().sketch; + let c = sk.constraints.iter().find(|c| c.id == id).unwrap(); + assert!(c.driven && c.param.is_none(), "{c:?}"); + assert!(matches!(c.kind, ConstraintKind::Length {value,..} if (value-20.).abs()<1e-6)); + assert_eq!(label(&c.kind, None, c.driven, "mm", 2), "(20.00)"); + assert!(!editing()); + assert_eq!(h.app.session.doc.to_json(), baseline); + h.step(&json!({"key":"Escape"})).unwrap(); + h.step(&json!({"key":"Escape"})).unwrap(); + cancel(); + } + #[test] fn placement_follows_real_pointer_without_editing_or_mutating_design() { cancel(); diff --git a/crates/ui-egui/src/dimension_create_ui_tests.rs b/crates/ui-egui/src/dimension_create_ui_tests.rs index d4277bee..3cca17cf 100644 --- a/crates/ui-egui/src/dimension_create_ui_tests.rs +++ b/crates/ui-egui/src/dimension_create_ui_tests.rs @@ -348,3 +348,33 @@ fn placing_a_two_point_dimension_picks_horizontal_vertical_or_aligned() { } } } + +/// A dimension that would over-constrain the sketch is placed as a driven (reference) one +/// instead of failing: added with no parameter or value editor, and removed again by one +/// undo (#320). +#[test] +fn an_over_constraining_dimension_is_placed_as_a_driven_one() { + let mut h = line([20., 0.]); + h.app.run("sketch.dimension", json!({"entities":["l1"],"value":"20 mm"})).unwrap(); + h.settle(); + let original = h.app.session.doc.to_json(); + let undo = h.app.session.undo.len(); + let params = h.app.session.doc.params.len(); + h.step(&json!({"start":"sketch.dimension"})).unwrap(); + h.step(&json!({"click":{"world":[10,0,0]}})).unwrap(); + h.step(&json!({"move":{"world":[10,6,0]}})).unwrap(); + h.settle(); + assert_eq!(crate::dim_view::drawn().len(), 1, "the preview is not a selectable dimension"); + h.step(&json!({"click":{"world":[10,6,0]}})).unwrap(); + assert!(!crate::dim_view::editing(), "a driven dimension has no value to edit"); + assert_eq!(h.app.session.undo.len(), undo + 1); + assert_eq!(h.app.session.doc.params.len(), params, "a driven dimension adds no parameter"); + let state = h.app.session.world_state(); + let solved = state.sketch(h.app.session.active_sketch.unwrap()).unwrap(); + assert!(solved.report.ok()); + let dims: Vec<_> = solved.sketch.constraints.iter().filter(|c| c.kind.is_dimension()).collect(); + assert_eq!(dims.len(), 2); + assert_eq!(dims.iter().filter(|c| c.driven && c.param.is_none()).count(), 1, "{dims:?}"); + h.app.run("edit.undo", json!({})).unwrap(); + assert_eq!(h.app.session.doc.to_json(), original); +} diff --git a/docs/book/src/guide/sketching.md b/docs/book/src/guide/sketching.md index 1584b737..19b6854f 100644 --- a/docs/book/src/guide/sketching.md +++ b/docs/book/src/guide/sketching.md @@ -112,6 +112,10 @@ Between two points, or on a line, that don't line up with an axis, where you pla dimension picks what it measures: across from them, the distance between them; above or below them, the horizontal distance; beside them, the vertical distance. +A dimension the sketch can't take, because the geometry is already fixed by other constraints +and dimensions, is added as a driven (reference) dimension instead: it is shown in +parentheses, measures the sketch, and has no value to edit. + For **Coincident**, pick two points, a point and a curve (in either order), or two distinct straight lines. Two lines join their closest pair of endpoints; the first picked line stays in place when the existing constraints allow it. Pick endpoints