forked from 1b-agent/opencadstudio-upstream
fix(undo): grip-drag history entry + mid-command Ctrl+Z steps (#332)
Grip drags mutated the document without ever pushing history — dragging a polyline vertex (or any grip) could not be undone. The release-commit now swaps the pre-drag backup in, snapshots that PRE state as a "GRIP" entry, swaps the dragged result back and clears the redo stack; the Esc-cancel path is unchanged and a no-move engaging click records nothing. Ctrl+Z during a drawing command steps the command instead of the document undo swallowing the in-progress object (which also killed the active command): new CadCommand::on_undo_step hook, consulted before undo_active_tab. PLINE pops its last vertex (shared with the U option), LINE reverts the last committed segment and keeps drawing from the previous point (rides its existing U/UndoDocument path), and ARC Continue reverts the last arc and resumes from the prior anchor with the correct tangent via a new anchor history. With nothing placed yet each falls through to the document undo. Also drops the two warnings the #336 work left behind (unused Entity import, dead Snapper::override_active). Closes #332 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
9b40881ab1
commit
9b28170d4b
8 files changed
+109
-34
No files matched your search
@@ -3709,6 +3709,17 @@ impl OpenCADStudio {
|
||||
Message::PspaceCommand => Task::done(Message::ExitViewport),
|
||||
|
||||
Message::Undo => {
|
||||
// Mid-command Ctrl+Z: a drawing command steps itself back
|
||||
// (PLINE pops the last vertex) instead of the document undo
|
||||
// swallowing the whole in-progress object.
|
||||
let i = self.active_tab;
|
||||
let step = self.tabs[i]
|
||||
.active_cmd
|
||||
.as_mut()
|
||||
.and_then(|c| c.on_undo_step());
|
||||
if let Some(r) = step {
|
||||
return self.apply_cmd_result(r);
|
||||
}
|
||||
self.undo_active_tab();
|
||||
Task::none()
|
||||
}
|
||||
|
||||
@@ -1829,11 +1829,29 @@ pub(super) fn on_tick(&mut self, t: Instant) -> Task<Message> {
|
||||
return Task::none();
|
||||
}
|
||||
self.tabs[i].active_grip = None;
|
||||
// Commit the grip drag: keep the doc's dragged geometry
|
||||
// (drop the cancel backup), un-hide the edited entity and
|
||||
// re-tessellate the base once, dropping the overlay preview.
|
||||
// Commit the grip drag: keep the doc's dragged geometry,
|
||||
// un-hide the edited entity and re-tessellate the base
|
||||
// once, dropping the overlay preview.
|
||||
if let Some(h) = self.grip_preview_handle.take() {
|
||||
self.grip_original = None;
|
||||
// Undo entry for the drag (#332): the pre-drag backup
|
||||
// swaps in, the PRE state is snapshotted, then the
|
||||
// dragged result swaps back. Without this a grip edit
|
||||
// (e.g. reshaping a polyline vertex) was un-undoable —
|
||||
// the drag mutated the document without any history.
|
||||
if let Some(orig) = self.grip_original.take() {
|
||||
if let Some(e) = self.tabs[i].scene.document.get_entity_mut(h) {
|
||||
let dragged = std::mem::replace(e, orig);
|
||||
let snap = self.capture_history_snapshot(i, "GRIP");
|
||||
if let Some(e2) =
|
||||
self.tabs[i].scene.document.get_entity_mut(h)
|
||||
{
|
||||
*e2 = dragged;
|
||||
}
|
||||
self.tabs[i].history.undo_stack.push(snap);
|
||||
self.tabs[i].history.redo_stack.clear();
|
||||
self.tabs[i].dirty = true;
|
||||
}
|
||||
}
|
||||
self.grip_text_verts = Vec::new();
|
||||
self.grip_text_slide = false;
|
||||
self.tabs[i].scene.hidden.remove(&h);
|
||||
|
||||
@@ -1218,6 +1218,14 @@ pub trait CadCommand: Send {
|
||||
/// during TRIM and trims during EXTEND, #336). Default no-op.
|
||||
fn set_shift(&mut self, _shift: bool) {}
|
||||
|
||||
/// Mid-command Ctrl+Z: a multi-point drawing command can take the undo
|
||||
/// itself (PLINE pops its last vertex) instead of the document undo
|
||||
/// swallowing the whole in-progress object. `None` (default) lets the
|
||||
/// normal document undo run.
|
||||
fn on_undo_step(&mut self) -> Option<CmdResult> {
|
||||
None
|
||||
}
|
||||
|
||||
/// Called when the user left-clicks in the viewport (point pick).
|
||||
fn on_point(&mut self, pt: DVec3) -> CmdResult;
|
||||
|
||||
|
||||
@@ -1334,6 +1334,10 @@ pub struct ArcContCommand {
|
||||
tangent: DVec3,
|
||||
/// Live Ctrl state (set via `set_ctrl`): flips the arc to the other way.
|
||||
ctrl: bool,
|
||||
/// Anchor history for mid-command Ctrl+Z: the (start, tangent) each
|
||||
/// committed arc was drawn from, so undoing a segment resumes the run
|
||||
/// from the prior anchor instead of ending it.
|
||||
history: Vec<(DVec3, DVec3)>,
|
||||
}
|
||||
|
||||
impl ArcContCommand {
|
||||
@@ -1342,6 +1346,7 @@ impl ArcContCommand {
|
||||
s,
|
||||
tangent,
|
||||
ctrl: false,
|
||||
history: Vec::new(),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1360,6 +1365,9 @@ impl CadCommand for ArcContCommand {
|
||||
match arc_continue(self.s, self.tangent, pt, self.ctrl) {
|
||||
Some((center, radius, sa, ea)) => {
|
||||
let arc = make_arc(center, radius, sa, ea);
|
||||
// Remember the anchor this arc was drawn from — mid-command
|
||||
// Ctrl+Z reverts the commit and resumes from here.
|
||||
self.history.push((self.s, self.tangent));
|
||||
// Keep drawing: re-anchor at the arc just committed so the next
|
||||
// click starts another tangent-continuing arc from its end,
|
||||
// until the user ends the run with Enter/Esc (#327).
|
||||
@@ -1374,6 +1382,15 @@ impl CadCommand for ArcContCommand {
|
||||
None => CmdResult::NeedPoint,
|
||||
}
|
||||
}
|
||||
fn on_undo_step(&mut self) -> Option<CmdResult> {
|
||||
// Ctrl+Z mid-run: revert the last committed arc and resume from the
|
||||
// anchor it was drawn from. With none committed this run, the
|
||||
// document undo takes over.
|
||||
let (s, tangent) = self.history.pop()?;
|
||||
self.s = s;
|
||||
self.tangent = tangent;
|
||||
Some(CmdResult::UndoDocument)
|
||||
}
|
||||
fn on_enter(&mut self) -> CmdResult {
|
||||
CmdResult::Cancel
|
||||
}
|
||||
|
||||
@@ -198,6 +198,16 @@ impl CadCommand for LineCommand {
|
||||
}
|
||||
}
|
||||
|
||||
fn on_undo_step(&mut self) -> Option<CmdResult> {
|
||||
// Ctrl+Z while drawing = the U option: revert the last committed
|
||||
// segment and keep drawing from the previous point. With nothing
|
||||
// placed yet the document undo takes over.
|
||||
if self.points.is_empty() {
|
||||
return None;
|
||||
}
|
||||
self.on_text_input("U")
|
||||
}
|
||||
|
||||
fn on_mouse_move(&mut self, pt: DVec3) -> Option<WireModel> {
|
||||
let last = *self.points.last()?;
|
||||
// With a deferred first tangent, slide the rubber band's start point
|
||||
|
||||
@@ -64,6 +64,36 @@ impl PlineCommand {
|
||||
}
|
||||
}
|
||||
|
||||
/// Drop the last placed vertex but keep drawing (#352): the U option and
|
||||
/// mid-command Ctrl+Z both land here. Repeatable down to (and including)
|
||||
/// the start point.
|
||||
fn undo_last_vertex(&mut self) -> CmdResult {
|
||||
if self.vertices.is_empty() {
|
||||
return CmdResult::NeedPoint;
|
||||
}
|
||||
self.vertices.pop();
|
||||
self.bulges.pop();
|
||||
// Restore the exit tangent of the segment that is now last so a
|
||||
// following Arc segment stays tangent-continuous.
|
||||
let n = self.vertices.len();
|
||||
self.last_tangent = if n >= 2 {
|
||||
seg_exit_tangent(self.vertices[n - 2], self.vertices[n - 1], self.bulges[n - 2])
|
||||
} else {
|
||||
None
|
||||
};
|
||||
match n {
|
||||
// Start point undone too — back to "Specify start point".
|
||||
0 => CmdResult::NeedPoint,
|
||||
// One vertex left: the live entity needs two, so it leaves the
|
||||
// document until the next point re-creates it.
|
||||
1 => match self.live_handle.take() {
|
||||
Some(h) => CmdResult::RemoveLiveEntity(h),
|
||||
None => CmdResult::NeedPoint,
|
||||
},
|
||||
_ => self.sync_live(false, false),
|
||||
}
|
||||
}
|
||||
|
||||
/// Build the result that publishes the current vertices to the document:
|
||||
/// create the live entity at the 2nd vertex, then replace it in place as
|
||||
/// more vertices land. `finish` ends the command (used for close/done).
|
||||
@@ -348,36 +378,21 @@ impl CadCommand for PlineCommand {
|
||||
// Undo: drop the last placed vertex but keep drawing (#352).
|
||||
// Repeatable — each U backs off one more segment, down to (and
|
||||
// including) the start point.
|
||||
"U" | "UNDO" => {
|
||||
if self.vertices.is_empty() {
|
||||
return Some(CmdResult::NeedPoint);
|
||||
}
|
||||
self.vertices.pop();
|
||||
self.bulges.pop();
|
||||
// Restore the exit tangent of the segment that is now last so
|
||||
// a following Arc segment stays tangent-continuous.
|
||||
let n = self.vertices.len();
|
||||
self.last_tangent = if n >= 2 {
|
||||
seg_exit_tangent(self.vertices[n - 2], self.vertices[n - 1], self.bulges[n - 2])
|
||||
} else {
|
||||
None
|
||||
};
|
||||
Some(match n {
|
||||
// Start point undone too — back to "Specify start point".
|
||||
0 => CmdResult::NeedPoint,
|
||||
// One vertex left: the live entity needs two, so it leaves
|
||||
// the document until the next point re-creates it.
|
||||
1 => match self.live_handle.take() {
|
||||
Some(h) => CmdResult::RemoveLiveEntity(h),
|
||||
None => CmdResult::NeedPoint,
|
||||
},
|
||||
_ => self.sync_live(false, false),
|
||||
})
|
||||
}
|
||||
"U" | "UNDO" => Some(self.undo_last_vertex()),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn on_undo_step(&mut self) -> Option<CmdResult> {
|
||||
// Ctrl+Z while drawing steps back one vertex; with nothing placed
|
||||
// yet the document undo takes over.
|
||||
if self.vertices.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(self.undo_last_vertex())
|
||||
}
|
||||
}
|
||||
|
||||
fn on_mouse_move(&mut self, pt: DVec3) -> Option<WireModel> {
|
||||
let pt = pt.as_vec3();
|
||||
// The committed vertices already render as a real document entity, so
|
||||
|
||||
@@ -2393,7 +2393,6 @@ fn fence_pieces(
|
||||
window: Option<[[f64; 2]; 2]>,
|
||||
extend: bool,
|
||||
) -> Option<Vec<EntityType>> {
|
||||
use acadrust::entities::Entity as _;
|
||||
// Sentinel handle: keeps the self-exclusion in *_seg_ts from matching any
|
||||
// real boundary geo while the piece is being re-picked.
|
||||
const TMP: u64 = u64::MAX - 7;
|
||||
|
||||
@@ -205,9 +205,6 @@ impl Snapper {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn override_active(&self) -> bool {
|
||||
self.override_saved.is_some()
|
||||
}
|
||||
|
||||
/// Whether temporary tracking points are being acquired and drawn: OTRACK
|
||||
/// on, or the Extension object snap on. Extension tracks a segment's line
|
||||
|
||||
Reference in new issue
Block a user