Edit Sketch: restore timeline and undo state when finishing (#10691) (#10791)

* claim: #10691

* Fix sketch edit completion history (#10691)
This commit is contained in:
Colin Faulkingham
2026-11-02 09:37:38 +00:00
committed by GitHub
parent 3ddd73728b
commit 3c806394b0
13 changed files with 434 additions and 47 deletions
+121 -12
View File
@@ -7,7 +7,9 @@ use solidcraft_sketch::{Analysis, Budget, ConstraintKind, CurveKind, Hold, Point
use super::{CommandSpec, in_sketch};
use crate::params::{bad, bool_, expr, num, req_expr, req_vec2, str_, string_list, vec2, vec3};
use crate::{EngineError, Result, Session, SketchEntry};
use crate::schema::sketch::{CancelResult, EditParams, EditResult, ExitParams, FinishResult};
use crate::schema::{TypedContract, output_schema_for};
use crate::{EngineError, Result, Session, SketchEntry, Snapshot};
#[path = "sketch_dimensioned.rs"]
mod dimensioned;
@@ -33,18 +35,28 @@ macro_rules! model_operand_doc {
};
}
const EDIT_CONTRACT: TypedContract<EditParams> = TypedContract::new();
const EXIT_CONTRACT: TypedContract<ExitParams> = TypedContract::new();
pub static COMMANDS: &[CommandSpec] = &[
CommandSpec::new("sketch.create", "Create Sketch", create_sketch)
.at("SOLID", "CREATE")
.icon("sketch")
.params("plane: XY|XZ|YZ (sketch x, y along world +X, +Y on XY; +X, -Z on XZ; +Y, +Z on YZ) | construction plane name or id | {origin?: [x,y,z] numbers in mm, default [0,0,0] (not an expression), x_dir, y_dir} (in world coordinates unless frame says otherwise; a named plane is the active component's own) | {face: [world x,y,z], body?: exact world body, index?: face index, path?: complete occurrence ids, fixed?: bool}; new face attachments require a unique certified source (a flat face of a sheet metal body attaches to its panel instead, in the flat pattern's coordinates), fixed:true explicitly freezes the selected plane; offset?: expr, flip?: bool | \"x\" | \"y\" (not on a face: sketch the plane from its other side, so the normal and the default extrude direction are reversed; a half turn about the plane's own x or y axis, true picks the one keeping the sketch's y axis upward; offset still moves along the unflipped normal), frame?: world (default) | component (which coordinates an explicit plane is in; the reply's `frame` says which, with a `note` when the active component is moved or turned), project_edges?: bool, activate_part?: bool (default false; on a face of a body of another component than the active one: activate that component at the picked placement first, as component.activate would, and make the sketch there; it stays active afterwards and the reply's `activated` names it), name?"),
CommandSpec::new("sketch.edit", "Edit Sketch", edit_sketch).noundo().icon("sketch").params("sketch: id or name"),
CommandSpec::new("sketch.finish", "Finish Sketch", finish_sketch).at("SKETCH", "FINISH SKETCH").icon("finish").enabled(in_sketch).noundo(),
CommandSpec::new("sketch.cancel", "Cancel Sketch", cancel_sketch)
CommandSpec::new("sketch.edit", "Edit Sketch", crate::checked!("sketch.edit", edit_sketch))
.noundo().icon("sketch")
.contract(&EDIT_CONTRACT).output(output_schema_for::<EditResult>)
.params("sketch: id or name; restore_marker?: nonnegative timeline position or null (end). When provided, temporarily roll to this sketch and restore that position on Finish or Cancel; Finish groups the edit into one Undo step. Omit to leave the timeline in place"),
CommandSpec::new("sketch.finish", "Finish Sketch", crate::checked!("sketch.finish", finish_sketch))
.at("SKETCH", "FINISH SKETCH").icon("finish").enabled(in_sketch).noundo()
.contract(&EXIT_CONTRACT).output(output_schema_for::<FinishResult>)
.params("restore_marker?: bool (default true); false transfers the temporary marker directly to another editor without rebuilding the intervening timeline"),
CommandSpec::new("sketch.cancel", "Cancel Sketch", crate::checked!("sketch.cancel", cancel_sketch))
.at("SKETCH", "FINISH SKETCH")
.icon("sketch_cancel")
.enabled(in_sketch)
.params("Leaves the sketch without keeping what changed since it was created or opened for editing: the design goes back to how it was then (a new sketch is removed), as one step that Undo brings back. Refused when the undo history no longer leads back to that point (use Undo instead)"),
.contract(&EXIT_CONTRACT).output(output_schema_for::<CancelResult>)
.params("Leaves the sketch without keeping what changed since it was created or opened for editing: the design goes back to how it was then (a new sketch is removed), as one step that Undo brings back. Refused when the undo history no longer leads back to that point (use Undo instead). restore_marker?: bool (default true); false transfers the temporary marker to another editor"),
CommandSpec::new("sketch.line", "Line", draw_line)
.at("SKETCH", "CREATE")
.icon("line")
@@ -946,6 +958,16 @@ pub(super) fn create_sketch_with_plane(s: &mut Session, p: &Value, plane: PlaneR
}
fn edit_sketch(s: &mut Session, p: &Value) -> Result<Value> {
let restore_marker = p
.get("restore_marker")
.map(|v| {
serde_json::from_value::<Option<usize>>(v.clone())
.map_err(|_| bad("sketch.edit", "`restore_marker` must be a nonnegative timeline position or null"))
})
.transpose()?;
if restore_marker.flatten().is_some_and(|m| m > s.doc.features.len()) {
return Err(bad("sketch.edit", "`restore_marker` is past the end of the timeline"));
}
let key = match p.get("sketch") {
Some(Value::Number(n)) => n.to_string(),
Some(Value::String(x)) => x.clone(),
@@ -967,7 +989,16 @@ fn edit_sketch(s: &mut Session, p: &Value) -> Result<Value> {
s.active_occurrence_path = (!path.is_empty()).then_some(path);
}
if s.active_sketch != Some(id) || s.sketch_entry.as_ref().is_none_or(|entry| entry.sketch != id) {
s.sketch_entry = Some(SketchEntry { sketch: id, ..entry_point(s) });
s.sketch_entry = Some(SketchEntry { sketch: id, restore_marker, ..entry_point(s) });
if restore_marker.is_some()
&& let Some(index) = s.doc.feature_index(id)
{
let marker = (index + 1 < s.doc.features.len()).then_some(index + 1);
if s.doc.marker != marker {
s.doc_mut().marker = marker;
}
s.selection.clear();
}
}
s.active_sketch = Some(id);
s.revision += 1;
@@ -976,15 +1007,32 @@ fn edit_sketch(s: &mut Session, p: &Value) -> Result<Value> {
/// The design and undo history as they are now, where a sketch is entered (its id set after).
fn entry_point(s: &Session) -> SketchEntry {
SketchEntry { sketch: 0, doc: s.doc.clone(), undo_len: s.undo.len(), undo_top: s.undo.last().map(|snap| snap.doc.clone()) }
SketchEntry {
sketch: 0,
doc: s.doc.clone(),
undo_len: s.undo.len(),
undo_top: s.undo.last().map(|snap| snap.doc.clone()),
restore_marker: None,
pending_moves: s.pending_moves.clone(),
visibility: s.visibility.clone(),
}
}
fn restore_on_exit(cmd: &str, p: &Value) -> Result<bool> {
match p.get("restore_marker") {
None => Ok(true),
Some(Value::Bool(restore)) => Ok(*restore),
_ => Err(bad(cmd, "`restore_marker` must be a boolean")),
}
}
/// Cancel Sketch (#3838): the design as it was when the open sketch was entered. The entry is
/// used only while the undo history still runs through it: the same steps before it, and every
/// step since taken in this sketch. Otherwise the steps since can't be told apart from others,
/// and Undo is the way back.
fn cancel_sketch(s: &mut Session, _p: &Value) -> Result<Value> {
fn cancel_sketch(s: &mut Session, p: &Value) -> Result<Value> {
let cmd = "sketch.cancel";
let restore = restore_on_exit(cmd, p)?;
let id = sketch_store::active(s).ok_or_else(|| bad(cmd, "no sketch is being edited"))?;
let entry = s.sketch_entry.clone().filter(|entry| entry.sketch == id);
let current = entry.filter(|entry| {
@@ -1004,23 +1052,84 @@ fn cancel_sketch(s: &mut Session, _p: &Value) -> Result<Value> {
return Err(bad(cmd, "the undo history no longer leads back to where this sketch was opened: use Undo to go back"));
};
let created = entry.doc.feature(id).is_none();
let marker = if restore { entry.restore_marker.unwrap_or(entry.doc.marker) } else { s.doc.marker };
// As Undo does: the session evaluates the design it is given.
// Undo of Cancel reopens this edit, so retain its return position for a later Finish.
s.sketch_entry = entry.restore_marker.map(|_| entry.clone());
s.doc = entry.doc;
let marker = marker.filter(|m| *m < s.doc.features.len());
if s.doc.marker != marker {
s.doc_mut().marker = marker;
}
s.active_sketch = None;
s.sketch_entry = None;
s.selection.clear();
s.revision += 1;
Ok(json!({"sketch": id, "removed": created}))
}
fn finish_sketch(s: &mut Session, _p: &Value) -> Result<Value> {
fn finish_sketch(s: &mut Session, p: &Value) -> Result<Value> {
let restore = restore_on_exit("sketch.finish", p)?;
let id = s.active_sketch.take();
s.sketch_entry = None;
s.revision += 1;
let profiles = id.and_then(|i| s.model.state().sketch(i).map(|ss| ss.profiles.len())).unwrap_or(0);
if let Some(entry) = s.sketch_entry.take().filter(|entry| Some(entry.sketch) == id)
&& let Some(marker) = entry.restore_marker
{
finish_edit_history(s, &entry, marker);
if restore {
let marker = marker.filter(|m| *m < s.doc.features.len());
if s.doc.marker != marker {
s.doc_mut().marker = marker;
}
}
s.selection.clear();
}
s.revision += 1;
Ok(json!({"sketch": id, "profiles": profiles}))
}
/// Fold only the steps still belonging to this edit. Its snapshots describe the complete
/// design, never the temporary marker. The anchor may have moved when the undo limit trimmed
/// older steps; if it is gone, only a stack entirely from this sketch can be folded safely.
fn finish_edit_history(s: &mut Session, entry: &SketchEntry, marker: Option<usize>) {
use std::sync::Arc;
let start = match &entry.undo_top {
Some(top) => s.undo.iter().rposition(|snap| Arc::ptr_eq(&snap.doc, top)).map(|i| i + 1),
None => Some(0),
}
.or_else(|| (s.undo.len() == crate::MAX_UNDO && s.undo.iter().all(|snap| snap.active_sketch == Some(entry.sketch))).then_some(0));
let Some(start) =
start.filter(|&start| s.undo.get(start..).is_some_and(|steps| steps.iter().all(|snap| snap.active_sketch == Some(entry.sketch))))
else {
return;
};
let mut before = (*entry.doc).clone();
before.marker = marker.filter(|m| *m < before.features.len());
let mut after = (*s.doc).clone();
after.marker = marker.filter(|m| *m < after.features.len());
let changed = before != after || entry.pending_moves != s.pending_moves || entry.visibility != s.visibility;
s.undo.truncate(start);
if changed {
s.undo.push(Snapshot {
label: "Edit Sketch".into(),
doc: Arc::new(before),
active_sketch: None,
pending_moves: entry.pending_moves.clone(),
visibility: entry.visibility.clone(),
});
if s.undo.len() > crate::MAX_UNDO {
s.undo.remove(0);
}
}
// Redo of a step undone while editing must not reopen a finished temporary rollback.
for snap in &mut s.redo {
if snap.active_sketch == Some(entry.sketch) {
let doc = Arc::make_mut(&mut snap.doc);
doc.marker = marker.filter(|m| *m < doc.features.len());
snap.active_sketch = None;
}
}
}
// ---------------------------------------------------------------------------------------------
// Drawing
+49
View File
@@ -1800,6 +1800,55 @@ fn budget_limited_cursor_progress_requests_a_frame_with_the_same_target() {
// ---------------------------------------------------------------------------------------------
// Cancel Sketch (#3838)
fn sketch_below_a_solid() -> (Session, u64) {
let mut s = Session::default();
let id = run(&mut s, "sketch.create", json!({"plane": "XY"}))["sketch"].as_u64().unwrap();
run(&mut s, "sketch.rectangle.two_point", json!({"p0": [0, 0], "p1": [20, 10]}));
run(&mut s, "sketch.finish", json!({}));
run(&mut s, "solid.extrude", json!({"distance": 5}));
(s, id)
}
#[test]
fn finishing_a_temporary_sketch_edit_at_the_undo_limit_keeps_the_original_design() {
let (mut s, id) = sketch_below_a_solid();
for i in 0..crate::MAX_UNDO {
run(&mut s, "browser.visibility", json!({"items": ["Body1"], "visible": i % 2 != 0}));
}
assert_eq!(s.undo.len(), crate::MAX_UNDO);
let before = s.doc.to_json();
run(&mut s, "sketch.edit", json!({"sketch": id, "restore_marker": null}));
for y in [20, 30] {
run(&mut s, "sketch.line", json!({"points": [[0, y], [10, y]]}));
}
run(&mut s, "sketch.finish", json!({}));
assert!(s.undo.len() <= crate::MAX_UNDO);
run(&mut s, "edit.undo", json!({}));
assert_eq!(s.doc.to_json(), before);
assert_eq!(s.active_sketch, None);
assert_eq!(s.model.state().bodies.len(), 1);
run(&mut s, "edit.redo", json!({}));
assert_eq!(s.doc.marker, None);
assert_eq!(s.doc.sketch(id).unwrap().curves.len(), 6);
}
#[test]
fn undoing_cancel_then_finishing_restores_the_temporary_sketch_marker() {
let (mut s, id) = sketch_below_a_solid();
run(&mut s, "sketch.edit", json!({"sketch": id, "restore_marker": null}));
run(&mut s, "sketch.line", json!({"points": [[0, 20], [10, 20]]}));
run(&mut s, "sketch.cancel", json!({}));
assert_eq!(s.doc.marker, None);
assert_eq!(s.doc.sketch(id).unwrap().curves.len(), 4);
run(&mut s, "edit.undo", json!({}));
assert_eq!(s.active_sketch, Some(id));
run(&mut s, "sketch.finish", json!({}));
assert_eq!(s.doc.marker, None);
assert_eq!(s.active_sketch, None);
assert_eq!(s.doc.sketch(id).unwrap().curves.len(), 5);
assert_eq!(s.model.state().bodies.len(), 1);
}
#[test]
fn cancel_removes_a_new_sketch_with_its_dimensions_and_undo_brings_it_back() {
let mut s = Session::default();
+11
View File
@@ -297,6 +297,10 @@ pub(crate) struct SketchEntry {
pub(crate) doc: Arc<Document>,
pub(crate) undo_len: usize,
pub(crate) undo_top: Option<Arc<Document>>,
/// A temporary editor rollback, restored by the engine even without a window.
pub(crate) restore_marker: Option<Option<usize>>,
pub(crate) pending_moves: std::collections::BTreeMap<u64, solidcraft_doc::Mat>,
pub(crate) visibility: Visibility,
}
/// A copy of a session that evaluates commands ahead of it ([`Session::ahead`]).
@@ -931,6 +935,12 @@ impl Session {
}
}
/// The marker an active temporary sketch edit returns to. `Some(None)` means the end of
/// the timeline; `None` means this sketch was opened without a temporary rollback.
pub fn sketch_restore_marker(&self) -> Option<Option<usize>> {
self.sketch_entry.as_ref().filter(|entry| Some(entry.sketch) == self.active_sketch).and_then(|entry| entry.restore_marker)
}
/// Mutable document (copy-on-write).
pub fn doc_mut(&mut self) -> &mut Document {
self.forget_document_caches();
@@ -1999,6 +2009,7 @@ impl Session {
let mut copy = self.scratch();
copy.undo.clone_from(&self.undo);
copy.redo.clone_from(&self.redo);
copy.sketch_entry.clone_from(&self.sketch_entry);
copy.model.keep(Some(Arc::default()));
Ahead(copy)
}
+1
View File
@@ -68,6 +68,7 @@ pub mod render_lights;
pub mod render_ui;
pub mod section;
pub mod sheet;
pub mod sketch;
pub mod sketch_constraints;
pub mod sketch_create;
pub mod sketch_equation;
+51
View File
@@ -0,0 +1,51 @@
//! Contracts and result types for sketch editing and completion in `cmd/sketch.rs`.
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
use super::types::{IdOrName, present};
#[derive(Debug, Deserialize, JsonSchema)]
#[schemars(deny_unknown_fields)]
pub struct EditParams {
pub sketch: IdOrName,
/// When present, temporarily roll to this sketch and restore this position when the edit
/// ends. Null means the end of the timeline; omission leaves the timeline in place.
#[serde(default, deserialize_with = "present", skip_serializing_if = "Option::is_none")]
#[schemars(with = "Option<usize>")]
pub restore_marker: Option<Option<usize>>,
}
#[derive(Debug, Deserialize, JsonSchema)]
#[schemars(deny_unknown_fields)]
pub struct ExitParams {
/// Restore the temporary editor marker (default true). False transfers it directly to
/// another editor without rebuilding the intermediate timeline.
#[serde(default, deserialize_with = "present", skip_serializing_if = "Option::is_none")]
#[schemars(with = "bool")]
pub restore_marker: Option<bool>,
}
#[derive(Debug, Serialize, JsonSchema)]
pub struct EditResult {
pub sketch: u64,
pub name: String,
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub param_warnings: Vec<String>,
}
#[derive(Debug, Serialize, JsonSchema)]
pub struct FinishResult {
pub sketch: Option<u64>,
pub profiles: usize,
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub param_warnings: Vec<String>,
}
#[derive(Debug, Serialize, JsonSchema)]
pub struct CancelResult {
pub sketch: u64,
pub removed: bool,
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub param_warnings: Vec<String>,
}
+35
View File
@@ -1130,3 +1130,38 @@ fn plane_names_match_check_plane_name() {
}
assert!(parse::<PlaneName>(json!(5)).is_none());
}
#[test]
fn sketch_edit_contracts_validate_markers_before_changing_the_session() {
use crate::schema::sketch::{CancelResult, EditParams, EditResult, ExitParams, FinishResult};
use crate::schema::{ParamCheck, input_schema_for, output_schema_for};
assert_keys(input_schema_for::<EditParams>(), &["restore_marker", "sketch"]);
assert_keys(input_schema_for::<ExitParams>(), &["restore_marker"]);
let mut s = crate::Session::default();
let run = |s: &mut crate::Session, id: &str, p: serde_json::Value| s.execute(id, &p).unwrap();
let id = run(&mut s, "sketch.create", json!({"plane": "XY"}))["sketch"].as_u64().unwrap();
run(&mut s, "sketch.rectangle.two_point", json!({"p0": [0, 0], "p1": [20, 10]}));
run(&mut s, "sketch.finish", json!({}));
run(&mut s, "solid.extrude", json!({"distance": 5}));
s.param_check = ParamCheck::Strict;
let before = s.doc.to_json();
for marker in [json!(-1), json!(3), json!(1.5), json!(true), json!([])] {
assert!(s.execute("sketch.edit", &json!({"sketch": id, "restore_marker": marker})).is_err());
assert_eq!(s.doc.to_json(), before);
assert_eq!(s.active_sketch, None);
}
for exit in ["sketch.finish", "sketch.cancel"] {
let r = run(&mut s, "sketch.edit", json!({"sketch": id, "restore_marker": null}));
let schema = output_schema_for::<EditResult>();
assert!(conforms(schema.as_value(), schema.as_value(), &r));
for params in [json!({"restore_marker": "yes"}), json!({"unknown": true})] {
assert!(s.execute(exit, &params).is_err());
assert_eq!(s.active_sketch, Some(id));
assert_eq!(s.doc.marker, Some(1));
}
let r = run(&mut s, exit, json!({}));
let schema = if exit == "sketch.finish" { output_schema_for::<FinishResult>() } else { output_schema_for::<CancelResult>() };
assert!(conforms(schema.as_value(), schema.as_value(), &r));
assert_eq!(s.doc.to_json(), before);
}
}
-3
View File
@@ -237,7 +237,6 @@ sketch.auto_project
sketch.blend_curve
sketch.break
sketch.break_link
sketch.cancel
sketch.centerline
sketch.chain
sketch.chamfer.distance_angle
@@ -273,13 +272,11 @@ sketch.dimension_text
sketch.drag
sketch.drag_end
sketch.draw.dimensioned
sketch.edit
sketch.edit_text
sketch.ellipse
sketch.export_dxf
sketch.extend
sketch.fillet
sketch.finish
sketch.finish_auto_constrain
sketch.fit_curves_to_section
sketch.glyphs
+13
View File
@@ -133,6 +133,19 @@ fn replaced_by_a_later_one(id: &str) -> bool {
}
impl SolveApp {
/// A direct exit or editor switch supersedes any queued sketch exit. A late worker must
/// never finish a different sketch that was opened while it computed.
pub(crate) fn take_sketch_exit(&mut self) -> Option<String> {
let is_exit = |id: &str| matches!(id, "sketch.finish" | "sketch.cancel");
let pending = self.deferred.as_ref().filter(|job| is_exit(&job.command.0)).map(|job| job.command.0.clone());
if pending.is_some() {
self.deferred = None;
}
let queued = self.deferred_queue.iter().find(|(id, _)| is_exit(id)).map(|(id, _)| id.clone());
self.deferred_queue.retain(|(id, _)| !is_exit(id));
pending.or(queued)
}
/// Run a command that may recompute much of the history, off the window thread when it takes
/// a while (see the module notes). `Ok(Null)` when it is left computing or waiting its turn:
/// its outcome then shows as any command's does (an error in the status line).
-3
View File
@@ -41,7 +41,6 @@ pub struct DocSlot {
pub(crate) autosave: Option<solidcraft_engine::recovery::Autosaver>,
pub(crate) big_operation: bool,
cam: Camera,
pre_sketch_marker: Option<Option<usize>>,
pub(crate) pending_feature_edit: Option<crate::feature_edit::PendingFeatureEdit>,
pre_sketch_cam: Option<crate::PreSketchView>,
pre_sketch_component: Option<crate::PreSketchComponent>,
@@ -135,7 +134,6 @@ impl DocSlot {
autosave: None,
big_operation: false,
cam: Camera::default(),
pre_sketch_marker: None,
pending_feature_edit: None,
pre_sketch_cam: None,
pre_sketch_component: None,
@@ -241,7 +239,6 @@ fn swap_in(app: &mut SolveApp, mut slot: DocSlot) -> DocSlot {
autosave: app.autosave.as_mut().and_then(|a| std::mem::replace(&mut a.active, slot.autosave)),
big_operation: std::mem::replace(&mut app.big_operation, slot.big_operation),
cam: std::mem::replace(&mut app.cam, slot.cam),
pre_sketch_marker: std::mem::replace(&mut app.pre_sketch_marker, slot.pre_sketch_marker),
pending_feature_edit: std::mem::replace(&mut app.pending_feature_edit, slot.pending_feature_edit),
pre_sketch_cam: std::mem::replace(&mut app.pre_sketch_cam, slot.pre_sketch_cam),
pre_sketch_component: std::mem::replace(&mut app.pre_sketch_component, slot.pre_sketch_component),
@@ -316,6 +316,47 @@ fn switching_sketches_keeps_the_original_marker_with_background_rebuilds() {
}
}
#[test]
fn finishing_a_sketch_in_the_background_restores_the_design_and_its_undo_snapshot() {
let (mut h, sketches) = sketches_with_later_bodies();
let before = h.app.session.doc.to_json();
h.app.edit_sketch(sketches[0]);
h.app.run("sketch.line", json!({"points": [[0, 2], [10, 2]]})).unwrap();
h.app.finish_sketch();
assert!(h.app.deferred.is_some());
until_settled(&mut h);
assert_eq!(marker(&h), None);
assert_eq!(h.app.session.active_sketch, None);
h.app.run("edit.undo", json!({})).unwrap();
assert_eq!(h.app.session.doc.to_json(), before);
assert_eq!(h.app.session.active_sketch, None);
}
#[test]
fn a_pending_sketch_exit_cannot_close_the_next_editor() {
for cancel in [false, true] {
let (mut h, sketches) = sketches_with_later_bodies();
let curves = h.app.session.doc.sketch(sketches[0]).unwrap().curves.len();
h.app.edit_sketch(sketches[0]);
h.app.run("sketch.line", json!({"points": [[0, 2], [10, 2]]})).unwrap();
if cancel {
h.app.cancel_sketch();
} else {
h.app.finish_sketch();
}
assert!(h.app.deferred.is_some());
h.app.edit_sketch(sketches[1]);
until_settled(&mut h);
assert_eq!(h.app.session.active_sketch, Some(sketches[1]));
assert_eq!(marker(&h), Some(3));
assert_eq!(h.app.session.doc.sketch(sketches[0]).unwrap().curves.len(), curves + usize::from(!cancel));
h.app.finish_sketch();
until_settled(&mut h);
assert_eq!(h.app.session.active_sketch, None);
assert_eq!(marker(&h), None);
}
}
#[test]
fn switching_from_a_sketch_to_a_feature_keeps_the_original_marker_with_background_rebuilds() {
let (mut h, sketches) = sketches_with_later_bodies();
+27 -26
View File
@@ -521,9 +521,6 @@ pub struct SolveApp {
pub cam_anim: Option<CameraAnim>,
/// Seconds since start, as of this frame.
pub now: f64,
/// The history marker before Edit Sketch rolled the timeline to the sketch; Finish Sketch
/// restores it.
pub pre_sketch_marker: Option<Option<usize>>,
/// The views around opening the open sketch. Leaving it goes back to the earlier one as the
/// "When a sketch ends" preference says (#7038).
pub pre_sketch_cam: Option<PreSketchView>,
@@ -790,7 +787,6 @@ impl SolveApp {
cam: Camera::default(),
cam_anim: None,
now: 0.0,
pre_sketch_marker: None,
pre_sketch_cam: None,
pre_extrude_cam: None,
pre_sketch_component: None,
@@ -1229,6 +1225,7 @@ impl SolveApp {
};
if let Ok(v) = &r {
match id {
"sketch.finish" | "sketch.cancel" => self.sketch_closed(v.get("sketch").and_then(Value::as_u64)),
"view.fit" | "view.home" => self.frame_view(id == "view.home"),
"view.tool" => {
self.viewport.nav = match v.get("tool").and_then(Value::as_str) {
@@ -2152,22 +2149,19 @@ impl SolveApp {
if self.session.active_sketch == Some(id) {
return;
}
let Some(idx) = self.session.doc.feature_index(id) else { return };
if self.session.doc.feature_index(id).is_none() {
return;
}
let marker = self.finish_sketch_for_edit();
let before = self.settled_cam();
let context = (self.session.active_component, self.session.active_occurrence_path.clone());
self.tool = None;
self.dialog = None;
activate(self);
let rolled = self.session.doc.marker != Some(idx + 1) && self.run("timeline.roll_to", json!({ "position": idx + 1 })).is_ok();
if self.run("sketch.edit", json!({ "sketch": id })).is_err() {
if rolled || marker != self.session.doc.marker {
self.restore_marker(marker);
}
if self.run("sketch.edit", json!({ "sketch": id, "restore_marker": marker })).is_err() {
self.restore_component(context.0, context.1);
return;
}
self.pre_sketch_marker = (rolled || marker != self.session.doc.marker).then_some(marker);
// `sketch.edit` itself activates the component a face-attached sketch was drawn in.
let opened_in = self.session.active_component;
let moved = (opened_in, &self.session.active_occurrence_path) != (context.0, &context.1);
@@ -2236,9 +2230,11 @@ impl SolveApp {
.pending_feature_edit
.take()
.map(|pending| pending.marker)
.unwrap_or_else(|| self.pre_sketch_marker.take().unwrap_or(self.session.doc.marker));
.unwrap_or_else(|| self.session.sketch_restore_marker().unwrap_or(self.session.doc.marker));
if self.session.active_sketch.is_some() {
self.finish_sketch();
let command = self.take_sketch_exit().unwrap_or_else(|| "sketch.finish".into());
self.prepare_sketch_exit();
let _ = self.run(&command, json!({"restore_marker": false}));
}
marker
}
@@ -2257,19 +2253,30 @@ impl SolveApp {
}
fn leave_sketch(&mut self, command: &str) {
self.prepare_sketch_exit();
if self.session.sketch_restore_marker().is_some() {
// Finishing now restores the marker in the engine: evaluate the complete
// command ahead so rebuilding later features stays off the window thread.
let _ = self.run_heavy(command, json!({}));
} else {
let _ = self.run(command, json!({}));
}
}
fn prepare_sketch_exit(&mut self) {
sketch_text_move::cancel_drag(self);
if sketch_text::editing(self) || self.dialog.as_ref().is_some_and(|d| matches!(d.kind, dialogs::Kind::TextMove(_))) {
dialogs::cancel(self);
}
dim_view::cancel();
self.tool = None;
let open = self.session.active_sketch;
if self.run(command, json!({})).is_err() && command != "sketch.finish" {
return;
}
if let Some(marker) = self.pre_sketch_marker.take() {
self.restore_marker(marker);
}
}
/// Successful UI and programmatic exits share the same window cleanup.
fn sketch_closed(&mut self, open: Option<u64>) {
self.take_sketch_exit();
self.tool = None;
dim_view::cancel();
// Unless another component was activated meanwhile: that one stays.
if let Some(pre) = self.pre_sketch_component.take().filter(|pre| Some(pre.sketch) == open && pre.opened_in == self.session.active_component) {
self.restore_component(pre.component, pre.path);
@@ -2282,12 +2289,6 @@ impl SolveApp {
}
}
fn restore_marker(&mut self, marker: Option<usize>) {
// Rolling forward past the features after the sketch recomputes them (an edited sketch
// under a text cut): off the window thread when it takes a while.
let _ = self.run_heavy("timeline.roll_to", marker.map(|m| json!({ "position": m })).unwrap_or_else(|| json!({})));
}
/// Visibility in the editor's scene: hidden face-profile sources are temporarily shown
/// without changing the session's visibility or either history stack.
pub(crate) fn body_visible(&self, name: &str) -> bool {
+83 -2
View File
@@ -8,6 +8,86 @@ fn first_sketch(app: &SolveApp) -> u64 {
app.session.doc.features.iter().find(|f| matches!(f.kind, FeatureKind::Sketch { .. })).map(|f| f.id).unwrap()
}
fn plate_and_cylinder() -> crate::scenario::Harness {
use serde_json::json;
let mut h = crate::scenario::Harness::new();
h.app.home.open = false;
for (command, params) in [
("sketch.create", json!({"plane": "XY", "name": "Sketch1"})),
("sketch.rectangle.two_point", json!({"p0": [0, 0], "p1": [40, 30]})),
("sketch.finish", json!({})),
("solid.extrude", json!({"name": "Plate", "distance": 10})),
("solid.cylinder", json!({"name": "Cyl", "base": [60, 15, 0], "radius": 8, "height": 20, "operation": "new"})),
] {
h.app.run(command, params).unwrap();
}
h
}
#[test]
fn finishing_an_edited_sketch_undoes_the_edit_with_the_complete_design_visible() {
use serde_json::json;
for engine_finish in [false, true] {
let mut h = plate_and_cylinder();
let before = h.app.session.doc.to_json();
let id = first_sketch(&h.app);
h.app.edit_sketch(id);
h.app.run("sketch.rectangle.two_point", json!({"p0": [10, 10], "p1": [20, 20]})).unwrap();
if engine_finish {
h.app.run("sketch.finish", json!({})).unwrap();
} else {
h.app.finish_sketch();
}
assert_eq!(h.app.session.doc.marker, None);
let edited = h.app.session.doc.to_json();
h.app.run("edit.undo", json!({})).unwrap();
assert_eq!(h.app.session.doc.marker, None, "Undo must not expose the temporary sketch rollback");
assert_eq!(h.app.session.active_sketch, None);
assert_eq!(h.app.session.doc.to_json(), before, "one Undo takes back the sketch edit");
assert_eq!(h.app.session.model.state().bodies.len(), 2);
let volume = h.app.run("inspect.measure", json!({"bodies": ["Body1"]})).unwrap();
assert!((volume["total"]["volume_mm3"].as_f64().unwrap() - 12000.0).abs() < 1e-6);
h.app.run("edit.redo", json!({})).unwrap();
assert_eq!(h.app.session.doc.to_json(), edited);
assert_eq!(h.app.session.active_sketch, None);
assert_eq!(h.app.session.model.state().bodies.len(), 2);
}
}
#[test]
fn engine_finish_restores_the_marker_from_a_ui_sketch_edit() {
use serde_json::json;
for marker in [None, Some(0), Some(2)] {
let mut h = plate_and_cylinder();
h.app.run("timeline.roll_to", json!({"position": marker})).unwrap();
let before = h.app.session.doc.to_json();
let bodies = h.app.session.model.state().bodies.len();
let id = first_sketch(&h.app);
h.app.edit_sketch(id);
// Deliberately bypass the window's completion callback: the engine owns restoration.
h.app.session.execute("sketch.finish", &json!({})).unwrap();
assert_eq!(h.app.session.active_sketch, None);
assert_eq!(h.app.session.doc.to_json(), before);
assert_eq!(h.app.session.model.state().bodies.len(), bodies);
}
}
#[test]
fn unchanged_sketch_edit_keeps_undo_and_redo() {
use serde_json::json;
let mut h = plate_and_cylinder();
h.app.run("edit.undo", json!({})).unwrap();
let before = h.app.session.doc.to_json();
let history = (h.app.session.undo.len(), h.app.session.redo.len());
let id = first_sketch(&h.app);
h.app.edit_sketch(id);
h.app.finish_sketch();
assert_eq!(h.app.session.doc.to_json(), before);
assert_eq!((h.app.session.undo.len(), h.app.session.redo.len()), history);
h.app.run("edit.redo", json!({})).unwrap();
assert_eq!(h.app.session.model.state().bodies.len(), 2);
}
#[test]
fn edit_sketch_rolls_back_and_finish_restores() {
let mut app = sample_app();
@@ -456,9 +536,10 @@ mod exit_view_preference {
#[test]
fn a_sketch_opened_by_a_command_leaves_the_view_alone() {
let (mut h, _) = sketching(SketchExitView::Previous);
// Finished by the command, so the views saved for this sketch stay behind.
// A programmatic finish clears this sketch's saved view too. A later sketch opened
// entirely by commands must not inherit that view.
h.app.run("sketch.finish", json!({})).unwrap();
assert!(h.app.pre_sketch_cam.is_some());
assert!(h.app.pre_sketch_cam.is_none());
h.app.run("sketch.create", json!({"plane": "XZ"})).unwrap();
h.app.cam.orbit(60.0, -40.0);
h.app.cam_anim = None;
+2 -1
View File
@@ -336,7 +336,7 @@ fn a_feature_started_while_the_marker_rolls_back_waits_for_it() {
h.app.edit_feature(sketch);
h.app.apply_off_thread = Some(std::time::Duration::ZERO);
h.app.start("solid.extrude");
assert!(h.app.session.active_sketch.is_none());
assert_eq!(h.app.session.active_sketch, Some(sketch), "Finish and the marker restore commit together after evaluation");
assert!(h.app.deferred.is_some(), "the marker is not rolling back off the window thread");
assert!(h.app.dialog.is_none(), "Extrude opened before the marker was back");
for _ in 0..500 {
@@ -347,6 +347,7 @@ fn a_feature_started_while_the_marker_rolls_back_waits_for_it() {
std::thread::sleep(std::time::Duration::from_millis(10));
}
assert_eq!(h.app.session.doc.marker, None);
assert_eq!(h.app.session.active_sketch, None);
let items = h.app.dialog.as_ref().and_then(|d| d.inputs.first()).map(|i| i.items.clone()).unwrap_or_default();
assert_eq!(items, vec![Sel::Profile { sketch, index: 0 }], "Extrude did not open once the marker was back");
}