From 044457e6bbd7e82e84027b88788ee08a3971d93a Mon Sep 17 00:00:00 2001 From: Alain Emilia Anna Zscheile Date: Thu, 2 Jan 2025 19:43:00 +0100 Subject: [PATCH] fix(egui): BBox selection should be solely triggered by drag To complete the interface, menu buttons for selecting everything and nothing were added. Trigger short-keys were taken from KiCAD. Requested-By: @mikolaj https://codeberg.org/topola/topola/pulls/139#issuecomment-2561084 --- crates/topola-egui/src/actions.rs | 32 ++++++-------- crates/topola-egui/src/menu_bar.rs | 15 +++---- crates/topola-egui/src/overlay.rs | 68 +++++++++++++----------------- crates/topola-egui/src/viewport.rs | 12 ++---- locales/en-US/main.ftl | 5 +-- 5 files changed, 53 insertions(+), 79 deletions(-) diff --git a/crates/topola-egui/src/actions.rs b/crates/topola-egui/src/actions.rs index 7402b21..c9dfbdc 100644 --- a/crates/topola-egui/src/actions.rs +++ b/crates/topola-egui/src/actions.rs @@ -78,9 +78,8 @@ pub struct EditActions { pub undo: Trigger, pub redo: Trigger, pub abort: Trigger, - pub reset_bbox: Trigger, - pub reselect_bbox: Trigger, - pub toggle_all_in_bbox: Trigger, + pub select_all: Trigger, + pub unselect_all: Trigger, pub remove_bands: Trigger, } @@ -105,21 +104,15 @@ impl EditActions { egui::Key::Escape, ) .into_trigger(), - reset_bbox: Action::new( - tr.text("tr-menu-edit-reset-bbox"), + select_all: Action::new( + tr.text("tr-menu-edit-select-all"), egui::Modifiers::CTRL, - egui::Key::B, + egui::Key::A, // taken from KiCAD ) .into_trigger(), - reselect_bbox: Action::new( - tr.text("tr-menu-edit-reselect-bbox"), - egui::Modifiers::NONE, - egui::Key::B, - ) - .into_trigger(), - toggle_all_in_bbox: Action::new( - tr.text("tr-menu-edit-toggle-all-in-bbox"), - egui::Modifiers::NONE, + unselect_all: Action::new( + tr.text("tr-menu-edit-unselect-all"), + egui::Modifiers::CTRL | egui::Modifiers::SHIFT, egui::Key::A, ) .into_trigger(), @@ -149,9 +142,10 @@ impl EditActions { ui.separator(); - self.reset_bbox.button(ctx, ui); - self.reselect_bbox.button(ctx, ui); - self.toggle_all_in_bbox.button(ctx, ui); + self.select_all.button(ctx, ui); + self.unselect_all.button(ctx, ui); + + ui.separator(); //ui.add_enabled_ui(workspace_activities_enabled, |ui| { self.remove_bands.button(ctx, ui); @@ -199,7 +193,7 @@ impl RouteActions { autoroute: Action::new( tr.text("tr-menu-route-autoroute"), egui::Modifiers::CTRL, - egui::Key::A, + egui::Key::R, ) .into_trigger(), } diff --git a/crates/topola-egui/src/menu_bar.rs b/crates/topola-egui/src/menu_bar.rs index 62764c0..1caeeed 100644 --- a/crates/topola-egui/src/menu_bar.rs +++ b/crates/topola-egui/src/menu_bar.rs @@ -232,17 +232,12 @@ impl MenuBar { workspace.interactor.redo(); } else if actions.edit.abort.consume_key_triggered(ctx, ui) { workspace.interactor.abort(); - } else if actions.edit.reset_bbox.consume_key_triggered(ctx, ui) { - workspace.overlay.reset_selected_bbox(); - } else if actions.edit.reselect_bbox.consume_key_triggered(ctx, ui) { - workspace.overlay.start_bbox_reselect(); - } else if actions - .edit - .toggle_all_in_bbox - .consume_key_triggered(ctx, ui) - { + } else if actions.edit.unselect_all.consume_key_triggered(ctx, ui) { + // NOTE: we need to check `unselect` first because `Ctrl+A` would also match `Ctrl+Shift+A` + workspace.overlay.unselect_all(); + } else if actions.edit.select_all.consume_key_triggered(ctx, ui) { let board = workspace.interactor.invoker().autorouter().board(); - workspace.overlay.toggle_all_in_selected_bbox(board); + workspace.overlay.select_all(board); } else if actions.place.place_via.consume_key_enabled( ctx, ui, diff --git a/crates/topola-egui/src/overlay.rs b/crates/topola-egui/src/overlay.rs index 269392b..7aba370 100644 --- a/crates/topola-egui/src/overlay.rs +++ b/crates/topola-egui/src/overlay.rs @@ -25,18 +25,17 @@ use topola::{ pub struct Overlay { ratsnest: Ratsnest, selection: Selection, - pub selected_bbox: AABB<[f64; 2]>, - reselect_bbox: Option>, + reselect_bbox: Option, active_layer: usize, } +const INF: f64 = f64::INFINITY; + impl Overlay { pub fn new(board: &Board) -> Result { - const INF: f64 = f64::INFINITY; Ok(Self { ratsnest: Ratsnest::new(board.layout())?, selection: Selection::new(), - selected_bbox: AABB::from_corners([-INF, -INF], [INF, INF]), reselect_bbox: None, active_layer: 0, }) @@ -46,44 +45,38 @@ impl Overlay { core::mem::replace(&mut self.selection, Selection::new()) } - pub fn clear_selection(&mut self) { + pub fn select_all(&mut self, board: &Board) { + self.select_all_in_bbox(board, &AABB::from_corners([-INF, -INF], [INF, INF])); + } + + pub fn unselect_all(&mut self) { self.selection = Selection::new(); - } - - pub fn reset_selected_bbox(&mut self) { - const INF: f64 = f64::INFINITY; - self.selected_bbox = AABB::from_corners([-INF, -INF], [INF, INF]); - } - - pub fn start_bbox_reselect(&mut self) { - self.reselect_bbox = Some(None); + self.reselect_bbox = None; } pub fn drag_start(&mut self, board: &Board, at: Point) { - if let None | Some(None) = self.reselect_bbox { + if self.reselect_bbox.is_none() { // handle bounding box selection - self.reselect_bbox = Some(Some(at)); + self.reselect_bbox = Some(at); } } pub fn drag_stop(&mut self, board: &Board, at: Point) { if let Some(aabb) = self.get_bbox_reselect(at) { // handle bounding box selection - self.selected_bbox = aabb; + self.select_all_in_bbox(board, &aabb); self.reselect_bbox = None; } } pub fn click(&mut self, board: &Board, at: Point) { - if let Some(rsbb) = self.reselect_bbox.take() { + if let Some(pt) = self.reselect_bbox.take() { // handle bounding box selection (takes precendence over other interactions) - self.reselect_bbox = match rsbb { - None => Some(Some(at)), - Some(pt) => { - self.selected_bbox = AABB::from_corners([pt.x(), pt.y()], [at.x(), at.y()]); - None - } - }; + // this is mostly in order to allow the user to recover from a missed/dropped drag_stop event + self.select_all_in_bbox( + board, + &AABB::from_corners([pt.x(), pt.y()], [at.x(), at.y()]), + ); return; } @@ -107,11 +100,13 @@ impl Overlay { } } - pub fn toggle_all_in_selected_bbox(&mut self, board: &Board) { + pub fn select_all_in_bbox( + &mut self, + board: &Board, + aabb: &AABB<[f64; 2]>, + ) { use rstar::Envelope; - let aabb = self.selected_bbox; - let mut temp_selection = Selection::new(); - for node in board + for &geom in board .layout() .drawing() .rtree() @@ -119,15 +114,14 @@ impl Overlay { [aabb.lower()[0], aabb.lower()[1], -f64::INFINITY], [aabb.upper()[0], aabb.upper()[1], f64::INFINITY], )) - .map(|&geom| geom.data) - .filter(|&node| { - aabb.contains_envelope(&board.layout().node_bbox(node)) - && board.layout().is_node_in_layer(node, self.active_layer) - }) { - temp_selection.select_at_node(board, node); + let node = geom.data; + if aabb.contains_envelope(&board.layout().node_bbox(node)) + && board.layout().is_node_in_layer(node, self.active_layer) + { + self.selection.select_at_node(board, node); + } } - self.selection ^= &temp_selection; } pub fn ratsnest(&self) -> &Ratsnest { @@ -141,8 +135,6 @@ impl Overlay { /// Returns the currently selected bounding box of a bounding-box reselect pub fn get_bbox_reselect(&self, at: Point) -> Option> { self.reselect_bbox - .as_ref() - .and_then(|pt| *pt) .map(|pt| AABB::from_corners([pt.x(), pt.y()], [at.x(), at.y()])) } } diff --git a/crates/topola-egui/src/viewport.rs b/crates/topola-egui/src/viewport.rs index 17b1e70..d7c15fa 100644 --- a/crates/topola-egui/src/viewport.rs +++ b/crates/topola-egui/src/viewport.rs @@ -74,7 +74,7 @@ impl Viewport { let latest_point = point! {x: latest_pos.x as f64, y: -latest_pos.y as f64}; let board = workspace.interactor.invoker().autorouter().board(); - if response.clicked() { + if response.clicked_by(egui::PointerButton::Primary) { if menu_bar.is_placing_via { workspace.interactor.execute(Command::PlaceVia(ViaWeight { from_layer: 0, @@ -92,9 +92,9 @@ impl Viewport { } else { overlay.click(board, latest_point); } - } else if response.drag_started() { + } else if response.drag_started_by(egui::PointerButton::Primary) { overlay.drag_start(board, latest_point); - } else if response.drag_stopped() { + } else if response.drag_stopped_by(egui::PointerButton::Primary) { overlay.drag_stop(board, latest_point); } else if let Some(cur_bbox) = overlay.get_bbox_reselect(latest_point) { painter.paint_bbox_with_color(cur_bbox, egui::Color32::RED); @@ -271,12 +271,6 @@ impl Viewport { [root_bbox3d.upper()[0], root_bbox3d.upper()[1]].into(), ); painter.paint_bbox(root_bbox); - - let selected_bbox = overlay.selected_bbox; - const INF: f64 = f64::INFINITY; - if selected_bbox != AABB::from_corners([-INF, -INF], [INF, INF]) { - painter.paint_bbox_with_color(selected_bbox, egui::Color32::BLUE); - } } if let Some(activity) = workspace.interactor.maybe_activity() { diff --git a/locales/en-US/main.ftl b/locales/en-US/main.ftl index eb490da..f693756 100644 --- a/locales/en-US/main.ftl +++ b/locales/en-US/main.ftl @@ -12,9 +12,8 @@ tr-menu-edit = Edit tr-menu-edit-undo = Undo tr-menu-edit-redo = Redo tr-menu-edit-abort = Abort -tr-menu-edit-reset-bbox = Reset selected BBox -tr-menu-edit-reselect-bbox = (Re-)select BBox -tr-menu-edit-toggle-all-in-bbox = Toggle all entries in selected BBox +tr-menu-edit-select-all = Select All +tr-menu-edit-unselect-all = Unselect All tr-menu-edit-remove-bands = Remove Bands tr-menu-view = View