From 457fc2319ecb10fc707adba305945ae9dc1c2eba Mon Sep 17 00:00:00 2001 From: Tanish Manku Date: Tue, 3 Nov 2026 15:14:10 -0700 Subject: [PATCH] View twist: review fixes - Every engine twist (DVIEW, view.set, opening a drawing, the untiled view) goes through norm_angle, so a tiny negative twist is 0, not a full turn. - VIEW Window in a twisted view stores the window's size along the view's axes (View::twisted_box), so a restore shows all of it. - A deferred tangent or perpendicular snap on an arc in a twisted layout viewport turns the arc's angles with its centre. - The active viewport's cached drawing is rebuilt when its twist changes; moving and scaling it can't turn it. --- crates/engine/src/cmd/view.rs | 2 +- crates/engine/src/cmd/view/dview.rs | 3 +-- crates/engine/src/cmd/view/named_views.rs | 6 ++++-- crates/engine/src/doc_state.rs | 2 +- crates/engine/src/session/files.rs | 4 +--- crates/engine/src/snap/viewport.rs | 4 +++- crates/ui-egui/src/canvas/viewport.rs | 5 ++++- 7 files changed, 15 insertions(+), 11 deletions(-) diff --git a/crates/engine/src/cmd/view.rs b/crates/engine/src/cmd/view.rs index 29581f79e..7161308df 100644 --- a/crates/engine/src/cmd/view.rs +++ b/crates/engine/src/cmd/view.rs @@ -293,7 +293,7 @@ fn run_view_set(s: &mut Session, p: &Value) -> Result { dview::set_twist(s, t, twist)?; s.state()?.view().twist } - Some(dview::Target::Model) => twist.rem_euclid(std::f64::consts::TAU), + Some(dview::Target::Model) => cadcraft_geom::norm_angle(twist), None => 0.0, }; if !s.state_mut()?.set_view(View { center: c, height: h, twist }) { diff --git a/crates/engine/src/cmd/view/dview.rs b/crates/engine/src/cmd/view/dview.rs index 5c1b1e1c5..9a421c8a9 100644 --- a/crates/engine/src/cmd/view/dview.rs +++ b/crates/engine/src/cmd/view/dview.rs @@ -13,7 +13,6 @@ //! ZOOM (view history, not undo). A layout viewport's twist belongs to the viewport entity, so a //! DVIEW that changed it is one undo step. -use std::f64::consts::TAU; use std::sync::Arc; use cadcraft_doc::{Entity, EntityKind, Handle, Insert, Line, Space}; @@ -69,7 +68,7 @@ pub(super) fn target(s: &Session) -> Result { /// A twist kept as files hold it: [0, 2π). fn normal(twist: f64) -> f64 { - twist.rem_euclid(TAU) + cadcraft_geom::norm_angle(twist) } /// The active viewport's twist, if it is `h`. diff --git a/crates/engine/src/cmd/view/named_views.rs b/crates/engine/src/cmd/view/named_views.rs index 34908fe23..e7c055fe2 100644 --- a/crates/engine/src/cmd/view/named_views.rs +++ b/crates/engine/src/cmd/view/named_views.rs @@ -119,7 +119,9 @@ fn save(s: &mut Session, name: &str, window: Option, snapshot: bool) -> if !(b.width() > 1e-12 && b.height() > 1e-12 && b.min.is_finite() && b.max.is_finite()) { return Err(EngineError::Other("The window has no area.".into())); } - (b.center(), b.height(), b.width()) + // In a twisted view the window's box on screen is larger than its world box. + let (center, width, height) = crate::View::twisted_box(b, twist); + (center, height, width) } None => { let f = frame; @@ -184,7 +186,7 @@ fn restore(s: &mut Session, name: &str) -> Result { // In an unlocked MSPACE viewport the saved twist turns the viewport (an edit of the // viewport, recorded as the command's undo step). let aspect = st.zoom_aspect(w / h.max(1.0)).max(1e-9); - let twist = if v.twist.is_finite() { v.twist.rem_euclid(std::f64::consts::TAU) } else { 0.0 }; + let twist = cadcraft_geom::norm_angle(v.twist); st.set_zoom_frame(View { center: v.center, height: v.height.max(v.width / aspect), twist }); } } diff --git a/crates/engine/src/doc_state.rs b/crates/engine/src/doc_state.rs index 411e160d2..f3862f90a 100644 --- a/crates/engine/src/doc_state.rs +++ b/crates/engine/src/doc_state.rs @@ -203,7 +203,7 @@ impl DocState { } fn keep_untiled_twist(&mut self, v: View) { use cadcraft_doc::{VportConfig, VportTile}; - let twist = if v.twist.is_finite() { v.twist.rem_euclid(std::f64::consts::TAU) } else { 0.0 }; + let twist = cadcraft_geom::norm_angle(v.twist); let active = self.doc.vports.iter().find(|c| c.is_active()); if active.is_some_and(|c| c.tiles.len() > 1) { return; diff --git a/crates/engine/src/session/files.rs b/crates/engine/src/session/files.rs index 961dabea4..6704b2674 100644 --- a/crates/engine/src/session/files.rs +++ b/crates/engine/src/session/files.rs @@ -19,9 +19,7 @@ impl Session { let mut st = DocState::new(d, title, path); let ext = st.doc.extents(&Space::Model); // The current tile (the first), or the lone untiled view, keeps its saved twist. - let twist = - st.doc.vports.iter().find(|c| c.is_active()).and_then(|c| c.tiles.first()).map_or(0.0, |t| t.twist).rem_euclid(std::f64::consts::TAU); - let twist = if twist.is_finite() { twist } else { 0.0 }; + let twist = cadcraft_geom::norm_angle(st.doc.vports.iter().find(|c| c.is_active()).and_then(|c| c.tiles.first()).map_or(0.0, |t| t.twist)); if ext.is_empty() { st.set_view_quiet(View { twist, ..st.view() }); } else { diff --git a/crates/engine/src/snap/viewport.rs b/crates/engine/src/snap/viewport.rs index 20decf8d4..c6ef9963c 100644 --- a/crates/engine/src/snap/viewport.rs +++ b/crates/engine/src/snap/viewport.rs @@ -77,7 +77,9 @@ impl<'a> Projection<'a> { if !(center.is_finite() && radius.is_finite() && radius > 0.0) { return None; } - SnapCurve::Arc { center, radius, start, end } + // The arc turns on the sheet with the viewport's twist. + let twist = self.viewport.view_twist(); + SnapCurve::Arc { center, radius, start: start + twist, end: end + twist } } SnapCurve::Line { a, b } => { let (a, b) = (self.to_paper(a), self.to_paper(b)); diff --git a/crates/ui-egui/src/canvas/viewport.rs b/crates/ui-egui/src/canvas/viewport.rs index 6eafc6a33..db1f42642 100644 --- a/crates/ui-egui/src/canvas/viewport.rs +++ b/crates/ui-egui/src/canvas/viewport.rs @@ -35,6 +35,8 @@ pub(super) struct VpContent { pub(super) key: Option<(u64, u64, usize, Handle, i32, bool, bool)>, /// The viewport's view (centre, height) the list was built for, and the paper area it covers. view: (Vec2, f64), + /// The view twist it was built for: [`vp_motion`] moves and scales a list, it can't turn it. + twist: f64, area: Bounds2, /// The view on the previous frame and when it last changed (ms). last_view: (Vec2, f64), @@ -106,7 +108,7 @@ pub(super) fn ensure_vp_content(app: &mut CadApp, xf_paper: &Xf, ctx: &egui::Con c.last_view = view; c.moved_at = now; } - let rebuild = match vp_motion(&vp, c.view).filter(|_| c.key == Some(key)) { + let rebuild = match vp_motion(&vp, c.view).filter(|_| c.key == Some(key) && c.twist == vp.view_twist()) { Some((k, b)) => { // The viewport in the list's own coordinates. let visible = Bounds2::new((rect.min - b) / k, (rect.max - b) / k); @@ -141,6 +143,7 @@ pub(super) fn ensure_vp_content(app: &mut CadApp, xf_paper: &Xf, ctx: &egui::Con c.list = cadcraft_render::viewport_content(&st.doc, &st.space, h, area, &opts); c.key = Some(key); c.view = view; + c.twist = vp.view_twist(); c.area = area; c.builds = c.builds.wrapping_add(1); app.canvas.build_ms = crate::now_ms() - t0;