From 59ae38ae1a15f2a29558f9c1b3cc4cf8eebb2df8 Mon Sep 17 00:00:00 2001 From: Felix Riestra Date: Tue, 15 Sep 2026 00:31:41 +0200 Subject: [PATCH] Add per-constraint delete; fix named-parameter reference bugs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Constraint delete: a Properties-panel row delete button, viewport click-to-select on a constraint glyph pill with Delete-key support, and a right-click "Delete Constraint" menu item — all routed through one undoable Scene::selected_constraint-aware path. Fixes two named-parameter bugs found while testing that feature: - DIMCONSTRAINT: a parameter named like a mode-switch keyword ("r", "d", "a", "x", "y") could never be referenced, since the keyword check ran before the parameter-name check. - PARAMETERS modal Apply rebuilt Scene::named_parameters but never refreshed the Properties panel, leaving renamed/deleted rows stale until an unrelated refresh (e.g. a selection change) happened to occur. Co-Authored-By: Claude Sonnet 5 --- src/app/command_driver.rs | 72 ++++++++++++++++++++++++++ src/app/mod.rs | 6 +++ src/app/properties.rs | 2 +- src/app/update/mod.rs | 11 ++++ src/app/update/viewport.rs | 26 ++++++++++ src/app/view/mod.rs | 76 +++++---------------------- src/app/view/overlay.rs | 10 ++++ src/modules/parametric/value.rs | 47 +++++++++++++++++ src/scene/mod.rs | 8 +++ src/scene/model/object.rs | 9 +++- src/scene/selection.rs | 4 ++ src/scene/sketch_constraints.rs | 92 +++++++++++++++++++++++++++++++++ src/ui/overlay.rs | 61 +++++++++++++++++++--- src/ui/properties.rs | 23 +++++++-- 14 files changed, 370 insertions(+), 77 deletions(-) diff --git a/src/app/command_driver.rs b/src/app/command_driver.rs index af720ea76..58a71c3c3 100644 --- a/src/app/command_driver.rs +++ b/src/app/command_driver.rs @@ -970,6 +970,41 @@ impl OpenCADStudio { } } + /// Removes one persistent sketch constraint by id (undoable), regardless + /// of whether it's flagged as conflicting. Shared by the Properties + /// panel's per-row delete button, the viewport's Delete key when a + /// constraint glyph is selected, and its right-click "Delete Constraint" + /// menu item — see `resolve_one_sketch_conflict` above for the same + /// undo pattern applied to the solver's own auto-removal. + pub(super) fn delete_sketch_constraint( + &mut self, + id: crate::scene::sketch_constraints::ConstraintId, + ) { + let i = self.active_tab; + let scope = self.tabs[i].current_sketch_scope(); + let Some(set) = self.tabs[i].scene.sketch_constraint_set(scope) else { return }; + let Some(constraint) = set.get(id) else { return }; + let touched: Vec = constraint.refs.iter().map(|r| r.entity).collect(); + let label = "Delete constraint"; + + let constraints_before = set.clone(); + let pending = self.begin_undo(i, label, touched.len(), true); + self.tabs[i] + .scene + .record_undo_sketch_constraints_before(scope, constraints_before); + self.tabs[i].scene.sketch_constraint_set_mut(scope).remove(id); + if self.tabs[i].scene.selected_constraint == Some(id) { + self.tabs[i].scene.selected_constraint = None; + } + let changes: Vec<(Handle, crate::scene::ChangeKind)> = + touched.into_iter().map(|h| (h, crate::scene::ChangeKind::Modified)).collect(); + self.tabs[i].scene.bump_entities(&changes); + if let Some(pd) = pending { + self.commit_undo_delta(i, pd); + } + self.refresh_properties(); + } + /// Rebuilds the active tab's `Scene::named_parameters` from the /// editor's working buffer and re-solves dependent constraints. pub(super) fn apply_named_parameter_editor_rows(&mut self) { @@ -7455,6 +7490,43 @@ mod sketch_constraint_undo_tests { assert_eq!(set.constraints.len(), 2, "undo should restore the removed constraint"); } + /// `delete_sketch_constraint` is the non-conflict counterpart to + /// `resolve_one_sketch_conflict` above: it removes an arbitrary + /// constraint by id (undoable), and also clears + /// `Scene::selected_constraint` when the deleted id was the one + /// selected — otherwise a stale id would linger and the viewport's + /// Delete key / right-click menu would target a constraint that no + /// longer exists. + #[test] + fn delete_sketch_constraint_removes_an_arbitrary_constraint_and_is_undoable() { + let mut app = OpenCADStudio::new_for_test(); + let _ = app.automation_op(r#"{"op":"new"}"#); + let line = add_line(&mut app, 0.0, 0.0, 10.0, 3.0); + + let _ = app.apply_cmd_result(CmdResult::AddSketchConstraint { + kind: ConstraintKind::Horizontal, + refs: vec![SketchRef::whole(line)], + driving_param: None, + label: "Horizontal constraint", + }); + let set = app.tabs[app.active_tab].scene.sketch_constraint_set(SketchScope::ModelSpace).unwrap(); + assert_eq!(set.constraints.len(), 1); + let id = set.constraints[0].id; + app.tabs[app.active_tab].scene.selected_constraint = Some(id); + + app.delete_sketch_constraint(id); + let set = app.tabs[app.active_tab].scene.sketch_constraint_set(SketchScope::ModelSpace).unwrap(); + assert_eq!(set.constraints.len(), 0, "the constraint should have been removed"); + assert_eq!( + app.tabs[app.active_tab].scene.selected_constraint, None, + "deleting the selected constraint should clear the selection" + ); + + app.undo_steps(1); + let set = app.tabs[app.active_tab].scene.sketch_constraint_set(SketchScope::ModelSpace).unwrap(); + assert_eq!(set.constraints.len(), 1, "undo should restore the deleted constraint"); + } + /// `named_parameters_design.md` stage 4: applying the PARAMETERS editor's /// working buffer both defines the parameter and re-solves whatever /// constraint already references it by name — the entire point of a diff --git a/src/app/mod.rs b/src/app/mod.rs index 0ad215b09..ea624615f 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -2908,6 +2908,12 @@ pub enum Message { /// A Constraints-section row was clicked: select every entity in the /// list (replacing the current selection) in the viewport. PropConstraintLinkClick(Vec), + /// A Constraints-section row's delete button was clicked, the Delete key + /// was pressed with the constraint's viewport glyph selected, or its + /// right-click "Delete Constraint" menu item was chosen: remove that one + /// persistent sketch constraint (undoable), independent of the entities + /// it touches. + PropConstraintDelete(crate::scene::sketch_constraints::ConstraintId), // ── About window ──────────────────────────────────────────────────── AboutOpen, // ── Graphics warning ──────────────────────────────────────────────── diff --git a/src/app/properties.rs b/src/app/properties.rs index 7096e7b03..2f3223b0f 100644 --- a/src/app/properties.rs +++ b/src/app/properties.rs @@ -2224,7 +2224,7 @@ impl OpenCADStudio { crate::scene::model::object::Property { label, field: "sketch_constraint", - value: crate::scene::model::object::PropValue::EntityLink { handles, conflicting }, + value: crate::scene::model::object::PropValue::EntityLink { id: c.id, handles, conflicting }, } }) .collect(); diff --git a/src/app/update/mod.rs b/src/app/update/mod.rs index 9cfbe5769..578090a8e 100644 --- a/src/app/update/mod.rs +++ b/src/app/update/mod.rs @@ -4287,6 +4287,13 @@ impl OpenCADStudio { } let i = self.active_tab; self.tabs[i].scene.selection.borrow_mut().context_menu = None; + // A selected constraint-glyph pill takes Delete before entity + // erase — the two selections are mutually exclusive (see + // `Scene::selected_constraint`). + if let Some(id) = self.tabs[i].scene.selected_constraint { + self.delete_sketch_constraint(id); + return Task::none(); + } let handles: Vec<_> = self.tabs[i].scene.selected.iter().cloned().collect(); if !handles.is_empty() { // Erase is delta-safe unless a target is in a group (group @@ -6218,6 +6225,10 @@ impl OpenCADStudio { self.refresh_properties(); Task::none() } + Message::PropConstraintDelete(id) => { + self.delete_sketch_constraint(id); + Task::none() + } // ── Options / About windows ─────────────────────────────────── Message::OptionsOpen => { diff --git a/src/app/update/viewport.rs b/src/app/update/viewport.rs index c57bb947c..58146a792 100644 --- a/src/app/update/viewport.rs +++ b/src/app/update/viewport.rs @@ -4451,6 +4451,31 @@ properties={:.1}ms picked={}", sel.box_current = None; } else { if box_anchor.is_none() { + // A sketch-constraint glyph pill draws on top of the + // geometry it touches and takes priority over it when + // clicked (design mirrors `has_selection` gating the + // viewport right-click menu, and `DeleteSelected` + // preferring a selected pill over entity erase). + let glyph_hit = (self.tabs[i].scene.current_layout == "Model") + .then(|| { + let scope = self.tabs[i].current_sketch_scope(); + self.tabs[i].scene.constraint_glyph_hit( + scope, + canvas_sz, + self.show_constraint_values, + p_full, + ) + }) + .flatten(); + if let Some(id) = glyph_hit { + if !selection_pick_add && !self.shift_down { + self.tabs[i].scene.deselect_all(); + } + self.tabs[i].scene.selected_constraint = Some(id); + self.refresh_properties(); + selection_just_completed = true; + } + if glyph_hit.is_none() { let (view_rot, eye, all_wires) = self.pick_view(i, &edit_cam, bounds); let click_world = self.cursor_model_point(i, &edit_cam, p, bounds); let t_arm = crate::perf::enabled().then(Instant::now); @@ -4611,6 +4636,7 @@ was_selected={}", } } } + } } else { let a = box_anchor.unwrap(); let box_points = [ diff --git a/src/app/view/mod.rs b/src/app/view/mod.rs index 285905fcb..2245b295f 100644 --- a/src/app/view/mod.rs +++ b/src/app/view/mod.rs @@ -817,72 +817,21 @@ bg={bg_ms:.1}ms n={view_count}" let point_cursor = tab.active_cmd.as_ref().is_some_and(|cmd| { !cmd.needs_entity_pick() && !cmd.is_selection_gathering() }); - // Constraint glyphs are displayed in model space only. - let constraint_glyphs: Vec<(iced::Point, [f32; 2], String, bool)> = if is_paper { + // Persistent sketch-constraint glyphs (design doc §6.3/§7) — model + // space only, like the constraints themselves; paper space shows + // none (out of scope per the design doc §3.1). + let constraint_glyphs: Vec<(iced::Point, [f32; 2], String, bool, bool)> = if is_paper { Vec::new() } else { - let (vw, vh) = sel_ref.vp_size; let scope = tab.current_sketch_scope(); - match tab.scene.sketch_constraint_set(scope) { - Some(set) if !set.constraints.is_empty() => { - let edit_frame = tab.scene.viewport_edit_frame((vw, vh)); - let bounds = match &edit_frame { - Some((_, full)) => *full, - None => tab.scene.active_model_tile_bounds(vw, vh), - }; - let (view_rot, eye) = if let Some((cam, _)) = &edit_frame { - (cam.view_proj_rte(bounds), cam.eye()) - } else { - let cam = tab.scene.camera.borrow(); - (cam.view_proj_rte(bounds), cam.eye()) - }; - set.constraints - .iter() - .filter(|c| c.enabled) - .filter(|c| tab.scene.is_sketch_constraint_visible(scope, c.id)) - .filter_map(|c| { - let (anchor, outward) = - crate::scene::sketch_constraints::glyph_placement( - &tab.scene.document, - c, - )?; - let screen = crate::scene::pick::grip::project_rte( - glam::DVec3::new(anchor.x, anchor.y, anchor.z), - view_rot, - eye, - bounds, - )?; - let outward_screen = crate::scene::pick::grip::project_rte( - glam::DVec3::new( - anchor.x + outward.x, - anchor.y + outward.y, - anchor.z + outward.z, - ), - view_rot, - eye, - bounds, - )?; - let direction = (outward_screen - screen) - .normalize_or(glam::Vec2::NEG_Y); - let point = iced::Point::new( - bounds.x + screen.x, - bounds.y + screen.y, - ); - let is_conflicting = - set.conflicts.iter().any(|(id, _)| *id == c.id); - let label = if self.show_constraint_values { - crate::scene::sketch_constraints::glyph_label(c) - } else { - c.kind.glyph_symbol().to_string() - }; - point.x.is_finite().then(|| { - (point, direction.to_array(), label, is_conflicting) - }) - }) - .collect() - } - _ => Vec::new(), - } + tab.scene + .constraint_glyph_placements_screen(scope, sel_ref.vp_size, self.show_constraint_values) + .into_iter() + .map(|(id, point, direction, label, is_conflicting)| { + let selected = tab.scene.selected_constraint == Some(id); + (point, direction, label, is_conflicting, selected) + }) + .collect() }; crate::ui::overlay::selection_overlay( std::sync::Arc::clone(&tab.scene.selection), @@ -1811,6 +1760,7 @@ bg={bg_ms:.1}ms n={view_count}" command_line_inset, has_cmd, has_selection, + tab.scene.selected_constraint, isolation_active, last_cmds, draworder_open, diff --git a/src/app/view/overlay.rs b/src/app/view/overlay.rs index 3192b9128..275097766 100644 --- a/src/app/view/overlay.rs +++ b/src/app/view/overlay.rs @@ -1114,6 +1114,7 @@ pub(super) fn viewport_context_menu_overlay( bottom_inset: f32, has_cmd: bool, has_selection: bool, + selected_constraint: Option, isolation_active: bool, last_cmds: Vec, draworder_open: bool, @@ -1250,6 +1251,15 @@ pub(super) fn viewport_context_menu_overlay( t!("Invert Selection").into_owned(), Message::InvertSelection, )); + } else if let Some(id) = selected_constraint { + // A selected constraint-glyph pill gets its own minimal menu — + // none of the entity-selection actions above (Move/Copy/Isolate/ + // Select Similar) apply to it. + items.push(item( + t!("Delete Constraint").into_owned(), + Message::PropConstraintDelete(id), + )); + items.push(sep()); } if isolation_active { items.push(item( diff --git a/src/modules/parametric/value.rs b/src/modules/parametric/value.rs index 068923701..7b16e4c13 100644 --- a/src/modules/parametric/value.rs +++ b/src/modules/parametric/value.rs @@ -302,6 +302,18 @@ impl CadCommand for DistanceConstraintCommand { } fn on_text_input(&mut self, text: &str) -> Option { + // A token that names an existing parameter always wins over the + // mode-switch keywords below — otherwise a parameter literally + // named "r", "d", "a", "x", or "y" (all entirely reasonable names, + // and exactly the single-letter keywords this command recognizes) + // could never be referenced here: e.g. on a circle, typing "r" + // would always be swallowed as the Radius mode-switch instead of + // resolving to the parameter. + if let Some(value @ DrivingValue::Named(_)) = + parse_driving_value(text, &self.known_param_names) + { + return self.build(value); + } // Mode-switch keywords re-prompt instead of building — consumed // inputs return `Some(NeedPoint)` so the driver doesn't re-offer // the same token a second time (matches e.g. `trim.rs`'s @@ -584,6 +596,41 @@ mod tests { )); } + /// A parameter literally named "r" (or "d"/"a"/"x"/"y", the other + /// single-letter mode-switch keywords) must still resolve to the + /// parameter, not be swallowed as a keyword — regression test for the + /// bug where typing a circle's radius-driving parameter named "r" was + /// always misread as the "switch to Radius mode" keyword and silently + /// re-prompted instead of building a constraint. + #[test] + fn a_parameter_named_like_a_mode_keyword_still_resolves_by_name() { + let mut scene = Scene::new(); + let circle = add_circle(&mut scene); + scene.named_parameters_mut().set("r", "5").unwrap(); + let mut cmd = DistanceConstraintCommand::new(&scene, circle).unwrap(); + assert!(matches!( + cmd.on_text_input("R"), + Some(CmdResult::AddSketchConstraint { + kind: ConstraintKind::Radius, + driving_param: Some(DrivingValue::Named(name)), + .. + }) if name == "r" + )); + + let mut scene2 = Scene::new(); + let line = add_line(&mut scene2); + scene2.named_parameters_mut().set("x", "5").unwrap(); + let mut cmd2 = DistanceConstraintCommand::new(&scene2, line).unwrap(); + assert!(matches!( + cmd2.on_text_input("X"), + Some(CmdResult::AddSketchConstraint { + kind: ConstraintKind::Distance, + driving_param: Some(DrivingValue::Named(name)), + .. + }) if name == "x" + )); + } + #[test] fn ordinary_distances_reject_nonpositive_and_nonfinite_values() { let mut scene = Scene::new(); diff --git a/src/scene/mod.rs b/src/scene/mod.rs index d36fef59f..42961d23e 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -1627,6 +1627,13 @@ pub struct Scene { /// Currently selected entity handles. pub selected: HashSet, selected_order: Vec, + /// The sketch-constraint glyph pill currently selected in the viewport + /// (click-to-select — see `sketch_constraints::constraint_glyph_hit`), + /// independent of `selected`'s entity handles. Cleared whenever entity + /// selection changes so the two stay mutually exclusive; validated + /// against the current scope's constraint set at use, since switching + /// sketch scope can leave a stale id behind. + pub selected_constraint: Option, /// Session-only ISOLATEOBJECTS / HIDEOBJECTS state. Never written to DWG/DXF. pub object_isolation: ObjectIsolationState, /// Entity handles temporarily removed from the base render while an @@ -2171,6 +2178,7 @@ impl Scene { lighting_cache: RefCell::new(HashMap::default()), selected: HashSet::default(), selected_order: Vec::new(), + selected_constraint: None, object_isolation: ObjectIsolationState::default(), preview_hidden: HashSet::default(), command_preview_hidden: HashSet::default(), diff --git a/src/scene/model/object.rs b/src/scene/model/object.rs index e5aed1d5d..98c637415 100644 --- a/src/scene/model/object.rs +++ b/src/scene/model/object.rs @@ -62,8 +62,13 @@ pub enum PropValue { /// row for one persistent sketch constraint. Clicking it selects every /// entity in `handles` in the viewport. `conflicting` mirrors /// `SketchConstraintSet::conflicts`, tinting the row the same danger - /// color the constraint's glyph pill already uses. - EntityLink { handles: Vec, conflicting: bool }, + /// color the constraint's glyph pill already uses. `id` routes the + /// row's own delete button back to this specific constraint. + EntityLink { + id: crate::scene::sketch_constraints::ConstraintId, + handles: Vec, + conflicting: bool, + }, /// One row of the document-wide named-parameter table (Parameters /// section, shown when nothing is selected). `index` is the row's /// position in `ParameterTable::iter()` order — stable across edits to diff --git a/src/scene/selection.rs b/src/scene/selection.rs index 46fdc260f..d30b0de2e 100644 --- a/src/scene/selection.rs +++ b/src/scene/selection.rs @@ -72,6 +72,7 @@ impl Scene { if handles.is_empty() { return; } + self.selected_constraint = None; let expanded = self.expanded_with_leaders(handles); let mut changed = false; for handle in expanded { @@ -111,6 +112,7 @@ impl Scene { expanded } pub fn select_entity(&mut self, handle: Handle, exclusive: bool) { + self.selected_constraint = None; let handles = self.handles_expanded_for_leader_annotations(&[handle]); let mut changed = false; @@ -133,6 +135,7 @@ impl Scene { } pub fn deselect_all(&mut self) { + self.selected_constraint = None; if self.selected.is_empty() { return; } @@ -215,6 +218,7 @@ impl Scene { /// only when its contents actually changed. History/file/command paths must /// use this instead of assigning `selected` directly. pub(crate) fn replace_selection(&mut self, selected: HashSet) { + self.selected_constraint = None; let handles: Vec = selected.iter().copied().collect(); let selected: HashSet = self .handles_expanded_for_leader_annotations(&handles) diff --git a/src/scene/sketch_constraints.rs b/src/scene/sketch_constraints.rs index 661c9ab9a..2907ccc1a 100644 --- a/src/scene/sketch_constraints.rs +++ b/src/scene/sketch_constraints.rs @@ -640,6 +640,98 @@ impl super::Scene { } } + /// Screen-projected sketch-constraint glyph placements for `scope`: + /// `(id, anchor, outward_screen_direction, label, is_conflicting)` for + /// every enabled, visible constraint whose glyph projects on-screen. + /// `vp_size` is the full canvas size (as `SelectionState::vp_size` + /// reports it), matching what `viewport_edit_frame`/ + /// `active_model_tile_bounds` expect. Mirrors the projection + /// `crate::app::view` builds its own render list with, and is reused by + /// [`constraint_glyph_hit`](Self::constraint_glyph_hit) — both feed the + /// same `(anchor, outward, label)` triples into + /// `crate::ui::overlay::constraint_glyph_box`/`constraint_glyph_offsets`, + /// so hit-testing can never drift from what's actually drawn. + pub fn constraint_glyph_placements_screen( + &self, + scope: SketchScope, + vp_size: (f32, f32), + show_values: bool, + ) -> Vec<(ConstraintId, iced::Point, [f32; 2], String, bool)> { + let Some(set) = self.sketch_constraint_set(scope) else { + return Vec::new(); + }; + if set.constraints.is_empty() { + return Vec::new(); + } + let edit_frame = self.viewport_edit_frame(vp_size); + let bounds = match &edit_frame { + Some((_, full)) => *full, + None => self.active_model_tile_bounds(vp_size.0, vp_size.1), + }; + let (view_rot, eye) = if let Some((cam, _)) = &edit_frame { + (cam.view_proj_rte(bounds), cam.eye()) + } else { + let cam = self.camera.borrow(); + (cam.view_proj_rte(bounds), cam.eye()) + }; + set.constraints + .iter() + .filter(|c| c.enabled) + .filter(|c| self.is_sketch_constraint_visible(scope, c.id)) + .filter_map(|c| { + let (anchor, outward) = glyph_placement(&self.document, c)?; + let screen = crate::scene::pick::grip::project_rte( + glam::DVec3::new(anchor.x, anchor.y, anchor.z), + view_rot, + eye, + bounds, + )?; + let outward_screen = crate::scene::pick::grip::project_rte( + glam::DVec3::new( + anchor.x + outward.x, + anchor.y + outward.y, + anchor.z + outward.z, + ), + view_rot, + eye, + bounds, + )?; + let direction = (outward_screen - screen).normalize_or(glam::Vec2::NEG_Y); + let point = iced::Point::new(bounds.x + screen.x, bounds.y + screen.y); + let is_conflicting = set.conflicts.iter().any(|(id, _)| *id == c.id); + let label = if show_values { + glyph_label(c) + } else { + c.kind.glyph_symbol().to_string() + }; + point.x.is_finite().then_some((c.id, point, direction.to_array(), label, is_conflicting)) + }) + .collect() + } + + /// Hit-tests screen point `p` (same coordinate space as `p_full` in the + /// viewport click handler) against the glyph pills from + /// [`constraint_glyph_placements_screen`](Self::constraint_glyph_placements_screen), + /// via `crate::ui::overlay::constraint_glyph_hit_test`'s shared layout + /// math, so a click only registers where the pill is actually drawn. + pub fn constraint_glyph_hit( + &self, + scope: SketchScope, + vp_size: (f32, f32), + show_values: bool, + p: iced::Point, + ) -> Option { + let placements = self.constraint_glyph_placements_screen(scope, vp_size, show_values); + let glyphs: Vec<(iced::Point, [f32; 2], String, bool)> = placements + .iter() + .map(|(_, point, direction, label, is_conflicting)| { + (*point, *direction, label.clone(), *is_conflicting) + }) + .collect(); + let index = crate::ui::overlay::constraint_glyph_hit_test(&glyphs, p)?; + Some(placements[index].0) + } + /// Handle remapping lives in each command that duplicates /// entities — `Scene::copy_entities`' `handle_map` (COPY/ARRAY/MIRROR, /// `src/scene/modify.rs`) and `OpenCADStudio::finalize_paste`'s own diff --git a/src/ui/overlay.rs b/src/ui/overlay.rs index 4cd3418d2..e7b403fdc 100644 --- a/src/ui/overlay.rs +++ b/src/ui/overlay.rs @@ -94,6 +94,34 @@ fn constraint_glyph_offsets(glyphs: &[(Point, [f32; 2], String, bool)]) -> Vec Option { + let offsets = constraint_glyph_offsets(glyphs); + glyphs + .iter() + .zip(offsets) + .enumerate() + .rev() + .find_map(|(index, ((anchor, outward, label, _), tangent_offset))| { + let (top_left, size) = constraint_glyph_box(*anchor, *outward, label, tangent_offset); + let within = p.x >= top_left.x + && p.x <= top_left.x + size.width + && p.y >= top_left.y + && p.y <= top_left.y + size.height; + within.then_some(index) + }) +} + /// Convert CURSORSIZE to a screen-space arm length while keeping the original /// 60 px cursor at the default value and the full-viewport result at 100. pub(crate) fn crosshair_arm_px(bounds: iced::Rectangle, value: i32) -> f32 { @@ -723,7 +751,7 @@ pub fn selection_overlay<'a>( crosshair_bg: [f32; 4], crosshair: CrosshairOptions, selection_visual: SelectionVisualOptions, - constraint_glyphs: Vec<(Point, [f32; 2], String, bool)>, + constraint_glyphs: Vec<(Point, [f32; 2], String, bool, bool)>, ) -> Element<'a, Message> { canvas(SelectionCanvas { selection, @@ -806,8 +834,10 @@ struct SelectionCanvas { crosshair_bg: [f32; 4], crosshair: CrosshairOptions, selection_visual: SelectionVisualOptions, - /// Constraint glyph anchor, outward screen direction, label, and conflict state. - constraint_glyphs: Vec<(Point, [f32; 2], String, bool)>, + /// Constraint glyph anchor, outward screen direction, label, conflict + /// state, and whether the pill itself is the current click-to-select + /// target (`Scene::selected_constraint`) — for the current scope. + constraint_glyphs: Vec<(Point, [f32; 2], String, bool, bool)>, } fn draw_grip_marker( @@ -1703,9 +1733,21 @@ impl canvas::Program for SelectionCanvas { frame.stroke(&b1, stroke.clone()); frame.stroke(&b2, stroke); } - // Constraint glyphs are visual-only and have no hit testing. + // Click-to-select hit testing against these pills lives in + // `Scene::constraint_glyph_hit`, sharing this same size formula so + // the clickable area never drifts from what's drawn here. if !self.constraint_glyphs.is_empty() { - let offsets = constraint_glyph_offsets(&self.constraint_glyphs); + // `constraint_glyph_offsets` only needs the anchor/outward/label/ + // conflict quadruple it was written against; project away the + // trailing `is_selected` flag rather than widen its signature. + let glyphs_for_offsets: Vec<(Point, [f32; 2], String, bool)> = self + .constraint_glyphs + .iter() + .map(|(anchor, outward, label, is_conflicting, _)| { + (*anchor, *outward, label.clone(), *is_conflicting) + }) + .collect(); + let offsets = constraint_glyph_offsets(&glyphs_for_offsets); let normal_bg = theme.palette().primary.base.color; let normal_fg = theme.palette().primary.base.text; // A redundant or conflicting constraint gets the danger palette @@ -1714,7 +1756,8 @@ impl canvas::Program for SelectionCanvas { // would show, surfaced right on the geometry. let conflict_bg = theme.palette().danger.base.color; let conflict_fg = theme.palette().danger.base.text; - for ((anchor, outward, label, is_conflicting), tangent_offset) in + let selected_ring = theme.palette().primary.strong.color; + for ((anchor, outward, label, is_conflicting, is_selected), tangent_offset) in self.constraint_glyphs.iter().zip(offsets) { if !anchor.x.is_finite() || !anchor.y.is_finite() { @@ -1729,6 +1772,12 @@ impl canvas::Program for SelectionCanvas { (size.height * 0.5).into(), ); frame.fill(&pill, bg); + if *is_selected { + frame.stroke( + &pill, + canvas::Stroke::default().with_color(selected_ring).with_width(2.0), + ); + } frame.fill_text(canvas::Text { content: label.clone(), position: Point::new( diff --git a/src/ui/properties.rs b/src/ui/properties.rs index 507c150d6..ef04d6785 100644 --- a/src/ui/properties.rs +++ b/src/ui/properties.rs @@ -865,8 +865,8 @@ impl PropertiesPanel { self.render_hatch_pattern_row(label, current) } PropValue::AttrText { tag, value } => self.render_attr_row(tag, value), - PropValue::EntityLink { handles, conflicting } => { - render_entity_link_row(label, handles.clone(), *conflicting) + PropValue::EntityLink { id, handles, conflicting } => { + render_entity_link_row(label, *id, handles.clone(), *conflicting) } PropValue::ParamRow { index, name, formula, resolved } => { self.render_param_row(*index, name, formula, resolved) @@ -1933,8 +1933,15 @@ fn render_ro_with_tooltip_row<'a>( /// field, so this doesn't use the usual `prop_row_widget` label|value split. /// Clicking it selects every entity in `handles`; a conflicting/redundant /// constraint (mirrors the viewport glyph pill's own color cue) tints red. -fn render_entity_link_row<'a>(label: &'a str, handles: Vec, conflicting: bool) -> Element<'a, Message> { - let btn = button(text(label).size(FONT_SZ).width(Length::Fill)) +/// A trailing `✕` deletes the constraint outright (undoable), matching +/// `PropParamDelete`'s own row-delete button in the Parameters section. +fn render_entity_link_row<'a>( + label: &'a str, + id: crate::scene::sketch_constraints::ConstraintId, + handles: Vec, + conflicting: bool, +) -> Element<'a, Message> { + let link_btn = button(text(label).size(FONT_SZ).width(Length::Fill)) .on_press(Message::PropConstraintLinkClick(handles)) .style(move |theme: &Theme, status| { let palette = theme.palette(); @@ -1955,7 +1962,13 @@ fn render_entity_link_row<'a>(label: &'a str, handles: Vec, conflicting: }) .padding([3, 8]) .width(Length::Fill); - container(btn).width(Length::Fill).into() + let delete_btn = button(text("\u{2715}").size(FONT_SZ)) + .on_press(Message::PropConstraintDelete(id)) + .style(button::text) + .padding([2, 6]); + container(row![link_btn, delete_btn].spacing(2).align_y(iced::Center)) + .width(Length::Fill) + .into() } // ── Parameters section: "+ Add parameter" row ──────────────────────────────