From d50373951f742c375ff65a9a96b766da88e94eef Mon Sep 17 00:00:00 2001 From: Tobias Hunger Date: Sun, 21 Jan 2024 22:12:03 +0100 Subject: [PATCH] lsp: Use a more "visual" element selection for Preview UI Try a more visual selection approach over the rather technical tree-based one from earlier. In this commit, a click selects the element that was rendered at the click position *last*. I use rendered loosely here: It even takes elements into account that are invisible but still cover the clicked location. A double-click punshes through to the item rendered earlier (behind) the currently selected item that also covers the clicked position. A shift-double-click moves towards the later rendered elements again, usually undoing the last double-click. --- tools/lsp/preview.rs | 4 +- tools/lsp/preview/element_selection.rs | 248 ++++++++++++------------- tools/lsp/preview/native.rs | 11 +- tools/lsp/preview/ui.rs | 3 +- tools/lsp/preview/wasm.rs | 11 +- tools/lsp/ui/draw-area.slint | 11 +- tools/lsp/ui/main.slint | 6 +- 7 files changed, 142 insertions(+), 152 deletions(-) diff --git a/tools/lsp/preview.rs b/tools/lsp/preview.rs index 2c22ce3a9..5d83e9884 100644 --- a/tools/lsp/preview.rs +++ b/tools/lsp/preview.rs @@ -390,7 +390,7 @@ pub fn reset_selections(ui: &ui::PreviewUi) { pub fn set_selections( ui: Option<&ui::PreviewUi>, - element_position: Option<(&ElementRc, LogicalRect)>, + element_position: Option<(&ElementRc, LogicalRect, usize)>, positions: ComponentPositions, ) { let Some(ui) = ui else { @@ -402,7 +402,7 @@ pub fn set_selections( positions.geometries.len() + if element_position.is_some() { 1 } else { 0 }, ); - if let Some((e, primary_position)) = element_position.as_ref() { + if let Some((e, primary_position, _)) = element_position.as_ref() { let border_color = if e.borrow().layout.is_some() { i_slint_core::Color::from_argb_encoded(0xffff0000) } else { diff --git a/tools/lsp/preview/element_selection.rs b/tools/lsp/preview/element_selection.rs index f4e2b5cc4..4a3625d2e 100644 --- a/tools/lsp/preview/element_selection.rs +++ b/tools/lsp/preview/element_selection.rs @@ -63,74 +63,26 @@ pub fn unselect_element() { } fn select_element( - x: f32, - y: f32, component_instance: &ComponentInstance, - selected_element: Option<&ElementRc>, -) -> Option { - let click_position = LogicalPoint::from_lengths(LogicalLength::new(x), LogicalLength::new(y)); - - let Some(c) = selected_element else { - unselect_element(); - return None; + selected_element: &ElementRc, + layer: usize, +) { + eprintln!(" select_element({}, {layer})", selected_element.borrow().id); + let Some(position) = component_instance.element_position(&selected_element) else { + return; }; - let Some(position) = component_instance.element_position(&c) else { - return None; + let secondary_positions = if let Some((path, offset)) = element_offset(selected_element) { + component_instance.component_positions(path, offset) + } else { + ComponentPositions::default() }; - if position.contains(click_position) { - let secondary_positions = if let Some((path, offset)) = element_offset(&c) { - component_instance.component_positions(path, offset) - } else { - ComponentPositions::default() - }; - super::set_selected_element(Some((&c, position)), secondary_positions); - let document_position = lsp_element_position(&c); - if !document_position.0.is_empty() { - super::ask_editor_to_show_document(document_position.0, document_position.1); - } - return Some(c.clone()); + super::set_selected_element(Some((&selected_element, position, layer)), secondary_positions); + let document_position = lsp_element_position(&selected_element); + if !document_position.0.is_empty() { + super::ask_editor_to_show_document(document_position.0, document_position.1); } - - None -} - -// triggered from the UI, running in UI thread -pub fn select_element_at_impl( - x: f32, - y: f32, - component_instance: &ComponentInstance, - root_element: &ElementRc, - current_element: Option<&ElementRc>, - reverse: bool, -) -> Option { - let re = root_element.borrow(); - let mut fw_iter = re.children.iter(); - let mut bw_iter = re.children.iter().rev(); - - let iterator: &mut dyn Iterator = - if reverse { &mut bw_iter } else { &mut fw_iter }; - - let mut skip = current_element.is_some(); - for c in &mut *iterator { - let c = self_or_embedded_component_root(c); - - if skip { - if let Some(ce) = current_element { - if Rc::ptr_eq(ce, &c) { - skip = false; - } - } - continue; - } - - if let Some(result) = select_element(x, y, component_instance, Some(&c)) { - return Some(result); - } - } - - None } fn element_offset(element: &ElementRc) -> Option<(PathBuf, u32)> { @@ -176,23 +128,84 @@ fn root_element(component_instance: &ComponentInstance) -> ElementRc { } } -fn parent_element(root_element: &ElementRc, element: &ElementRc) -> Option { - for c in &root_element.borrow().children { - if Rc::ptr_eq(c, element) { - return Some(root_element.clone()); +fn visit_tree_element( + x: f32, + y: f32, + component_instance: &ComponentInstance, + root_element: &ElementRc, + current_element: &ElementRc, + target_layer: usize, + current_layer: usize, + switch_files: bool, + previous: &(usize, Option), +) -> ((usize, Option), (usize, Option)) { + let ce = self_or_embedded_component_root(current_element); + + let mut current_layer = current_layer; + let mut previous = previous.clone(); + + for c in ce.borrow().children.iter().rev() { + let (p, (ncl, fe)) = visit_tree_element( + x, + y, + component_instance, + root_element, + c, + target_layer, + current_layer, + switch_files, + &previous, + ); + + current_layer = ncl; + previous = p; + + if fe.is_some() { + return (previous, (current_layer, fe)); } } - for c in &root_element.borrow().children { - if let Some(p) = parent_element(c, element) { - return Some(p); + if element_covers_point(x, y, component_instance, &ce) + && !Rc::ptr_eq(current_element, root_element) + { + current_layer += 1; + + let same_source = (|| { + let Some(re) = &root_element.borrow().node else { + return false; + }; + let Some(ce) = &ce.borrow().node else { + return false; + }; + Rc::ptr_eq(&re.source_file, &ce.source_file) + })(); + let file_ok = switch_files || same_source; + + if file_ok && current_layer < target_layer && current_layer > previous.0 { + eprintln!( + " visit: {x},{y} (target: {target_layer}/{current_layer}): {} => Found prev candidate in self!", + ce.borrow().id + ); + previous = (current_layer, Some(ce.clone())) + } + + if file_ok && current_layer > target_layer { + eprintln!( + " visit: {x},{y} (target: {target_layer}/{current_layer}): {} => Found next in self!", + ce.borrow().id + ); + return (previous, (current_layer, Some(ce))); + } + + if file_ok && current_layer < target_layer { + previous = (current_layer, Some(ce.clone())); } } - None + // eprintln!(" visit: {x},{y} (target: {target_layer}/{current_layer}): {} => mot found", current_element.borrow().id); + (previous, (current_layer, None)) } -// triggered from the UI, running in UI thread pub fn select_element_at(x: f32, y: f32) { let Some(component_instance) = super::component_instance() else { return; @@ -200,75 +213,54 @@ pub fn select_element_at(x: f32, y: f32) { let root_element = root_element(&component_instance); - if let Some(selected_element) = super::selected_element() { + if let Some((selected_element, _)) = super::selected_element() { if element_covers_point(x, y, &component_instance, &selected_element) { // We clicked on the already selected element: Do nothing! return; } - - let mut parent = parent_element(&root_element, &selected_element); - while let Some(p) = &parent { - if select_element_at_impl(x, y, &component_instance, p, None, true).is_some() { - return; - } - parent = parent_element(&root_element, p); - } } - select_element_at_impl(x, y, &component_instance, &root_element, None, true); + let (_, (layer, next)) = visit_tree_element( + x, + y, + &component_instance, + &root_element, + &root_element, + 0, + 0, + false, + &(0, None), + ); + if let Some(n) = next { + select_element(&component_instance, &n, layer); + } } -// triggered from the UI, running in UI thread -pub fn select_element_down(x: f32, y: f32, reverse: bool) { +pub fn select_element_behind(x: f32, y: f32, switch_files: bool, reverse: bool) { let Some(component_instance) = super::component_instance() else { return; }; - // We have an actively selected element (via the earlier click-event :-): - let Some(selected_element) = super::selected_element() else { - return; - }; + let root_element = root_element(&component_instance); + let target_layer = super::selected_element().map(|(_, l)| l).unwrap_or_default(); + eprintln!("select_element_behind: {x},{y} (switch: {switch_files}, reverse: {reverse}), target: {target_layer}"); - if !reverse { - let _ = select_element_at_impl(x, y, &component_instance, &selected_element, None, true); - } else { - if element_covers_point(x, y, &component_instance, &selected_element) { - let _ = select_element( - x, - y, - &component_instance, - parent_element(&root_element(&component_instance), &selected_element).as_ref(), - ); - } - } -} - -// triggered from the UI, running in UI thread -pub fn select_element_front_to_back(x: f32, y: f32, reverse: bool) { - let Some(component_instance) = super::component_instance() else { - return; - }; - - // We have an actively selected element (via the earlier click-event :-): - let Some(selected_element) = super::selected_element() else { - return; - }; - - if element_covers_point(x, y, &component_instance, &selected_element) { - let Some(parent_element) = - parent_element(&root_element(&component_instance), &selected_element) - else { - return; - }; - // We clicked on the already selected element: Do nothing! - let _ = select_element_at_impl( - x, - y, - &component_instance, - &parent_element, - Some(&selected_element), - !reverse, - ); - return; + let (previous, next) = visit_tree_element( + x, + y, + &component_instance, + &root_element, + &root_element, + target_layer, + 0, + switch_files, + &(0, None), + ); + eprintln!("select_element_behind: {x},{y} (switch: {switch_files}, reverse: {reverse}) => Prev: {:?}, Next: {:?}", previous.1.as_ref().map(|e| e.borrow().id.clone()), next.1.as_ref().map(|e| e.borrow().id.clone())); + let to_select = if reverse { previous } else { next }; + eprintln!("select_element_behind: {x},{y} (switch: {switch_files}, reverse: {reverse}) => To select: {:?}", to_select.1.as_ref().map(|e| e.borrow().id.clone())); + if let (layer, Some(ts)) = to_select { + eprintln!("select_element_behind: {x},{y} (switch: {switch_files}, reverse: {reverse}) => SETTING {}@{layer}", ts.borrow().id); + select_element(&component_instance, &ts, layer); } } diff --git a/tools/lsp/preview/native.rs b/tools/lsp/preview/native.rs index 6d84ae517..5bad06f96 100644 --- a/tools/lsp/preview/native.rs +++ b/tools/lsp/preview/native.rs @@ -211,33 +211,36 @@ struct PreviewState { ui: Option, handle: Rc>>, selected_element: Option, + selected_layer: usize, } thread_local! {static PREVIEW_STATE: std::cell::RefCell = Default::default();} pub fn set_selected_element( - element_position: Option<(&ElementRc, LogicalRect)>, + element_position: Option<(&ElementRc, LogicalRect, usize)>, positions: slint_interpreter::highlight::ComponentPositions, ) { PREVIEW_STATE.with(move |preview_state| { let mut preview_state = preview_state.borrow_mut(); - if let Some((e, _)) = element_position.as_ref() { + if let Some((e, _, l)) = element_position.as_ref() { preview_state.selected_element = Some(Rc::downgrade(e)); + preview_state.selected_layer = *l; } else { preview_state.selected_element = None; + preview_state.selected_layer = 0; } super::set_selections(preview_state.ui.as_ref(), element_position, positions); }) } -pub fn selected_element() -> Option { +pub fn selected_element() -> Option<(ElementRc, usize)> { PREVIEW_STATE.with(move |preview_state| { let preview_state = preview_state.borrow(); let Some(weak) = &preview_state.selected_element else { return None; }; - Weak::upgrade(&weak) + Weak::upgrade(&weak).map(|e| (e, preview_state.selected_layer)) }) } diff --git a/tools/lsp/preview/ui.rs b/tools/lsp/preview/ui.rs index 017af6da3..55f38a84f 100644 --- a/tools/lsp/preview/ui.rs +++ b/tools/lsp/preview/ui.rs @@ -44,8 +44,7 @@ pub fn create_ui(style: String) -> Result { }); ui.on_unselect(super::element_selection::unselect_element); ui.on_select_at(super::element_selection::select_element_at); - ui.on_select_front_to_back(super::element_selection::select_element_front_to_back); - ui.on_select_down(super::element_selection::select_element_down); + ui.on_select_behind(super::element_selection::select_element_behind); ui.on_can_drop(super::can_drop_component); ui.on_drop(super::drop_component); diff --git a/tools/lsp/preview/wasm.rs b/tools/lsp/preview/wasm.rs index 1f009136d..d9d96652d 100644 --- a/tools/lsp/preview/wasm.rs +++ b/tools/lsp/preview/wasm.rs @@ -48,16 +48,17 @@ struct PreviewState { lsp_notifier: Option, resource_url_mapper: Option, selected_element: Option, + selected_layer: usize, } thread_local! {static PREVIEW_STATE: std::cell::RefCell = Default::default();} -pub fn selected_element() -> Option { +pub fn selected_element() -> Option<(ElementRc, usize)> { PREVIEW_STATE.with(move |preview_state| { let preview_state = preview_state.borrow(); let Some(weak) = &preview_state.selected_element else { return None; }; - Weak::upgrade(&weak) + Weak::upgrade(&weak).map(|e| (e, preview_state.selected_layer)) }) } @@ -68,15 +69,17 @@ pub fn component_instance() -> Option { } pub fn set_selected_element( - element_position: Option<(&ElementRc, LogicalRect)>, + element_position: Option<(&ElementRc, LogicalRect, usize)>, positions: ComponentPositions, ) { PREVIEW_STATE.with(move |preview_state| { let mut preview_state = preview_state.borrow_mut(); - if let Some((e, _)) = element_position.as_ref() { + if let Some((e, _, l)) = element_position.as_ref() { preview_state.selected_element = Some(Rc::downgrade(e)); + preview_state.selected_layer = *l; } else { preview_state.selected_element = None; + preview_state.selected_layer = 0; } super::set_selections(preview_state.ui.as_ref(), element_position, positions); diff --git a/tools/lsp/ui/draw-area.slint b/tools/lsp/ui/draw-area.slint index 432633011..ad9f0c728 100644 --- a/tools/lsp/ui/draw-area.slint +++ b/tools/lsp/ui/draw-area.slint @@ -36,8 +36,7 @@ export component DrawArea { out property preview-area-height: preview-visible ? i-preview-area-container.height : 0px; callback select-at(/* x */ length, /* y */ length); - callback select-front-to-back(/* x */ length, /* y */ length, /* reverse? */ bool); - callback select-down(/* x */ length, /* y */ length, /* reverse? */ bool); + callback select-behind(/* x */ length, /* y */ length, /* stay_in_file? */ bool, /* reverse */ bool); callback show-document(/* url */ string, /* line */ int, /* column */ int); callback unselect(); @@ -121,13 +120,9 @@ export component DrawArea { // This needs to fire AFTER clicked and double-clicked to work :-/ if (event.kind == PointerEventKind.up) { if (self.selection-kind == SelectionKind.select_up_or_down) { - root.select_down(self.selection-x, self.selection-y, event.modifiers.shift); + root.select_behind(self.selection-x, self.selection-y, event.modifiers.control, event.modifiers.shift); } else if (self.selection-kind == SelectionKind.select-at) { - if (event.modifiers.control) { - root.select-front-to-back(self.selection-x, self.selection-y, event.modifiers.shift); - } else { - root.select-at(self.selection-x, self.selection-y); - } + root.select-at(self.selection-x, self.selection-y); } } self.selection-kind = SelectionKind.none; diff --git a/tools/lsp/ui/main.slint b/tools/lsp/ui/main.slint index 0765402b6..af3bcd144 100644 --- a/tools/lsp/ui/main.slint +++ b/tools/lsp/ui/main.slint @@ -26,8 +26,7 @@ export component PreviewUi inherits Window { callback can-drop(/* component_type */ string, /* x */ length, /* y */ length) -> bool; callback drop(/* component_type */ string, /* x */ length, /* y */ length); callback select-at(/* x */ length, /* y */ length); - callback select-down(/* x */ length, /* y */ length, /* upwards? */ bool); - callback select-front-to-back(/* x */ length, /* y */ length, /* backwards? */ bool); + callback select-behind(/* x */ length, /* y */ length, /* stay_in_file* */ bool, /* reverse */ bool); callback show-document(/* url */ string, /* line */ int, /* column */ int); callback style-changed(); callback unselect(); @@ -110,8 +109,7 @@ export component PreviewUi inherits Window { selections <=> root.selections; select-at(x, y) => { root.select-at(x, y); } - select-front-to-back(x, y, front) => { root.select-front-to-back(x, y, front); } - select-down(x, y, up) => { root.select-down(x, y, up); } + select-behind(x, y, stay_in_file, reverse) => { root.select-behind(x, y, stay_in_file, reverse); } show-document(url, line, column) => { root.show-document(url, line, column); } unselect() => { root.unselect(); } }