diff --git a/src/app/update/viewport.rs b/src/app/update/viewport.rs index 7dabd8108..9e045ff9b 100644 --- a/src/app/update/viewport.rs +++ b/src/app/update/viewport.rs @@ -48,6 +48,11 @@ enum ScrollIntent { Pan { dx: f32, dy: f32 }, } +/// How far a ZOOM Dynamic drag must travel vertically before the magnifier +/// cursor flips between "+" and "-". Without the dead zone a hand holding +/// still at the top of a drag strobes the glyph on every jittered pixel. +const ZOOM_DIR_FLIP_PX: f32 = 2.0; + /// A wheel notch zooms; a trackpad's two fingers pan. /// /// A wheel reports notches, a precise-scrolling device reports pixels, and that @@ -1238,7 +1243,14 @@ impl OpenCADStudio { self.tabs[i] .scene .record_nav_perf(crate::scene::NavPerfOp::Zoom, move_started); - self.tabs[i].scene.selection.borrow_mut().middle_last_pos = Some(p); + let mut sel = self.tabs[i].scene.selection.borrow_mut(); + // Latch the direction for the magnifier cursor. A dead + // zone keeps the +/- glyph from strobing when a slow drag + // wobbles a pixel either side of where it started. + if dy.abs() >= ZOOM_DIR_FLIP_PX { + sel.zoom_dir_out = dy > 0.0; + } + sel.middle_last_pos = Some(p); return Task::none(); } // Shift+MMB drag orbits the model view instead of panning @@ -3473,6 +3485,9 @@ impl OpenCADStudio { sel.box_current = Some(p); sel.box_crossing = false; sel.box_crossing_locked = true; + // A fresh drag starts neutral: the magnifier shows "+" until + // the pointer has actually moved far enough to mean zoom out. + sel.zoom_dir_out = false; } return Task::none(); } @@ -3742,6 +3757,7 @@ impl OpenCADStudio { sel.box_anchor_world = None; sel.box_current = None; sel.box_crossing_locked = false; + sel.zoom_dir_out = false; drop(sel); self.arm_hover_after_navigation(i); return Task::none(); @@ -6729,6 +6745,94 @@ mod scroll_intent_tests { } } +#[cfg(test)] +mod zoom_dynamic_cursor_tests { + use super::*; + use crate::app::OpenCADStudio; + use crate::ui::overlay::NavCursor; + + fn zoom_armed_app() -> OpenCADStudio { + let mut app = OpenCADStudio::new_for_test(); + app.automation_op(r#"{"op":"new"}"#); + app.tabs[0].scene.selection.borrow_mut().vp_size = (800.0, 600.0); + let _ = app.dispatch_command("ZOOM DYNAMIC"); + app + } + + fn drag_to(app: &mut OpenCADStudio, y: f32) { + let _ = app.update(Message::ViewportMove(Point::new(400.0, y))); + } + + fn zoom_dir_out(app: &OpenCADStudio) -> bool { + app.tabs[0].scene.selection.borrow().zoom_dir_out + } + + /// The magnifier has to carry the sign of the zoom actually under way: + /// dragging up zooms in, dragging back down zooms out. + #[test] + fn the_magnifier_follows_the_drag_direction() { + let mut app = zoom_armed_app(); + assert!(app.tabs[0].zoom_dynamic_mode); + drag_to(&mut app, 300.0); + let _ = app.update(Message::ViewportLeftPress); + // A fresh drag starts neutral — "+" until told otherwise. + assert!(!zoom_dir_out(&app)); + + drag_to(&mut app, 200.0); + assert!(!zoom_dir_out(&app), "dragging up zooms in"); + + drag_to(&mut app, 400.0); + assert!(zoom_dir_out(&app), "dragging down zooms out"); + + drag_to(&mut app, 250.0); + assert!(!zoom_dir_out(&app), "dragging up again zooms in"); + + // Releasing disarms the direction, so the next hover shows "+". + let _ = app.update(Message::ViewportLeftRelease); + assert!(!zoom_dir_out(&app)); + } + + /// Sub-pixel wobble while a hand holds still must not strobe the glyph. + #[test] + fn jitter_below_the_dead_zone_does_not_flip_the_glyph() { + let mut app = zoom_armed_app(); + drag_to(&mut app, 300.0); + let _ = app.update(Message::ViewportLeftPress); + drag_to(&mut app, 400.0); + assert!(zoom_dir_out(&app)); + + // Back up by less than ZOOM_DIR_FLIP_PX: still zooming out. + drag_to(&mut app, 399.0); + assert!(zoom_dir_out(&app), "1 px of wobble must not flip the sign"); + } + + /// Each navigation tool owns its own cursor. They used to share the pan + /// hand, which read as "drag the sheet" during a zoom and an orbit. + #[test] + fn each_navigation_tool_reports_its_own_mode() { + let mut app = OpenCADStudio::new_for_test(); + app.automation_op(r#"{"op":"new"}"#); + for (command, want) in [ + ("PAN", NavCursor::Pan), + ("ORBIT", NavCursor::Orbit), + ("ZOOM DYNAMIC", NavCursor::Zoom), + ] { + let _ = app.dispatch_command(command); + assert_eq!( + crate::app::view::nav_cursor(&app.tabs[0]), + want, + "{command} must have its own cursor" + ); + let _ = app.update(Message::CommandEscape); + assert_eq!( + crate::app::view::nav_cursor(&app.tabs[0]), + NavCursor::None, + "Esc must disarm {command}" + ); + } + } +} + #[cfg(test)] mod snap_elevation_tests { use super::snap_keeps_elevation; diff --git a/src/app/view/mod.rs b/src/app/view/mod.rs index 6399320d8..e7ab2370e 100644 --- a/src/app/view/mod.rs +++ b/src/app/view/mod.rs @@ -45,6 +45,23 @@ fn crosshair_background(tab: &DocumentTab, is_paper: bool) -> [f32; 4] { tab.scene.paper_bg_color } +/// Which navigation tool the viewport cursor should advertise. The three flags +/// are mutually exclusive — every command entry point clears all of them before +/// arming one (`crate::app::commands`) — so the order here only settles a state +/// that cannot occur. +pub(in crate::app) fn nav_cursor(tab: &DocumentTab) -> crate::ui::overlay::NavCursor { + use crate::ui::overlay::NavCursor; + if tab.pan_mode { + NavCursor::Pan + } else if tab.orbit_mode { + NavCursor::Orbit + } else if tab.zoom_dynamic_mode { + NavCursor::Zoom + } else { + NavCursor::None + } +} + /// Clear gap (px) kept between the render-mode bar (top-left) and the ViewCube /// (top-right) before the cube is judged to collide and hides. const VIEWCUBE_GAP: f32 = 12.0; @@ -894,7 +911,7 @@ bg={bg_ms:.1}ms n={view_count}" dividers, pane_move_rect, pane_drop_rect, - tab.pan_mode || tab.orbit_mode || tab.zoom_dynamic_mode, + nav_cursor(tab), self.ribbon.open_dropdown.is_some(), hover_locked, crosshair_background(tab, is_paper), diff --git a/src/scene/camera_ops.rs b/src/scene/camera_ops.rs index b9eed710d..a1f0ffc2e 100644 --- a/src/scene/camera_ops.rs +++ b/src/scene/camera_ops.rs @@ -81,6 +81,26 @@ fn rendered_wire_bounds( } } +/// Merge `pair` into `bounds`, dropping coordinates that are parser junk +/// rather than geometry: DXF writes ±1e20 as its "no extents" sentinel, and +/// past 1e16 an f32 has no resolution left to place a point with. +fn fold_sane_bounds( + bounds: &mut Option<(glam::DVec3, glam::DVec3)>, + pair: Option<(glam::DVec3, glam::DVec3)>, +) { + const SANE_EXTENT: f64 = 1.0e16; + let Some((lo, hi)) = pair else { + return; + }; + if lo.abs().max_element() >= SANE_EXTENT || hi.abs().max_element() >= SANE_EXTENT { + return; + } + *bounds = Some(match *bounds { + Some((min, max)) => (min.min(lo), max.max(hi)), + None => (lo, hi), + }); +} + fn rendered_wire_center(wires: &[WireModel], fallback_z: f64) -> Option { rendered_wire_bounds(wires, fallback_z).map(|(min, max)| (min + max) * 0.5) } @@ -1321,6 +1341,155 @@ impl Scene { self.camera_generation += 1; } + /// Absolute-WCS bounds of everything visible in `block` — the one extents + /// walk behind ZOOM EXTENTS and the drawing-extents readers. + /// + /// Nothing is rejected here on a guess. An earlier version dropped any wire + /// whose centroid fell outside the inter-quartile consensus of the others, + /// to stop one junk wire from a bad parse poisoning the box. It also + /// dropped legitimately remote geometry — an outlying detail, a survey + /// point, a block placed far from the rest — so ZOOM EXTENTS quietly + /// refused to reach it. `SANE_EXTENT` and the finite test are the only + /// filters now; the ±1e6 display segments that motivated the old heuristic + /// come from XLine / Ray, which are handled explicitly below. + pub(super) fn visible_block_bounds(&self, block: Handle) -> Option<(glam::DVec3, glam::DVec3)> { + if block.is_null() { + return None; + } + let mut bounds: Option<(glam::DVec3, glam::DVec3)> = None; + macro_rules! fold { + ($pair:expr) => { + fold_sane_bounds(&mut bounds, $pair) + }; + } + + // Drawing extents must not depend on where the camera happens to be + // looking: the result is cached per geometry epoch, and a floating + // viewport's own camera is derived FROM the extents + // (`camera_for_viewport`), so reading the camera here recurses. A wire + // with no precise position falls back to the z = 0 plane. + let fallback_z = 0.0; + let scale = crate::scene::annotative::scale_handle_by_name( + &self.document, + &self.document.header.current_annotation_scale, + ); + // The resident set is the full, un-culled, LOD-free tessellation the + // block actually renders from. Culled input would fit only what is + // already on screen, so each ZOOM EXTENTS would zoom out a little and + // reveal more, converging on the true extent after several uses (#51). + let wires = self.resident_wires_for(block, None, scale, None, None); + // Infinite display segments do not contribute to drawing extents. + // Keep their base points as a fallback for otherwise empty drawings. + let mut infinite_base: Vec = Vec::new(); + for wire in wires.iter() { + let is_infinite = Self::handle_from_wire_name(&wire.name) + .and_then(|handle| self.document.get_entity(handle)) + .is_some_and(|entity| matches!(entity, EntityType::XLine(_) | EntityType::Ray(_))); + if is_infinite { + infinite_base.extend( + wire.key_vertices + .iter() + .map(|&[x, y, z]| glam::DVec3::new(x, y, z)) + .filter(|point| point.is_finite()), + ); + continue; + } + fold!(rendered_wire_bounds(std::slice::from_ref(wire), fallback_z)); + } + + // 3D solids render as meshes, not wires: a drawing of only solids has + // no wire to fit. + for (&handle, set) in &self.meshes { + let Some(entity) = self.document.get_entity(handle) else { + continue; + }; + if !self.mesh_entity_visible(handle) + || !self.belongs_to_visible_block(handle, entity.common().owner_handle, block) + { + continue; + } + let [x0, y0, x1, y1] = set.world_aabb; + let [z0, z1] = set.z_aabb; + let lo = glam::DVec3::new(x0 as f64, y0 as f64, z0 as f64); + let hi = glam::DVec3::new(x1 as f64, y1 as f64, z1 as f64); + if lo.is_finite() && hi.is_finite() { + fold!(Some((lo.min(hi), lo.max(hi)))); + } + } + + // Raster images, OLE frames and underlays draw as GPU quads; with + // their frames hidden they carry no wire outline at all. + for (&handle, image) in &self.images { + let Some(entity) = self.document.get_entity(handle) else { + continue; + }; + if !self.mesh_entity_visible(handle) + || !self.belongs_to_visible_block(handle, entity.common().owner_handle, block) + { + continue; + } + for (corner, low) in image.corners.iter().zip(image.corners_low.iter()) { + let point = glam::DVec3::new( + corner[0] as f64 + low[0] as f64, + corner[1] as f64 + low[1] as f64, + corner[2] as f64 + low[2] as f64, + ); + if point.is_finite() { + fold!(Some((point, point))); + } + } + } + + // HATCH tessellates to no wires at all (#131), so a drawing whose + // outermost geometry is a fill — or which is nothing but fills — has + // to pick its bounds up from the fill models themselves. The paper + // sheet already does this; model space did not. + // The renderer already holds the fill models for the block it is + // drawing, keyed by geometry epoch — reuse that rather than exploding + // every INSERT again on each ZOOM EXTENTS. Only a block that is not + // the one on screen (model space while a BEDIT is open) has to build + // its own set. + let hatches = if block == self.content_render_block_handle() { + // Tinted, matching the renderer's own call: the two then share one + // cache entry. Asking for an untinted set would key the same slot + // with a different selection signature and the two would evict + // each other on every frame. Tint is colour; it moves no vertex. + self.hatch_models_arc_for_view(true) + } else { + std::sync::Arc::new(self.synced_hatch_models( + block, + None, + scale, + self.annotation_all_visible(), + None, + false, + )) + }; + for hatch in hatches.iter() { + for &[x, y] in hatch.boundary.iter() { + // NaN-NaN separators between disconnected paths fall out on + // the finite test — they must not be treated as vertices. + let point = glam::DVec3::new( + hatch.world_origin[0] + x as f64, + hatch.world_origin[1] + y as f64, + 0.0, + ); + if point.is_finite() { + fold!(Some((point, point))); + } + } + } + + // Drawing holds only infinite construction geometry: fall back to its + // base points rather than reporting nothing. + if bounds.is_none() { + for point in infinite_base { + fold!(Some((point, point))); + } + } + bounds + } + pub fn fit_all(&mut self) { if self.active_viewport.is_some() { if let Some((mut min, mut max)) = self.model_space_extents() { @@ -1337,225 +1506,19 @@ impl Scene { return; } - // Use the FULL, un-culled wire set — not `entity_wires()`, which is - // frustum-culled to the current view. Culled input would fit only the - // entities already on screen, so each call would zoom out a little and - // reveal more, converging on the true extent only after several uses - // (issue #51). `wpp = None` also tessellates at a fixed tolerance so - // the bounds don't drift with zoom-adaptive curve sampling. - let layout_block = self.current_layout_block_handle(); - let scale = if self.current_layout == "Model" { - crate::scene::annotative::scale_handle_by_name( - &self.document, - &self.document.header.current_annotation_scale, - ) - } else { - self.paper_annotation_scale_handle() + let Some((min, mut max)) = self.visible_block_bounds(self.current_layout_block_handle()) + else { + // Nothing usable to fit: leave the camera where it is. + return; }; - let mut wires = self.wires_for_block_culled( - layout_block, - None, - None, - None, - None, - scale, - self.annotation_all_visible(), - None, - ); - // Infinite display segments do not contribute to drawing extents. - // Preserve their base points as a fallback for otherwise empty drawings. - let mut infinite_base_pts: Vec = Vec::new(); - wires.retain(|w| { - let is_infinite = Self::handle_from_wire_name(&w.name) - .and_then(|h| self.document.get_entity(h)) - .map(|e| matches!(e, EntityType::XLine(_) | EntityType::Ray(_))) - .unwrap_or(false); - if is_infinite { - infinite_base_pts.extend( - w.key_vertices - .iter() - .map(|v| glam::Vec3::new(v[0] as f32, v[1] as f32, v[2] as f32)), - ); - } - !is_infinite - }); - // 3D solids render as meshes, not wires, so collect their complete - // world boxes separately — a drawing of only solids has no wires to fit. - let mesh_aabbs: Vec<([f32; 4], [f32; 2])> = self - .meshes - .iter() - .filter(|(h, _)| { - let handle = **h; - self.mesh_entity_visible(handle) - && self.document.get_entity(handle).is_some_and(|entity| { - self.belongs_to_visible_block( - handle, - entity.common().owner_handle, - layout_block, - ) - }) - }) - .map(|(_, set)| (set.world_aabb, set.z_aabb)) - .filter(|(xy, z)| { - xy[0].is_finite() - && xy[1].is_finite() - && xy[2].is_finite() - && xy[3].is_finite() - && z[0].is_finite() - && z[1].is_finite() - }) - .collect(); - // Raster images and underlays draw as GPU quads; with their frames - // hidden they carry no wire outline, so fold their corners in too. - let image_corners: Vec = self - .images - .iter() - .filter(|(h, _)| { - let handle = **h; - self.mesh_entity_visible(handle) - && self.document.get_entity(handle).is_some_and(|entity| { - self.belongs_to_visible_block(handle, entity.common().owner_handle, layout_block) - }) - }) - .flat_map(|(_, image)| { - image.corners.iter().zip(image.corners_low.iter()).map(|(c, low)| { - glam::Vec3::new(c[0] + low[0], c[1] + low[1], c[2] + low[2]) - }) - }) - .filter(|p| p.is_finite()) - .collect(); - if wires.is_empty() && mesh_aabbs.is_empty() && infinite_base_pts.is_empty() && image_corners.is_empty() { - return; - } - - // Collect wire centroids for robust outlier rejection. - struct WireCent { - idx: usize, - cx: f32, - cy: f32, - } - let mut cents: Vec = Vec::with_capacity(wires.len()); - for (idx, wire) in wires.iter().enumerate() { - let mut sx = 0.0_f64; - let mut sy = 0.0_f64; - let mut n = 0_usize; - for &[x, y, _] in &wire.points { - if !x.is_finite() || !y.is_finite() { - continue; - } - sx += x as f64; - sy += y as f64; - n += 1; - } - // SDF text is carried as textured glyph quads, not ordinary wire - // points. Include those vertices so a text-only wire participates - // in the robust-centre filter used by ZOOM EXTENTS. - for vertex in &wire.text_verts { - let x = vertex.pos[0] as f64 + vertex.pos_low[0] as f64; - let y = vertex.pos[1] as f64 + vertex.pos_low[1] as f64; - if !x.is_finite() || !y.is_finite() { - continue; - } - sx += x; - sy += y; - n += 1; - } - if n > 0 { - cents.push(WireCent { - idx, - cx: (sx / n as f64) as f32, - cy: (sy / n as f64) as f32, - }); - } - } - if cents.is_empty() && mesh_aabbs.is_empty() && infinite_base_pts.is_empty() && image_corners.is_empty() { - return; - } - - // Quartile rejection needs enough samples to be meaningful. With fewer - // wires, every finite visible point belongs to the drawing extents. - let (rx_lo, rx_hi, ry_lo, ry_hi) = if cents.len() >= 8 { - let mut xs: Vec = cents.iter().map(|c| c.cx).collect(); - let mut ys: Vec = cents.iter().map(|c| c.cy).collect(); - xs.sort_by(|a, b| a.partial_cmp(b).unwrap_or(std::cmp::Ordering::Equal)); - ys.sort_by(|a, b| a.partial_cmp(b).unwrap_or(std::cmp::Ordering::Equal)); - let q = |v: &[f32], frac: f32| v[((v.len() as f32 - 1.0) * frac) as usize]; - let q1x = q(&xs, 0.25); - let q3x = q(&xs, 0.75); - let q1y = q(&ys, 0.25); - let q3y = q(&ys, 0.75); - // Keep sparse annotations while rejecting isolated corrupt wires. - const K: f32 = 10.0; - let dx = (q3x - q1x).max(1.0) * K; - let dy = (q3y - q1y).max(1.0) * K; - (q1x - dx, q3x + dx, q1y - dy, q3y + dy) - } else { - ( - f32::NEG_INFINITY, - f32::INFINITY, - f32::NEG_INFINITY, - f32::INFINITY, - ) - }; - - let mut min = glam::Vec3::splat(f32::MAX); - let mut max = glam::Vec3::splat(f32::MIN); - for c in ¢s { - if c.cx < rx_lo || c.cx > rx_hi || c.cy < ry_lo || c.cy > ry_hi { - continue; - } - let wire = &wires[c.idx]; - let hw = (wire.world_width * 0.5) as f32; - for &[x, y, z] in &wire.points { - if !x.is_finite() || !y.is_finite() || !z.is_finite() { - continue; - } - if hw > 0.0 { - min = min.min(glam::Vec3::new(x - hw, y - hw, z - hw)); - max = max.max(glam::Vec3::new(x + hw, y + hw, z + hw)); - } else { - min = min.min(glam::Vec3::new(x, y, z)); - max = max.max(glam::Vec3::new(x, y, z)); - } - } - for vertex in &wire.text_verts { - let point = glam::Vec3::new( - vertex.pos[0] + vertex.pos_low[0], - vertex.pos[1] + vertex.pos_low[1], - vertex.pos[2] + vertex.pos_low[2], - ); - if point.is_finite() { - min = min.min(point); - max = max.max(point); - } - } - } - // Fold in 3D-solid mesh AABBs (not subject to the wire IQR reject). - for ([ax, ay, bx, by], [az, bz]) in &mesh_aabbs { - min = min.min(glam::Vec3::new(*ax, *ay, *az)); - max = max.max(glam::Vec3::new(*bx, *by, *bz)); - } - for p in &image_corners { - min = min.min(*p); - max = max.max(*p); - } - // Drawing holds only infinite construction geometry: fit the view to - // its base points rather than leaving the camera unchanged. - if min.x > max.x { - for p in &infinite_base_pts { - min = min.min(*p); - max = max.max(*p); - } - } - // If no usable points found, leave the camera unchanged. - if min.x > max.x { - return; - } if min == max { - max += glam::Vec3::splat(1.0); + max += glam::DVec3::splat(1.0); } let aspect = self.active_camera_aspect(); - self.camera.borrow_mut().fit_to_bounds(min, max, aspect); + // f64 all the way into the camera: a UTM-scale drawing loses whole + // metres to an f32 round-trip, which shows up as a fit that sits + // slightly off the geometry it was asked to frame. + self.camera.borrow_mut().fit_to_bounds_f64(min, max, aspect); self.projection_bounds_epoch.set(self.geometry_epoch); self.camera_generation += 1; } diff --git a/src/scene/mod.rs b/src/scene/mod.rs index d9594a945..a08c183b8 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -11953,78 +11953,15 @@ vis_index={:.1} visible_probe={:.1}", fn compute_model_space_extents(&self) -> Option<(glam::Vec3, glam::Vec3)> { let model_block = self.model_space_block_handle(); - if model_block.is_null() { - return None; - } - // Reuse the full resident source in every layout. The kernel bounds - // its world-space key vertices; camera fitting never tessellates a - // second copy or reads the current paper sheet's wire cache. - let scale = crate::scene::annotative::scale_handle_by_name( - &self.document, - &self.document.header.current_annotation_scale, - ); - let wires = - self.resident_wires_for(model_block, Some(self.annotation_scale), scale, None, None); - let mut min = [f64::INFINITY; 3]; - let mut max = [f64::NEG_INFINITY; 3]; - let mut has_points = false; - - for wire in wires.iter() { - for &[x, y, z] in &wire.key_vertices { - if (x as f32).is_finite() && (y as f32).is_finite() && (z as f32).is_finite() { - min[0] = min[0].min(x); - min[1] = min[1].min(y); - min[2] = min[2].min(z); - max[0] = max[0].max(x); - max[1] = max[1].max(y); - max[2] = max[2].max(z); - has_points = true; - } - } - } - - if !self.meshes.is_empty() { - for (&handle, set) in &self.meshes { - let Some(entity) = self.document.get_entity(handle) else { - continue; - }; - if !self.mesh_entity_visible(handle) - || !self.belongs_to_visible_block( - handle, - entity.common().owner_handle, - model_block, - ) - { - continue; - } - let [ax, ay, bx, by] = set.world_aabb; - let [az, bz] = set.z_aabb; - let (ax, ay, bx, by, az, bz) = ( - ax as f64, ay as f64, bx as f64, by as f64, az as f64, bz as f64, - ); - if (ax as f32).is_finite() - && (ay as f32).is_finite() - && (az as f32).is_finite() - && (bx as f32).is_finite() - && (by as f32).is_finite() - && (bz as f32).is_finite() - { - min[0] = min[0].min(ax.min(bx)); - min[1] = min[1].min(ay.min(by)); - min[2] = min[2].min(az.min(bz)); - max[0] = max[0].max(ax.max(bx)); - max[1] = max[1].max(ay.max(by)); - max[2] = max[2].max(az.max(bz)); - has_points = true; - } - } - } - - if has_points { - return Some(( - glam::Vec3::new(min[0] as f32, min[1] as f32, min[2] as f32), - glam::Vec3::new(max[0] as f32, max[1] as f32, max[2] as f32), - )); + // Shared with ZOOM EXTENTS so the two never disagree about what the + // drawing covers. The previous walk here read only `key_vertices`, + // which is empty for every tessellated curve (see `WireModel`) — so + // circles, arcs, ellipses and splines contributed nothing, and + // neither did hatches or images. Everything that reads drawing + // extents (plot Extents, sheet-set framing, viewport auto-fit, + // SpaceMouse model bounds) inherited that blind spot. + if let Some((min, max)) = self.visible_block_bounds(model_block) { + return Some((min.as_vec3(), max.as_vec3())); } // Last resort: saved EXTMIN/EXTMAX before the wire cache is built. const SANE_EXTENT: f64 = 1.0e16; @@ -13663,42 +13600,27 @@ mod layout_cache_tests { s.add_entity(EntityType::LwPolyline(pl)); let bounds = s.model_space_extents().expect("Extents must exist"); - // Key vertices come from entities with distinct vertex positions (Line, LwPolyline). - // Line vertices: (0, 0, 10), (50, 100, 20) - // LwPolyline vertices: (300, -200, 0), (400, -100, 0) - // Min X = 0.0, Max X = 400.0 - // Min Y = -200.0, Max Y = 100.0 - // Min Z = 0.0, Max Z = 20.0 - assert!( - (bounds.0.x - 0.0).abs() < 1e-3, - "min.x mismatch: {}", - bounds.0.x - ); - assert!( - (bounds.1.x - 400.0).abs() < 1e-3, - "max.x mismatch: {}", - bounds.1.x - ); - assert!( - (bounds.0.y - (-200.0)).abs() < 1e-3, - "min.y mismatch: {}", - bounds.0.y - ); - assert!( - (bounds.1.y - 100.0).abs() < 1e-3, - "max.y mismatch: {}", - bounds.1.y - ); - assert!( - (bounds.0.z - 0.0).abs() < 1e-3, - "min.z mismatch: {}", - bounds.0.z - ); - assert!( - (bounds.1.z - 20.0).abs() < 1e-3, - "max.z mismatch: {}", - bounds.1.z - ); + // Every entity counts, curves included. The circle and the arc used to + // contribute nothing at all: the walk read only `key_vertices`, which + // is empty for anything tessellated, so the drawing looked 275 units + // narrower and 150 shorter than it is. + // Line: (0, 0, 10) .. (50, 100, 20) + // Circle: (150, 150, 0) .. (250, 250, 0) centre 200,200 r50 + // Arc: (-125, -50, -5) .. (-75, -25, -5) centre -100,-50 r25, 0..PI + // LwPolyline: (300, -200, 0) .. (400, -100, 0) + for (got, want, axis) in [ + (bounds.0.x, -125.0, "min.x"), + (bounds.1.x, 400.0, "max.x"), + (bounds.0.y, -200.0, "min.y"), + (bounds.1.y, 250.0, "max.y"), + (bounds.0.z, -5.0, "min.z"), + (bounds.1.z, 20.0, "max.z"), + ] { + assert!( + (got - want).abs() < 1e-3, + "{axis} mismatch: got {got}, want {want}" + ); + } } #[test] diff --git a/src/scene/pick/selection_state.rs b/src/scene/pick/selection_state.rs index 22faca9a5..b4fc62baa 100644 --- a/src/scene/pick/selection_state.rs +++ b/src/scene/pick/selection_state.rs @@ -59,6 +59,11 @@ pub struct SelectionState { pub middle_down: bool, pub middle_last_pos: Option, pub middle_last_press_time: Option, + /// Which way a ZOOM Dynamic drag is currently zooming: `true` while it is + /// zooming out. Only the magnifier cursor reads it. The sign is latched + /// rather than taken from the live `dy` so that the sub-pixel jitter of a + /// slow drag cannot strobe the `+` / `−` glyph. + pub zoom_dir_out: bool, } /// Transient state of the open right-click context menu. diff --git a/src/ui/overlay.rs b/src/ui/overlay.rs index c6d38fa84..adde3f65d 100644 --- a/src/ui/overlay.rs +++ b/src/ui/overlay.rs @@ -213,6 +213,18 @@ pub(crate) fn pick_box_aperture_px(value: i32) -> f32 { aperture.max(1.0) } +/// Which interactive navigation tool is armed. Each one owns the whole +/// viewport and hides the CAD crosshair, but they do not share a cursor: the +/// hand reads as "drag the sheet", which is wrong for a zoom or an orbit. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Default)] +pub enum NavCursor { + #[default] + None, + Pan, + Orbit, + Zoom, +} + #[derive(Clone, Copy)] pub struct CrosshairOptions { pub size_percent: i32, @@ -852,7 +864,7 @@ pub fn selection_overlay<'a>( dividers: Vec, pane_move_rect: Option, pane_drop_rect: Option, - pan_mode: bool, + nav: NavCursor, suppressed: bool, hover_locked: bool, crosshair_bg: [f32; 4], @@ -882,7 +894,7 @@ pub fn selection_overlay<'a>( dividers, pane_move_rect, pane_drop_rect, - pan_mode, + nav, suppressed, hover_locked, crosshair_bg, @@ -936,9 +948,10 @@ struct SelectionCanvas { pane_move_rect: Option, /// The pane under the cursor during a pane move (drop target), highlighted. pane_drop_rect: Option, - /// Interactive PAN mode: the crosshair is hidden and the cursor becomes a - /// hand so the viewport reads as a draggable surface. - pan_mode: bool, + /// Which interactive navigation tool is armed. Any of them hides the + /// crosshair — the viewport is being driven, not drawn on — and each + /// picks its own cursor in `mouse_interaction`. + nav: NavCursor, /// A ribbon dropdown (or similar overlay) is open over the viewport. The /// crosshair is not drawn and the OS cursor is shown normally so the panel /// is usable instead of the cursor vanishing over it. (#227) @@ -1197,13 +1210,30 @@ impl canvas::Program for SelectionCanvas { if self.suppressed { return mouse::Interaction::default(); } - // PAN mode owns the whole viewport: an open hand when hovering, a - // closed hand while dragging. - if self.pan_mode && cursor.is_over(bounds) { - return if self.selection.borrow().middle_down { - mouse::Interaction::Grabbing - } else { - mouse::Interaction::Grab + // An armed navigation tool owns the whole viewport, and the cursor + // says which one: a hand to drag the sheet, four-way arrows to swing + // the model, a magnifier that carries the sign of the zoom actually + // under way. Hovering before the drag starts shows the neutral "+" + // magnifier rather than guessing a direction. + if self.nav != NavCursor::None && cursor.is_over(bounds) { + let dragging = self.selection.borrow().middle_down; + return match self.nav { + NavCursor::Pan => { + if dragging { + mouse::Interaction::Grabbing + } else { + mouse::Interaction::Grab + } + } + NavCursor::Orbit => mouse::Interaction::AllScroll, + NavCursor::Zoom => { + if dragging && self.selection.borrow().zoom_dir_out { + mouse::Interaction::ZoomOut + } else { + mouse::Interaction::ZoomIn + } + } + NavCursor::None => unreachable!("guarded above"), }; } if self.show_viewcube { @@ -1367,8 +1397,14 @@ impl canvas::Program for SelectionCanvas { ); } - if let (Some(a), Some(b)) = (self.selection.borrow().box_anchor, self.selection.borrow().box_current) { - draw_marquee(&mut frame, a, b, self.selection.borrow().box_crossing, theme, &self.selection_visual, self.crosshair_bg); + // ZOOM Dynamic reuses `box_anchor` to remember where the drag began, + // so the marquee would otherwise rubber-band across the screen while + // the view zooms. Suppress the drawing, not the state — the anchor is + // still re-projected as the camera moves. + if self.nav != NavCursor::Zoom { + if let (Some(a), Some(b)) = (self.selection.borrow().box_anchor, self.selection.borrow().box_current) { + draw_marquee(&mut frame, a, b, self.selection.borrow().box_crossing, theme, &self.selection_visual, self.crosshair_bg); + } } // Preview marquee for point-picked windows (STRETCH) — same look, no pick. if let Some((a, b, crossing)) = self.selection.borrow().preview_box { @@ -1826,10 +1862,10 @@ impl canvas::Program for SelectionCanvas { // arrow (see `mouse_interaction`); drawing the CAD crosshair on // top of it would double up the visual feedback. let over_divider = self.divider_under(cursor, bounds); - // PAN mode replaces the crosshair with a hand cursor. + // An armed navigation tool replaces the crosshair with its own cursor. if !over_viewcube && !over_divider - && !self.pan_mode + && self.nav == NavCursor::None && !self.suppressed && self.crosshair.cursor_type == CursorType::Crosshair { diff --git a/tests/integration.rs b/tests/integration.rs index d695e4ffb..dd6d13d09 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -72,5 +72,8 @@ mod viewport_wide_polyline_plot_width; #[path = "xclip_plot_test.rs"] mod xclip_plot_test; +#[path = "zoom_extents_coverage.rs"] +mod zoom_extents_coverage; + #[path = "zoom_extents_xline.rs"] mod zoom_extents_xline; diff --git a/tests/zoom_extents_coverage.rs b/tests/zoom_extents_coverage.rs new file mode 100644 index 000000000..455349760 --- /dev/null +++ b/tests/zoom_extents_coverage.rs @@ -0,0 +1,123 @@ +//! ZOOM EXTENTS must frame every visible entity, the way commercial solutions +//! do — no heuristic allowed to decide that part of the drawing does not count. +//! +//! Two things used to stop it. A quartile filter dropped any wire whose +//! centroid sat far from the consensus of the others, so a detail parked away +//! from the main body was silently left outside the fit. And HATCH tessellates +//! to no wires at all, so a drawing whose outermost geometry is a fill had +//! nothing for the model-space walk to read. +#![cfg(not(target_arch = "wasm32"))] + +use codec::entities::hatch::{BoundaryEdge, BoundaryPath, LineEdge}; +use codec::entities::{Hatch, Line}; +use codec::types::{Vector2, Vector3}; +use codec::EntityType; +use OpenCADStudio::scene::Scene; + +fn add_line(scene: &mut Scene, x1: f64, y1: f64, x2: f64, y2: f64) { + let mut line = Line::new(); + line.start = Vector3::new(x1, y1, 0.0); + line.end = Vector3::new(x2, y2, 0.0); + scene.add_entity(EntityType::Line(line)); +} + +/// A solid square fill with corners (x, y) .. (x + size, y + size). +fn square_hatch(x: f64, y: f64, size: f64) -> Hatch { + let mut path = BoundaryPath::new(); + for (sx, sy, ex, ey) in [ + (0.0, 0.0, size, 0.0), + (size, 0.0, size, size), + (size, size, 0.0, size), + (0.0, size, 0.0, 0.0), + ] { + path.edges.push(BoundaryEdge::Line(LineEdge { + start: Vector2::new(x + sx, y + sy), + end: Vector2::new(x + ex, y + ey), + })); + } + let mut hatch = Hatch::new(); + hatch.paths.push(path); + hatch +} + +#[test] +fn fit_all_reaches_an_entity_parked_far_from_the_rest() { + let mut scene = Scene::new(); + // A dense consensus cluster — more than the eight wires the old quartile + // filter needed before it switched itself on. + for i in 0..12 { + let x = i as f64; + add_line(&mut scene, x, 0.0, x, 10.0); + } + // …and one line far outside it. Nothing about it is invalid: it is simply + // somewhere else, which is exactly what the filter could not tell apart + // from parser junk. + add_line(&mut scene, 50_000.0, 50_000.0, 50_010.0, 50_010.0); + + scene.fit_all(); + + // The fit centres on the extents box, so the target alone says whether the + // remote line counted: dropping it leaves the camera on the cluster at + // about (5.5, 5) instead of halfway out to (25005, 25005). + let cam = scene.camera.borrow(); + assert!( + (cam.target.x - 25_005.0).abs() < 50.0 && (cam.target.y - 25_005.0).abs() < 50.0, + "the fit must span cluster and remote line, target = {:?}", + cam.target + ); + assert!( + cam.ortho_size() >= 25_005.0, + "the view must be wide enough to hold both, ortho_size = {}", + cam.ortho_size() + ); +} + +#[test] +fn fit_all_frames_a_hatch_only_drawing() { + let mut scene = Scene::new(); + scene.add_entity(EntityType::Hatch(square_hatch(100.0, 200.0, 40.0))); + + scene.fit_all(); + + // Nothing else is in the drawing, so without the fill the camera never + // moves and stays on the origin. + let cam = scene.camera.borrow(); + assert!( + (cam.target.x - 120.0).abs() < 1.0 && (cam.target.y - 220.0).abs() < 1.0, + "a hatch is geometry: ZOOM EXTENTS must centre on it, target = {:?}", + cam.target + ); + assert!( + cam.ortho_size() >= 20.0, + "the view must hold the whole fill, ortho_size = {}", + cam.ortho_size() + ); +} + +#[test] +fn fit_all_keeps_a_hatch_outside_the_wires_in_view() { + let mut scene = Scene::new(); + for i in 0..12 { + let x = i as f64; + add_line(&mut scene, x, 0.0, x, 10.0); + } + // The fill is the outermost thing in the drawing; the wires alone would + // frame a 12 x 10 strip and leave it off screen. + scene.add_entity(EntityType::Hatch(square_hatch(500.0, 500.0, 50.0))); + + scene.fit_all(); + + // Wires alone centre near (5.5, 5); including the fill pulls the centre + // out to about (275, 275). + let cam = scene.camera.borrow(); + assert!( + cam.target.x > 200.0 && cam.target.y > 200.0, + "the fill must widen the fit past the wires, target = {:?}", + cam.target + ); + assert!( + cam.ortho_size() >= 275.0, + "the view must reach the far corner of the fill, ortho_size = {}", + cam.ortho_size() + ); +}