forked from 1b-agent/opencadstudio-upstream
Add per-constraint delete; fix named-parameter reference bugs
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 <noreply@anthropic.com>
This commit is contained in:
1 parent
753df67ee4
commit
59ae38ae1a
14 files changed
+370
-77
No files matched your search
@@ -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<Handle> = 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
|
||||
|
||||
@@ -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<acadrust::Handle>),
|
||||
/// 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 ────────────────────────────────────────────────
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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 => {
|
||||
|
||||
@@ -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 = [
|
||||
|
||||
+13
-63
@@ -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,
|
||||
|
||||
@@ -1114,6 +1114,7 @@ pub(super) fn viewport_context_menu_overlay(
|
||||
bottom_inset: f32,
|
||||
has_cmd: bool,
|
||||
has_selection: bool,
|
||||
selected_constraint: Option<crate::scene::sketch_constraints::ConstraintId>,
|
||||
isolation_active: bool,
|
||||
last_cmds: Vec<String>,
|
||||
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(
|
||||
|
||||
Reference in new issue
Block a user