From 033ee15e0146eac3a686c832580fdd804542fe13 Mon Sep 17 00:00:00 2001 From: GJoe2 <71229258+GJoe2@users.noreply.github.com> Date: Tue, 13 Oct 2026 05:16:24 -0500 Subject: [PATCH] Fix double-clicking layout viewports creating persistent selection rectangle (#830) Double-clicking a viewport to enter MSPACE or sheet to enter PSPACE triggers a primary click on the first click that starts an idle selection window (pending_window). When the second click triggers dbl_primary, the handler executed an early return, leaving pending_window open and rendering an unwanted crossing/window rectangle across the screen. Reset pending_window and drag_window on double clicks and suppress primary click dispatch. Co-authored-by: Brandon Thomas Co-authored-by: Claude Opus 5.5 --- contributors/people.toml | 3 ++ crates/engine/src/cmd/layout.rs | 2 + crates/engine/src/cmd/view.rs | 1 + crates/ui-egui/src/canvas.rs | 13 ++++- .../tests/viewport_double_click_selection.rs | 54 +++++++++++++++++++ 5 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 crates/ui-egui/tests/viewport_double_click_selection.rs diff --git a/contributors/people.toml b/contributors/people.toml index 86860a02..17f56c2a 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -49,6 +49,9 @@ sync_github_name = true [people.Losminek] sync_github_name = true +[people.GJoe2] +sync_github_name = true + [people.cyfung1031] real_name = "Chester Fung" display_name = "Chester Fung" diff --git a/crates/engine/src/cmd/layout.rs b/crates/engine/src/cmd/layout.rs index eecb2aa5..cac58520 100644 --- a/crates/engine/src/cmd/layout.rs +++ b/crates/engine/src/cmd/layout.rs @@ -136,6 +136,7 @@ fn run_mspace(s: &mut Session, p: &Value) -> Result { let st = s.state_mut()?; st.mspace = Some(pick); st.selection.clear(); + s.pending_window = None; s.echo("MSPACE"); Ok(json!({ "viewport": pick.hex() })) } @@ -147,6 +148,7 @@ fn run_pspace(s: &mut Session, _p: &Value) -> Result { let st = s.state_mut()?; st.mspace = None; st.selection.clear(); + s.pending_window = None; Ok(json!({ "space": "paper" })) } diff --git a/crates/engine/src/cmd/view.rs b/crates/engine/src/cmd/view.rs index 47762b32..1c7cbb64 100644 --- a/crates/engine/src/cmd/view.rs +++ b/crates/engine/src/cmd/view.rs @@ -279,6 +279,7 @@ fn run_layout_set(s: &mut Session, p: &Value) -> Result { let l = s.doc()?.layout(name).ok_or_else(|| bad("layout.set", format!("no layout `{name}`")))?; Space::Paper(l.name.clone()) }; + s.pending_window = None; let st = s.state_mut()?; st.space = space.clone(); st.mspace = None; diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 0b18f0a3..28e17c3a 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -1286,9 +1286,13 @@ pub fn show(app: &mut CadApp, ui: &mut egui::Ui) { let in_active = active_vp.as_ref().is_some_and(|v| active_clip.as_ref().map_or(in_rect(v), |c| c.contains(pp))); if !in_active { app.canvas.hot_grips.clear(); + app.session.pending_window = None; + app.canvas.drag_window = false; if app.run("mspace", serde_json::json!({"at": [pp.x, pp.y]})).is_err() && active_vp.is_some() { let _ = app.run("pspace", serde_json::json!({})); } + app.session.pending_window = None; + app.canvas.drag_window = false; app.canvas.list = None; return; } @@ -1400,7 +1404,14 @@ pub fn show(app: &mut CadApp, ui: &mut egui::Ui) { None => app.cmd.options = None, } } - let pressed_primary = pressed_primary && !picked && !hatch_dialog; + // A double-click with no command running (on a layout viewport, say) leaves no selection + // window behind; while a command asks for points, every click still counts (#815). + let idle_dbl = dbl_primary && app.session.running.is_none(); + if idle_dbl { + app.session.pending_window = None; + app.canvas.drag_window = false; + } + let pressed_primary = pressed_primary && !picked && !hatch_dialog && !idle_dbl; // Clicks. if inside && pressed_primary diff --git a/crates/ui-egui/tests/viewport_double_click_selection.rs b/crates/ui-egui/tests/viewport_double_click_selection.rs new file mode 100644 index 00000000..66bcae18 --- /dev/null +++ b/crates/ui-egui/tests/viewport_double_click_selection.rs @@ -0,0 +1,54 @@ +use cadcraft_doc::{Common, EntityKind, Space, Viewport}; +use cadcraft_engine::Session; +use cadcraft_geom::Vec2; +use cadcraft_ui_egui::{CadApp, Services}; +use egui::vec2; +use egui_kittest::Harness; +use serde_json::json; + +#[test] +fn double_click_viewport_does_not_leave_pending_window() { + let mut session = Session::new(); + session.new_drawing(true); + session.viewport_px = (1600.0, 1000.0); + + let paper = Space::Paper("Layout1".into()); + let vp = Viewport { + id: 2, + center: cadcraft_geom::Vec3::new(70.0, 60.0, 0.0), + width: 100.0, + height: 80.0, + view_center: Vec2::new(500.0, 250.0), + view_height: 500.0, + ..Default::default() + }; + session.doc_mut().unwrap().add(&paper, Common::default(), EntityKind::Viewport(vp)).unwrap(); + session.execute("layout.set", &json!({"name": "Layout1"})).unwrap(); + + let mut app = CadApp::new(session, Services::default()); + app.canvas.zoom_pending = false; + + let mut h = Harness::builder().with_size(vec2(1600.0, 1000.0)).build_ui_state( + |ui, app: &mut CadApp| { + app.logic(ui.ctx()); + app.ui(ui); + }, + app, + ); + h.run_steps(3); + + // Simulate double click at viewport center + let screen_pos = h.state().canvas.xf.unwrap().to_screen(Vec2::new(70.0, 60.0)); + // First click sets pending window + h.event(egui::Event::PointerMoved(screen_pos)); + h.event(egui::Event::PointerButton { pos: screen_pos, button: egui::PointerButton::Primary, pressed: true, modifiers: egui::Modifiers::NONE }); + h.event(egui::Event::PointerButton { pos: screen_pos, button: egui::PointerButton::Primary, pressed: false, modifiers: egui::Modifiers::NONE }); + h.run_steps(1); + + // Second click (egui triggers double click) + h.event(egui::Event::PointerButton { pos: screen_pos, button: egui::PointerButton::Primary, pressed: true, modifiers: egui::Modifiers::NONE }); + h.event(egui::Event::PointerButton { pos: screen_pos, button: egui::PointerButton::Primary, pressed: false, modifiers: egui::Modifiers::NONE }); + h.run_steps(3); + + assert!(h.state().session.pending_window.is_none(), "double clicking a viewport must not leave a pending selection window"); +}