From a1a4df28532c9a250c8def95207d748429bdce25 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler <49431240+abey79@users.noreply.github.com> Date: Thu, 1 Feb 2024 18:21:33 +0100 Subject: [PATCH] Improve how the root container is displayed and handled in the blueprint tree (#4989) ### What This PR changes how the root container is displayed and handled: - It's labeled as "Viewport", since it defines the top-level organisation of the viewport. - It's not collapsible, so it doesn't have a triangle. This reduces the right-ward drift. - It (still) cannot be dragged (but now the difference with other containers is more obvious). It now behaves better when dragged over though. - It cannot be removed. Other than that, it still behaves mostly as other containers (in particular, it can be selected and edited). Some changes were needed in the drag-and-drop code, making the "if-statement-of-death" incrementally more complicated, sadly. * Fixes #4909 image ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [x] I've included a screenshot or gif (if applicable) * [x] I have tested the web demo (if applicable): * Using newly built examples: [app.rerun.io](https://app.rerun.io/pr/4989/index.html) * Using examples from latest `main` build: [app.rerun.io](https://app.rerun.io/pr/4989/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json) * Using full set of examples from `nightly` build: [app.rerun.io](https://app.rerun.io/pr/4989/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json) * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG - [PR Build Summary](https://build.rerun.io/pr/4989) - [Docs preview](https://rerun.io/preview/f0e2e0f5a8b95c99a75fa09038187289ff3a3173/docs) - [Examples preview](https://rerun.io/preview/f0e2e0f5a8b95c99a75fa09038187289ff3a3173/examples) - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) --- crates/re_ui/examples/re_ui_example.rs | 14 +- crates/re_ui/src/drag_and_drop.rs | 260 +++++++++++------- .../re_viewport/src/viewport_blueprint_ui.rs | 113 +++++++- 3 files changed, 271 insertions(+), 116 deletions(-) diff --git a/crates/re_ui/examples/re_ui_example.rs b/crates/re_ui/examples/re_ui_example.rs index 411f418b9d05..2ac13726f988 100644 --- a/crates/re_ui/examples/re_ui_example.rs +++ b/crates/re_ui/examples/re_ui_example.rs @@ -1153,9 +1153,17 @@ mod hierarchical_drag_and_drop { let item_desc = re_ui::drag_and_drop::ItemContext { id: item_id, - is_container, - parent_id, - position_index_in_parent, + item_kind: if is_container { + re_ui::drag_and_drop::ItemKind::Container { + parent_id, + position_index_in_parent, + } + } else { + re_ui::drag_and_drop::ItemKind::Leaf { + parent_id, + position_index_in_parent, + } + }, previous_container_id, }; diff --git a/crates/re_ui/src/drag_and_drop.rs b/crates/re_ui/src/drag_and_drop.rs index 510d4f466a10..38577c360d74 100644 --- a/crates/re_ui/src/drag_and_drop.rs +++ b/crates/re_ui/src/drag_and_drop.rs @@ -2,6 +2,26 @@ //! //! Works well in combination with [`crate::list_item::ListItem`]. +pub enum ItemKind { + /// Root container item. + /// + /// Root container don't have a parent and are restricted when hovered as drop target (the dragged item can only go + /// _in_ the root container, not before or after). + RootContainer, + + /// Container item. + Container { + parent_id: ItemId, + position_index_in_parent: usize, + }, + + /// Leaf item. + Leaf { + parent_id: ItemId, + position_index_in_parent: usize, + }, +} + /// Context information about the hovered item. /// /// This is used by [`find_drop_target`] to compute the [`DropTarget`], if any. @@ -9,14 +29,8 @@ pub struct ItemContext { /// 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 of this item. - pub parent_id: ItemId, - - /// Position of this item within its parent. - pub position_index_in_parent: usize, + /// What kind of item is this? + pub item_kind: ItemKind, /// ID of the container just before this item within the parent, if such a container exists. pub previous_container_id: Option, @@ -137,11 +151,23 @@ impl DropTarget { /// ┌─▼─── ║ ║ │ /// │ ║ ║ │ /// └───── ╚═════════════════════════════════════════════╝ ─┘ +/// +/// not a valid drop zone +/// │ +/// ╔═════════════════════════════════▼══════════════════╗ +/// root container ║ ║ +/// item ║ ────────────────────────────────────────────────── ║ +/// ║ ║ +/// ╚════════════════════════▲═══════════════════════════╝ +/// │ +/// insert inside me +/// at pos = 0 /// ``` /// /// 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. +/// - The top parts of the item are treated the same in most cases (root container is an +/// exception). /// - 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". @@ -158,20 +184,25 @@ pub fn find_drop_target( ) -> Option> { let indent = ui.spacing().indent; 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; + let item_kind = &item_context.item_kind; + let is_non_root_container = matches!(item_kind, ItemKind::Container { .. }); // For both leaf and containers we have two drag zones on the upper half of the item. - 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); + let (mut top, mut bottom) = item_rect.split_top_bottom_at_fraction(0.5); + + let mut left_top = egui::Rect::NOTHING; + if matches!(item_kind, ItemKind::RootContainer) { + top = egui::Rect::NOTHING; + } else { + (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 leaf and root container items, 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 { + if is_non_root_container { (left_bottom, bottom) = bottom.split_left_right_at_x(bottom.left() + indent); } @@ -181,7 +212,7 @@ pub fn find_drop_target( // - 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_rect) = body_rect { - if item_context.is_container { + if is_non_root_container { egui::Rect::from_two_pos( body_rect.left_bottom() + egui::vec2(indent, -item_height / 2.0), body_rect.left_bottom(), @@ -226,95 +257,120 @@ pub fn find_drop_target( } } - /* ===== TOP SECTIONS (same leaf/container items) ==== */ - if ui.rect_contains_pointer(left_top) { - // insert before me - Some(DropTarget::new( - item_rect.x_range(), - top.top(), + match *item_kind { + ItemKind::RootContainer => { + // we just need to test the bottom section in this case. + if ui.rect_contains_pointer(bottom) { + Some(DropTarget::new( + item_rect.x_range(), + bottom.bottom(), + item_id, + 0, + )) + } else { + None + } + } + + ItemKind::Container { 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_context.previous_container_id { - Some(DropTarget::new( - (item_rect.left() + indent..=item_rect.right()).into(), - top.top(), - previous_container_id, - usize::MAX, - )) - } else { - Some(DropTarget::new( - item_rect.x_range(), - top.top(), - parent_id, - pos_in_parent, - )) + position_index_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( - item_rect.x_range(), - body_insert_after_me_area.bottom(), + | ItemKind::Leaf { 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() - }; + position_index_in_parent, + } => { + /* ===== TOP SECTIONS (same leaf/container items) ==== */ + if ui.rect_contains_pointer(left_top) { + // insert before me + Some(DropTarget::new( + item_rect.x_range(), + top.top(), + parent_id, + position_index_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_context.previous_container_id { + Some(DropTarget::new( + (item_rect.left() + indent..=item_rect.right()).into(), + top.top(), + previous_container_id, + usize::MAX, + )) + } else { + Some(DropTarget::new( + item_rect.x_range(), + top.top(), + parent_id, + position_index_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( + item_rect.x_range(), + body_insert_after_me_area.bottom(), + parent_id, + position_index_in_parent + 1, + )) + } + /* ==== BOTTOM SECTIONS (leaf item) ==== */ + else if !is_non_root_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( - item_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 + // insert after me + Some(DropTarget::new( + item_rect.x_range(), + position_y, + parent_id, + position_index_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( + item_rect.x_range(), + left_bottom.bottom(), + parent_id, + position_index_in_parent + 1, + )) + } else if ui.rect_contains_pointer(bottom) { + // insert at pos = 0 inside me + Some(DropTarget::new( + (item_rect.left() + indent..=item_rect.right()).into(), + bottom.bottom(), + item_id, + 0, + )) + } + /* ==== Who knows where else the mouse cursor might wander… ¯\_(ツ)_/¯ ==== */ + else { + None + } } - } else if ui.rect_contains_pointer(left_bottom) { - // insert after me in my parent - Some(DropTarget::new( - item_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( - (item_rect.left() + indent..=item_rect.right()).into(), - bottom.bottom(), - item_id, - 0, - )) - } - /* ==== Who knows where else the mouse cursor might wander… ¯\_(ツ)_/¯ ==== */ - else { - None } } diff --git a/crates/re_viewport/src/viewport_blueprint_ui.rs b/crates/re_viewport/src/viewport_blueprint_ui.rs index 5daac27cde35..2903d7a4743c 100644 --- a/crates/re_viewport/src/viewport_blueprint_ui.rs +++ b/crates/re_viewport/src/viewport_blueprint_ui.rs @@ -24,9 +24,7 @@ impl Viewport<'_, '_> { .auto_shrink([true, false]) .show(ui, |ui| { ctx.re_ui.panel_content(ui, |_, ui| { - if let Some(root_container) = self.blueprint.root_container { - self.contents_ui(ctx, ui, &Contents::Container(root_container), true); - } + self.root_container_tree_ui(ctx, ui); let empty_space_response = ui.allocate_response(ui.available_size(), egui::Sense::click()); @@ -68,6 +66,49 @@ impl Viewport<'_, '_> { }; } + /// Display the root container. + /// + /// The root container is different from other containers in that it cannot be removed or dragged, and it cannot be + /// collapsed, so it's drawn without a collapsing triangle.. + fn root_container_tree_ui(&self, ctx: &ViewerContext<'_>, ui: &mut egui::Ui) { + let Some(container_id) = self.blueprint.root_container else { + // nothing to draw if there is no root container + return; + }; + + let Some(container_blueprint) = self.blueprint.containers.get(&container_id) else { + re_log::warn_once!("Cannot find root container {container_id}"); + return; + }; + + let item = Item::Container(container_id); + + let item_response = ListItem::new( + ctx.re_ui, + format!("Viewport ({:?})", container_blueprint.container_kind), + ) + .selected(ctx.selection().contains_item(&item)) + .draggable(false) + .drop_target_style(self.state.is_candidate_drop_parent_container(&container_id)) + .label_style(re_ui::LabelStyle::Unnamed) + .with_icon(crate::icon_for_container_kind( + &container_blueprint.container_kind, + )) + .show(ui); + + for child in &container_blueprint.contents { + self.contents_ui(ctx, ui, child, true); + } + + ctx.select_hovered_on_click(&item_response, item); + + self.handle_root_container_drag_and_drop_interaction( + ui, + Contents::Container(container_id), + &item_response, + ); + } + fn container_tree_ui( &self, ctx: &ViewerContext<'_>, @@ -518,6 +559,49 @@ impl Viewport<'_, '_> { // ---------------------------------------------------------------------------- // drag and drop support + fn handle_root_container_drag_and_drop_interaction( + &self, + ui: &egui::Ui, + contents: Contents, + response: &egui::Response, + ) { + // + // check if a drag is in progress and set the cursor accordingly + // + + let Some(dragged_item_id) = egui::DragAndDrop::payload(ui.ctx()).map(|payload| *payload) + else { + // nothing is being dragged, so nothing to do + return; + }; + + ui.ctx().set_cursor_icon(egui::CursorIcon::Grabbing); + + // + // 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::ItemContext { + id: contents, + item_kind: re_ui::drag_and_drop::ItemKind::RootContainer, + previous_container_id: None, + }; + + let drop_target = re_ui::drag_and_drop::find_drop_target( + ui, + &item_desc, + response.rect, + None, + ReUi::list_item_height(), + ); + + if let Some(drop_target) = drop_target { + self.handle_drop_target(ui, dragged_item_id, &drop_target); + } + } + fn handle_drag_and_drop_interaction( &self, ctx: &ViewerContext<'_>, @@ -551,16 +635,16 @@ impl Viewport<'_, '_> { // find our parent, our position within parent, and the previous container (if any) // - let Some((parent_container_id, pos_in_parent)) = + let Some((parent_container_id, position_index_in_parent)) = self.blueprint.find_parent_and_position_index(&contents) else { return; }; - let previous_container = if pos_in_parent > 0 { + let previous_container = if position_index_in_parent > 0 { self.blueprint .container(&parent_container_id) - .map(|container| container.contents[pos_in_parent - 1]) + .map(|container| container.contents[position_index_in_parent - 1]) .filter(|contents| matches!(contents, Contents::Container(_))) } else { None @@ -572,11 +656,19 @@ 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::ItemContext { id: contents, - is_container: matches!(contents, Contents::Container(_)), - parent_id: Contents::Container(parent_container_id), - position_index_in_parent: pos_in_parent, + item_kind: match contents { + Contents::Container(_) => re_ui::drag_and_drop::ItemKind::Container { + parent_id: Contents::Container(parent_container_id), + position_index_in_parent, + }, + Contents::SpaceView(_) => re_ui::drag_and_drop::ItemKind::Leaf { + parent_id: Contents::Container(parent_container_id), + position_index_in_parent, + }, + }, previous_container_id: previous_container, }; @@ -618,8 +710,7 @@ impl Viewport<'_, '_> { 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.x_range(), empty_space.top(), Contents::Container(root_container_id), usize::MAX,