diff --git a/crates/engine/src/cmd/view.rs b/crates/engine/src/cmd/view.rs index 36bef26ee..b3ef77c88 100644 --- a/crates/engine/src/cmd/view.rs +++ b/crates/engine/src/cmd/view.rs @@ -265,7 +265,9 @@ fn run_view_set(s: &mut Session, p: &Value) -> Result { if h <= 0.0 { return Err(bad("view.set", "height must be positive")); } - s.state_mut()?.set_view(View { center: c, height: h }); + if !s.state_mut()?.set_view(View { center: c, height: h }) { + return Err(bad("view.set", "the view is too large to show. Nothing was changed")); + } ok() } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 3d43e67cd..9c55f1dfe 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -79,6 +79,10 @@ impl Default for View { } } +fn usable_view(v: &View) -> bool { + v.center.is_finite() && v.height.is_finite() && v.height > 1e-12 +} + /// Tiled model viewports (VPORTS): the session side of the drawing's `*Active` configuration. #[derive(Clone, Debug, Default, PartialEq)] pub struct ModelTiles { @@ -220,7 +224,9 @@ impl DocState { } } /// Map a requested screen view to the paper view, or (in MSPACE) to the viewport's model view. - fn store_view(&mut self, v: View) { + /// A finite request can still map to a view too large to represent: then nothing changes and + /// this returns false. + fn store_view(&mut self, v: View) -> bool { if let Some((h, vp)) = self.active_viewport() { let pv = self.paper_view(); let k = v.height / pv.height.max(1e-12); @@ -228,10 +234,17 @@ impl DocState { // A locked viewport keeps its scale: pan/zoom the sheet instead. let k0 = vp.view_height / vp.height; let pc = vp.center.xy() + (v.center - vp.view_center) / k0; - self.put_paper_view(View { center: pc, height: v.height / k0 }); + let paper = View { center: pc, height: v.height / k0 }; + if !usable_view(&paper) { + return false; + } + self.put_paper_view(paper); } else { let center = v.center - (pv.center - vp.center.xy()) * k; let height = vp.height * k; + if !usable_view(&View { center, height }) { + return false; + } let doc = Arc::make_mut(&mut self.doc); if let Some(store) = doc.space_mut(&self.space.clone()) { store.modify(h, |e| { @@ -242,9 +255,10 @@ impl DocState { }); } } - return; + return true; } self.put_paper_view(v); + true } fn put_paper_view(&mut self, v: View) { match self.views.iter_mut().find(|(s, _)| *s == self.space) { @@ -252,18 +266,23 @@ impl DocState { None => self.views.push((self.space.clone(), v)), } } - pub fn set_view(&mut self, v: View) { - if !(v.center.is_finite() && v.height.is_finite() && v.height > 1e-12) { - return; + /// Set the view, recording view history. Returns false, changing nothing, when the view (or, + /// in MSPACE, the viewport view it maps to) isn't finite with a positive height. + pub fn set_view(&mut self, v: View) -> bool { + if !usable_view(&v) { + return false; } let old = self.view(); + if !self.store_view(v) { + return false; + } if old != v { self.view_history.push(old); if self.view_history.len() > 50 { self.view_history.remove(0); } } - self.store_view(v); + true } /// The active MSPACE viewport when ZOOM works on its own view, as in AutoCAD: extents and /// windows fit the viewport, and zooms keep its centre. A locked viewport keeps its scale, so diff --git a/crates/engine/tests/zoom_viewport.rs b/crates/engine/tests/zoom_viewport.rs index f33758e96..c3867df1c 100644 --- a/crates/engine/tests/zoom_viewport.rs +++ b/crates/engine/tests/zoom_viewport.rs @@ -66,6 +66,24 @@ fn zoom_inside_a_viewport_acts_on_the_viewport() { assert_eq!(s.state().unwrap().paper_view(), paper); } +#[test] +fn a_view_too_large_for_the_viewport_is_refused_and_keeps_the_viewport() { + // A small paper view makes a finite but huge request map to an infinite model view (#714). + let (mut s, h) = session(); + s.execute("pspace", &json!({})).unwrap(); + s.execute("view.set", &json!({"center": [70, 60], "height": 0.5})).unwrap(); + s.execute("mspace", &json!({"handle": h})).unwrap(); + let before = viewport(&s, &h); + assert!(s.execute("view.set", &json!({"center": [0, 0], "height": 1e308})).is_err()); + let after = viewport(&s, &h); + assert_eq!(after, before); + assert!(after.view_center.is_finite() && after.view_height.is_finite()); + // A representable view still applies. + s.execute("view.set", &json!({"center": [0, 0], "height": 1})).unwrap(); + let v = viewport(&s, &h); + assert!(v.view_height.is_finite() && v.view_height != before.view_height, "{}", v.view_height); +} + #[test] fn zoom_in_a_locked_viewport_or_on_the_model_tab_is_unchanged() { // A locked viewport keeps its view: ZOOM moves the sheet instead.