From bd1c51c68224c61a321caec37c984faac1a77eff Mon Sep 17 00:00:00 2001 From: Hakan Seven Date: Wed, 24 Jun 2026 23:14:46 +0300 Subject: [PATCH] fix(viewport): correct ViewCube flip + activation/border for off-screen viewports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ViewCube "already there → flip to opposite" tested the main (paper) camera, which always looks straight down, so inside a floating viewport clicking "top" landed on "bottom". Route the check through active_gaze_dir() — the gaze of the camera the cube actually reflects. Off-screen viewports: - The active-viewport outline clamped only its origin while keeping full width/height, shifting the whole border inward. Clip it to the visible canvas (origin AND size) so it hugs the real viewport edge. - Double-click activation tested each viewport's full paper rect, so a click on empty space beside a viewport that runs off-screen matched its partly-off-canvas rect and switched to it. Activation now keys on the visible on-screen rect (viewport_at_screen_point), picks the smallest (topmost) on overlap, and accepts the border-wire pick only when the click also lands inside that viewport's visible rect — so a far viewport's edge passing near the cursor can't steal the activation. Co-Authored-By: Claude Opus 4.8 --- src/app/update.rs | 43 +++++++++++++------- src/app/view.rs | 12 +++++- src/scene/mod.rs | 99 +++++++++++++++++++++++++++++++++++++---------- 3 files changed, 117 insertions(+), 37 deletions(-) diff --git a/src/app/update.rs b/src/app/update.rs index e4be20084..dea9af13a 100644 --- a/src/app/update.rs +++ b/src/app/update.rs @@ -4200,7 +4200,7 @@ impl OpenCADStudio { }; // 1) Try direct wire hit — works when the border is clicked. - let hit_vp: Option = { + let wire_hit: Option = { let (view_rot, eye) = { let c = self.tabs[i].scene.camera.borrow(); (c.view_proj_rte(bounds), c.eye()) }; let all_wires = self.tabs[i].scene.hit_test_wires(); scene::pick::hit_test::click_hit(p, &all_wires[..], view_rot, eye, bounds) @@ -4220,19 +4220,35 @@ impl OpenCADStudio { }) }; - // 2) Geometric fallback: check if the cursor is inside any - // viewport's bounding rectangle in paper space. This handles - // double-clicks on model-entity content wires or empty areas. - let hit_vp = hit_vp.or_else(|| { - let paper_pt = self.tabs[i] - .scene - .camera - .borrow() - .pick_on_target_plane(p, bounds); + // 2) Decide which viewport (if any) the click enters, + // keyed on the *visible* on-screen rectangle. Screen-space + // (not the full paper rect) so a click on the empty area + // beside an off-screen viewport doesn't match its + // partly-off-canvas rect and switch to it by mistake. + // The wire hit only refines *which* visible viewport + // (border precision / overlap), and only when the click + // actually lands inside THAT viewport's visible rect — + // otherwise its border-wire pick tolerance could match a + // far viewport whose edge merely passes near the cursor + // (e.g. clicking the overlap of two viewports while a + // third's border runs nearby) and enter the wrong one. + let screen_hit = self + .tabs[i] + .scene + .viewport_at_screen_point(p.x, p.y, (vw, vh)); + let wire_in_visible = wire_hit.filter(|&h| { self.tabs[i] .scene - .viewport_at_paper_point(paper_pt.x as f32, paper_pt.y as f32) + .viewport_screen_rect(h, (vw, vh)) + .is_some_and(|r| { + let x0 = r.x.max(0.0); + let y0 = r.y.max(0.0); + let x1 = (r.x + r.width).min(vw); + let y1 = (r.y + r.height).min(vh); + p.x >= x0 && p.x <= x1 && p.y >= y0 && p.y <= y1 + }) }); + let hit_vp = wire_in_visible.or(screen_hit); if let Some(handle) = hit_vp { return Task::done(Message::EnterViewport(handle)); @@ -4421,10 +4437,7 @@ impl OpenCADStudio { // "Already there → flip to opposite" check: compare the // current gaze direction with the region's target gaze. let target_dir = r_ucs.transform_vector3(region.snap_direction()); - let cur_dir = { - let cam = self.tabs[i].scene.camera.borrow(); - cam.rotation * glam::Vec3::Z - }; + let cur_dir = self.tabs[i].scene.active_gaze_dir(); if cur_dir.dot(target_dir) > 0.9999 { region = region.opposite(); } diff --git a/src/app/view.rs b/src/app/view.rs index 1d7c57d5b..0bc010dcf 100644 --- a/src/app/view.rs +++ b/src/app/view.rs @@ -469,8 +469,16 @@ impl OpenCADStudio { None }; if let Some(rect) = active_vp_rect { + // Clip the outline to the visible canvas. Clamping only the origin + // (max(0.0)) while keeping the full width/height shifted the whole + // outline inward when the viewport ran off the top/left edge, so + // its drawn border no longer matched the real viewport — clicks + // that looked outside landed in (and activated) another viewport. + let (cw, ch) = tab.scene.selection.borrow().vp_size; let x = rect.x.max(0.0); let y = rect.y.max(0.0); + let vw = ((rect.x + rect.width).min(cw) - x).max(1.0); + let vh = ((rect.y + rect.height).min(ch) - y).max(1.0); // Highlight the active viewport with a 2-px border so its // boundary is always visible over the GPU shader. const VP_BORDER: Color = Color { @@ -481,8 +489,8 @@ impl OpenCADStudio { }; let border_frame = container( Space::new() - .width(iced::Length::Fixed(rect.width.max(1.0))) - .height(iced::Length::Fixed(rect.height.max(1.0))), + .width(iced::Length::Fixed(vw)) + .height(iced::Length::Fixed(vh)), ) .style(move |_: &Theme| container::Style { border: iced::Border { diff --git a/src/scene/mod.rs b/src/scene/mod.rs index 8588e16f0..6cb810e20 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -3929,6 +3929,20 @@ impl Scene { } } + /// Current gaze direction (canonical +Z eye dir, world space) of whichever + /// camera owns the ViewCube — the active floating viewport in MSPACE, else + /// the main camera. Used by the ViewCube "already there → flip to opposite" + /// check, which must test the camera the cube actually reflects (the paper + /// camera always looks straight down, so testing it flipped every snap). + pub fn active_gaze_dir(&self) -> glam::Vec3 { + if let Some(h) = self.active_viewport { + if let Some(cam) = self.camera_for_viewport(h) { + return cam.rotation * glam::Vec3::Z; + } + } + self.camera.borrow().rotation * glam::Vec3::Z + } + /// View-rotation matrix for the active viewport (MSPACE), or the /// paper-space camera's matrix when not in MSPACE. /// Used by ViewCube hit-testing so clicks map to the correct camera. @@ -3959,28 +3973,73 @@ impl Scene { } /// Return the handle of the user viewport whose bounding rectangle contains - /// the given paper-space point, or `None` if no viewport matches. + /// the given paper-space point, or `None` if no viewport matches. When + /// viewports overlap (e.g. a large viewport that runs off-screen spanning + /// smaller ones), pick the SMALLEST-area match — the topmost one the user + /// actually sees and means to click, not the first in document order. pub fn viewport_at_paper_point(&self, px: f32, py: f32) -> Option { let layout_block = self.current_layout_block_handle(); - self.document.entities().find_map(|e| { - let EntityType::Viewport(vp) = e else { - return None; - }; - if !self.is_content_viewport_in_layout(vp, layout_block) - || !vp.status.is_on - { - return None; - } - let hw = (vp.width / 2.0) as f32; - let hh = (vp.height / 2.0) as f32; - let cx = vp.center.x as f32; - let cy = vp.center.y as f32; - if px >= cx - hw && px <= cx + hw && py >= cy - hh && py <= cy + hh { - Some(vp.common.handle) - } else { - None - } - }) + self.document + .entities() + .filter_map(|e| { + let EntityType::Viewport(vp) = e else { + return None; + }; + if !self.is_content_viewport_in_layout(vp, layout_block) || !vp.status.is_on { + return None; + } + let hw = (vp.width / 2.0) as f32; + let hh = (vp.height / 2.0) as f32; + let cx = vp.center.x as f32; + let cy = vp.center.y as f32; + if px >= cx - hw && px <= cx + hw && py >= cy - hh && py <= cy + hh { + Some((vp.common.handle, (vp.width * vp.height).abs())) + } else { + None + } + }) + .min_by(|a, b| a.1.partial_cmp(&b.1).unwrap_or(std::cmp::Ordering::Equal)) + .map(|(h, _)| h) + } + + /// Like [`viewport_at_paper_point`] but tested against each viewport's + /// *visible* on-screen rectangle (clamped to the canvas), in screen pixels. + /// Viewport activation goes through this so a click only enters a viewport + /// when it lands on the part the user can actually see — clicking the empty + /// area beside a viewport that runs off-screen no longer matches its full + /// (partly off-canvas) paper rect and switches to it by mistake. + pub fn viewport_at_screen_point( + &self, + px: f32, + py: f32, + canvas: (f32, f32), + ) -> Option { + let layout_block = self.current_layout_block_handle(); + self.document + .entities() + .filter_map(|e| { + let EntityType::Viewport(vp) = e else { + return None; + }; + if !self.is_content_viewport_in_layout(vp, layout_block) || !vp.status.is_on { + return None; + } + let rect = self.viewport_screen_rect(vp.common.handle, canvas)?; + let x0 = rect.x.max(0.0); + let y0 = rect.y.max(0.0); + let x1 = (rect.x + rect.width).min(canvas.0); + let y1 = (rect.y + rect.height).min(canvas.1); + if x1 <= x0 || y1 <= y0 { + return None; // fully off-canvas → nothing to click + } + if px >= x0 && px <= x1 && py >= y0 && py <= y1 { + Some((vp.common.handle, (x1 - x0) * (y1 - y0))) + } else { + None + } + }) + .min_by(|a, b| a.1.partial_cmp(&b.1).unwrap_or(std::cmp::Ordering::Equal)) + .map(|(h, _)| h) } /// Return the handle of the first active user viewport in the current layout,