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 <[email protected]>
Co-authored-by: Claude Opus 5.5 <[email protected]>
This commit is contained in:
authored and GitHub committed 2026-10-13 06:16:24 -04:00
1 parent 4a9dd31a58
commit 033ee15e01
5 files changed
+72 -1

No files matched your search

+3
View File
@@ -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"
+2
View File
@@ -136,6 +136,7 @@ fn run_mspace(s: &mut Session, p: &Value) -> Result<Value> {
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<Value> {
let st = s.state_mut()?;
st.mspace = None;
st.selection.clear();
s.pending_window = None;
Ok(json!({ "space": "paper" }))
}
+1
View File
@@ -279,6 +279,7 @@ fn run_layout_set(s: &mut Session, p: &Value) -> Result<Value> {
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;
+12 -1
View File
@@ -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
@@ -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");
}