From 990e4210cc215c3d77f0111d1c2e800803b61355 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Thu, 25 Jan 2024 15:15:36 +0100 Subject: [PATCH 01/28] WIP - `ViewportBlueprint`: added a bunch of tree-walking search functions - updated blueprint tree ui - fully isolated drop logic in a no-dependency file TODO: - ListItem: dont show icons when a drag is ongoing - emit Move tree action - highlight receiving container - remove arrow for root container --- crates/re_ui/src/drag_and_drop.rs | 304 ++++++++++++++++++ crates/re_ui/src/lib.rs | 1 + crates/re_viewer_context/src/blueprint_id.rs | 12 + crates/re_viewport/src/container.rs | 25 +- crates/re_viewport/src/viewport_blueprint.rs | 99 ++++++ .../re_viewport/src/viewport_blueprint_ui.rs | 160 ++++++++- 6 files changed, 591 insertions(+), 10 deletions(-) create mode 100644 crates/re_ui/src/drag_and_drop.rs diff --git a/crates/re_ui/src/drag_and_drop.rs b/crates/re_ui/src/drag_and_drop.rs new file mode 100644 index 000000000000..93bbc0098132 --- /dev/null +++ b/crates/re_ui/src/drag_and_drop.rs @@ -0,0 +1,304 @@ +//! Helpers for drag and drop support. Works well in combination with [`crate::list_item::ListItem`]. + +#[derive(Clone, Debug)] +pub struct DropTarget { + /// Range of X coordinates for the drag target indicator + pub indicator_span_x: egui::Rangef, + + /// Y coordinate for drag target indicator + pub indicator_position_y: f32, + + /// Destination container ID + pub target_parent_id: ItemId, + + /// Destination position within the container + pub target_position_index: usize, +} + +impl DropTarget { + fn new( + indicator_span_x: egui::Rangef, + indicator_position_y: f32, + target_parent_id: ItemId, + target_position_index: usize, + ) -> Self { + Self { + indicator_span_x, + indicator_position_y, + target_parent_id, + target_position_index, + } + } +} + +pub struct DropItemDescription { + /// ID of the item being hovered during drag + pub id: ItemId, + /// Can this item "contain" the currently dragged item? + pub is_container: bool, + /// ID of the parent if this item. + pub parent_id: ItemId, + /// Position of this item within its parent. + pub position_index_in_parent: usize, + /// ID of the container just before this item within the parent, if such a container exists. + pub previous_container_id: Option, +} + +/// Compute the geometry of the drag cursor and where the dragged item should be inserted. +/// +/// This function implements the following logic: +/// ```text +/// +/// insert insert last in container before me +/// before me (if any) or insert before me +/// │ │ +/// ╔═══▼═════════════════════════════▼══════════════════╗ +/// ║ │ ║ +/// leaf item ║ ─────┴──────────────────────────────────────────── ║ +/// ║ ║ +/// ╚═════════════════════▲══════════════════════════════╝ +/// │ +/// insert after me +/// +/// +/// insert insert last in container before me +/// before me (if any) or insert before me +/// │ │ +/// ╔═══▼═════════════════════════════▼══════════════════╗ +/// leaf item ║ │ ║ +/// with body ║ ─────┴──────────────────────────────────────────── ║ +/// ║ ║ +/// ╚══════╦══════════════════════════════════════▲══════╣ ─┐ +/// │ ║ │ ║ │ +/// │ ║ insert ║ │ +/// │ ║ after me ║ │ +/// │ ╠══ ══╣ │ +/// │ ║ no insertion possible ║ │ +/// │ ║ here by definition of ║ │ body +/// │ ║ parent being a leaf ║ │ +/// │ ╠══ ══╣ │ +/// │ ║ ║ │ +/// │ ║ ║ │ +/// │ ║ ║ │ +/// └──▲── ╚══════════════════════════▲══════════════════╝ ─┘ +/// │ │ +/// insert insert +/// after me after me +/// +/// +/// insert insert last in container before me +/// before me (if any) or insert before me +/// │ │ +/// ╔═══▼═════════════════════════════▼══════════════════╗ +/// container item ║ │ ║ +/// (empty/collapsed ║ ─────┼──────────────────────────────────────────── ║ +/// body) ║ │ ║ +/// ╚═══▲═════════════════════════════▲══════════════════╝ +/// │ │ +/// insert insert inside me +/// after me at pos = 0 +/// +/// +/// insert insert last in container before me +/// before me (if any) or insert before me +/// │ │ +/// ╔═══▼═════════════════════════════▼══════════════════╗ +/// container item ║ │ ║ +/// with body ║ ─────┴──────────────────────────────────────────── ║ +/// ║ ║ +/// ╚═▲════╦═════════════════════════════════════════════╣ ─┐ +/// │ ║ ║ │ +/// insert ║ ║ │ +/// inside me ║ ║ │ +/// at pos = 0 ╠══ ══╣ │ +/// ║ same logic ║ │ +/// ║ recursively ║ │ body +/// insert ║ applied here ║ │ +/// after me ╠══ ══╣ │ +/// │ ║ ║ │ +/// ┌─▼─── ║ ║ │ +/// │ ║ ║ │ +/// └───── ╚═════════════════════════════════════════════╝ ─┘ +/// ``` +/// +/// Here are a few observations of the above that help navigate the "if-statement-of-death" +/// in the implementation: +/// - The top parts of the item are treated the same in all four cases. +/// - Handling of the body can be simplified by making the sensitive area either a small +/// corner (container case), or the entire body (leaf case). Then, that area always maps +/// to "insert after me". +/// - The bottom parts have the most difference between cases and need case-by-case handling. +/// In both leaf item cases, the entire bottom part maps to "insert after me", though. +/// +/// +/// +/// **Note**: in debug builds, press `Alt` to visualize the drag zones while dragging. +pub fn find_drop_target( + ui: &egui::Ui, + item_desc: &DropItemDescription, + response: &egui::Response, + body_response: Option<&egui::Response>, + item_height: f32, +) -> Option> { + let indent = ui.spacing().indent; + let item_id = item_desc.id; + let is_container = item_desc.is_container; + let parent_id = item_desc.parent_id; + let pos_in_parent = item_desc.position_index_in_parent; + + // For both leaf and containers we have two drag zones on the upper half of the item. + let (top, mut bottom) = response.rect.split_top_bottom_at_fraction(0.5); + let (left_top, top) = top.split_left_right_at_x(top.left() + indent); + + // For the lower part of the item, the story is more complicated: + // - for leaf item, we have a single drag zone on the entire lower half + // - for container item, we must distinguish between the indent part and the rest, plus check some area in the + // body + let mut left_bottom = egui::Rect::NOTHING; + if is_container { + (left_bottom, bottom) = bottom.split_left_right_at_x(bottom.left() + indent); + } + + // For the body area we have two cases: + // - container item: it's handled recursively by the nested items, so we only need to check a small area down + // left, which maps to "insert after me" + // - leaf item: the entire body area, if any, cannot receive a drag (by definition) and thus homogeneously maps + // to "insert after me" + let body_insert_after_me_area = if let Some(body_response) = body_response { + if item_desc.is_container { + egui::Rect::from_two_pos( + body_response.rect.left_bottom() + egui::vec2(indent, -item_height / 2.0), + body_response.rect.left_bottom(), + ) + } else { + body_response.rect + } + } else { + egui::Rect::NOTHING + }; + + // body rect, if any AND it actually contains something + let non_empty_body_rect = body_response.map(|r| r.rect).filter(|r| r.height() > 0.0); + + // visualize the drag zones in debug builds, when the `Alt` key is pressed during drag + #[cfg(debug_assertions)] + { + // Visualize the drag zones + if ui.input(|i| i.modifiers.alt) { + ui.ctx() + .debug_painter() + .debug_rect(top, egui::Color32::RED, "t"); + ui.ctx() + .debug_painter() + .debug_rect(bottom, egui::Color32::GREEN, "b"); + + ui.ctx().debug_painter().debug_rect( + left_top, + egui::Color32::RED.gamma_multiply(0.5), + "lt", + ); + ui.ctx().debug_painter().debug_rect( + left_bottom, + egui::Color32::GREEN.gamma_multiply(0.5), + "lb", + ); + ui.ctx().debug_painter().debug_rect( + body_insert_after_me_area, + egui::Color32::YELLOW, + "bdy", + ); + } + } + + /* ===== TOP SECTIONS (same leaf/container items) ==== */ + if ui.rect_contains_pointer(left_top) { + // insert before me + Some(DropTarget::new( + response.rect.x_range(), + top.top(), + parent_id, + pos_in_parent, + )) + } else if ui.rect_contains_pointer(top) { + // insert last in the previous container if any, else insert before me + if let Some(previous_container_id) = item_desc.previous_container_id { + Some(DropTarget::new( + (response.rect.left() + indent..=response.rect.right()).into(), + top.top(), + previous_container_id, + usize::MAX, + )) + } else { + Some(DropTarget::new( + response.rect.x_range(), + top.top(), + parent_id, + pos_in_parent, + )) + } + } + /* ==== BODY SENSE AREA ==== */ + else if ui.rect_contains_pointer(body_insert_after_me_area) { + // insert after me in my parent + Some(DropTarget::new( + response.rect.x_range(), + body_insert_after_me_area.bottom(), + parent_id, + pos_in_parent + 1, + )) + } + /* ==== BOTTOM SECTIONS (leaf item) ==== */ + else if !is_container { + if ui.rect_contains_pointer(bottom) { + let position_y = if let Some(body_rect) = non_empty_body_rect { + body_rect.bottom() + } else { + bottom.bottom() + }; + + // insert after me + Some(DropTarget::new( + response.rect.x_range(), + position_y, + parent_id, + pos_in_parent + 1, + )) + } else { + None + } + } + /* ==== BOTTOM SECTIONS (container item) ==== */ + else if let Some(body_rect) = non_empty_body_rect { + if ui.rect_contains_pointer(left_bottom) || ui.rect_contains_pointer(bottom) { + // insert at pos = 0 inside me + Some(DropTarget::new( + (body_rect.left() + indent..=body_rect.right()).into(), + left_bottom.bottom(), + item_id, + 0, + )) + } else { + None + } + } else if ui.rect_contains_pointer(left_bottom) { + // insert after me in my parent + Some(DropTarget::new( + response.rect.x_range(), + left_bottom.bottom(), + parent_id, + pos_in_parent + 1, + )) + } else if ui.rect_contains_pointer(bottom) { + // insert at pos = 0 inside me + Some(DropTarget::new( + (response.rect.left() + indent..=response.rect.right()).into(), + bottom.bottom(), + item_id, + 0, + )) + } + /* ==== Who knows where else the mouse cursor might wander… ¯\_(ツ)_/¯ ==== */ + else { + None + } +} diff --git a/crates/re_ui/src/lib.rs b/crates/re_ui/src/lib.rs index 6fe28ae5b443..2a15b7ab66ec 100644 --- a/crates/re_ui/src/lib.rs +++ b/crates/re_ui/src/lib.rs @@ -3,6 +3,7 @@ mod command; mod command_palette; mod design_tokens; +pub mod drag_and_drop; pub mod egui_helpers; pub mod icons; mod layout_job_builder; diff --git a/crates/re_viewer_context/src/blueprint_id.rs b/crates/re_viewer_context/src/blueprint_id.rs index 6870dda084e1..addd0bbf96ef 100644 --- a/crates/re_viewer_context/src/blueprint_id.rs +++ b/crates/re_viewer_context/src/blueprint_id.rs @@ -179,6 +179,18 @@ define_blueprint_id_type!(SpaceViewId, SpaceViewIdRegistry, "space_view"); define_blueprint_id_type!(DataQueryId, DataQueryIdRegistry, "data_query"); define_blueprint_id_type!(ContainerId, ContainerIdRegistry, "container"); +impl ContainerId { + pub fn to_drag_id(&self) -> egui::Id { + egui::Id::new("container_id").with(self.id) + } +} + +impl SpaceViewId { + pub fn to_drag_id(&self) -> egui::Id { + egui::Id::new("space_view_id").with(self.id) + } +} + // ---------------------------------------------------------------------------- // Tests #[cfg(test)] diff --git a/crates/re_viewport/src/container.rs b/crates/re_viewport/src/container.rs index 4f70870cf26e..6905da98ef75 100644 --- a/crates/re_viewport/src/container.rs +++ b/crates/re_viewport/src/container.rs @@ -8,13 +8,13 @@ use re_query::query_archetype; use re_types::blueprint::components::Visible; use re_types_core::{archetypes::Clear, ArrowBuffer}; use re_viewer_context::{ - blueprint_timepoint_for_writes, BlueprintId, BlueprintIdRegistry, ContainerId, SpaceViewId, - SystemCommand, SystemCommandSender as _, ViewerContext, + blueprint_timepoint_for_writes, BlueprintId, BlueprintIdRegistry, ContainerId, Item, + SpaceViewId, SystemCommand, SystemCommandSender as _, ViewerContext, }; use crate::blueprint::components::GridColumns; -#[derive(Clone, Debug)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum Contents { Container(ContainerId), SpaceView(SpaceViewId), @@ -47,6 +47,25 @@ impl Contents { } } + /// Convert to drag id. + /// + /// Drag ids are used to track items during drag-and-drop operations. + #[inline] + pub fn to_drag_id(&self) -> egui::Id { + match self { + Contents::Container(id) => id.to_drag_id(), + Contents::SpaceView(id) => id.to_drag_id(), + } + } + + #[inline] + pub fn to_item(&self) -> Item { + match self { + Contents::Container(container_id) => Item::Container(*container_id), + Contents::SpaceView(space_view_id) => Item::SpaceView(*space_view_id), + } + } + #[inline] pub fn as_container_id(&self) -> Option { match self { diff --git a/crates/re_viewport/src/viewport_blueprint.rs b/crates/re_viewport/src/viewport_blueprint.rs index 6aedc1a4d90e..5eaf95b2b75e 100644 --- a/crates/re_viewport/src/viewport_blueprint.rs +++ b/crates/re_viewport/src/viewport_blueprint.rs @@ -305,6 +305,105 @@ impl ViewportBlueprint { new_ids } + /// Given a predicate, finds the (first) matching contents by recursively walking from the root + /// container. + pub fn find_contents_by(&self, predicate: &impl Fn(&Contents) -> bool) -> Option { + if let Some(root_container) = self.root_container { + self.find_contents_in_container_by(predicate, &root_container) + } else { + None + } + } + + /// Given a predicate, finds the (first) matching contents by recursively walking from the given + /// container. + pub fn find_contents_in_container_by( + &self, + predicate: &impl Fn(&Contents) -> bool, + container_id: &ContainerId, + ) -> Option { + if predicate(&Contents::Container(*container_id)) { + return Some(Contents::Container(*container_id)); + } + + let Some(container) = self.container(&container_id) else { + return None; + }; + + for contents in &container.contents { + if predicate(&contents) { + return Some(*contents); + } + + match contents { + Contents::Container(container_id) => { + let res = self.find_contents_in_container_by(predicate, container_id); + if res.is_some() { + return res; + } + } + Contents::SpaceView(_) => {} + } + } + + return None; + } + + /// Checks if some content is (directly or indirectly) contained in the given container. + pub fn is_contents_in_container( + &self, + contents: &Contents, + container_id: &ContainerId, + ) -> bool { + self.find_contents_in_container_by(&|c| c == contents, container_id) + .is_some() + } + + /// Given a container or a space view, find its enclosing container and its position within it. + pub fn find_parent_and_position_index( + &self, + contents: &Contents, + ) -> Option<(ContainerId, usize)> { + if let Some(container_id) = self.root_container { + if *contents == Contents::Container(container_id) { + // root doesn't have a parent + return None; + } + self.find_parent_and_position_index_impl(contents, &container_id) + } else { + None + } + } + + fn find_parent_and_position_index_impl( + &self, + contents: &Contents, + container_id: &ContainerId, + ) -> Option<(ContainerId, usize)> { + let Some(container) = self.container(&container_id) else { + return None; + }; + + for (pos, child_contents) in container.contents.iter().enumerate() { + if child_contents == contents { + return Some((*container_id, pos)); + } + + match child_contents { + Contents::Container(child_container_id) => { + let res = + self.find_parent_and_position_index_impl(contents, child_container_id); + if res.is_some() { + return res; + } + } + Contents::SpaceView(_) => {} + } + } + + None + } + /// Add a container of the provided kind. /// /// The container is added to the root container or, if provided, to the given parent container. diff --git a/crates/re_viewport/src/viewport_blueprint_ui.rs b/crates/re_viewport/src/viewport_blueprint_ui.rs index cad80df88867..b95837c6f717 100644 --- a/crates/re_viewport/src/viewport_blueprint_ui.rs +++ b/crates/re_viewport/src/viewport_blueprint_ui.rs @@ -86,12 +86,16 @@ impl Viewport<'_, '_> { let default_open = true; - let response = ListItem::new( + let re_ui::list_item::ShowCollapsingResponse { + item_response: response, + body_response, + } = ListItem::new( ctx.re_ui, format!("{:?}", container_blueprint.container_kind), ) .subdued(!container_visible) .selected(ctx.selection().contains_item(&item)) + .draggable(container_id.to_drag_id()) .with_icon(crate::icon_for_container_kind( &container_blueprint.container_kind, )) @@ -108,11 +112,18 @@ impl Viewport<'_, '_> { for child in &container_blueprint.contents { self.contents_ui(ctx, ui, child, container_visible); } - }) - .item_response; + }); item_ui::select_hovered_on_click(ctx, &response, item); + self.handle_drag_and_drop_interaction( + ctx, + ui, + Contents::Container(*container_id), + &response, + body_response.as_ref().map(|r| &r.response), + ); + if remove { self.blueprint.mark_user_interaction(ctx); self.blueprint @@ -165,10 +176,14 @@ impl Viewport<'_, '_> { let space_view_name = space_view.display_name_or_default(); - let response = ListItem::new(ctx.re_ui, space_view_name.as_ref()) + let re_ui::list_item::ShowCollapsingResponse { + item_response: mut response, + body_response, + } = ListItem::new(ctx.re_ui, space_view_name.as_ref()) .label_style(space_view_name.style()) .with_icon(space_view.class(ctx.space_view_class_registry).icon()) .selected(ctx.selection().contains_item(&item)) + .draggable(space_view_id.to_drag_id()) .subdued(!space_view_visible) .force_hovered(is_item_hovered) .with_buttons(|re_ui, ui| { @@ -200,9 +215,9 @@ impl Viewport<'_, '_> { } else { ui.label("No results"); } - }) - .item_response - .on_hover_text("Space View"); + }); + + response = response.on_hover_text("Space View"); if response.clicked() { self.blueprint.focus_tab(space_view.id); @@ -210,6 +225,14 @@ impl Viewport<'_, '_> { item_ui::select_hovered_on_click(ctx, &response, item); + self.handle_drag_and_drop_interaction( + ctx, + ui, + Contents::SpaceView(*space_view_id), + &response, + body_response.as_ref().map(|r| &r.response), + ); + if visibility_changed { if self.blueprint.auto_layout { re_log::trace!("Space view visibility changed - will no longer auto-layout"); @@ -491,6 +514,129 @@ impl Viewport<'_, '_> { .response .on_hover_text("Add new Space View"); } + + // ---------------------------------------------------------------------------- + // drag and drop support + + fn handle_drag_and_drop_interaction( + &self, + ctx: &ViewerContext<'_>, + ui: &egui::Ui, + contents: Contents, + response: &egui::Response, + body_response: Option<&egui::Response>, + ) { + // + // are we really dragging something? if so, set the appropriate cursor + // + + let anything_being_decidedly_dragged = ui.memory(|mem| mem.is_anything_being_dragged()) + && ui.input(|i| i.pointer.is_decidedly_dragging()); + + if !anything_being_decidedly_dragged { + // nothing to do + return; + } + + ui.ctx().set_cursor_icon(egui::CursorIcon::Grabbing); + + // + // force single-selection on drag-and-drop + // + + if response.decidedly_dragged() { + ctx.selection_state().set_selection(contents.to_item()); + } + + // + // find the item being dragged + // + + let Some(dragged_item_id) = ui.memory(|mem| { + self.blueprint + .find_contents_by(&|contents| mem.is_being_dragged(contents.to_drag_id())) + }) else { + // this shouldn't happen + return; + }; + + // + // find our parent, our position within parent, and the previous container (if any) + // + + let Some((parent_container_id, pos_in_parent)) = + self.blueprint.find_parent_and_position_index(&contents) + else { + return; + }; + + let previous_container = if pos_in_parent > 0 { + self.blueprint + .container(&parent_container_id) + .map(|container| container.contents[pos_in_parent - 1]) + .filter(|contents| matches!(contents, Contents::Container(_))) + } else { + None + }; + + // + // find the drop target + // + + // Prepare the item description structure needed by `find_drop_target`. Here, we use + // `Contents` for the "ItemId" generic type parameter. + let item_desc = re_ui::drag_and_drop::DropItemDescription { + id: contents, + is_container: matches!(contents, Contents::Container(_)), + parent_id: Contents::Container(parent_container_id), + position_index_in_parent: pos_in_parent, + previous_container_id: previous_container, + }; + + let drop_target = re_ui::drag_and_drop::find_drop_target( + ui, + &item_desc, + response, + body_response, + ReUi::list_item_height(), + ); + + if let Some(drop_target) = drop_target { + // We cannot allow the target location to be "inside" the dragged item, because that would amount moving + // myself inside of me. + if let Contents::Container(container_id) = &contents { + if self + .blueprint + .is_contents_in_container(&drop_target.target_parent_id, container_id) + { + return; + } + } + + ui.painter().hline( + drop_target.indicator_span_x, + drop_target.indicator_position_y, + (2.0, egui::Color32::WHITE), + ); + + // TODO(emilk/egui#3882): it would be nice to have a drag specific API for `ctx().drag_stopped()`. + if ui.input(|i| i.pointer.any_released()) { + //TODO: emit a move tree action + + // self.send_command(Command::MoveItem { + // moved_item_id: dragged_item_id, + // target_container_id: drag_target.target_parent_id, + // target_position_index: drag_target.target_position_index, + // }); + } else { + //TODO: the target container should be highlighted + + // self.send_command(Command::HighlightTargetContainer( + // drag_target.target_parent_id, + // )); + } + } + } } // ---------------------------------------------------------------------------- From b73a8cbb25313820c4bda1ec2c40a81c5022aab2 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Thu, 25 Jan 2024 16:55:23 +0100 Subject: [PATCH 02/28] First working drag and drop!! :tada: TODO: - ListItem: dont show icons when a drag is ongoing - highlight receiving container - remove arrow for root container - back port re_::drag_and_drop::* to re_ui_example --- Cargo.lock | 3 +- Cargo.toml | 2 +- crates/re_viewport/src/viewport.rs | 30 +++++++++++++++++-- crates/re_viewport/src/viewport_blueprint.rs | 14 +++++++++ .../re_viewport/src/viewport_blueprint_ui.rs | 16 +++++----- 5 files changed, 53 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a0cb3c04fc5b..b12b3c92a395 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1702,8 +1702,7 @@ dependencies = [ [[package]] name = "egui_tiles" version = "0.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c0255c0209b349b1a4a67a344556501e75accae669f3a25be6e07deb30fefa91" +source = "git+https://github.com/rerun-io/egui_tiles?rev=3cb9547e6de4433d619f7d579182a709ab5a0dc5#3cb9547e6de4433d619f7d579182a709ab5a0dc5" dependencies = [ "ahash", "egui", diff --git a/Cargo.toml b/Cargo.toml index 9f88a39481df..381f1428e455 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -293,6 +293,6 @@ emath = { git = "https://github.com/emilk/egui.git", rev = "bcf032a08f7d7b510901 # egui-wgpu = { path = "../../egui/crates/egui-wgpu" } # emath = { path = "../../egui/crates/emath" } -# egui_tiles = { git = "https://github.com/rerun-io/egui_tiles", rev = "b6e4fd457b2eee2c671747ead12f4a20feb380e8" } # Merge of: https://github.com/rerun-io/egui_tiles/pull/41 +egui_tiles = { git = "https://github.com/rerun-io/egui_tiles", rev = "3cb9547e6de4433d619f7d579182a709ab5a0dc5" } # from https://github.com/rerun-io/egui_tiles/pull/44 # egui_commonmark = { git = "https://github.com/rerun-io/egui_commonmark", rev = "3d83a92f995a1d18ab1172d0b129d496e0eedaae" } # Update to egui 0.25 https://github.com/lampsitter/egui_commonmark/pull/27 diff --git a/crates/re_viewport/src/viewport.rs b/crates/re_viewport/src/viewport.rs index 6b5ee182f5c8..869b59273fc5 100644 --- a/crates/re_viewport/src/viewport.rs +++ b/crates/re_viewport/src/viewport.rs @@ -5,12 +5,11 @@ use std::collections::BTreeMap; use ahash::HashMap; - use egui_tiles::Behavior as _; use once_cell::sync::Lazy; + use re_data_ui::item_ui; use re_entity_db::EntityPropertyMap; - use re_ui::{Icon, ReUi}; use re_viewer_context::{ AppOptions, ContainerId, Item, SpaceViewClassIdentifier, SpaceViewClassRegistry, SpaceViewId, @@ -88,6 +87,13 @@ pub enum TreeAction { /// Simplify the container with the provided options SimplifyContainer(ContainerId, egui_tiles::SimplificationOptions), + + /// Move some contents to a different container + MoveContents { + contents_to_move: Contents, + target_container: ContainerId, + target_position_in_container: usize, + }, } fn tree_simplification_option_for_app_options( @@ -421,6 +427,26 @@ impl<'a, 'b> Viewport<'a, 'b> { self.tree.simplify_children_of_tile(tile_id, &options); self.tree_edited = true; } + TreeAction::MoveContents { + contents_to_move, + target_container, + target_position_in_container, + } => { + re_log::trace!( + "Moving {contents_to_move:?} to container {target_container:?} at pos \ + {target_position_in_container}" + ); + + let contents_tile_id = contents_to_move.to_tile_id(); + let target_container_tile_id = blueprint_id_to_tile_id(&target_container); + + self.tree.move_tile_to_container( + contents_tile_id, + target_container_tile_id, + target_position_in_container, + ); + self.tree_edited = true; + } } } diff --git a/crates/re_viewport/src/viewport_blueprint.rs b/crates/re_viewport/src/viewport_blueprint.rs index 5eaf95b2b75e..f020dedbcc73 100644 --- a/crates/re_viewport/src/viewport_blueprint.rs +++ b/crates/re_viewport/src/viewport_blueprint.rs @@ -420,6 +420,20 @@ impl ViewportBlueprint { self.send_tree_action(TreeAction::RemoveContents(contents)); } + /// Move the `contents` container or space view to the specified target container and position. + pub fn move_contents( + &self, + contents: Contents, + target_container: ContainerId, + target_position_in_container: usize, + ) { + self.send_tree_action(TreeAction::MoveContents { + contents_to_move: contents, + target_container, + target_position_in_container, + }) + } + /// Make sure the tab corresponding to this space view is focused. pub fn focus_tab(&self, space_view_id: SpaceViewId) { self.send_tree_action(TreeAction::FocusTab(space_view_id)); diff --git a/crates/re_viewport/src/viewport_blueprint_ui.rs b/crates/re_viewport/src/viewport_blueprint_ui.rs index b95837c6f717..3d162f32aefe 100644 --- a/crates/re_viewport/src/viewport_blueprint_ui.rs +++ b/crates/re_viewport/src/viewport_blueprint_ui.rs @@ -619,15 +619,17 @@ impl Viewport<'_, '_> { (2.0, egui::Color32::WHITE), ); - // TODO(emilk/egui#3882): it would be nice to have a drag specific API for `ctx().drag_stopped()`. if ui.input(|i| i.pointer.any_released()) { - //TODO: emit a move tree action + let Contents::Container(target_container_id) = drop_target.target_parent_id else { + // this shouldn't append + return; + }; - // self.send_command(Command::MoveItem { - // moved_item_id: dragged_item_id, - // target_container_id: drag_target.target_parent_id, - // target_position_index: drag_target.target_position_index, - // }); + self.blueprint.move_contents( + dragged_item_id, + target_container_id, + drop_target.target_position_index, + ); } else { //TODO: the target container should be highlighted From 73efd74df31e82f4a1dab69e3451da4fbd2ddf2e Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Thu, 25 Jan 2024 16:58:53 +0100 Subject: [PATCH 03/28] `ListItem`: don't show icons when a drag is ongoing TODO: - highlight receiving container - remove arrow for root container - back port re_::drag_and_drop::* to re_ui_example --- crates/re_ui/src/list_item.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/re_ui/src/list_item.rs b/crates/re_ui/src/list_item.rs index 869f972d6759..03228e04287b 100644 --- a/crates/re_ui/src/list_item.rs +++ b/crates/re_ui/src/list_item.rs @@ -456,7 +456,12 @@ impl<'a> ListItem<'a> { } // Handle buttons - let button_response = if self.active && ui.rect_contains_pointer(rect) { + let anything_being_decidedly_dragged = ui.memory(|mem| mem.is_anything_being_dragged()) + && ui.input(|i| i.pointer.is_decidedly_dragging()); + let button_response = if self.active + && ui.rect_contains_pointer(rect) + && !anything_being_decidedly_dragged + { if let Some(buttons) = self.buttons_fn { let mut ui = ui.child_ui(rect, egui::Layout::right_to_left(egui::Align::Center)); From 3b6597b47db92d0fe6a44db5a552220d12bf197c Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Thu, 25 Jan 2024 17:31:44 +0100 Subject: [PATCH 04/28] Highlight receiving container TODO: - remove arrow for root container - back port re_::drag_and_drop::* to re_ui_example --- crates/re_viewport/src/viewport.rs | 19 +++++++++++++++++++ crates/re_viewport/src/viewport_blueprint.rs | 8 ++++++++ .../re_viewport/src/viewport_blueprint_ui.rs | 17 +++++++---------- 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/crates/re_viewport/src/viewport.rs b/crates/re_viewport/src/viewport.rs index 869b59273fc5..e601678a8be9 100644 --- a/crates/re_viewport/src/viewport.rs +++ b/crates/re_viewport/src/viewport.rs @@ -38,6 +38,9 @@ pub struct PerSpaceViewState { pub struct ViewportState { space_view_entity_window: SpaceViewEntityPicker, space_view_states: HashMap, + + /// when a drag is in progress, this is the currently identified drop target to be highlighted + drop_target_container_id: Option, } static DEFAULT_PROPS: Lazy = Lazy::::new(Default::default); @@ -64,6 +67,10 @@ impl ViewportState { .get(&space_view_id) .map_or(&DEFAULT_PROPS, |state| &state.auto_properties) } + + pub fn is_drop_target(&self, container_id: &ContainerId) -> bool { + self.drop_target_container_id.as_ref() == Some(container_id) + } } /// Mutation actions to perform on the tree at the end of the frame. These messages are sent by the mutation APIs from @@ -94,6 +101,12 @@ pub enum TreeAction { target_container: ContainerId, target_position_in_container: usize, }, + + /// Set the container that is currently identified as the drop target of an ongoing drag. + /// + /// This is used for highlighting the drop target in the UI. Note that the drop target container is reset at every + /// frame, so this command must be re-sent every frame as long as a drop target is identified. + SetDropTarget(ContainerId), } fn tree_simplification_option_for_app_options( @@ -325,6 +338,9 @@ impl<'a, 'b> Viewport<'a, 'b> { let mut reset = false; + // always reset the drop target + self.state.drop_target_container_id = None; + // TODO(#4687): Be extra careful here. If we mark edited inappropriately we can create an infinite edit loop. for tree_action in self.tree_action_receiver.try_iter() { match tree_action { @@ -447,6 +463,9 @@ impl<'a, 'b> Viewport<'a, 'b> { ); self.tree_edited = true; } + TreeAction::SetDropTarget(container_id) => { + self.state.drop_target_container_id = Some(container_id); + } } } diff --git a/crates/re_viewport/src/viewport_blueprint.rs b/crates/re_viewport/src/viewport_blueprint.rs index f020dedbcc73..7ddea198a104 100644 --- a/crates/re_viewport/src/viewport_blueprint.rs +++ b/crates/re_viewport/src/viewport_blueprint.rs @@ -463,6 +463,14 @@ impl ViewportBlueprint { )); } + /// Set the container that is currently identified as the drop target of an ongoing drag. + /// + /// This is used for highlighting the drop target in the UI. Note that the drop target container is reset at every + /// frame, so this command must be re-sent every frame as long as a drop target is identified. + pub fn set_drop_target(&self, container_id: &ContainerId) { + self.send_tree_action(TreeAction::SetDropTarget(*container_id)); + } + #[allow(clippy::unused_self)] pub fn space_views_containing_entity_path( &self, diff --git a/crates/re_viewport/src/viewport_blueprint_ui.rs b/crates/re_viewport/src/viewport_blueprint_ui.rs index 3d162f32aefe..a1e3e232342e 100644 --- a/crates/re_viewport/src/viewport_blueprint_ui.rs +++ b/crates/re_viewport/src/viewport_blueprint_ui.rs @@ -96,6 +96,7 @@ impl Viewport<'_, '_> { .subdued(!container_visible) .selected(ctx.selection().contains_item(&item)) .draggable(container_id.to_drag_id()) + .drop_target_style(self.state.is_drop_target(container_id)) .with_icon(crate::icon_for_container_kind( &container_blueprint.container_kind, )) @@ -619,23 +620,19 @@ impl Viewport<'_, '_> { (2.0, egui::Color32::WHITE), ); - if ui.input(|i| i.pointer.any_released()) { - let Contents::Container(target_container_id) = drop_target.target_parent_id else { - // this shouldn't append - return; - }; + let Contents::Container(target_container_id) = drop_target.target_parent_id else { + // this shouldn't append + return; + }; + if ui.input(|i| i.pointer.any_released()) { self.blueprint.move_contents( dragged_item_id, target_container_id, drop_target.target_position_index, ); } else { - //TODO: the target container should be highlighted - - // self.send_command(Command::HighlightTargetContainer( - // drag_target.target_parent_id, - // )); + self.blueprint.set_drop_target(&target_container_id); } } } From b76345889a23eaf7ab4a32bd673ebefa93525e49 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Thu, 25 Jan 2024 17:44:43 +0100 Subject: [PATCH 05/28] Lint TODO: - drag in one-self is not possible - remove arrow for root container - back port re_::drag_and_drop::* to re_ui_example --- crates/re_ui/src/drag_and_drop.rs | 140 +++++++++--------- crates/re_viewer_context/src/blueprint_id.rs | 4 +- crates/re_viewport/src/container.rs | 20 +-- crates/re_viewport/src/viewport.rs | 4 +- crates/re_viewport/src/viewport_blueprint.rs | 10 +- .../re_viewport/src/viewport_blueprint_ui.rs | 8 +- 6 files changed, 95 insertions(+), 91 deletions(-) diff --git a/crates/re_ui/src/drag_and_drop.rs b/crates/re_ui/src/drag_and_drop.rs index 93bbc0098132..67ef2da7e05a 100644 --- a/crates/re_ui/src/drag_and_drop.rs +++ b/crates/re_ui/src/drag_and_drop.rs @@ -34,12 +34,16 @@ impl DropTarget { pub struct DropItemDescription { /// ID of the item being hovered during drag pub id: ItemId, + /// Can this item "contain" the currently dragged item? pub is_container: bool, + /// ID of the parent if this item. pub parent_id: ItemId, + /// Position of this item within its parent. pub position_index_in_parent: usize, + /// ID of the container just before this item within the parent, if such a container exists. pub previous_container_id: Option, } @@ -49,76 +53,76 @@ pub struct DropItemDescription { /// This function implements the following logic: /// ```text /// -/// insert insert last in container before me -/// before me (if any) or insert before me -/// │ │ -/// ╔═══▼═════════════════════════════▼══════════════════╗ -/// ║ │ ║ -/// leaf item ║ ─────┴──────────────────────────────────────────── ║ -/// ║ ║ -/// ╚═════════════════════▲══════════════════════════════╝ -/// │ -/// insert after me -/// -/// -/// insert insert last in container before me -/// before me (if any) or insert before me -/// │ │ -/// ╔═══▼═════════════════════════════▼══════════════════╗ -/// leaf item ║ │ ║ -/// with body ║ ─────┴──────────────────────────────────────────── ║ -/// ║ ║ -/// ╚══════╦══════════════════════════════════════▲══════╣ ─┐ -/// │ ║ │ ║ │ -/// │ ║ insert ║ │ -/// │ ║ after me ║ │ -/// │ ╠══ ══╣ │ -/// │ ║ no insertion possible ║ │ +/// insert insert last in container before me +/// before me (if any) or insert before me +/// │ │ +/// ╔═══▼═════════════════════════════▼══════════════════╗ +/// ║ │ ║ +/// leaf item ║ ─────┴──────────────────────────────────────────── ║ +/// ║ ║ +/// ╚═════════════════════▲══════════════════════════════╝ +/// │ +/// insert after me +/// +/// +/// insert insert last in container before me +/// before me (if any) or insert before me +/// │ │ +/// ╔═══▼═════════════════════════════▼══════════════════╗ +/// leaf item ║ │ ║ +/// with body ║ ─────┴──────────────────────────────────────────── ║ +/// ║ ║ +/// ╚══════╦══════════════════════════════════════▲══════╣ ─┐ +/// │ ║ │ ║ │ +/// │ ║ insert ║ │ +/// │ ║ after me ║ │ +/// │ ╠══ ══╣ │ +/// │ ║ no insertion possible ║ │ /// │ ║ here by definition of ║ │ body -/// │ ║ parent being a leaf ║ │ -/// │ ╠══ ══╣ │ -/// │ ║ ║ │ -/// │ ║ ║ │ -/// │ ║ ║ │ -/// └──▲── ╚══════════════════════════▲══════════════════╝ ─┘ -/// │ │ -/// insert insert -/// after me after me -/// -/// -/// insert insert last in container before me -/// before me (if any) or insert before me -/// │ │ -/// ╔═══▼═════════════════════════════▼══════════════════╗ -/// container item ║ │ ║ -/// (empty/collapsed ║ ─────┼──────────────────────────────────────────── ║ -/// body) ║ │ ║ -/// ╚═══▲═════════════════════════════▲══════════════════╝ -/// │ │ -/// insert insert inside me -/// after me at pos = 0 -/// -/// -/// insert insert last in container before me -/// before me (if any) or insert before me -/// │ │ -/// ╔═══▼═════════════════════════════▼══════════════════╗ -/// container item ║ │ ║ -/// with body ║ ─────┴──────────────────────────────────────────── ║ -/// ║ ║ -/// ╚═▲════╦═════════════════════════════════════════════╣ ─┐ -/// │ ║ ║ │ -/// insert ║ ║ │ -/// inside me ║ ║ │ -/// at pos = 0 ╠══ ══╣ │ -/// ║ same logic ║ │ +/// │ ║ parent being a leaf ║ │ +/// │ ╠══ ══╣ │ +/// │ ║ ║ │ +/// │ ║ ║ │ +/// │ ║ ║ │ +/// └──▲── ╚══════════════════════════▲══════════════════╝ ─┘ +/// │ │ +/// insert insert +/// after me after me +/// +/// +/// insert insert last in container before me +/// before me (if any) or insert before me +/// │ │ +/// ╔═══▼═════════════════════════════▼══════════════════╗ +/// container item ║ │ ║ +/// (empty/collapsed ║ ─────┼──────────────────────────────────────────── ║ +/// body) ║ │ ║ +/// ╚═══▲═════════════════════════════▲══════════════════╝ +/// │ │ +/// insert insert inside me +/// after me at pos = 0 +/// +/// +/// insert insert last in container before me +/// before me (if any) or insert before me +/// │ │ +/// ╔═══▼═════════════════════════════▼══════════════════╗ +/// container item ║ │ ║ +/// with body ║ ─────┴──────────────────────────────────────────── ║ +/// ║ ║ +/// ╚═▲════╦═════════════════════════════════════════════╣ ─┐ +/// │ ║ ║ │ +/// insert ║ ║ │ +/// inside me ║ ║ │ +/// at pos = 0 ╠══ ══╣ │ +/// ║ same logic ║ │ /// ║ recursively ║ │ body -/// insert ║ applied here ║ │ -/// after me ╠══ ══╣ │ -/// │ ║ ║ │ -/// ┌─▼─── ║ ║ │ -/// │ ║ ║ │ -/// └───── ╚═════════════════════════════════════════════╝ ─┘ +/// insert ║ applied here ║ │ +/// after me ╠══ ══╣ │ +/// │ ║ ║ │ +/// ┌─▼─── ║ ║ │ +/// │ ║ ║ │ +/// └───── ╚═════════════════════════════════════════════╝ ─┘ /// ``` /// /// Here are a few observations of the above that help navigate the "if-statement-of-death" diff --git a/crates/re_viewer_context/src/blueprint_id.rs b/crates/re_viewer_context/src/blueprint_id.rs index addd0bbf96ef..a0281ab214cb 100644 --- a/crates/re_viewer_context/src/blueprint_id.rs +++ b/crates/re_viewer_context/src/blueprint_id.rs @@ -180,13 +180,13 @@ define_blueprint_id_type!(DataQueryId, DataQueryIdRegistry, "data_query"); define_blueprint_id_type!(ContainerId, ContainerIdRegistry, "container"); impl ContainerId { - pub fn to_drag_id(&self) -> egui::Id { + pub fn as_drag_id(&self) -> egui::Id { egui::Id::new("container_id").with(self.id) } } impl SpaceViewId { - pub fn to_drag_id(&self) -> egui::Id { + pub fn as_drag_id(&self) -> egui::Id { egui::Id::new("space_view_id").with(self.id) } } diff --git a/crates/re_viewport/src/container.rs b/crates/re_viewport/src/container.rs index 6905da98ef75..bc005997bd3d 100644 --- a/crates/re_viewport/src/container.rs +++ b/crates/re_viewport/src/container.rs @@ -32,7 +32,7 @@ impl Contents { } #[inline] - fn to_entity_path(&self) -> EntityPath { + fn as_entity_path(&self) -> EntityPath { match self { Self::Container(id) => id.as_entity_path(), Self::SpaceView(id) => id.as_entity_path(), @@ -40,7 +40,7 @@ impl Contents { } #[inline] - pub fn to_tile_id(&self) -> TileId { + pub fn as_tile_id(&self) -> TileId { match self { Self::Container(id) => blueprint_id_to_tile_id(id), Self::SpaceView(id) => blueprint_id_to_tile_id(id), @@ -51,15 +51,15 @@ impl Contents { /// /// Drag ids are used to track items during drag-and-drop operations. #[inline] - pub fn to_drag_id(&self) -> egui::Id { + pub fn as_drag_id(&self) -> egui::Id { match self { - Contents::Container(id) => id.to_drag_id(), - Contents::SpaceView(id) => id.to_drag_id(), + Contents::Container(id) => id.as_drag_id(), + Contents::SpaceView(id) => id.as_drag_id(), } } #[inline] - pub fn to_item(&self) -> Item { + pub fn as_item(&self) -> Item { match self { Contents::Container(container_id) => Item::Container(*container_id), Contents::SpaceView(space_view_id) => Item::SpaceView(*space_view_id), @@ -215,7 +215,7 @@ impl ContainerBlueprint { grid_columns, } = self; - let contents: Vec<_> = contents.iter().map(|item| item.to_entity_path()).collect(); + let contents: Vec<_> = contents.iter().map(|item| item.as_entity_path()).collect(); let col_shares: ArrowBuffer<_> = col_shares.clone().into(); let row_shares: ArrowBuffer<_> = row_shares.clone().into(); @@ -230,7 +230,7 @@ impl ContainerBlueprint { // TODO(jleibs): The need for this pattern is annoying. Should codegen // a version of this that can take an Option. if let Some(active_tab) = &active_tab { - arch = arch.with_active_tab(&active_tab.to_entity_path()); + arch = arch.with_active_tab(&active_tab.as_entity_path()); } if let Some(cols) = grid_columns { @@ -378,7 +378,7 @@ impl ContainerBlueprint { let children = self .contents .iter() - .map(|item| item.to_tile_id()) + .map(|item| item.as_tile_id()) .collect::>(); let container = match self.container_kind { @@ -387,7 +387,7 @@ impl ContainerBlueprint { tabs.active = self .active_tab .as_ref() - .map(|id| id.to_tile_id()) + .map(|id| id.as_tile_id()) .or_else(|| tabs.children.first().copied()); egui_tiles::Container::Tabs(tabs) } diff --git a/crates/re_viewport/src/viewport.rs b/crates/re_viewport/src/viewport.rs index e601678a8be9..b0388156d0e6 100644 --- a/crates/re_viewport/src/viewport.rs +++ b/crates/re_viewport/src/viewport.rs @@ -421,7 +421,7 @@ impl<'a, 'b> Viewport<'a, 'b> { self.tree_edited = true; } TreeAction::RemoveContents(contents) => { - let tile_id = contents.to_tile_id(); + let tile_id = contents.as_tile_id(); for tile in self.tree.remove_recursively(tile_id) { re_log::trace!("Removing tile {tile_id:?}"); @@ -453,7 +453,7 @@ impl<'a, 'b> Viewport<'a, 'b> { {target_position_in_container}" ); - let contents_tile_id = contents_to_move.to_tile_id(); + let contents_tile_id = contents_to_move.as_tile_id(); let target_container_tile_id = blueprint_id_to_tile_id(&target_container); self.tree.move_tile_to_container( diff --git a/crates/re_viewport/src/viewport_blueprint.rs b/crates/re_viewport/src/viewport_blueprint.rs index 7ddea198a104..2cc225ba507b 100644 --- a/crates/re_viewport/src/viewport_blueprint.rs +++ b/crates/re_viewport/src/viewport_blueprint.rs @@ -326,12 +326,12 @@ impl ViewportBlueprint { return Some(Contents::Container(*container_id)); } - let Some(container) = self.container(&container_id) else { + let Some(container) = self.container(container_id) else { return None; }; for contents in &container.contents { - if predicate(&contents) { + if predicate(contents) { return Some(*contents); } @@ -346,7 +346,7 @@ impl ViewportBlueprint { } } - return None; + None } /// Checks if some content is (directly or indirectly) contained in the given container. @@ -380,7 +380,7 @@ impl ViewportBlueprint { contents: &Contents, container_id: &ContainerId, ) -> Option<(ContainerId, usize)> { - let Some(container) = self.container(&container_id) else { + let Some(container) = self.container(container_id) else { return None; }; @@ -431,7 +431,7 @@ impl ViewportBlueprint { contents_to_move: contents, target_container, target_position_in_container, - }) + }); } /// Make sure the tab corresponding to this space view is focused. diff --git a/crates/re_viewport/src/viewport_blueprint_ui.rs b/crates/re_viewport/src/viewport_blueprint_ui.rs index a1e3e232342e..6e8b1b827b07 100644 --- a/crates/re_viewport/src/viewport_blueprint_ui.rs +++ b/crates/re_viewport/src/viewport_blueprint_ui.rs @@ -95,7 +95,7 @@ impl Viewport<'_, '_> { ) .subdued(!container_visible) .selected(ctx.selection().contains_item(&item)) - .draggable(container_id.to_drag_id()) + .draggable(container_id.as_drag_id()) .drop_target_style(self.state.is_drop_target(container_id)) .with_icon(crate::icon_for_container_kind( &container_blueprint.container_kind, @@ -184,7 +184,7 @@ impl Viewport<'_, '_> { .label_style(space_view_name.style()) .with_icon(space_view.class(ctx.space_view_class_registry).icon()) .selected(ctx.selection().contains_item(&item)) - .draggable(space_view_id.to_drag_id()) + .draggable(space_view_id.as_drag_id()) .subdued(!space_view_visible) .force_hovered(is_item_hovered) .with_buttons(|re_ui, ui| { @@ -546,7 +546,7 @@ impl Viewport<'_, '_> { // if response.decidedly_dragged() { - ctx.selection_state().set_selection(contents.to_item()); + ctx.selection_state().set_selection(contents.as_item()); } // @@ -555,7 +555,7 @@ impl Viewport<'_, '_> { let Some(dragged_item_id) = ui.memory(|mem| { self.blueprint - .find_contents_by(&|contents| mem.is_being_dragged(contents.to_drag_id())) + .find_contents_by(&|contents| mem.is_being_dragged(contents.as_drag_id())) }) else { // this shouldn't happen return; From d37f60bfb388817512d093bc3025724da59a0757 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Thu, 25 Jan 2024 17:55:33 +0100 Subject: [PATCH 06/28] Update egui_tile commit ref TODO: - drag in one-self is not possible - remove arrow for root container - back port re_::drag_and_drop::* to re_ui_example --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 381f1428e455..3e5890995c67 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -293,6 +293,6 @@ emath = { git = "https://github.com/emilk/egui.git", rev = "bcf032a08f7d7b510901 # egui-wgpu = { path = "../../egui/crates/egui-wgpu" } # emath = { path = "../../egui/crates/emath" } -egui_tiles = { git = "https://github.com/rerun-io/egui_tiles", rev = "3cb9547e6de4433d619f7d579182a709ab5a0dc5" } # from https://github.com/rerun-io/egui_tiles/pull/44 +egui_tiles = { git = "https://github.com/rerun-io/egui_tiles", rev = "f40606f43157a2d03ffe30e272014da1461c54f8" } # master 2024-01-25 # egui_commonmark = { git = "https://github.com/rerun-io/egui_commonmark", rev = "3d83a92f995a1d18ab1172d0b129d496e0eedaae" } # Update to egui 0.25 https://github.com/lampsitter/egui_commonmark/pull/27 From e8cfa301c670f36f74765489b096c8cf012a1d5a Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Thu, 25 Jan 2024 18:23:30 +0100 Subject: [PATCH 07/28] Fixed self-drag bug + cargo bug from main TODO: - some bugs when self dragging - remove arrow for root container - back port re_::drag_and_drop::* to re_ui_example - check after last items --- Cargo.lock | 3 ++- crates/re_space_view_text_log/Cargo.toml | 1 + crates/re_viewport/src/viewport_blueprint_ui.rs | 4 ++-- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ab008b2bf1a1..63e9e188d261 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1702,7 +1702,7 @@ dependencies = [ [[package]] name = "egui_tiles" version = "0.6.0" -source = "git+https://github.com/rerun-io/egui_tiles?rev=3cb9547e6de4433d619f7d579182a709ab5a0dc5#3cb9547e6de4433d619f7d579182a709ab5a0dc5" +source = "git+https://github.com/rerun-io/egui_tiles?rev=f40606f43157a2d03ffe30e272014da1461c54f8#f40606f43157a2d03ffe30e272014da1461c54f8" dependencies = [ "ahash", "egui", @@ -4787,6 +4787,7 @@ dependencies = [ "re_entity_db", "re_log", "re_log_types", + "re_query_cache", "re_renderer", "re_tracing", "re_types", diff --git a/crates/re_space_view_text_log/Cargo.toml b/crates/re_space_view_text_log/Cargo.toml index 6f5b3d0178e4..7f990c89b3be 100644 --- a/crates/re_space_view_text_log/Cargo.toml +++ b/crates/re_space_view_text_log/Cargo.toml @@ -21,6 +21,7 @@ re_data_ui.workspace = true re_entity_db.workspace = true re_log.workspace = true re_log_types.workspace = true +re_query_cache.workspace = true re_renderer.workspace = true re_tracing.workspace = true re_types.workspace = true diff --git a/crates/re_viewport/src/viewport_blueprint_ui.rs b/crates/re_viewport/src/viewport_blueprint_ui.rs index 6e8b1b827b07..875ff03524e6 100644 --- a/crates/re_viewport/src/viewport_blueprint_ui.rs +++ b/crates/re_viewport/src/viewport_blueprint_ui.rs @@ -605,10 +605,10 @@ impl Viewport<'_, '_> { if let Some(drop_target) = drop_target { // We cannot allow the target location to be "inside" the dragged item, because that would amount moving // myself inside of me. - if let Contents::Container(container_id) = &contents { + if let Contents::Container(dragged_container_id) = &dragged_item_id { if self .blueprint - .is_contents_in_container(&drop_target.target_parent_id, container_id) + .is_contents_in_container(&drop_target.target_parent_id, dragged_container_id) { return; } From 9a967a2665690e894b9279938765be5c16481d22 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Fri, 26 Jan 2024 08:57:03 +0100 Subject: [PATCH 08/28] Back-port `re_ui::drag_and_drop::find_drop_target` to `re_ui_example` TODO: - check area after last items --- crates/re_ui/examples/re_ui_example.rs | 284 +++---------------------- 1 file changed, 35 insertions(+), 249 deletions(-) diff --git a/crates/re_ui/examples/re_ui_example.rs b/crates/re_ui/examples/re_ui_example.rs index 42c05e11365d..6f4acbc561c9 100644 --- a/crates/re_ui/examples/re_ui_example.rs +++ b/crates/re_ui/examples/re_ui_example.rs @@ -1154,20 +1154,46 @@ mod hierarchical_drag_and_drop { ui.ctx().set_cursor_icon(egui::CursorIcon::Grabbing); - let drag_target = - self.find_drop_target(ui, item_id, is_container, response, body_response); + let Some((parent_id, position_index_in_parent)) = self.parent_and_pos(item_id) else { + // this shouldn't happen + return; + }; + + let previous_container_id = if position_index_in_parent > 0 { + self.container(parent_id) + .map(|c| c[position_index_in_parent - 1]) + .filter(|id| self.container(*id).is_some()) + } else { + None + }; + + let item_desc = re_ui::drag_and_drop::DropItemDescription { + id: item_id, + is_container, + parent_id, + position_index_in_parent, + previous_container_id, + }; - if let Some(drag_target) = drag_target { + let drop_target = re_ui::drag_and_drop::find_drop_target( + ui, + &item_desc, + response, + body_response, + ReUi::list_item_height(), + ); + + if let Some(drop_target) = drop_target { // We cannot allow the target location to be "inside" the dragged item, because that would amount moving // myself inside of me. - if self.contains(dragged_item_id, drag_target.target_parent_id) { + if self.contains(dragged_item_id, drop_target.target_parent_id) { return; } ui.painter().hline( - drag_target.indicator_span_x, - drag_target.indicator_position_y, + drop_target.indicator_span_x, + drop_target.indicator_position_y, (2.0, egui::Color32::WHITE), ); @@ -1175,255 +1201,15 @@ mod hierarchical_drag_and_drop { if ui.input(|i| i.pointer.any_released()) { self.send_command(Command::MoveItem { moved_item_id: dragged_item_id, - target_container_id: drag_target.target_parent_id, - target_position_index: drag_target.target_position_index, + target_container_id: drop_target.target_parent_id, + target_position_index: drop_target.target_position_index, }); } else { self.send_command(Command::HighlightTargetContainer( - drag_target.target_parent_id, + drop_target.target_parent_id, )); } } } - - /// Compute the geometry of the drag cursor and where the dragged item should be inserted. - /// - /// This function implements the following logic: - /// ```text - /// - /// insert insert last in container before me - /// before me (if any) or insert before me - /// │ │ - /// ╔═══▼═════════════════════════════▼══════════════════╗ - /// ║ │ ║ - /// leaf item ║ ─────┴──────────────────────────────────────────── ║ - /// ║ ║ - /// ╚═════════════════════▲══════════════════════════════╝ - /// │ - /// insert after me - /// - /// - /// - /// insert insert last in container before me - /// before me (if any) or insert before me - /// │ │ - /// ╔═══▼═════════════════════════════▼══════════════════╗ - /// container item ║ │ ║ - /// (no/collapsed ║ ─────┼──────────────────────────────────────────── ║ - /// body) ║ │ ║ - /// ╚═══▲═════════════════════════════▲══════════════════╝ - /// │ │ - /// insert insert inside me - /// after me at pos = 0 - /// - /// - /// - /// insert insert last in container before me - /// before me (if any) or insert before me - /// │ │ - /// ╔═══▼═════════════════════════════▼══════════════════╗ - /// container item ║ │ ║ - /// with body ║ ─────┴──────────────────────────────────────────── ║ - /// ║ ║ - /// ╚══▲═══╦═════════════════════════════════════════════╣ ─┐ - /// │ ║ ║ │ - /// insert ║ ║ │ - /// inside me ║ ║ │ - /// at pos = 0 ╠══ ══╣ │ - /// ║ same logic ║ │ - /// ║ recursively ║ │ body - /// insert ║ applied here ║ │ - /// after me ╠══ ══╣ │ - /// │ ║ ║ │ - /// ┌──▼── ║ ║ │ - /// │ ║ ║ │ - /// └───── ╚═════════════════════════════════════════════╝ ─┘ - /// - /// ``` - /// - /// **Note**: press `Alt` to visualize the drag zones while dragging. - fn find_drop_target( - &self, - ui: &egui::Ui, - item_id: ItemId, - is_container: bool, - response: &egui::Response, - body_response: Option<&egui::Response>, - ) -> Option { - let indent = ui.spacing().indent; - - // For both leaf and containers we have two drag zones on the upper half of the item. - let (top, mut bottom) = response.rect.split_top_bottom_at_fraction(0.5); - let (left_top, top) = top.split_left_right_at_x(top.left() + indent); - - // For the lower part of the item, the story is more complicated: - // - for leaf item, we have a single drag zone on the entire lower half - // - for container item, we must distinguish between the indent part and the rest, plus check some area in the - // body - let mut left_bottom = egui::Rect::NOTHING; - if is_container { - (left_bottom, bottom) = bottom.split_left_right_at_x(bottom.left() + indent); - } - - let mut content_left_bottom = egui::Rect::NOTHING; - if let Some(body_response) = body_response { - content_left_bottom = egui::Rect::from_two_pos( - body_response.rect.left_bottom() - + egui::vec2(indent, -ReUi::list_item_height() / 2.0), - body_response.rect.left_bottom(), - ); - } - - // Visualize the drag zones - if ui.input(|i| i.modifiers.alt) { - ui.ctx() - .debug_painter() - .debug_rect(top, egui::Color32::RED, "t"); - ui.ctx() - .debug_painter() - .debug_rect(bottom, egui::Color32::GREEN, "b"); - - ui.ctx().debug_painter().debug_rect( - left_top, - egui::Color32::RED.gamma_multiply(0.5), - "lt", - ); - ui.ctx().debug_painter().debug_rect( - left_bottom, - egui::Color32::GREEN.gamma_multiply(0.5), - "lb", - ); - ui.ctx().debug_painter().debug_rect( - content_left_bottom, - egui::Color32::YELLOW, - "c", - ); - } - - let Some((parent_id, pos_in_parent)) = self.parent_and_pos(item_id) else { - // this shouldn't happen - return None; - }; - - if ui.rect_contains_pointer(left_top) { - // insert before me - Some(DropTarget::new( - response.rect.x_range(), - top.top(), - parent_id, - pos_in_parent, - )) - } else if ui.rect_contains_pointer(top) { - // insert last in the previous container if any, else insert before me - let previous_container_id = if pos_in_parent > 0 { - self.container(parent_id) - .map(|c| c[pos_in_parent - 1]) - .filter(|id| self.container(*id).is_some()) - } else { - None - }; - - if let Some(previous_container_id) = previous_container_id { - Some(DropTarget::new( - (response.rect.left() + indent..=response.rect.right()).into(), - top.top(), - previous_container_id, - usize::MAX, - )) - } else { - Some(DropTarget::new( - response.rect.x_range(), - top.top(), - parent_id, - pos_in_parent, - )) - } - } else if !is_container { - if ui.rect_contains_pointer(bottom) { - // insert after me - Some(DropTarget::new( - response.rect.x_range(), - bottom.bottom(), - parent_id, - pos_in_parent + 1, - )) - } else { - None - } - } else { - let body_rect = body_response.map(|r| r.rect).filter(|r| r.width() > 0.0); - if let Some(body_rect) = body_rect { - if ui.rect_contains_pointer(left_bottom) || ui.rect_contains_pointer(bottom) { - // insert at pos = 0 inside me - Some(DropTarget::new( - (body_rect.left() + indent..=body_rect.right()).into(), - left_bottom.bottom(), - item_id, - 0, - )) - } else if ui.rect_contains_pointer(content_left_bottom) { - // insert after me in my parent - Some(DropTarget::new( - response.rect.x_range(), - content_left_bottom.bottom(), - parent_id, - pos_in_parent + 1, - )) - } else { - None - } - } else if ui.rect_contains_pointer(left_bottom) { - // insert after me in my parent - Some(DropTarget::new( - response.rect.x_range(), - left_bottom.bottom(), - parent_id, - pos_in_parent + 1, - )) - } else if ui.rect_contains_pointer(bottom) { - // insert at pos = 0 inside me - Some(DropTarget::new( - (response.rect.left() + indent..=response.rect.right()).into(), - bottom.bottom(), - item_id, - 0, - )) - } else { - None - } - } - } - } - - /// Gather information about a drop target, including the geometry of the drop indicator that should be - /// displayed, and the destination where the dragged items should be moved. - struct DropTarget { - /// Range of X coordinates for the drag target indicator - indicator_span_x: egui::Rangef, - - /// Y coordinate for drag target indicator - indicator_position_y: f32, - - /// Destination container ID - target_parent_id: ItemId, - - /// Destination position within the container - target_position_index: usize, - } - - impl DropTarget { - fn new( - indicator_span_x: egui::Rangef, - indicator_position_y: f32, - target_parent_id: ItemId, - target_position_index: usize, - ) -> Self { - Self { - indicator_span_x, - indicator_position_y, - target_parent_id, - target_position_index, - } - } } } From 914634564ab60ddd6df06a8fb1f936a5236f03fd Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Fri, 26 Jan 2024 09:08:34 +0100 Subject: [PATCH 09/28] Updated to last egui_tile + minor code improvement TODO: - check area after last items --- Cargo.lock | 2 +- Cargo.toml | 2 +- crates/re_viewport/src/viewport.rs | 1 + crates/re_viewport/src/viewport_blueprint_ui.rs | 13 +++++++++---- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d2aa37cc0f3e..cc8e93ce4e2b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1702,7 +1702,7 @@ dependencies = [ [[package]] name = "egui_tiles" version = "0.6.0" -source = "git+https://github.com/rerun-io/egui_tiles?rev=f40606f43157a2d03ffe30e272014da1461c54f8#f40606f43157a2d03ffe30e272014da1461c54f8" +source = "git+https://github.com/rerun-io/egui_tiles?rev=35e711283e7a021ca425d9fbd8e7581548971f49#35e711283e7a021ca425d9fbd8e7581548971f49" dependencies = [ "ahash", "egui", diff --git a/Cargo.toml b/Cargo.toml index 3e5890995c67..e44998ef04a1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -293,6 +293,6 @@ emath = { git = "https://github.com/emilk/egui.git", rev = "bcf032a08f7d7b510901 # egui-wgpu = { path = "../../egui/crates/egui-wgpu" } # emath = { path = "../../egui/crates/emath" } -egui_tiles = { git = "https://github.com/rerun-io/egui_tiles", rev = "f40606f43157a2d03ffe30e272014da1461c54f8" } # master 2024-01-25 +egui_tiles = { git = "https://github.com/rerun-io/egui_tiles", rev = "35e711283e7a021ca425d9fbd8e7581548971f49" } # master 2024-01-26 # egui_commonmark = { git = "https://github.com/rerun-io/egui_commonmark", rev = "3d83a92f995a1d18ab1172d0b129d496e0eedaae" } # Update to egui 0.25 https://github.com/lampsitter/egui_commonmark/pull/27 diff --git a/crates/re_viewport/src/viewport.rs b/crates/re_viewport/src/viewport.rs index b0388156d0e6..5070b58e5421 100644 --- a/crates/re_viewport/src/viewport.rs +++ b/crates/re_viewport/src/viewport.rs @@ -460,6 +460,7 @@ impl<'a, 'b> Viewport<'a, 'b> { contents_tile_id, target_container_tile_id, target_position_in_container, + true, ); self.tree_edited = true; } diff --git a/crates/re_viewport/src/viewport_blueprint_ui.rs b/crates/re_viewport/src/viewport_blueprint_ui.rs index 875ff03524e6..c30735fbf0b4 100644 --- a/crates/re_viewport/src/viewport_blueprint_ui.rs +++ b/crates/re_viewport/src/viewport_blueprint_ui.rs @@ -553,10 +553,15 @@ impl Viewport<'_, '_> { // find the item being dragged // - let Some(dragged_item_id) = ui.memory(|mem| { - self.blueprint - .find_contents_by(&|contents| mem.is_being_dragged(contents.as_drag_id())) - }) else { + let Some(egui_dragged_id) = ui.memory(|mem| mem.dragged_id()) else { + // this shouldn't happen + return; + }; + + let Some(dragged_item_id) = self + .blueprint + .find_contents_by(&|contents| contents.as_drag_id() == egui_dragged_id) + else { // this shouldn't happen return; }; From dea19c9f0cf868544a7f3e8a102d8b1066dd3bb1 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Fri, 26 Jan 2024 09:28:12 +0100 Subject: [PATCH 10/28] Accept drag in the empty space bellow the tree --- crates/re_ui/src/drag_and_drop.rs | 2 +- .../re_viewport/src/viewport_blueprint_ui.rs | 133 +++++++++++++----- 2 files changed, 102 insertions(+), 33 deletions(-) diff --git a/crates/re_ui/src/drag_and_drop.rs b/crates/re_ui/src/drag_and_drop.rs index 67ef2da7e05a..ba43cdb5c9c1 100644 --- a/crates/re_ui/src/drag_and_drop.rs +++ b/crates/re_ui/src/drag_and_drop.rs @@ -16,7 +16,7 @@ pub struct DropTarget { } impl DropTarget { - fn new( + pub fn new( indicator_span_x: egui::Rangef, indicator_position_y: f32, target_parent_id: ItemId, diff --git a/crates/re_viewport/src/viewport_blueprint_ui.rs b/crates/re_viewport/src/viewport_blueprint_ui.rs index c30735fbf0b4..f05d7a896d13 100644 --- a/crates/re_viewport/src/viewport_blueprint_ui.rs +++ b/crates/re_viewport/src/viewport_blueprint_ui.rs @@ -5,8 +5,7 @@ use re_data_ui::item_ui; use re_entity_db::InstancePath; use re_log_types::{EntityPath, EntityPathRule}; use re_space_view::DataQueryBlueprint; -use re_ui::list_item::ListItem; -use re_ui::ReUi; +use re_ui::{drag_and_drop::DropTarget, list_item::ListItem, ReUi}; use re_viewer_context::{ ContainerId, DataQueryResult, DataResultNode, HoverHighlight, Item, SpaceViewId, ViewerContext, }; @@ -31,13 +30,19 @@ impl Viewport<'_, '_> { self.contents_ui(ctx, ui, &Contents::Container(root_container), true); } + let empty_space_response = + ui.allocate_response(ui.available_size(), egui::Sense::click()); + // clear selection upon clicking on empty space - if ui - .allocate_response(ui.available_size(), egui::Sense::click()) - .clicked() - { + if empty_space_response.clicked() { ctx.selection_state().clear_current(); } + + // handle drag and drop interaction on empty space + self.handle_empty_space_drag_and_drop_interaction( + ui, + empty_space_response.rect, + ); }); }); } @@ -608,38 +613,102 @@ impl Viewport<'_, '_> { ); if let Some(drop_target) = drop_target { - // We cannot allow the target location to be "inside" the dragged item, because that would amount moving - // myself inside of me. - if let Contents::Container(dragged_container_id) = &dragged_item_id { - if self - .blueprint - .is_contents_in_container(&drop_target.target_parent_id, dragged_container_id) - { - return; - } - } + self.handle_drop_target(ui, dragged_item_id, &drop_target); + } + } + + fn handle_empty_space_drag_and_drop_interaction(&self, ui: &egui::Ui, empty_space: egui::Rect) { + // + // are we really dragging something? if so, set the appropriate cursor + // + + let anything_being_decidedly_dragged = ui.memory(|mem| mem.is_anything_being_dragged()) + && ui.input(|i| i.pointer.is_decidedly_dragging()); + + if !anything_being_decidedly_dragged { + // nothing to do + return; + } + + ui.ctx().set_cursor_icon(egui::CursorIcon::Grabbing); + + // + // find the item being dragged + // + + let Some(egui_dragged_id) = ui.memory(|mem| mem.dragged_id()) else { + // this shouldn't happen + return; + }; - ui.painter().hline( - drop_target.indicator_span_x, - drop_target.indicator_position_y, - (2.0, egui::Color32::WHITE), + let Some(dragged_item_id) = self + .blueprint + .find_contents_by(&|contents| contents.as_drag_id() == egui_dragged_id) + else { + // this shouldn't happen + return; + }; + + // + // prepare a drop target corresponding to "insert last in root container" + // + // TODO(ab): this is a rather primitive behavior. Ideally we should allow dropping in the last container based + // on the horizontal position of the cursor. + + let Some(root_container_id) = self.blueprint.root_container else { + return; + }; + + if ui.rect_contains_pointer(empty_space) { + let drop_target = re_ui::drag_and_drop::DropTarget::new( + // TODO(#4909): this indent is a visual hack that should be remove once #4909 is done + (empty_space.left() + ui.spacing().indent..=empty_space.right()).into(), + empty_space.top(), + Contents::Container(root_container_id), + usize::MAX, ); - let Contents::Container(target_container_id) = drop_target.target_parent_id else { - // this shouldn't append - return; - }; + self.handle_drop_target(ui, dragged_item_id, &drop_target); + } + } - if ui.input(|i| i.pointer.any_released()) { - self.blueprint.move_contents( - dragged_item_id, - target_container_id, - drop_target.target_position_index, - ); - } else { - self.blueprint.set_drop_target(&target_container_id); + fn handle_drop_target( + &self, + ui: &Ui, + dragged_item_id: Contents, + drop_target: &DropTarget, + ) { + // We cannot allow the target location to be "inside" the dragged item, because that would amount moving + // myself inside of me. + if let Contents::Container(dragged_container_id) = &dragged_item_id { + if self + .blueprint + .is_contents_in_container(&drop_target.target_parent_id, dragged_container_id) + { + return; } } + + ui.painter().hline( + drop_target.indicator_span_x, + drop_target.indicator_position_y, + (2.0, egui::Color32::WHITE), + ); + + let Contents::Container(target_container_id) = drop_target.target_parent_id else { + // this shouldn't append + return; + }; + + if ui.input(|i| i.pointer.any_released()) { + self.blueprint.move_contents( + dragged_item_id, + target_container_id, + drop_target.target_position_index, + ); + } else { + self.blueprint.set_drop_target(&target_container_id); + } } } From 492b7c5dc0e7903e5b50a6bc7b648c651a79d3a0 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Fri, 26 Jan 2024 17:06:26 +0100 Subject: [PATCH 11/28] Post merge fixes --- crates/re_viewport/src/viewport_blueprint_ui.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/re_viewport/src/viewport_blueprint_ui.rs b/crates/re_viewport/src/viewport_blueprint_ui.rs index f05d7a896d13..25f64c343aff 100644 --- a/crates/re_viewport/src/viewport_blueprint_ui.rs +++ b/crates/re_viewport/src/viewport_blueprint_ui.rs @@ -550,7 +550,7 @@ impl Viewport<'_, '_> { // force single-selection on drag-and-drop // - if response.decidedly_dragged() { + if response.drag_started() { ctx.selection_state().set_selection(contents.as_item()); } From 59489b110a21b0aa08a220ec5d95ce58ac290724 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Fri, 26 Jan 2024 14:03:28 +0100 Subject: [PATCH 12/28] WIP --- crates/re_ui/examples/re_ui_example.rs | 37 +++++++------------ crates/re_ui/src/list_item.rs | 22 ++++++----- .../re_viewport/src/viewport_blueprint_ui.rs | 4 +- 3 files changed, 28 insertions(+), 35 deletions(-) diff --git a/crates/re_ui/examples/re_ui_example.rs b/crates/re_ui/examples/re_ui_example.rs index d4f31be05d0c..80bcef9a63b6 100644 --- a/crates/re_ui/examples/re_ui_example.rs +++ b/crates/re_ui/examples/re_ui_example.rs @@ -666,21 +666,18 @@ mod drag_and_drop { impl ExampleDragAndDrop { pub fn ui(&mut self, re_ui: &crate::ReUi, ui: &mut egui::Ui) { - let mut source_item_position_index = None; - let mut target_item_position_index = None; + let mut swap: Option<(usize, usize)> = None; for (i, item_id) in self.items.iter().enumerate() { // // Draw the item // - let id = egui::Id::new("drag_demo").with(*item_id); - let label = format!("Item {}", item_id.0); let response = re_ui .list_item(label.as_str()) .selected(self.selected_items.contains(item_id)) - .draggable(id) + .draggable(true) .show(ui); // @@ -702,24 +699,20 @@ mod drag_and_drop { } // Drag-and-drop of multiple items not (yet?) supported, so dragging resets selection to single item. - if response.dragged() { + if response.decidedly_dragged() { self.selected_items.clear(); self.selected_items.insert(*item_id); + + response.dnd_set_drag_payload(i); } // - // Detect end-of-drag situation and prepare the swap command. + // Detect drag situation and run the swap if it ends. // - if response.dragged() || response.drag_released() { - source_item_position_index = Some(i); - } + let source_item_position_index = egui::DragAndDrop::payload(ui.ctx()).map(|i| *i); - // TODO(emilk/egui#3882): this feels like a common enough pattern that is should deserve its own API. - let anything_being_decidedly_dragged = ui - .memory(|mem| mem.is_anything_being_dragged()) - && ui.input(|i| i.pointer.is_decidedly_dragging()); - if anything_being_decidedly_dragged { + if let Some(source_item_position_index) = source_item_position_index { ui.ctx().set_cursor_icon(egui::CursorIcon::Grabbing); let (top, bottom) = response.rect.split_top_bottom_at_fraction(0.5); @@ -732,16 +725,16 @@ mod drag_and_drop { (None, None) }; - if let Some(insert_y) = insert_y { + if let (Some(insert_y), Some(target)) = (insert_y, target) { ui.painter().hline( ui.cursor().x_range(), insert_y, (2.0, egui::Color32::WHITE), ); - // TODO(emilk/egui#3882): it would be nice to have a drag specific API for that if ui.input(|i| i.pointer.any_released()) { - target_item_position_index = target; + swap = Some((source_item_position_index, target)); + egui::DragAndDrop::clear_payload(ui.ctx()); } } } @@ -751,9 +744,7 @@ mod drag_and_drop { // Handle the swap command (if any) // - if let (Some(source), Some(target)) = - (source_item_position_index, target_item_position_index) - { + if let Some((source, target)) = swap { if source != target { let item = self.items.remove(source); @@ -1054,7 +1045,7 @@ mod hierarchical_drag_and_drop { .list_item(format!("Container {item_id:?}")) .subdued(true) .selected(self.selected(item_id)) - .draggable(item_id.into()) + .draggable(true) .drop_target_style(self.target_container == Some(item_id)) .show_collapsing(ui, item_id.into(), true, |re_ui, ui| { self.container_children_ui(re_ui, ui, children); @@ -1092,7 +1083,7 @@ mod hierarchical_drag_and_drop { let response = re_ui .list_item(label) .selected(self.selected(item_id)) - .draggable(item_id.into()) + .draggable(true) .show(ui); self.handle_interaction(ui, item_id, false, &response, None); diff --git a/crates/re_ui/src/list_item.rs b/crates/re_ui/src/list_item.rs index 03228e04287b..0dd14f75c940 100644 --- a/crates/re_ui/src/list_item.rs +++ b/crates/re_ui/src/list_item.rs @@ -113,7 +113,7 @@ pub struct ListItem<'a> { re_ui: &'a ReUi, active: bool, selected: bool, - drag_id: Option, + draggable: bool, drag_target: bool, subdued: bool, weak: bool, @@ -136,7 +136,7 @@ impl<'a> ListItem<'a> { re_ui, active: true, selected: false, - drag_id: None, + draggable: false, drag_target: false, subdued: false, weak: false, @@ -167,8 +167,8 @@ impl<'a> ListItem<'a> { /// Make the item draggable and set its persistent ID. #[inline] - pub fn draggable(mut self, drag_id: egui::Id) -> Self { - self.drag_id = Some(drag_id); + pub fn draggable(mut self, draggable: bool) -> Self { + self.draggable = draggable; self } @@ -383,12 +383,14 @@ impl<'a> ListItem<'a> { }; let desired_size = egui::vec2(desired_width, self.height); - let (rect, mut response) = ui.allocate_at_least(desired_size, egui::Sense::click()); - - // handle dragging - if let Some(drag_id) = self.drag_id { - response = ui.interact(response.rect, drag_id, egui::Sense::drag()); - } + let (rect, mut response) = ui.allocate_at_least( + desired_size, + if self.draggable { + egui::Sense::drag() + } else { + egui::Sense::click() + }, + ); // compute the full-span background rect let mut bg_rect = rect; diff --git a/crates/re_viewport/src/viewport_blueprint_ui.rs b/crates/re_viewport/src/viewport_blueprint_ui.rs index 25f64c343aff..bf015a2bb517 100644 --- a/crates/re_viewport/src/viewport_blueprint_ui.rs +++ b/crates/re_viewport/src/viewport_blueprint_ui.rs @@ -100,7 +100,7 @@ impl Viewport<'_, '_> { ) .subdued(!container_visible) .selected(ctx.selection().contains_item(&item)) - .draggable(container_id.as_drag_id()) + .draggable(true) .drop_target_style(self.state.is_drop_target(container_id)) .with_icon(crate::icon_for_container_kind( &container_blueprint.container_kind, @@ -189,7 +189,7 @@ impl Viewport<'_, '_> { .label_style(space_view_name.style()) .with_icon(space_view.class(ctx.space_view_class_registry).icon()) .selected(ctx.selection().contains_item(&item)) - .draggable(space_view_id.as_drag_id()) + .draggable(true) .subdued(!space_view_visible) .force_hovered(is_item_hovered) .with_buttons(|re_ui, ui| { From 128add05361a36dc0bed3d83bdffd7c2398554a5 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Fri, 26 Jan 2024 17:20:07 +0100 Subject: [PATCH 13/28] Fix stuff due to egui bump --- Cargo.lock | 20 +++++++++---------- Cargo.toml | 14 ++++++------- .../src/space_view_class.rs | 2 +- crates/re_viewer/src/ui/memory_panel.rs | 4 ++-- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b91f6ecfa13e..b2a0ac3a08c4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1559,7 +1559,7 @@ checksum = "68b0cf012f1230e43cd00ebb729c6bb58707ecfa8ad08b52ef3a4ccd2697fc30" [[package]] name = "ecolor" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=6b0782c96b76349da9be785d0e9054f87cd7b00a#6b0782c96b76349da9be785d0e9054f87cd7b00a" +source = "git+https://github.com/emilk/egui.git?rev=da95adf466f4d85675c014a41593a12caa2de55e#da95adf466f4d85675c014a41593a12caa2de55e" dependencies = [ "bytemuck", "serde", @@ -1568,7 +1568,7 @@ dependencies = [ [[package]] name = "eframe" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=6b0782c96b76349da9be785d0e9054f87cd7b00a#6b0782c96b76349da9be785d0e9054f87cd7b00a" +source = "git+https://github.com/emilk/egui.git?rev=da95adf466f4d85675c014a41593a12caa2de55e#da95adf466f4d85675c014a41593a12caa2de55e" dependencies = [ "bytemuck", "cocoa", @@ -1601,7 +1601,7 @@ dependencies = [ [[package]] name = "egui" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=6b0782c96b76349da9be785d0e9054f87cd7b00a#6b0782c96b76349da9be785d0e9054f87cd7b00a" +source = "git+https://github.com/emilk/egui.git?rev=da95adf466f4d85675c014a41593a12caa2de55e#da95adf466f4d85675c014a41593a12caa2de55e" dependencies = [ "accesskit", "ahash", @@ -1617,7 +1617,7 @@ dependencies = [ [[package]] name = "egui-wgpu" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=6b0782c96b76349da9be785d0e9054f87cd7b00a#6b0782c96b76349da9be785d0e9054f87cd7b00a" +source = "git+https://github.com/emilk/egui.git?rev=da95adf466f4d85675c014a41593a12caa2de55e#da95adf466f4d85675c014a41593a12caa2de55e" dependencies = [ "bytemuck", "egui", @@ -1633,7 +1633,7 @@ dependencies = [ [[package]] name = "egui-winit" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=6b0782c96b76349da9be785d0e9054f87cd7b00a#6b0782c96b76349da9be785d0e9054f87cd7b00a" +source = "git+https://github.com/emilk/egui.git?rev=da95adf466f4d85675c014a41593a12caa2de55e#da95adf466f4d85675c014a41593a12caa2de55e" dependencies = [ "accesskit_winit", "arboard", @@ -1662,7 +1662,7 @@ dependencies = [ [[package]] name = "egui_extras" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=6b0782c96b76349da9be785d0e9054f87cd7b00a#6b0782c96b76349da9be785d0e9054f87cd7b00a" +source = "git+https://github.com/emilk/egui.git?rev=da95adf466f4d85675c014a41593a12caa2de55e#da95adf466f4d85675c014a41593a12caa2de55e" dependencies = [ "egui", "ehttp", @@ -1677,7 +1677,7 @@ dependencies = [ [[package]] name = "egui_glow" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=6b0782c96b76349da9be785d0e9054f87cd7b00a#6b0782c96b76349da9be785d0e9054f87cd7b00a" +source = "git+https://github.com/emilk/egui.git?rev=da95adf466f4d85675c014a41593a12caa2de55e#da95adf466f4d85675c014a41593a12caa2de55e" dependencies = [ "bytemuck", "egui", @@ -1694,7 +1694,7 @@ dependencies = [ [[package]] name = "egui_plot" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=6b0782c96b76349da9be785d0e9054f87cd7b00a#6b0782c96b76349da9be785d0e9054f87cd7b00a" +source = "git+https://github.com/emilk/egui.git?rev=da95adf466f4d85675c014a41593a12caa2de55e#da95adf466f4d85675c014a41593a12caa2de55e" dependencies = [ "egui", ] @@ -1736,7 +1736,7 @@ checksum = "a26ae43d7bcc3b814de94796a5e736d4029efb0ee900c12e2d54c993ad1a1e07" [[package]] name = "emath" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=6b0782c96b76349da9be785d0e9054f87cd7b00a#6b0782c96b76349da9be785d0e9054f87cd7b00a" +source = "git+https://github.com/emilk/egui.git?rev=da95adf466f4d85675c014a41593a12caa2de55e#da95adf466f4d85675c014a41593a12caa2de55e" dependencies = [ "bytemuck", "serde", @@ -1837,7 +1837,7 @@ dependencies = [ [[package]] name = "epaint" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=6b0782c96b76349da9be785d0e9054f87cd7b00a#6b0782c96b76349da9be785d0e9054f87cd7b00a" +source = "git+https://github.com/emilk/egui.git?rev=da95adf466f4d85675c014a41593a12caa2de55e#da95adf466f4d85675c014a41593a12caa2de55e" dependencies = [ "ab_glyph", "ahash", diff --git a/Cargo.toml b/Cargo.toml index c55b30d3a4bb..6abcb11886c0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -277,13 +277,13 @@ debug = true # As a last resport, patch with a commit to our own repository. # ALWAYS document what PR the commit hash is part of, or when it was merged into the upstream trunk. -ecolor = { git = "https://github.com/emilk/egui.git", rev = "6b0782c96b76349da9be785d0e9054f87cd7b00a" } # egui master 2024-01-26 -eframe = { git = "https://github.com/emilk/egui.git", rev = "6b0782c96b76349da9be785d0e9054f87cd7b00a" } # egui master 2024-01-26 -egui = { git = "https://github.com/emilk/egui.git", rev = "6b0782c96b76349da9be785d0e9054f87cd7b00a" } # egui master 2024-01-26 -egui_extras = { git = "https://github.com/emilk/egui.git", rev = "6b0782c96b76349da9be785d0e9054f87cd7b00a" } # egui master 2024-01-26 -egui_plot = { git = "https://github.com/emilk/egui.git", rev = "6b0782c96b76349da9be785d0e9054f87cd7b00a" } # egui master 2024-01-26 -egui-wgpu = { git = "https://github.com/emilk/egui.git", rev = "6b0782c96b76349da9be785d0e9054f87cd7b00a" } # egui master 2024-01-26 -emath = { git = "https://github.com/emilk/egui.git", rev = "6b0782c96b76349da9be785d0e9054f87cd7b00a" } # egui master 2024-01-26 +ecolor = { git = "https://github.com/emilk/egui.git", rev = "da95adf466f4d85675c014a41593a12caa2de55e" } # egui master 2024-01-26 +eframe = { git = "https://github.com/emilk/egui.git", rev = "da95adf466f4d85675c014a41593a12caa2de55e" } # egui master 2024-01-26 +egui = { git = "https://github.com/emilk/egui.git", rev = "da95adf466f4d85675c014a41593a12caa2de55e" } # egui master 2024-01-26 +egui_extras = { git = "https://github.com/emilk/egui.git", rev = "da95adf466f4d85675c014a41593a12caa2de55e" } # egui master 2024-01-26 +egui_plot = { git = "https://github.com/emilk/egui.git", rev = "da95adf466f4d85675c014a41593a12caa2de55e" } # egui master 2024-01-26 +egui-wgpu = { git = "https://github.com/emilk/egui.git", rev = "da95adf466f4d85675c014a41593a12caa2de55e" } # egui master 2024-01-26 +emath = { git = "https://github.com/emilk/egui.git", rev = "da95adf466f4d85675c014a41593a12caa2de55e" } # egui master 2024-01-26 # Useful while developing: # ecolor = { path = "../../egui/crates/ecolor" } diff --git a/crates/re_space_view_time_series/src/space_view_class.rs b/crates/re_space_view_time_series/src/space_view_class.rs index e4e0dc61c037..d5a73ca4fc24 100644 --- a/crates/re_space_view_time_series/src/space_view_class.rs +++ b/crates/re_space_view_time_series/src/space_view_class.rs @@ -260,7 +260,7 @@ impl SpaceViewClass for TimeSeriesSpaceView { .x_axis_formatter(move |time, _, _| { format_time( time_type, - time as i64 + time_offset, + time.value as i64 + time_offset, time_zone_for_timestamps, ) }) diff --git a/crates/re_viewer/src/ui/memory_panel.rs b/crates/re_viewer/src/ui/memory_panel.rs index 6fe51f103d68..2401bbd9816d 100644 --- a/crates/re_viewer/src/ui/memory_panel.rs +++ b/crates/re_viewer/src/ui/memory_panel.rs @@ -540,8 +540,8 @@ impl MemoryPanel { egui_plot::Plot::new("mem_history_plot") .min_size(egui::Vec2::splat(200.0)) .label_formatter(|name, value| format!("{name}: {}", format_bytes(value.y))) - .x_axis_formatter(|time, _, _| format!("{time} s")) - .y_axis_formatter(|bytes, _, _| format_bytes(bytes)) + .x_axis_formatter(|time, _, _| format!("{} s", time.value)) + .y_axis_formatter(|bytes, _, _| format_bytes(bytes.value)) .show_x(false) .legend(egui_plot::Legend::default().position(egui_plot::Corner::LeftTop)) .include_y(0.0) From 39adb172c677b97a507b92c824ecd33ee49c9934 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Fri, 26 Jan 2024 17:41:40 +0100 Subject: [PATCH 14/28] Update the dnd demo to the new APIs --- crates/re_ui/examples/re_ui_example.rs | 31 ++++++++++---------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/crates/re_ui/examples/re_ui_example.rs b/crates/re_ui/examples/re_ui_example.rs index 80bcef9a63b6..99f8dcb83bc3 100644 --- a/crates/re_ui/examples/re_ui_example.rs +++ b/crates/re_ui/examples/re_ui_example.rs @@ -699,7 +699,7 @@ mod drag_and_drop { } // Drag-and-drop of multiple items not (yet?) supported, so dragging resets selection to single item. - if response.decidedly_dragged() { + if response.drag_started() { self.selected_items.clear(); self.selected_items.insert(*item_id); @@ -732,6 +732,8 @@ mod drag_and_drop { (2.0, egui::Color32::WHITE), ); + // note: can't use `response.drag_released()` because we not the item which + // started the drag if ui.input(|i| i.pointer.any_released()) { swap = Some((source_item_position_index, target)); egui::DragAndDrop::clear_payload(ui.ctx()); @@ -1113,33 +1115,23 @@ mod hierarchical_drag_and_drop { // handle drag // - if response.dragged() { + if response.drag_started() { // Here, we support dragging a single item at a time, so we set the selection to the dragged item // if/when we're dragging it proper. self.send_command(Command::SetSelection(item_id)); + + egui::DragAndDrop::set_payload(ui.ctx(), item_id); } // // handle drop // - let anything_being_decidedly_dragged = ui.memory(|mem| mem.is_anything_being_dragged()) - && ui.input(|i| i.pointer.is_decidedly_dragging()); - - if !anything_being_decidedly_dragged { - // nothing to do - return; - } - // find the item being dragged - // TODO(ab): `mem.dragged_id()` now exists but there is no easy way to get the value out of and egui::Id - let Some(dragged_item_id) = ui.memory(|mem| { - self.items - .keys() - .find(|item_id| mem.is_being_dragged((**item_id).into())) - .copied() - }) else { - // this shouldn't happen + let Some(dragged_item_id) = + egui::DragAndDrop::payload(ui.ctx()).map(|payload| (*payload)) + else { + // nothing is being dragged, we're done here return; }; @@ -1188,7 +1180,8 @@ mod hierarchical_drag_and_drop { (2.0, egui::Color32::WHITE), ); - // TODO(emilk/egui#3882): it would be nice to have a drag specific API for `ctx().drag_stopped()`. + // note: can't use `response.drag_released()` because we not the item which + // started the drag if ui.input(|i| i.pointer.any_released()) { self.send_command(Command::MoveItem { moved_item_id: dragged_item_id, From 12ccfb1c18189ba18e670cca539430bd00e19342 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Fri, 26 Jan 2024 17:56:40 +0100 Subject: [PATCH 15/28] Update the dnd demo to the new APIs --- crates/re_ui/examples/re_ui_example.rs | 3 + .../re_viewport/src/viewport_blueprint_ui.rs | 63 +++++-------------- 2 files changed, 17 insertions(+), 49 deletions(-) diff --git a/crates/re_ui/examples/re_ui_example.rs b/crates/re_ui/examples/re_ui_example.rs index 99f8dcb83bc3..3d6f5fa1c417 100644 --- a/crates/re_ui/examples/re_ui_example.rs +++ b/crates/re_ui/examples/re_ui_example.rs @@ -736,6 +736,7 @@ mod drag_and_drop { // started the drag if ui.input(|i| i.pointer.any_released()) { swap = Some((source_item_position_index, target)); + egui::DragAndDrop::clear_payload(ui.ctx()); } } @@ -1188,6 +1189,8 @@ mod hierarchical_drag_and_drop { target_container_id: drop_target.target_parent_id, target_position_index: drop_target.target_position_index, }); + + egui::DragAndDrop::clear_payload(ui.ctx()); } else { self.send_command(Command::HighlightTargetContainer( drop_target.target_parent_id, diff --git a/crates/re_viewport/src/viewport_blueprint_ui.rs b/crates/re_viewport/src/viewport_blueprint_ui.rs index bf015a2bb517..8ba64be010fe 100644 --- a/crates/re_viewport/src/viewport_blueprint_ui.rs +++ b/crates/re_viewport/src/viewport_blueprint_ui.rs @@ -533,44 +533,26 @@ impl Viewport<'_, '_> { body_response: Option<&egui::Response>, ) { // - // are we really dragging something? if so, set the appropriate cursor - // - - let anything_being_decidedly_dragged = ui.memory(|mem| mem.is_anything_being_dragged()) - && ui.input(|i| i.pointer.is_decidedly_dragging()); - - if !anything_being_decidedly_dragged { - // nothing to do - return; - } - - ui.ctx().set_cursor_icon(egui::CursorIcon::Grabbing); - - // - // force single-selection on drag-and-drop + // initiate drag and force single-selection // if response.drag_started() { ctx.selection_state().set_selection(contents.as_item()); + egui::DragAndDrop::set_payload(ui.ctx(), contents); } // - // find the item being dragged + // check if a drag is in progress and set the cursor accordingly // - let Some(egui_dragged_id) = ui.memory(|mem| mem.dragged_id()) else { - // this shouldn't happen - return; - }; - - let Some(dragged_item_id) = self - .blueprint - .find_contents_by(&|contents| contents.as_drag_id() == egui_dragged_id) + let Some(dragged_item_id) = egui::DragAndDrop::payload(ui.ctx()).map(|payload| *payload) else { - // this shouldn't happen + // nothing is being dragged, so nothing to do return; }; + ui.ctx().set_cursor_icon(egui::CursorIcon::Grabbing); + // // find our parent, our position within parent, and the previous container (if any) // @@ -619,36 +601,17 @@ impl Viewport<'_, '_> { fn handle_empty_space_drag_and_drop_interaction(&self, ui: &egui::Ui, empty_space: egui::Rect) { // - // are we really dragging something? if so, set the appropriate cursor + // check if a drag is in progress and set the cursor accordingly // - let anything_being_decidedly_dragged = ui.memory(|mem| mem.is_anything_being_dragged()) - && ui.input(|i| i.pointer.is_decidedly_dragging()); - - if !anything_being_decidedly_dragged { - // nothing to do - return; - } - - ui.ctx().set_cursor_icon(egui::CursorIcon::Grabbing); - - // - // find the item being dragged - // - - let Some(egui_dragged_id) = ui.memory(|mem| mem.dragged_id()) else { - // this shouldn't happen - return; - }; - - let Some(dragged_item_id) = self - .blueprint - .find_contents_by(&|contents| contents.as_drag_id() == egui_dragged_id) + let Some(dragged_item_id) = egui::DragAndDrop::payload(ui.ctx()).map(|payload| *payload) else { - // this shouldn't happen + // nothing is being dragged, so nothing to do return; }; + ui.ctx().set_cursor_icon(egui::CursorIcon::Grabbing); + // // prepare a drop target corresponding to "insert last in root container" // @@ -706,6 +669,8 @@ impl Viewport<'_, '_> { target_container_id, drop_target.target_position_index, ); + + egui::DragAndDrop::clear_payload(ui.ctx()); } else { self.blueprint.set_drop_target(&target_container_id); } From 21073dbd58a28f64218bca83e08f7300594be305 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Fri, 26 Jan 2024 18:01:04 +0100 Subject: [PATCH 16/28] Remove useless code --- crates/re_viewer_context/src/blueprint_id.rs | 12 ------------ crates/re_viewport/src/container.rs | 11 ----------- 2 files changed, 23 deletions(-) diff --git a/crates/re_viewer_context/src/blueprint_id.rs b/crates/re_viewer_context/src/blueprint_id.rs index a0281ab214cb..6870dda084e1 100644 --- a/crates/re_viewer_context/src/blueprint_id.rs +++ b/crates/re_viewer_context/src/blueprint_id.rs @@ -179,18 +179,6 @@ define_blueprint_id_type!(SpaceViewId, SpaceViewIdRegistry, "space_view"); define_blueprint_id_type!(DataQueryId, DataQueryIdRegistry, "data_query"); define_blueprint_id_type!(ContainerId, ContainerIdRegistry, "container"); -impl ContainerId { - pub fn as_drag_id(&self) -> egui::Id { - egui::Id::new("container_id").with(self.id) - } -} - -impl SpaceViewId { - pub fn as_drag_id(&self) -> egui::Id { - egui::Id::new("space_view_id").with(self.id) - } -} - // ---------------------------------------------------------------------------- // Tests #[cfg(test)] diff --git a/crates/re_viewport/src/container.rs b/crates/re_viewport/src/container.rs index bc005997bd3d..6675337c7788 100644 --- a/crates/re_viewport/src/container.rs +++ b/crates/re_viewport/src/container.rs @@ -47,17 +47,6 @@ impl Contents { } } - /// Convert to drag id. - /// - /// Drag ids are used to track items during drag-and-drop operations. - #[inline] - pub fn as_drag_id(&self) -> egui::Id { - match self { - Contents::Container(id) => id.as_drag_id(), - Contents::SpaceView(id) => id.as_drag_id(), - } - } - #[inline] pub fn as_item(&self) -> Item { match self { From 6478006acfe38f78ac302074a403a7e00797a6b1 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Fri, 26 Jan 2024 20:10:11 +0100 Subject: [PATCH 17/28] Improved docstring --- crates/re_ui/src/drag_and_drop.rs | 37 +++++++++++++++++-------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/crates/re_ui/src/drag_and_drop.rs b/crates/re_ui/src/drag_and_drop.rs index ba43cdb5c9c1..0bc2b3fe7371 100644 --- a/crates/re_ui/src/drag_and_drop.rs +++ b/crates/re_ui/src/drag_and_drop.rs @@ -1,5 +1,25 @@ //! Helpers for drag and drop support. Works well in combination with [`crate::list_item::ListItem`]. +/// Context information related to a candidate drop target, used by [`find_drop_target`] to compute the [`DropTarget`], +/// if any. +pub struct DropItemDescription { + /// ID of the item being hovered during drag + pub id: ItemId, + + /// Can this item "contain" the currently dragged item? + pub is_container: bool, + + /// ID of the parent if this item. + pub parent_id: ItemId, + + /// Position of this item within its parent. + pub position_index_in_parent: usize, + + /// ID of the container just before this item within the parent, if such a container exists. + pub previous_container_id: Option, +} + +/// Drop target information, including where to draw the drop indicator and where to insert the dragged item. #[derive(Clone, Debug)] pub struct DropTarget { /// Range of X coordinates for the drag target indicator @@ -31,23 +51,6 @@ impl DropTarget { } } -pub struct DropItemDescription { - /// ID of the item being hovered during drag - pub id: ItemId, - - /// Can this item "contain" the currently dragged item? - pub is_container: bool, - - /// ID of the parent if this item. - pub parent_id: ItemId, - - /// Position of this item within its parent. - pub position_index_in_parent: usize, - - /// ID of the container just before this item within the parent, if such a container exists. - pub previous_container_id: Option, -} - /// Compute the geometry of the drag cursor and where the dragged item should be inserted. /// /// This function implements the following logic: From 4536b47eee38dc42b26b8b78afc5dc3f48796518 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Sat, 27 Jan 2024 08:37:15 +0100 Subject: [PATCH 18/28] Unneeded whitespace --- crates/re_ui/src/drag_and_drop.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/re_ui/src/drag_and_drop.rs b/crates/re_ui/src/drag_and_drop.rs index 0bc2b3fe7371..61dd5bcd9acd 100644 --- a/crates/re_ui/src/drag_and_drop.rs +++ b/crates/re_ui/src/drag_and_drop.rs @@ -137,8 +137,6 @@ impl DropTarget { /// - The bottom parts have the most difference between cases and need case-by-case handling. /// In both leaf item cases, the entire bottom part maps to "insert after me", though. /// -/// -/// /// **Note**: in debug builds, press `Alt` to visualize the drag zones while dragging. pub fn find_drop_target( ui: &egui::Ui, From 75749e0862555a58a0634e9c681b24dacc378f52 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Mon, 29 Jan 2024 11:36:30 +0100 Subject: [PATCH 19/28] Bump egui commit to today's master --- Cargo.toml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 6abcb11886c0..a05895889602 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -277,13 +277,13 @@ debug = true # As a last resport, patch with a commit to our own repository. # ALWAYS document what PR the commit hash is part of, or when it was merged into the upstream trunk. -ecolor = { git = "https://github.com/emilk/egui.git", rev = "da95adf466f4d85675c014a41593a12caa2de55e" } # egui master 2024-01-26 -eframe = { git = "https://github.com/emilk/egui.git", rev = "da95adf466f4d85675c014a41593a12caa2de55e" } # egui master 2024-01-26 -egui = { git = "https://github.com/emilk/egui.git", rev = "da95adf466f4d85675c014a41593a12caa2de55e" } # egui master 2024-01-26 -egui_extras = { git = "https://github.com/emilk/egui.git", rev = "da95adf466f4d85675c014a41593a12caa2de55e" } # egui master 2024-01-26 -egui_plot = { git = "https://github.com/emilk/egui.git", rev = "da95adf466f4d85675c014a41593a12caa2de55e" } # egui master 2024-01-26 -egui-wgpu = { git = "https://github.com/emilk/egui.git", rev = "da95adf466f4d85675c014a41593a12caa2de55e" } # egui master 2024-01-26 -emath = { git = "https://github.com/emilk/egui.git", rev = "da95adf466f4d85675c014a41593a12caa2de55e" } # egui master 2024-01-26 +ecolor = { git = "https://github.com/emilk/egui.git", rev = "13b59fb0d4c4b032edc426abed4a98decd0163a6" } # egui master 2024-01-29 +eframe = { git = "https://github.com/emilk/egui.git", rev = "13b59fb0d4c4b032edc426abed4a98decd0163a6" } # egui master 2024-01-29 +egui = { git = "https://github.com/emilk/egui.git", rev = "13b59fb0d4c4b032edc426abed4a98decd0163a6" } # egui master 2024-01-29 +egui_extras = { git = "https://github.com/emilk/egui.git", rev = "13b59fb0d4c4b032edc426abed4a98decd0163a6" } # egui master 2024-01-29 +egui_plot = { git = "https://github.com/emilk/egui.git", rev = "13b59fb0d4c4b032edc426abed4a98decd0163a6" } # egui master 2024-01-29 +egui-wgpu = { git = "https://github.com/emilk/egui.git", rev = "13b59fb0d4c4b032edc426abed4a98decd0163a6" } # egui master 2024-01-29 +emath = { git = "https://github.com/emilk/egui.git", rev = "13b59fb0d4c4b032edc426abed4a98decd0163a6" } # egui master 2024-01-29 # Useful while developing: # ecolor = { path = "../../egui/crates/ecolor" } From cb3a755a7893ca365234cfb865368986c0f00abf Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Mon, 29 Jan 2024 11:41:54 +0100 Subject: [PATCH 20/28] Fix lint --- Cargo.lock | 20 ++++++++++---------- crates/re_viewer/src/ui/selection_panel.rs | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 255437e8ccac..986d2f6b40f9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1559,7 +1559,7 @@ checksum = "68b0cf012f1230e43cd00ebb729c6bb58707ecfa8ad08b52ef3a4ccd2697fc30" [[package]] name = "ecolor" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=da95adf466f4d85675c014a41593a12caa2de55e#da95adf466f4d85675c014a41593a12caa2de55e" +source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" dependencies = [ "bytemuck", "serde", @@ -1568,7 +1568,7 @@ dependencies = [ [[package]] name = "eframe" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=da95adf466f4d85675c014a41593a12caa2de55e#da95adf466f4d85675c014a41593a12caa2de55e" +source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" dependencies = [ "bytemuck", "cocoa", @@ -1601,7 +1601,7 @@ dependencies = [ [[package]] name = "egui" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=da95adf466f4d85675c014a41593a12caa2de55e#da95adf466f4d85675c014a41593a12caa2de55e" +source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" dependencies = [ "accesskit", "ahash", @@ -1617,7 +1617,7 @@ dependencies = [ [[package]] name = "egui-wgpu" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=da95adf466f4d85675c014a41593a12caa2de55e#da95adf466f4d85675c014a41593a12caa2de55e" +source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" dependencies = [ "bytemuck", "egui", @@ -1633,7 +1633,7 @@ dependencies = [ [[package]] name = "egui-winit" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=da95adf466f4d85675c014a41593a12caa2de55e#da95adf466f4d85675c014a41593a12caa2de55e" +source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" dependencies = [ "accesskit_winit", "arboard", @@ -1662,7 +1662,7 @@ dependencies = [ [[package]] name = "egui_extras" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=da95adf466f4d85675c014a41593a12caa2de55e#da95adf466f4d85675c014a41593a12caa2de55e" +source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" dependencies = [ "egui", "ehttp", @@ -1677,7 +1677,7 @@ dependencies = [ [[package]] name = "egui_glow" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=da95adf466f4d85675c014a41593a12caa2de55e#da95adf466f4d85675c014a41593a12caa2de55e" +source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" dependencies = [ "bytemuck", "egui", @@ -1694,7 +1694,7 @@ dependencies = [ [[package]] name = "egui_plot" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=da95adf466f4d85675c014a41593a12caa2de55e#da95adf466f4d85675c014a41593a12caa2de55e" +source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" dependencies = [ "egui", ] @@ -1736,7 +1736,7 @@ checksum = "a26ae43d7bcc3b814de94796a5e736d4029efb0ee900c12e2d54c993ad1a1e07" [[package]] name = "emath" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=da95adf466f4d85675c014a41593a12caa2de55e#da95adf466f4d85675c014a41593a12caa2de55e" +source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" dependencies = [ "bytemuck", "serde", @@ -1837,7 +1837,7 @@ dependencies = [ [[package]] name = "epaint" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=da95adf466f4d85675c014a41593a12caa2de55e#da95adf466f4d85675c014a41593a12caa2de55e" +source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" dependencies = [ "ab_glyph", "ahash", diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index 0fb59459e476..dcc4b74fff3c 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -752,7 +752,7 @@ fn show_list_item_for_container_child( if remove_contents { viewport.blueprint.mark_user_interaction(ctx); - viewport.blueprint.remove_contents(child_contents.clone()); + viewport.blueprint.remove_contents(*child_contents); } true From 628b0c9149fddfb7f7c82c3ff48859c218ef69e3 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler <49431240+abey79@users.noreply.github.com> Date: Mon, 29 Jan 2024 12:52:31 +0100 Subject: [PATCH 21/28] Update crates/re_ui/src/drag_and_drop.rs Co-authored-by: Emil Ernerfeldt --- crates/re_ui/src/drag_and_drop.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/re_ui/src/drag_and_drop.rs b/crates/re_ui/src/drag_and_drop.rs index 61dd5bcd9acd..3b2633103fb0 100644 --- a/crates/re_ui/src/drag_and_drop.rs +++ b/crates/re_ui/src/drag_and_drop.rs @@ -9,7 +9,7 @@ pub struct DropItemDescription { /// Can this item "contain" the currently dragged item? pub is_container: bool, - /// ID of the parent if this item. + /// ID of the parent of this item. pub parent_id: ItemId, /// Position of this item within its parent. From 8c0d55a3288aa51d528400f94eab5192bba3b92e Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Mon, 29 Jan 2024 13:14:26 +0100 Subject: [PATCH 22/28] Clarified `find_drop_target` docstring and improved its signature --- crates/re_ui/examples/re_ui_example.rs | 6 +- crates/re_ui/src/drag_and_drop.rs | 63 +++++++++++-------- .../re_viewport/src/viewport_blueprint_ui.rs | 6 +- 3 files changed, 43 insertions(+), 32 deletions(-) diff --git a/crates/re_ui/examples/re_ui_example.rs b/crates/re_ui/examples/re_ui_example.rs index 3d6f5fa1c417..411f418b9d05 100644 --- a/crates/re_ui/examples/re_ui_example.rs +++ b/crates/re_ui/examples/re_ui_example.rs @@ -1151,7 +1151,7 @@ mod hierarchical_drag_and_drop { None }; - let item_desc = re_ui::drag_and_drop::DropItemDescription { + let item_desc = re_ui::drag_and_drop::ItemContext { id: item_id, is_container, parent_id, @@ -1162,8 +1162,8 @@ mod hierarchical_drag_and_drop { let drop_target = re_ui::drag_and_drop::find_drop_target( ui, &item_desc, - response, - body_response, + response.rect, + body_response.map(|r| r.rect), ReUi::list_item_height(), ); diff --git a/crates/re_ui/src/drag_and_drop.rs b/crates/re_ui/src/drag_and_drop.rs index 3b2633103fb0..510d4f466a10 100644 --- a/crates/re_ui/src/drag_and_drop.rs +++ b/crates/re_ui/src/drag_and_drop.rs @@ -1,8 +1,11 @@ -//! Helpers for drag and drop support. Works well in combination with [`crate::list_item::ListItem`]. +//! Helpers for drag and drop support for reordering hierarchical lists. +//! +//! Works well in combination with [`crate::list_item::ListItem`]. -/// Context information related to a candidate drop target, used by [`find_drop_target`] to compute the [`DropTarget`], -/// if any. -pub struct DropItemDescription { +/// Context information about the hovered item. +/// +/// This is used by [`find_drop_target`] to compute the [`DropTarget`], if any. +pub struct ItemContext { /// ID of the item being hovered during drag pub id: ItemId, @@ -53,6 +56,14 @@ impl DropTarget { /// Compute the geometry of the drag cursor and where the dragged item should be inserted. /// +/// This function performs the following tasks: +/// - based on `item_rect` and `body_rect`, establish the geometry of actual drop zones (see below) +/// - test the mouse cursor against these zones +/// - if one is a match: +/// - compute the geometry of a drop insertion indicator +/// - use the context provided in `item_context` to return the "logical" drop target (ie. the target container and +/// position within it) +/// /// This function implements the following logic: /// ```text /// @@ -140,19 +151,19 @@ impl DropTarget { /// **Note**: in debug builds, press `Alt` to visualize the drag zones while dragging. pub fn find_drop_target( ui: &egui::Ui, - item_desc: &DropItemDescription, - response: &egui::Response, - body_response: Option<&egui::Response>, + item_context: &ItemContext, + item_rect: egui::Rect, + body_rect: Option, item_height: f32, ) -> Option> { let indent = ui.spacing().indent; - let item_id = item_desc.id; - let is_container = item_desc.is_container; - let parent_id = item_desc.parent_id; - let pos_in_parent = item_desc.position_index_in_parent; + let item_id = item_context.id; + let is_container = item_context.is_container; + let parent_id = item_context.parent_id; + let pos_in_parent = item_context.position_index_in_parent; // For both leaf and containers we have two drag zones on the upper half of the item. - let (top, mut bottom) = response.rect.split_top_bottom_at_fraction(0.5); + let (top, mut bottom) = item_rect.split_top_bottom_at_fraction(0.5); let (left_top, top) = top.split_left_right_at_x(top.left() + indent); // For the lower part of the item, the story is more complicated: @@ -169,21 +180,21 @@ pub fn find_drop_target( // left, which maps to "insert after me" // - leaf item: the entire body area, if any, cannot receive a drag (by definition) and thus homogeneously maps // to "insert after me" - let body_insert_after_me_area = if let Some(body_response) = body_response { - if item_desc.is_container { + let body_insert_after_me_area = if let Some(body_rect) = body_rect { + if item_context.is_container { egui::Rect::from_two_pos( - body_response.rect.left_bottom() + egui::vec2(indent, -item_height / 2.0), - body_response.rect.left_bottom(), + body_rect.left_bottom() + egui::vec2(indent, -item_height / 2.0), + body_rect.left_bottom(), ) } else { - body_response.rect + body_rect } } else { egui::Rect::NOTHING }; // body rect, if any AND it actually contains something - let non_empty_body_rect = body_response.map(|r| r.rect).filter(|r| r.height() > 0.0); + let non_empty_body_rect = body_rect.filter(|r| r.height() > 0.0); // visualize the drag zones in debug builds, when the `Alt` key is pressed during drag #[cfg(debug_assertions)] @@ -219,23 +230,23 @@ pub fn find_drop_target( if ui.rect_contains_pointer(left_top) { // insert before me Some(DropTarget::new( - response.rect.x_range(), + item_rect.x_range(), top.top(), parent_id, pos_in_parent, )) } else if ui.rect_contains_pointer(top) { // insert last in the previous container if any, else insert before me - if let Some(previous_container_id) = item_desc.previous_container_id { + if let Some(previous_container_id) = item_context.previous_container_id { Some(DropTarget::new( - (response.rect.left() + indent..=response.rect.right()).into(), + (item_rect.left() + indent..=item_rect.right()).into(), top.top(), previous_container_id, usize::MAX, )) } else { Some(DropTarget::new( - response.rect.x_range(), + item_rect.x_range(), top.top(), parent_id, pos_in_parent, @@ -246,7 +257,7 @@ pub fn find_drop_target( else if ui.rect_contains_pointer(body_insert_after_me_area) { // insert after me in my parent Some(DropTarget::new( - response.rect.x_range(), + item_rect.x_range(), body_insert_after_me_area.bottom(), parent_id, pos_in_parent + 1, @@ -263,7 +274,7 @@ pub fn find_drop_target( // insert after me Some(DropTarget::new( - response.rect.x_range(), + item_rect.x_range(), position_y, parent_id, pos_in_parent + 1, @@ -288,7 +299,7 @@ pub fn find_drop_target( } else if ui.rect_contains_pointer(left_bottom) { // insert after me in my parent Some(DropTarget::new( - response.rect.x_range(), + item_rect.x_range(), left_bottom.bottom(), parent_id, pos_in_parent + 1, @@ -296,7 +307,7 @@ pub fn find_drop_target( } else if ui.rect_contains_pointer(bottom) { // insert at pos = 0 inside me Some(DropTarget::new( - (response.rect.left() + indent..=response.rect.right()).into(), + (item_rect.left() + indent..=item_rect.right()).into(), bottom.bottom(), item_id, 0, diff --git a/crates/re_viewport/src/viewport_blueprint_ui.rs b/crates/re_viewport/src/viewport_blueprint_ui.rs index 8ba64be010fe..ee18fe68c736 100644 --- a/crates/re_viewport/src/viewport_blueprint_ui.rs +++ b/crates/re_viewport/src/viewport_blueprint_ui.rs @@ -578,7 +578,7 @@ impl Viewport<'_, '_> { // Prepare the item description structure needed by `find_drop_target`. Here, we use // `Contents` for the "ItemId" generic type parameter. - let item_desc = re_ui::drag_and_drop::DropItemDescription { + let item_desc = re_ui::drag_and_drop::ItemContext { id: contents, is_container: matches!(contents, Contents::Container(_)), parent_id: Contents::Container(parent_container_id), @@ -589,8 +589,8 @@ impl Viewport<'_, '_> { let drop_target = re_ui::drag_and_drop::find_drop_target( ui, &item_desc, - response, - body_response, + response.rect, + body_response.map(|r| r.rect), ReUi::list_item_height(), ); From 85d31a9024f66af77ef09f82cadcc60bd07febd0 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Mon, 29 Jan 2024 17:03:28 +0100 Subject: [PATCH 23/28] Review comments: `ListItem` code clean-up --- crates/re_ui/src/list_item.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/crates/re_ui/src/list_item.rs b/crates/re_ui/src/list_item.rs index 0dd14f75c940..21fc95529514 100644 --- a/crates/re_ui/src/list_item.rs +++ b/crates/re_ui/src/list_item.rs @@ -165,7 +165,7 @@ impl<'a> ListItem<'a> { self } - /// Make the item draggable and set its persistent ID. + /// Make the item draggable. #[inline] pub fn draggable(mut self, draggable: bool) -> Self { self.draggable = draggable; @@ -386,7 +386,7 @@ impl<'a> ListItem<'a> { let (rect, mut response) = ui.allocate_at_least( desired_size, if self.draggable { - egui::Sense::drag() + egui::Sense::click_and_drag() } else { egui::Sense::click() }, @@ -458,12 +458,9 @@ impl<'a> ListItem<'a> { } // Handle buttons - let anything_being_decidedly_dragged = ui.memory(|mem| mem.is_anything_being_dragged()) - && ui.input(|i| i.pointer.is_decidedly_dragging()); - let button_response = if self.active - && ui.rect_contains_pointer(rect) - && !anything_being_decidedly_dragged - { + // Note: `response.hovered()` always returns `false` when a drag is in process, which is the desired + // behavior as we don't want to display the buttons while dragging. + let button_response = if self.active && response.hovered() { if let Some(buttons) = self.buttons_fn { let mut ui = ui.child_ui(rect, egui::Layout::right_to_left(egui::Align::Center)); From 8dc3acafcda7a876bf9dfdd04b70afd024b7a4d9 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Mon, 29 Jan 2024 17:24:31 +0100 Subject: [PATCH 24/28] Review comments: highlighted container clarifications --- crates/re_viewport/src/viewport.rs | 18 ++++++++++++------ .../re_viewport/src/viewport_blueprint_ui.rs | 2 +- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/crates/re_viewport/src/viewport.rs b/crates/re_viewport/src/viewport.rs index 85e87c58e233..ac289400455e 100644 --- a/crates/re_viewport/src/viewport.rs +++ b/crates/re_viewport/src/viewport.rs @@ -38,8 +38,10 @@ pub struct ViewportState { space_view_entity_window: SpaceViewEntityPicker, space_view_states: HashMap, - /// when a drag is in progress, this is the currently identified drop target to be highlighted - drop_target_container_id: Option, + /// Current candidate parent container for the onging drop. + /// + /// See [`ViewportState::is_candidate_drop_parent_container`] for details. + candidate_drop_parent_container_id: Option, } static DEFAULT_PROPS: Lazy = Lazy::::new(Default::default); @@ -67,8 +69,12 @@ impl ViewportState { .map_or(&DEFAULT_PROPS, |state| &state.auto_properties) } - pub fn is_drop_target(&self, container_id: &ContainerId) -> bool { - self.drop_target_container_id.as_ref() == Some(container_id) + /// Is the provided container the current candidate parent container for the ongoing drag? + /// + /// When a drag is in progress, the candidate parent container for the dragged item should be highlighted. Note that + /// this can happen when hovering said container, its direct children, or even the item just after it. + pub fn is_candidate_drop_parent_container(&self, container_id: &ContainerId) -> bool { + self.candidate_drop_parent_container_id.as_ref() == Some(container_id) } } @@ -338,7 +344,7 @@ impl<'a, 'b> Viewport<'a, 'b> { let mut reset = false; // always reset the drop target - self.state.drop_target_container_id = None; + self.state.candidate_drop_parent_container_id = None; // TODO(#4687): Be extra careful here. If we mark edited inappropriately we can create an infinite edit loop. for tree_action in self.tree_action_receiver.try_iter() { @@ -464,7 +470,7 @@ impl<'a, 'b> Viewport<'a, 'b> { self.tree_edited = true; } TreeAction::SetDropTarget(container_id) => { - self.state.drop_target_container_id = Some(container_id); + self.state.candidate_drop_parent_container_id = Some(container_id); } } } diff --git a/crates/re_viewport/src/viewport_blueprint_ui.rs b/crates/re_viewport/src/viewport_blueprint_ui.rs index ee18fe68c736..8bc9e19777f1 100644 --- a/crates/re_viewport/src/viewport_blueprint_ui.rs +++ b/crates/re_viewport/src/viewport_blueprint_ui.rs @@ -101,7 +101,7 @@ impl Viewport<'_, '_> { .subdued(!container_visible) .selected(ctx.selection().contains_item(&item)) .draggable(true) - .drop_target_style(self.state.is_drop_target(container_id)) + .drop_target_style(self.state.is_candidate_drop_parent_container(container_id)) .with_icon(crate::icon_for_container_kind( &container_blueprint.container_kind, )) From e4b47c0860181730f2408c8e439af675dc8c3f79 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler <49431240+abey79@users.noreply.github.com> Date: Tue, 30 Jan 2024 09:45:00 +0100 Subject: [PATCH 25/28] Update crates/re_viewport/src/viewport.rs Co-authored-by: Emil Ernerfeldt --- crates/re_viewport/src/viewport.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/re_viewport/src/viewport.rs b/crates/re_viewport/src/viewport.rs index ac289400455e..b39340243ae6 100644 --- a/crates/re_viewport/src/viewport.rs +++ b/crates/re_viewport/src/viewport.rs @@ -38,7 +38,7 @@ pub struct ViewportState { space_view_entity_window: SpaceViewEntityPicker, space_view_states: HashMap, - /// Current candidate parent container for the onging drop. + /// Current candidate parent container for the ongoing drop. /// /// See [`ViewportState::is_candidate_drop_parent_container`] for details. candidate_drop_parent_container_id: Option, From 2040ab02c513739dda4769cb750d44851f7f495e Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Tue, 30 Jan 2024 09:56:54 +0100 Subject: [PATCH 26/28] Post merge --- Cargo.lock | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b68fcb506009..17ad8c7d750f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1559,7 +1559,7 @@ checksum = "68b0cf012f1230e43cd00ebb729c6bb58707ecfa8ad08b52ef3a4ccd2697fc30" [[package]] name = "ecolor" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=ab39420c2933d2e402999043375b64e7cf0ee9ed#ab39420c2933d2e402999043375b64e7cf0ee9ed" +source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" dependencies = [ "bytemuck", "serde", @@ -1568,7 +1568,7 @@ dependencies = [ [[package]] name = "eframe" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=ab39420c2933d2e402999043375b64e7cf0ee9ed#ab39420c2933d2e402999043375b64e7cf0ee9ed" +source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" dependencies = [ "bytemuck", "cocoa", @@ -1593,7 +1593,6 @@ dependencies = [ "wasm-bindgen", "wasm-bindgen-futures", "web-sys", - "web-time", "wgpu", "winapi", "winit", @@ -1602,7 +1601,7 @@ dependencies = [ [[package]] name = "egui" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=ab39420c2933d2e402999043375b64e7cf0ee9ed#ab39420c2933d2e402999043375b64e7cf0ee9ed" +source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" dependencies = [ "accesskit", "ahash", @@ -1618,7 +1617,7 @@ dependencies = [ [[package]] name = "egui-wgpu" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=ab39420c2933d2e402999043375b64e7cf0ee9ed#ab39420c2933d2e402999043375b64e7cf0ee9ed" +source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" dependencies = [ "bytemuck", "egui", @@ -1627,7 +1626,6 @@ dependencies = [ "puffin", "thiserror", "type-map", - "web-time", "wgpu", "winit", ] @@ -1635,7 +1633,7 @@ dependencies = [ [[package]] name = "egui-winit" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=ab39420c2933d2e402999043375b64e7cf0ee9ed#ab39420c2933d2e402999043375b64e7cf0ee9ed" +source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" dependencies = [ "accesskit_winit", "arboard", @@ -1664,7 +1662,7 @@ dependencies = [ [[package]] name = "egui_extras" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=ab39420c2933d2e402999043375b64e7cf0ee9ed#ab39420c2933d2e402999043375b64e7cf0ee9ed" +source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" dependencies = [ "egui", "ehttp", @@ -1679,7 +1677,7 @@ dependencies = [ [[package]] name = "egui_glow" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=ab39420c2933d2e402999043375b64e7cf0ee9ed#ab39420c2933d2e402999043375b64e7cf0ee9ed" +source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" dependencies = [ "bytemuck", "egui", @@ -1696,7 +1694,7 @@ dependencies = [ [[package]] name = "egui_plot" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=ab39420c2933d2e402999043375b64e7cf0ee9ed#ab39420c2933d2e402999043375b64e7cf0ee9ed" +source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" dependencies = [ "egui", ] @@ -1704,8 +1702,7 @@ dependencies = [ [[package]] name = "egui_tiles" version = "0.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c0255c0209b349b1a4a67a344556501e75accae669f3a25be6e07deb30fefa91" +source = "git+https://github.com/rerun-io/egui_tiles?rev=35e711283e7a021ca425d9fbd8e7581548971f49#35e711283e7a021ca425d9fbd8e7581548971f49" dependencies = [ "ahash", "egui", @@ -1739,7 +1736,7 @@ checksum = "a26ae43d7bcc3b814de94796a5e736d4029efb0ee900c12e2d54c993ad1a1e07" [[package]] name = "emath" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=ab39420c2933d2e402999043375b64e7cf0ee9ed#ab39420c2933d2e402999043375b64e7cf0ee9ed" +source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" dependencies = [ "bytemuck", "serde", @@ -1840,7 +1837,7 @@ dependencies = [ [[package]] name = "epaint" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=ab39420c2933d2e402999043375b64e7cf0ee9ed#ab39420c2933d2e402999043375b64e7cf0ee9ed" +source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" dependencies = [ "ab_glyph", "ahash", @@ -4651,7 +4648,6 @@ dependencies = [ name = "re_space_view" version = "0.13.0-alpha.3" dependencies = [ - "ahash", "egui", "itertools 0.12.0", "nohash-hasher", @@ -4735,7 +4731,6 @@ dependencies = [ "re_viewer_context", "serde", "smallvec", - "web-time", ] [[package]] @@ -4991,7 +4986,6 @@ dependencies = [ "eframe", "egui", "egui-wgpu", - "egui_extras", "egui_plot", "egui_tiles", "ehttp", From 3abcb574f1b3c14529e16165414777c87cd6b56a Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Tue, 30 Jan 2024 11:19:08 +0100 Subject: [PATCH 27/28] Post merge proper --- Cargo.lock | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 17ad8c7d750f..6e761715e4ed 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1559,7 +1559,7 @@ checksum = "68b0cf012f1230e43cd00ebb729c6bb58707ecfa8ad08b52ef3a4ccd2697fc30" [[package]] name = "ecolor" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" +source = "git+https://github.com/emilk/egui.git?rev=ab39420c2933d2e402999043375b64e7cf0ee9ed#ab39420c2933d2e402999043375b64e7cf0ee9ed" dependencies = [ "bytemuck", "serde", @@ -1568,7 +1568,7 @@ dependencies = [ [[package]] name = "eframe" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" +source = "git+https://github.com/emilk/egui.git?rev=ab39420c2933d2e402999043375b64e7cf0ee9ed#ab39420c2933d2e402999043375b64e7cf0ee9ed" dependencies = [ "bytemuck", "cocoa", @@ -1593,6 +1593,7 @@ dependencies = [ "wasm-bindgen", "wasm-bindgen-futures", "web-sys", + "web-time", "wgpu", "winapi", "winit", @@ -1601,7 +1602,7 @@ dependencies = [ [[package]] name = "egui" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" +source = "git+https://github.com/emilk/egui.git?rev=ab39420c2933d2e402999043375b64e7cf0ee9ed#ab39420c2933d2e402999043375b64e7cf0ee9ed" dependencies = [ "accesskit", "ahash", @@ -1617,7 +1618,7 @@ dependencies = [ [[package]] name = "egui-wgpu" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" +source = "git+https://github.com/emilk/egui.git?rev=ab39420c2933d2e402999043375b64e7cf0ee9ed#ab39420c2933d2e402999043375b64e7cf0ee9ed" dependencies = [ "bytemuck", "egui", @@ -1626,6 +1627,7 @@ dependencies = [ "puffin", "thiserror", "type-map", + "web-time", "wgpu", "winit", ] @@ -1633,7 +1635,7 @@ dependencies = [ [[package]] name = "egui-winit" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" +source = "git+https://github.com/emilk/egui.git?rev=ab39420c2933d2e402999043375b64e7cf0ee9ed#ab39420c2933d2e402999043375b64e7cf0ee9ed" dependencies = [ "accesskit_winit", "arboard", @@ -1662,7 +1664,7 @@ dependencies = [ [[package]] name = "egui_extras" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" +source = "git+https://github.com/emilk/egui.git?rev=ab39420c2933d2e402999043375b64e7cf0ee9ed#ab39420c2933d2e402999043375b64e7cf0ee9ed" dependencies = [ "egui", "ehttp", @@ -1677,7 +1679,7 @@ dependencies = [ [[package]] name = "egui_glow" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" +source = "git+https://github.com/emilk/egui.git?rev=ab39420c2933d2e402999043375b64e7cf0ee9ed#ab39420c2933d2e402999043375b64e7cf0ee9ed" dependencies = [ "bytemuck", "egui", @@ -1694,7 +1696,7 @@ dependencies = [ [[package]] name = "egui_plot" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" +source = "git+https://github.com/emilk/egui.git?rev=ab39420c2933d2e402999043375b64e7cf0ee9ed#ab39420c2933d2e402999043375b64e7cf0ee9ed" dependencies = [ "egui", ] @@ -1736,7 +1738,7 @@ checksum = "a26ae43d7bcc3b814de94796a5e736d4029efb0ee900c12e2d54c993ad1a1e07" [[package]] name = "emath" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" +source = "git+https://github.com/emilk/egui.git?rev=ab39420c2933d2e402999043375b64e7cf0ee9ed#ab39420c2933d2e402999043375b64e7cf0ee9ed" dependencies = [ "bytemuck", "serde", @@ -1837,7 +1839,7 @@ dependencies = [ [[package]] name = "epaint" version = "0.25.0" -source = "git+https://github.com/emilk/egui.git?rev=13b59fb0d4c4b032edc426abed4a98decd0163a6#13b59fb0d4c4b032edc426abed4a98decd0163a6" +source = "git+https://github.com/emilk/egui.git?rev=ab39420c2933d2e402999043375b64e7cf0ee9ed#ab39420c2933d2e402999043375b64e7cf0ee9ed" dependencies = [ "ab_glyph", "ahash", @@ -4648,6 +4650,7 @@ dependencies = [ name = "re_space_view" version = "0.13.0-alpha.3" dependencies = [ + "ahash", "egui", "itertools 0.12.0", "nohash-hasher", @@ -4731,6 +4734,7 @@ dependencies = [ "re_viewer_context", "serde", "smallvec", + "web-time", ] [[package]] @@ -4986,6 +4990,7 @@ dependencies = [ "eframe", "egui", "egui-wgpu", + "egui_extras", "egui_plot", "egui_tiles", "ehttp", From b4084fe32cd0466fe6acd4807a33545091c9e1ba Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Tue, 30 Jan 2024 15:55:38 +0100 Subject: [PATCH 28/28] Rolled back some `ListItem` changes that triggered unwanted interactions --- crates/re_ui/src/list_item.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/re_ui/src/list_item.rs b/crates/re_ui/src/list_item.rs index 21fc95529514..057159b2489b 100644 --- a/crates/re_ui/src/list_item.rs +++ b/crates/re_ui/src/list_item.rs @@ -458,9 +458,14 @@ impl<'a> ListItem<'a> { } // Handle buttons - // Note: `response.hovered()` always returns `false` when a drag is in process, which is the desired - // behavior as we don't want to display the buttons while dragging. - let button_response = if self.active && response.hovered() { + // Note: We should be able to just use `response.hovered()` here, which only returns `true` if no drag is in + // progress. Due to the response merging we do above, this breaks though. This is why we do an explicit + // rectangle and drag payload check. + //TODO(ab): refactor responses to address that. + let should_show_buttons = self.active + && ui.rect_contains_pointer(rect) + && !egui::DragAndDrop::has_any_payload(ui.ctx()); + let button_response = if should_show_buttons { if let Some(buttons) = self.buttons_fn { let mut ui = ui.child_ui(rect, egui::Layout::right_to_left(egui::Align::Center));