From 21ff44da7b16cb92093d54579c717181dd898711 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Sat, 26 Aug 2023 10:54:43 +0200 Subject: [PATCH 1/7] Use `ListItem` in blueprint tree UI --- crates/re_ui/src/list_item.rs | 82 +++- crates/re_viewer/src/app_state.rs | 14 +- crates/re_viewer/src/ui/blueprint_panel.rs | 22 +- crates/re_viewer/src/ui/recordings_panel.rs | 12 +- .../re_viewport/src/viewport_blueprint_ui.rs | 401 +++++++----------- 5 files changed, 228 insertions(+), 303 deletions(-) diff --git a/crates/re_ui/src/list_item.rs b/crates/re_ui/src/list_item.rs index 04e94f337b59..d128e7192260 100644 --- a/crates/re_ui/src/list_item.rs +++ b/crates/re_ui/src/list_item.rs @@ -7,6 +7,15 @@ struct ListItemResponse { collapse_response: Option, } +/// Responses returned by [`ListItem::show_collapsing`]. +pub struct ShowCollapsingResponse { + /// Response from the item itself. + pub item_response: Response, + + /// Response from the body, if it was displayed. + pub body_response: Option, +} + /// Generic widget for use in lists. /// /// Layout: @@ -45,6 +54,7 @@ pub struct ListItem<'a> { re_ui: &'a ReUi, active: bool, selected: bool, + subdued: bool, collapse_openness: Option, icon_fn: Option>, @@ -59,6 +69,7 @@ impl<'a> ListItem<'a> { re_ui, active: true, selected: false, + subdued: false, collapse_openness: None, icon_fn: None, buttons_fn: None, @@ -77,6 +88,16 @@ impl<'a> ListItem<'a> { self } + /// Set the subdued state of the item. + // TODO(ab): this is a hack to implement the behavior of the blueprint tree UI, where active + // widget are displayed in a subdued state (container, hidden space views/entities). One + // slightly more correct way would be to override the color using a (color, index) pair + // related to the design system table. + pub fn subdued(mut self, subdued: bool) -> Self { + self.subdued = subdued; + self + } + /// Provide an [`Icon`] to be displayed on the left of the item. pub fn with_icon(self, icon: &'a Icon) -> Self { self.with_icon_fn(|re_ui, ui, rect, visuals| { @@ -115,19 +136,20 @@ impl<'a> ListItem<'a> { /// Draw the item. pub fn show(self, ui: &mut Ui) -> Response { - self.ui(ui).response + self.ui(ui, None).response } /// Draw the item as a collapsing header. pub fn show_collapsing( mut self, ui: &mut Ui, + id: egui::Id, default_open: bool, add_body: impl FnOnce(&ReUi, &mut egui::Ui) -> R, - ) { + ) -> ShowCollapsingResponse { let mut state = egui::collapsing_header::CollapsingState::load_with_default_open( ui.ctx(), - ui.make_persistent_id(ui.id().with(self.text.text())), + id, default_open, ); @@ -135,7 +157,7 @@ impl<'a> ListItem<'a> { self.collapse_openness = Some(state.openness(ui.ctx())); let re_ui = self.re_ui; - let response = self.ui(ui); + let response = self.ui(ui, Some(id)); if let Some(collapse_response) = response.collapse_response { if collapse_response.clicked() { @@ -143,12 +165,16 @@ impl<'a> ListItem<'a> { } } - state.show_body_indented(&response.response, ui, |ui| { - add_body(re_ui, ui); - }); + let body_response = + state.show_body_indented(&response.response, ui, |ui| add_body(re_ui, ui)); + + ShowCollapsingResponse { + item_response: response.response, + body_response: body_response.map(|r| r.inner), + } } - fn ui(self, ui: &mut Ui) -> ListItemResponse { + fn ui(self, ui: &mut Ui, id: Option) -> ListItemResponse { let collapse_extra = if self.collapse_openness.is_some() { ReUi::collapsing_triangle_size().x + ReUi::text_to_icon_padding() } else { @@ -166,12 +192,17 @@ impl<'a> ListItem<'a> { let mut collapse_response = None; if ui.is_rect_visible(rect) { - let visuals = if self.active { + let mut visuals = if self.active { ui.style().interact_selectable(&response, self.selected) } else { ui.visuals().widgets.inactive }; + if self.subdued { + //TODO(ab): hack, see ['ListItem::subdued'] + visuals.fg_stroke.color = visuals.fg_stroke.color.gamma_multiply(0.5); + } + let mut bg_rect = rect; bg_rect.extend_with_x(ui.clip_rect().right()); bg_rect.extend_with_x(ui.clip_rect().left()); @@ -185,7 +216,11 @@ impl<'a> ListItem<'a> { )); let triangle_rect = egui::Rect::from_min_size(triangle_pos, ReUi::collapsing_triangle_size()); - let resp = ui.interact(triangle_rect, ui.id(), egui::Sense::click()); + let resp = ui.interact( + triangle_rect, + id.unwrap_or(ui.id()).with("collapsing_triangle"), + egui::Sense::click(), + ); ReUi::paint_collapsing_triangle(ui, openness, &resp); collapse_response = Some(resp); } @@ -201,18 +236,25 @@ impl<'a> ListItem<'a> { } // Handle buttons - let button_response = - if self.active && ui.interact(rect, ui.id(), egui::Sense::hover()).hovered() { - if let Some(buttons) = self.buttons_fn { - let mut ui = - ui.child_ui(rect, egui::Layout::right_to_left(egui::Align::Center)); - Some(buttons(self.re_ui, &mut ui)) - } else { - None - } + let button_response = if self.active + && ui + .interact( + rect, + id.unwrap_or(ui.id()).with("buttons"), + egui::Sense::hover(), + ) + .hovered() + { + if let Some(buttons) = self.buttons_fn { + let mut ui = + ui.child_ui(rect, egui::Layout::right_to_left(egui::Align::Center)); + Some(buttons(self.re_ui, &mut ui)) } else { None - }; + } + } else { + None + }; // Draw text next to the icon. let mut text_rect = rect; diff --git a/crates/re_viewer/src/app_state.rs b/crates/re_viewer/src/app_state.rs index 894064eee21c..470e8a11f738 100644 --- a/crates/re_viewer/src/app_state.rs +++ b/crates/re_viewer/src/app_state.rs @@ -167,21 +167,13 @@ impl AppState { // ListItem don't need vertical spacing so we disable it, but restore it // before drawing the blueprint panel. - // TODO(ab): remove this once the blueprint tree uses list items - let v_space = ui.spacing().item_spacing.y; ui.spacing_mut().item_spacing.y = 0.0; recordings_panel_ui(&mut ctx, ui); - // TODO(ab): remove this frame once the blueprint tree uses list items - egui::Frame { - inner_margin: re_ui::ReUi::panel_margin(), - ..Default::default() - } - .show(ui, |ui| { - ui.spacing_mut().item_spacing.y = v_space; - blueprint_panel_ui(&mut viewport.blueprint, &mut ctx, ui, &spaces_info); - }); + ui.add_space(4.0); + + blueprint_panel_ui(&mut viewport.blueprint, &mut ctx, ui, &spaces_info); }, ); diff --git a/crates/re_viewer/src/ui/blueprint_panel.rs b/crates/re_viewer/src/ui/blueprint_panel.rs index d010193af142..bb69534a87c5 100644 --- a/crates/re_viewer/src/ui/blueprint_panel.rs +++ b/crates/re_viewer/src/ui/blueprint_panel.rs @@ -8,17 +8,19 @@ pub fn blueprint_panel_ui( ui: &mut egui::Ui, spaces_info: &SpaceInfoCollection, ) { - ctx.re_ui.panel_title_bar_with_buttons( - ui, - "Blueprint", - Some("The Blueprint is where you can configure the Rerun Viewer"), - |ui| { - blueprint.add_new_spaceview_button_ui(ctx, ui, spaces_info); - reset_button_ui(blueprint, ctx, ui, spaces_info); - }, - ); + ctx.re_ui.panel_content(ui, |_, ui| { + ctx.re_ui.panel_title_bar_with_buttons( + ui, + "Blueprint", + Some("The Blueprint is where you can configure the Rerun Viewer"), + |ui| { + blueprint.add_new_spaceview_button_ui(ctx, ui, spaces_info); + reset_button_ui(blueprint, ctx, ui, spaces_info); + }, + ); - blueprint.tree_ui(ctx, ui); + blueprint.tree_ui(ctx, ui); + }); } fn reset_button_ui( diff --git a/crates/re_viewer/src/ui/recordings_panel.rs b/crates/re_viewer/src/ui/recordings_panel.rs index 3ddfea69ce9e..d9aece900499 100644 --- a/crates/re_viewer/src/ui/recordings_panel.rs +++ b/crates/re_viewer/src/ui/recordings_panel.rs @@ -72,10 +72,11 @@ fn recording_list_ui(ctx: &mut ViewerContext<'_>, ui: &mut egui::Ui) { .send_system(SystemCommand::SetRecordingId(store_db.store_id().clone())); } } else { - ctx.re_ui - .list_item(app_id) - .active(false) - .show_collapsing(ui, true, |_, ui| { + ctx.re_ui.list_item(app_id).active(false).show_collapsing( + ui, + ui.id().with(app_id), + true, + |_, ui| { for store_db in store_dbs { if recording_ui( ctx.re_ui, @@ -92,7 +93,8 @@ fn recording_list_ui(ctx: &mut ViewerContext<'_>, ui: &mut egui::Ui) { )); } } - }); + }, + ); } } } diff --git a/crates/re_viewport/src/viewport_blueprint_ui.rs b/crates/re_viewport/src/viewport_blueprint_ui.rs index 894e3f231927..7fc74c397df9 100644 --- a/crates/re_viewport/src/viewport_blueprint_ui.rs +++ b/crates/re_viewport/src/viewport_blueprint_ui.rs @@ -1,8 +1,9 @@ use itertools::Itertools; use re_data_store::InstancePath; -use re_data_ui::item_ui; +use re_data_ui::{item_ui, DataUi}; use re_space_view::DataBlueprintGroup; -use re_viewer_context::{DataBlueprintGroupHandle, Item, SpaceViewId, ViewerContext}; +use re_ui::list_item::ListItem; +use re_viewer_context::{DataBlueprintGroupHandle, Item, SpaceViewId, UiVerbosity, ViewerContext}; use crate::{ space_view_heuristics::all_possible_space_views, SpaceInfoCollection, SpaceViewBlueprint, @@ -95,33 +96,28 @@ impl ViewportBlueprint<'_> { let mut visible = self.tree.is_visible(tile_id); let default_open = true; - egui::collapsing_header::CollapsingState::load_with_default_open( - ui.ctx(), - egui::Id::new((tile_id, "tree")), - default_open, - ) - .show_header(ui, |ui| { - blueprint_row_with_buttons( - ctx.re_ui, - ui, - true, - visible, - false, - |ui| ui.label(format!("{:?}", container.kind())), - |re_ui, ui| { - visibility_changed = - visibility_button_ui(re_ui, ui, true, &mut visible).changed(); - if re_ui - .small_icon_button(ui, &re_ui::icons::REMOVE) - .on_hover_text("Remove container") - .clicked() - { - action = TreeAction::Remove; - } - }, - ); - }) - .body(|ui| container.retain(|child| self.tile_ui(ctx, ui, child) == TreeAction::Keep)); + + ListItem::new(ctx.re_ui, format!("{:?}", container.kind())) + .subdued(true) + .with_buttons(|re_ui, ui| { + //TODO(ab): deduplicate that + let vis_response = visibility_button_ui(re_ui, ui, true, &mut visible); + + visibility_changed = vis_response.changed(); + + let response = re_ui + .small_icon_button(ui, &re_ui::icons::REMOVE) + .on_hover_text("Remove container"); + + if response.clicked() { + action = TreeAction::Remove; + } + + response | vis_response + }) + .show_collapsing(ui, ui.id().with(tile_id), default_open, |_, ui| { + container.retain(|child| self.tile_ui(ctx, ui, child) == TreeAction::Keep); + }); if visibility_changed { self.has_been_user_edited = true; @@ -147,53 +143,50 @@ impl ViewportBlueprint<'_> { let mut visibility_changed = false; let mut action = TreeAction::Keep; let mut visible = self.tree.is_visible(tile_id); + let visible_child = visible; let item = Item::SpaceView(space_view.id); - let is_selected = ctx.selection().contains(&item); let root_group = space_view.data_blueprint.root_group(); let default_open = Self::default_open_for_group(root_group); let collapsing_header_id = ui.id().with(space_view.id); - egui::collapsing_header::CollapsingState::load_with_default_open( - ui.ctx(), - collapsing_header_id, - default_open, - ) - .show_header(ui, |ui| { - blueprint_row_with_buttons( - ctx.re_ui, - ui, - true, - visible, - is_selected, - |ui| { - let response = crate::item_ui::space_view_button(ctx, ui, space_view); - if response.clicked() { - focus_tab(&mut self.tree, space_view_id); - } - response - }, - |re_ui, ui| { - visibility_changed = - visibility_button_ui(re_ui, ui, true, &mut visible).changed(); - if re_ui - .small_icon_button(ui, &re_ui::icons::REMOVE) - .on_hover_text("Remove Space View from the Viewport") - .clicked() - { - action = TreeAction::Remove; - } - }, - ); - }) - .body(|ui| { - Self::data_blueprint_tree_ui( - ctx, - ui, - space_view.data_blueprint.root_handle(), - space_view, - visible, - ); - }); + + let response = ListItem::new(ctx.re_ui, space_view.display_name.clone()) + .selected(ctx.selection().contains(&item)) + .subdued(!visible) + .with_icon(space_view.class(ctx.space_view_class_registry).icon()) + .with_buttons(|re_ui, ui| { + //TODO(ab): deduplicate that + let vis_response = visibility_button_ui(re_ui, ui, true, &mut visible); + + visibility_changed = vis_response.changed(); + + let response = re_ui + .small_icon_button(ui, &re_ui::icons::REMOVE) + .on_hover_text("Remove Space View from the Viewport"); + + if response.clicked() { + action = TreeAction::Remove; + } + + response | vis_response + }) + .show_collapsing(ui, collapsing_header_id, default_open, |_, ui| { + Self::data_blueprint_tree_ui( + ctx, + ui, + space_view.data_blueprint.root_handle(), + space_view, + visible_child, + ); + }) + .item_response + .on_hover_text("Space View"); + + if response.clicked() { + focus_tab(&mut self.tree, space_view_id); + } + + item_ui::cursor_interact_with_selectable(ctx, response, item); if visibility_changed { self.has_been_user_edited = true; @@ -235,53 +228,48 @@ impl ViewportBlueprint<'_> { InstancePath::entity_splat(entity_path.clone()), )); - ui.horizontal(|ui| { - let mut properties = space_view - .data_blueprint - .data_blueprints_individual() - .get(entity_path); - blueprint_row_with_buttons( - ctx.re_ui, - ui, - group_is_visible, - properties.visible, - is_selected, - |ui| { - let name = entity_path.iter().last().unwrap().to_string(); - let label = format!("🔹 {name}"); - re_data_ui::item_ui::data_blueprint_button_to( - ctx, - ui, - label, - space_view.id, - entity_path, - ) - }, - |re_ui, ui| { - if visibility_button_ui( - re_ui, - ui, - group_is_visible, - &mut properties.visible, - ) - .changed() - { - space_view - .data_blueprint - .data_blueprints_individual() - .set(entity_path.clone(), properties); - } - if re_ui - .small_icon_button(ui, &re_ui::icons::REMOVE) - .on_hover_text("Remove Entity from the Space View") - .clicked() - { - space_view.data_blueprint.remove_entity(entity_path); - space_view.entities_determined_by_user = true; - } - }, - ); - }); + let item = Item::InstancePath( + Some(space_view.id), + InstancePath::entity_splat(entity_path.clone()), + ); + + let mut properties = space_view + .data_blueprint + .data_blueprints_individual() + .get(entity_path); + let name = entity_path.iter().last().unwrap().to_string(); + let label = format!("🔹 {name}"); + let response = ListItem::new(ctx.re_ui, label) + .selected(is_selected) + .subdued(!group_is_visible || !properties.visible) + .with_buttons(|re_ui, ui| { + let vis_response = + visibility_button_ui(re_ui, ui, group_is_visible, &mut properties.visible); + if vis_response.changed() { + space_view + .data_blueprint + .data_blueprints_individual() + .set(entity_path.clone(), properties); + } + let response = re_ui + .small_icon_button(ui, &re_ui::icons::REMOVE) + .on_hover_text("Remove Entity from the Space View"); + + if response.clicked() { + space_view.data_blueprint.remove_entity(entity_path); + space_view.entities_determined_by_user = true; + } + response | vis_response + }) + .show(ui) + //TODO(ab): refactor that (duplicated from data_blueprint_button_to) + .on_hover_ui(|ui| { + ui.strong("Space View Entity"); + ui.label(format!("Path: {entity_path}")); + entity_path.data_ui(ctx, ui, UiVerbosity::Reduced, &ctx.current_query()); + }); + + item_ui::cursor_interact_with_selectable(ctx, response, item); } for child_group_handle in &children { @@ -290,60 +278,53 @@ impl ViewportBlueprint<'_> { continue; }; - let is_selected = ctx.selection().contains(&Item::DataBlueprintGroup( - space_view.id, - *child_group_handle, - )); + let item = Item::DataBlueprintGroup(space_view.id, *child_group_handle); + let is_selected = ctx.selection().contains(&item); let mut remove_group = false; let default_open = Self::default_open_for_group(child_group); - egui::collapsing_header::CollapsingState::load_with_default_open( - ui.ctx(), - ui.id().with(child_group_handle), - default_open, - ) - .show_header(ui, |ui| { - blueprint_row_with_buttons( - ctx.re_ui, + + let mut child_group_visible = child_group.properties_individual.visible; + let response = ListItem::new(ctx.re_ui, child_group.display_name.clone()) + .selected(is_selected) + .subdued(!child_group_visible || !group_is_visible) + .with_icon(&re_ui::icons::CONTAINER) + .with_buttons(|re_ui, ui| { + let vis_response = + visibility_button_ui(re_ui, ui, group_is_visible, &mut child_group_visible); + let response = re_ui + .small_icon_button(ui, &re_ui::icons::REMOVE) + .on_hover_text("Remove Group and all its children from the Space View"); + if response.clicked() { + remove_group = true; + } + response | vis_response + }) + .show_collapsing( ui, - group_is_visible, - child_group.properties_individual.visible, - is_selected, - |ui| { - item_ui::data_blueprint_group_button_to( + ui.id().with(child_group_handle), + default_open, + |_, ui| { + Self::data_blueprint_tree_ui( ctx, ui, - child_group.display_name.clone(), - space_view.id, *child_group_handle, - ) - }, - |re_ui, ui| { - visibility_button_ui( - re_ui, - ui, - group_is_visible, - &mut child_group.properties_individual.visible, + space_view, + space_view_visible, ); - if re_ui - .small_icon_button(ui, &re_ui::icons::REMOVE) - .on_hover_text("Remove Group and all its children from the Space View") - .clicked() - { - remove_group = true; - } }, - ); - }) - .body(|ui| { - Self::data_blueprint_tree_ui( - ctx, - ui, - *child_group_handle, - space_view, - space_view_visible, - ); - }); + ) + .item_response + .on_hover_text("Group"); + + re_data_ui::item_ui::cursor_interact_with_selectable(ctx, response, item); + + // needed by the borrow checker + let Some(child_group) = space_view.data_blueprint.group_mut(*child_group_handle) else { + unreachable!("we did the same thing just above"); + }; + child_group.properties_individual.visible = child_group_visible; + if remove_group { space_view.data_blueprint.remove_group(*child_group_handle); space_view.entities_determined_by_user = true; @@ -402,100 +383,6 @@ fn focus_tab(tree: &mut egui_tiles::Tree, tab: &SpaceViewId) { }); } -/// Show a single button (`add_content`), justified, -/// and show a visibility button if the row is hovered. -/// -/// Returns true if visibility changed. -#[allow(clippy::fn_params_excessive_bools)] -fn blueprint_row_with_buttons( - re_ui: &re_ui::ReUi, - ui: &mut egui::Ui, - enabled: bool, - visible: bool, - selected: bool, - add_content: impl FnOnce(&mut egui::Ui) -> egui::Response, - add_on_hover_buttons: impl FnOnce(&re_ui::ReUi, &mut egui::Ui), -) { - let where_to_add_hover_rect = ui.painter().add(egui::Shape::Noop); - - // Make the main button span the whole width to make it easier to click: - let main_button_response = ui - .with_layout(egui::Layout::top_down_justified(egui::Align::LEFT), |ui| { - ui.style_mut().wrap = Some(false); - - // Turn off the background color of hovered buttons. - // Why? Because we add a manual hover-effect later. - // Why? Because we want that hover-effect even when only the visibility button is hovered. - let visuals = ui.visuals_mut(); - visuals.widgets.hovered.weak_bg_fill = egui::Color32::TRANSPARENT; - visuals.widgets.hovered.bg_fill = egui::Color32::TRANSPARENT; - visuals.widgets.active.weak_bg_fill = egui::Color32::TRANSPARENT; - visuals.widgets.active.bg_fill = egui::Color32::TRANSPARENT; - visuals.widgets.open.weak_bg_fill = egui::Color32::TRANSPARENT; - visuals.widgets.open.bg_fill = egui::Color32::TRANSPARENT; - - if ui - .interact(ui.max_rect(), ui.id(), egui::Sense::hover()) - .hovered() - { - // Clip the main button so that the on-hover buttons have room to cover it. - // Ideally we would only clip the button _text_, not the button background, but that's not possible. - let mut clip_rect = ui.max_rect(); - let on_hover_buttons_width = 36.0; - clip_rect.max.x -= on_hover_buttons_width; - ui.set_clip_rect(clip_rect); - } - - if !visible || !enabled { - // Dim the appearance of things added by `add_content`: - let widget_visuals = &mut ui.visuals_mut().widgets; - - fn dim_color(color: &mut egui::Color32) { - *color = color.gamma_multiply(0.5); - } - dim_color(&mut widget_visuals.noninteractive.fg_stroke.color); - dim_color(&mut widget_visuals.inactive.fg_stroke.color); - } - - add_content(ui) - }) - .inner; - - let main_button_rect = main_button_response.rect; - - // We check the same rectangle as the main button, - // but we will also catch hovers on the visibility button (if any). - let button_hovered = ui - .interact(main_button_rect, ui.id(), egui::Sense::hover()) - .hovered(); - if button_hovered { - // Just put the buttons on top of the existing ui: - let mut ui = ui.child_ui( - ui.max_rect(), - egui::Layout::right_to_left(egui::Align::Center), - ); - add_on_hover_buttons(re_ui, &mut ui); - } - - // The main button might have been highlighted because what it was referring - // to was hovered somewhere else, and then we also want it highlighted here. - if button_hovered || main_button_response.highlighted() || selected { - // Highlight the row: - let visuals = ui.visuals().widgets.hovered; - - let bg_fill = if selected { - ui.style().visuals.selection.bg_fill - } else { - visuals.bg_fill - }; - let hover_rect = main_button_rect.expand(visuals.expansion); - ui.painter().set( - where_to_add_hover_rect, - egui::Shape::rect_filled(hover_rect, visuals.rounding, bg_fill), - ); - } -} - fn visibility_button_ui( re_ui: &re_ui::ReUi, ui: &mut egui::Ui, From b0a5267da6b96bef5506fbf616c47f5120ae5723 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Mon, 28 Aug 2023 10:48:30 +0200 Subject: [PATCH 2/7] Fixe `re_ui_example` --- crates/re_ui/examples/re_ui_example.rs | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/crates/re_ui/examples/re_ui_example.rs b/crates/re_ui/examples/re_ui_example.rs index c563657d817f..ab64eea08d1b 100644 --- a/crates/re_ui/examples/re_ui_example.rs +++ b/crates/re_ui/examples/re_ui_example.rs @@ -317,15 +317,20 @@ impl eframe::App for ExampleApp { self.re_ui .list_item("Collapsing list item with icon") .with_icon(&re_ui::icons::SPACE_VIEW_2D) - .show_collapsing(ui, true, |_re_ui, ui| { - self.re_ui.list_item("Sub-item").show(ui); - self.re_ui.list_item("Sub-item").show(ui); - self.re_ui - .list_item("Sub-item with icon") - .with_icon(&re_ui::icons::SPACE_VIEW_TEXT) - .show(ui); - self.re_ui.list_item("Sub-item").show(ui); - }); + .show_collapsing( + ui, + "collapsing example".into(), + true, + |_re_ui, ui| { + self.re_ui.list_item("Sub-item").show(ui); + self.re_ui.list_item("Sub-item").show(ui); + self.re_ui + .list_item("Sub-item with icon") + .with_icon(&re_ui::icons::SPACE_VIEW_TEXT) + .show(ui); + self.re_ui.list_item("Sub-item").show(ui); + }, + ); }); }); }); From d859ed2c1b636b1d7d69dae94d4401815980d419 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Tue, 29 Aug 2023 11:09:54 +0200 Subject: [PATCH 3/7] Avoid using `egui::Response::highlight()` for the blueprint tree UI Instead, support for hover overriding is built in ListItem. This reduces the frame lag and related flicker. --- crates/re_ui/src/list_item.rs | 27 ++++++++++++++++--- .../re_viewport/src/viewport_blueprint_ui.rs | 19 ++++++++++--- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/crates/re_ui/src/list_item.rs b/crates/re_ui/src/list_item.rs index d128e7192260..0d5d500860ae 100644 --- a/crates/re_ui/src/list_item.rs +++ b/crates/re_ui/src/list_item.rs @@ -55,6 +55,7 @@ pub struct ListItem<'a> { active: bool, selected: bool, subdued: bool, + override_hover: bool, collapse_openness: Option, icon_fn: Option>, @@ -70,6 +71,7 @@ impl<'a> ListItem<'a> { active: true, selected: false, subdued: false, + override_hover: false, collapse_openness: None, icon_fn: None, buttons_fn: None, @@ -98,6 +100,16 @@ impl<'a> ListItem<'a> { self } + /// Override the hovered state even if the item is not actually hovered. + /// + /// Used to highlight items representing things that are hovered elsewhere in the UI. Note that + /// the [`egui::Response`] returned by [`Self::show`] and ]`Self::show_collapsing`] will still + /// reflect the actual hover state. + pub fn override_hover(mut self, override_hover: bool) -> Self { + self.override_hover = override_hover; + self + } + /// Provide an [`Icon`] to be displayed on the left of the item. pub fn with_icon(self, icon: &'a Icon) -> Self { self.with_icon_fn(|re_ui, ui, rect, visuals| { @@ -189,11 +201,18 @@ impl<'a> ListItem<'a> { let desired_size = egui::vec2(ui.available_width(), ReUi::list_item_height()); let (rect, response) = ui.allocate_at_least(desired_size, egui::Sense::click()); + // override_hover should not affect the returned response + let mut style_response = response.clone(); + if self.override_hover { + style_response.hovered = true; + } + let mut collapse_response = None; if ui.is_rect_visible(rect) { let mut visuals = if self.active { - ui.style().interact_selectable(&response, self.selected) + ui.style() + .interact_selectable(&style_response, self.selected) } else { ui.visuals().widgets.inactive }; @@ -289,9 +308,9 @@ impl<'a> ListItem<'a> { let bg_fill = if button_response.map_or(false, |r| r.hovered()) { Some(visuals.bg_fill) } else if self.selected - || response.hovered() - || response.highlighted() - || response.has_focus() + || style_response.hovered() + || style_response.highlighted() + || style_response.has_focus() { Some(visuals.weak_bg_fill) } else { diff --git a/crates/re_viewport/src/viewport_blueprint_ui.rs b/crates/re_viewport/src/viewport_blueprint_ui.rs index 7fc74c397df9..7537ca69d08f 100644 --- a/crates/re_viewport/src/viewport_blueprint_ui.rs +++ b/crates/re_viewport/src/viewport_blueprint_ui.rs @@ -3,7 +3,9 @@ use re_data_store::InstancePath; use re_data_ui::{item_ui, DataUi}; use re_space_view::DataBlueprintGroup; use re_ui::list_item::ListItem; -use re_viewer_context::{DataBlueprintGroupHandle, Item, SpaceViewId, UiVerbosity, ViewerContext}; +use re_viewer_context::{ + DataBlueprintGroupHandle, HoverHighlight, Item, SpaceViewId, UiVerbosity, ViewerContext, +}; use crate::{ space_view_heuristics::all_possible_space_views, SpaceInfoCollection, SpaceViewBlueprint, @@ -149,10 +151,13 @@ impl ViewportBlueprint<'_> { let root_group = space_view.data_blueprint.root_group(); let default_open = Self::default_open_for_group(root_group); let collapsing_header_id = ui.id().with(space_view.id); + let is_item_hovered = + ctx.selection_state().highlight_for_ui_element(&item) == HoverHighlight::Hovered; let response = ListItem::new(ctx.re_ui, space_view.display_name.clone()) .selected(ctx.selection().contains(&item)) .subdued(!visible) + .override_hover(is_item_hovered) .with_icon(space_view.class(ctx.space_view_class_registry).icon()) .with_buttons(|re_ui, ui| { //TODO(ab): deduplicate that @@ -186,7 +191,7 @@ impl ViewportBlueprint<'_> { focus_tab(&mut self.tree, space_view_id); } - item_ui::cursor_interact_with_selectable(ctx, response, item); + item_ui::select_hovered_on_click(ctx, &response, &[item]); if visibility_changed { self.has_been_user_edited = true; @@ -232,6 +237,8 @@ impl ViewportBlueprint<'_> { Some(space_view.id), InstancePath::entity_splat(entity_path.clone()), ); + let is_item_hovered = + ctx.selection_state().highlight_for_ui_element(&item) == HoverHighlight::Hovered; let mut properties = space_view .data_blueprint @@ -242,6 +249,7 @@ impl ViewportBlueprint<'_> { let response = ListItem::new(ctx.re_ui, label) .selected(is_selected) .subdued(!group_is_visible || !properties.visible) + .override_hover(is_item_hovered) .with_buttons(|re_ui, ui| { let vis_response = visibility_button_ui(re_ui, ui, group_is_visible, &mut properties.visible); @@ -269,7 +277,7 @@ impl ViewportBlueprint<'_> { entity_path.data_ui(ctx, ui, UiVerbosity::Reduced, &ctx.current_query()); }); - item_ui::cursor_interact_with_selectable(ctx, response, item); + item_ui::select_hovered_on_click(ctx, &response, &[item]); } for child_group_handle in &children { @@ -280,6 +288,8 @@ impl ViewportBlueprint<'_> { let item = Item::DataBlueprintGroup(space_view.id, *child_group_handle); let is_selected = ctx.selection().contains(&item); + let is_item_hovered = + ctx.selection_state().highlight_for_ui_element(&item) == HoverHighlight::Hovered; let mut remove_group = false; let default_open = Self::default_open_for_group(child_group); @@ -288,6 +298,7 @@ impl ViewportBlueprint<'_> { let response = ListItem::new(ctx.re_ui, child_group.display_name.clone()) .selected(is_selected) .subdued(!child_group_visible || !group_is_visible) + .override_hover(is_item_hovered) .with_icon(&re_ui::icons::CONTAINER) .with_buttons(|re_ui, ui| { let vis_response = @@ -317,7 +328,7 @@ impl ViewportBlueprint<'_> { .item_response .on_hover_text("Group"); - re_data_ui::item_ui::cursor_interact_with_selectable(ctx, response, item); + item_ui::select_hovered_on_click(ctx, &response, &[item]); // needed by the borrow checker let Some(child_group) = space_view.data_blueprint.group_mut(*child_group_handle) else { From 80ef7ee4369b1d71fc8212bf41f6ec5fee8b19ee Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Tue, 29 Aug 2023 11:15:24 +0200 Subject: [PATCH 4/7] Moved `space_view_button` closer to where it's still used --- crates/re_viewer/src/ui/selection_panel.rs | 24 +++++++++++++++-- crates/re_viewport/src/lib.rs | 30 ---------------------- 2 files changed, 22 insertions(+), 32 deletions(-) diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index 57f95a4e22c6..27efc46f870d 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -122,6 +122,26 @@ fn has_data_section(item: &Item) -> bool { } } +fn space_view_button( + ctx: &mut ViewerContext<'_>, + ui: &mut egui::Ui, + space_view: &re_viewport::SpaceViewBlueprint, +) -> egui::Response { + let item = Item::SpaceView(space_view.id); + let is_selected = ctx.selection().contains(&item); + + let response = ctx + .re_ui + .selectable_label_with_icon( + ui, + space_view.class(ctx.space_view_class_registry).icon(), + space_view.display_name.clone(), + is_selected, + ) + .on_hover_text("Space View"); + item_ui::cursor_interact_with_selectable(ctx, response, item) +} + /// What is selected? Not the contents, just the short id of it. fn what_is_selected_ui( ui: &mut egui::Ui, @@ -168,7 +188,7 @@ fn what_is_selected_ui( if let Some(space_view_id) = space_view_id { if let Some(space_view) = viewport.space_view_mut(space_view_id) { ui.label("In Space View"); - re_viewport::item_ui::space_view_button(ctx, ui, space_view); + space_view_button(ctx, ui, space_view); ui.end_row(); } } @@ -194,7 +214,7 @@ fn what_is_selected_ui( ui.end_row(); ui.label("In Space View"); - re_viewport::item_ui::space_view_button(ctx, ui, space_view); + space_view_button(ctx, ui, space_view); ui.end_row(); }); } diff --git a/crates/re_viewport/src/lib.rs b/crates/re_viewport/src/lib.rs index 602354878b1d..760334f6cdda 100644 --- a/crates/re_viewport/src/lib.rs +++ b/crates/re_viewport/src/lib.rs @@ -22,33 +22,3 @@ pub use viewport_blueprint::ViewportBlueprint; pub mod external { pub use re_space_view; } - -// --------------------------------------------------------------------------- - -// TODO(andreas): This should be part of re_data_ui::item_ui. -pub mod item_ui { - use re_data_ui::item_ui; - use re_viewer_context::{Item, ViewerContext}; - - use crate::space_view::SpaceViewBlueprint; - - pub fn space_view_button( - ctx: &mut ViewerContext<'_>, - ui: &mut egui::Ui, - space_view: &SpaceViewBlueprint, - ) -> egui::Response { - let item = Item::SpaceView(space_view.id); - let is_selected = ctx.selection().contains(&item); - - let response = ctx - .re_ui - .selectable_label_with_icon( - ui, - space_view.class(ctx.space_view_class_registry).icon(), - space_view.display_name.clone(), - is_selected, - ) - .on_hover_text("Space View"); - item_ui::cursor_interact_with_selectable(ctx, response, item) - } -} From a547ccde4801ac4dbfd6248add2bf72a54b20496 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Tue, 29 Aug 2023 11:22:42 +0200 Subject: [PATCH 5/7] No space between recording and blueprint header when there are no recordings (looks better) --- crates/re_viewer/src/app_state.rs | 6 ++++-- crates/re_viewer/src/ui/recordings_panel.rs | 18 +++++++++++++----- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/crates/re_viewer/src/app_state.rs b/crates/re_viewer/src/app_state.rs index 470e8a11f738..ec9f1c3c63c9 100644 --- a/crates/re_viewer/src/app_state.rs +++ b/crates/re_viewer/src/app_state.rs @@ -169,9 +169,11 @@ impl AppState { // before drawing the blueprint panel. ui.spacing_mut().item_spacing.y = 0.0; - recordings_panel_ui(&mut ctx, ui); + let recording_shown = recordings_panel_ui(&mut ctx, ui); - ui.add_space(4.0); + if recording_shown { + ui.add_space(4.0); + } blueprint_panel_ui(&mut viewport.blueprint, &mut ctx, ui, &spaces_info); }, diff --git a/crates/re_viewer/src/ui/recordings_panel.rs b/crates/re_viewer/src/ui/recordings_panel.rs index d9aece900499..df28851be8ac 100644 --- a/crates/re_viewer/src/ui/recordings_panel.rs +++ b/crates/re_viewer/src/ui/recordings_panel.rs @@ -7,7 +7,9 @@ static TIME_FORMAT_DESCRIPTION: once_cell::sync::Lazy< > = once_cell::sync::Lazy::new(|| format_description!(version = 2, "[hour]:[minute]:[second]Z")); /// Show the currently open Recordings in a selectable list. -pub fn recordings_panel_ui(ctx: &mut ViewerContext<'_>, ui: &mut egui::Ui) { +/// +/// Returns `true` if any recordings were shown. +pub fn recordings_panel_ui(ctx: &mut ViewerContext<'_>, ui: &mut egui::Ui) -> bool { ctx.re_ui.panel_content(ui, |re_ui, ui| { re_ui.panel_title_bar_with_buttons( ui, @@ -25,12 +27,16 @@ pub fn recordings_panel_ui(ctx: &mut ViewerContext<'_>, ui: &mut egui::Ui) { .max_height(300.) .show(ui, |ui| { ctx.re_ui - .panel_content(ui, |_re_ui, ui| recording_list_ui(ctx, ui)); - }); + .panel_content(ui, |_re_ui, ui| recording_list_ui(ctx, ui)) + }) + .inner } +/// Draw the recording list. +/// +/// Returns `true` if any recordings were shown. #[allow(clippy::blocks_in_if_conditions)] -fn recording_list_ui(ctx: &mut ViewerContext<'_>, ui: &mut egui::Ui) { +fn recording_list_ui(ctx: &mut ViewerContext<'_>, ui: &mut egui::Ui) -> bool { let ViewerContext { store_context, command_sender, @@ -46,7 +52,7 @@ fn recording_list_ui(ctx: &mut ViewerContext<'_>, ui: &mut egui::Ui) { } if store_dbs_map.is_empty() { - return; + return false; } for store_dbs in store_dbs_map.values_mut() { @@ -97,6 +103,8 @@ fn recording_list_ui(ctx: &mut ViewerContext<'_>, ui: &mut egui::Ui) { ); } } + + true } /// Show the UI for a single recording. From 60d54de0c51130fe980a0cb4f25ae3ea55d826fa Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Tue, 29 Aug 2023 11:40:21 +0200 Subject: [PATCH 6/7] 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 5f31a155942c..867fc8e5a9a4 100644 --- a/crates/re_viewport/src/viewport_blueprint_ui.rs +++ b/crates/re_viewport/src/viewport_blueprint_ui.rs @@ -336,7 +336,7 @@ impl ViewportBlueprint<'_> { item_ui::select_hovered_on_click(ctx, &response, &[item]); // needed by the borrow checker - let Some(child_group) = space_view.data_blueprint.group_mut(*child_group_handle) else { + let Some(child_group) = space_view.contents.group_mut(*child_group_handle) else { unreachable!("we did the same thing just above"); }; child_group.properties_individual.visible = child_group_visible; From a33ae6b49d223f5fb3ed15da9ff14740b1b4efae Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Tue, 29 Aug 2023 15:29:33 +0200 Subject: [PATCH 7/7] Misc clean-up and renaming --- crates/re_data_ui/src/item_ui.rs | 14 +++-- crates/re_ui/src/list_item.rs | 10 ++-- .../re_viewport/src/viewport_blueprint_ui.rs | 52 +++++++++---------- 3 files changed, 41 insertions(+), 35 deletions(-) diff --git a/crates/re_data_ui/src/item_ui.rs b/crates/re_data_ui/src/item_ui.rs index 17066520e0b2..38aaa98e031b 100644 --- a/crates/re_data_ui/src/item_ui.rs +++ b/crates/re_data_ui/src/item_ui.rs @@ -245,13 +245,21 @@ pub fn data_blueprint_button_to( let response = ui .selectable_label(ctx.selection().contains(&item), text) .on_hover_ui(|ui| { - ui.strong("Space View Entity"); - ui.label(format!("Path: {entity_path}")); - entity_path.data_ui(ctx, ui, UiVerbosity::Reduced, &ctx.current_query()); + data_blueprint_tooltip(ui, ctx, entity_path); }); cursor_interact_with_selectable(ctx, response, item) } +pub fn data_blueprint_tooltip( + ui: &mut egui::Ui, + ctx: &mut ViewerContext<'_>, + entity_path: &EntityPath, +) { + ui.strong("Space View Entity"); + ui.label(format!("Path: {entity_path}")); + entity_path.data_ui(ctx, ui, UiVerbosity::Reduced, &ctx.current_query()); +} + pub fn time_button( ctx: &mut ViewerContext<'_>, ui: &mut egui::Ui, diff --git a/crates/re_ui/src/list_item.rs b/crates/re_ui/src/list_item.rs index 0d5d500860ae..af887ea5fce4 100644 --- a/crates/re_ui/src/list_item.rs +++ b/crates/re_ui/src/list_item.rs @@ -55,7 +55,7 @@ pub struct ListItem<'a> { active: bool, selected: bool, subdued: bool, - override_hover: bool, + force_hovered: bool, collapse_openness: Option, icon_fn: Option>, @@ -71,7 +71,7 @@ impl<'a> ListItem<'a> { active: true, selected: false, subdued: false, - override_hover: false, + force_hovered: false, collapse_openness: None, icon_fn: None, buttons_fn: None, @@ -105,8 +105,8 @@ impl<'a> ListItem<'a> { /// Used to highlight items representing things that are hovered elsewhere in the UI. Note that /// the [`egui::Response`] returned by [`Self::show`] and ]`Self::show_collapsing`] will still /// reflect the actual hover state. - pub fn override_hover(mut self, override_hover: bool) -> Self { - self.override_hover = override_hover; + pub fn force_hovered(mut self, force_hovered: bool) -> Self { + self.force_hovered = force_hovered; self } @@ -203,7 +203,7 @@ impl<'a> ListItem<'a> { // override_hover should not affect the returned response let mut style_response = response.clone(); - if self.override_hover { + if self.force_hovered { style_response.hovered = true; } diff --git a/crates/re_viewport/src/viewport_blueprint_ui.rs b/crates/re_viewport/src/viewport_blueprint_ui.rs index 867fc8e5a9a4..3ad14eba78fa 100644 --- a/crates/re_viewport/src/viewport_blueprint_ui.rs +++ b/crates/re_viewport/src/viewport_blueprint_ui.rs @@ -1,10 +1,12 @@ +use egui::{Response, Ui}; use itertools::Itertools; use re_data_store::InstancePath; -use re_data_ui::{item_ui, DataUi}; +use re_data_ui::item_ui; use re_space_view::DataBlueprintGroup; use re_ui::list_item::ListItem; +use re_ui::ReUi; use re_viewer_context::{ - DataBlueprintGroupHandle, HoverHighlight, Item, SpaceViewId, UiVerbosity, ViewerContext, + DataBlueprintGroupHandle, HoverHighlight, Item, SpaceViewId, ViewerContext, }; use crate::{ @@ -104,15 +106,10 @@ impl ViewportBlueprint<'_> { ListItem::new(ctx.re_ui, format!("{:?}", container.kind())) .subdued(true) .with_buttons(|re_ui, ui| { - //TODO(ab): deduplicate that let vis_response = visibility_button_ui(re_ui, ui, true, &mut visible); - visibility_changed = vis_response.changed(); - let response = re_ui - .small_icon_button(ui, &re_ui::icons::REMOVE) - .on_hover_text("Remove container"); - + let response = remove_button_ui(re_ui, ui, "Remove container"); if response.clicked() { action = TreeAction::Remove; } @@ -159,18 +156,13 @@ impl ViewportBlueprint<'_> { let response = ListItem::new(ctx.re_ui, space_view.display_name.clone()) .selected(ctx.selection().contains(&item)) .subdued(!visible) - .override_hover(is_item_hovered) + .force_hovered(is_item_hovered) .with_icon(space_view.class(ctx.space_view_class_registry).icon()) .with_buttons(|re_ui, ui| { - //TODO(ab): deduplicate that let vis_response = visibility_button_ui(re_ui, ui, true, &mut visible); - visibility_changed = vis_response.changed(); - let response = re_ui - .small_icon_button(ui, &re_ui::icons::REMOVE) - .on_hover_text("Remove Space View from the Viewport"); - + let response = remove_button_ui(re_ui, ui, "Remove Space View from the Viewport"); if response.clicked() { action = TreeAction::Remove; } @@ -251,7 +243,7 @@ impl ViewportBlueprint<'_> { let response = ListItem::new(ctx.re_ui, label) .selected(is_selected) .subdued(!group_is_visible || !properties.visible) - .override_hover(is_item_hovered) + .force_hovered(is_item_hovered) .with_buttons(|re_ui, ui| { let vis_response = visibility_button_ui(re_ui, ui, group_is_visible, &mut properties.visible); @@ -261,22 +253,18 @@ impl ViewportBlueprint<'_> { .data_blueprints_individual() .set(entity_path.clone(), properties); } - let response = re_ui - .small_icon_button(ui, &re_ui::icons::REMOVE) - .on_hover_text("Remove Entity from the Space View"); + let response = remove_button_ui(re_ui, ui, "Remove Entity from the Space View"); if response.clicked() { space_view.contents.remove_entity(entity_path); space_view.entities_determined_by_user = true; } + response | vis_response }) .show(ui) - //TODO(ab): refactor that (duplicated from data_blueprint_button_to) .on_hover_ui(|ui| { - ui.strong("Space View Entity"); - ui.label(format!("Path: {entity_path}")); - entity_path.data_ui(ctx, ui, UiVerbosity::Reduced, &ctx.current_query()); + re_data_ui::item_ui::data_blueprint_tooltip(ui, ctx, entity_path); }); item_ui::select_hovered_on_click(ctx, &response, &[item]); @@ -303,17 +291,21 @@ impl ViewportBlueprint<'_> { let response = ListItem::new(ctx.re_ui, child_group.display_name.clone()) .selected(is_selected) .subdued(!child_group_visible || !group_is_visible) - .override_hover(is_item_hovered) + .force_hovered(is_item_hovered) .with_icon(&re_ui::icons::CONTAINER) .with_buttons(|re_ui, ui| { let vis_response = visibility_button_ui(re_ui, ui, group_is_visible, &mut child_group_visible); - let response = re_ui - .small_icon_button(ui, &re_ui::icons::REMOVE) - .on_hover_text("Remove Group and all its children from the Space View"); + + let response = remove_button_ui( + re_ui, + ui, + "Remove Group and all its children from the Space View", + ); if response.clicked() { remove_group = true; } + response | vis_response }) .show_collapsing( @@ -401,6 +393,12 @@ fn focus_tab(tree: &mut egui_tiles::Tree, tab: &SpaceViewId) { }); } +fn remove_button_ui(re_ui: &ReUi, ui: &mut Ui, tooltip: &str) -> Response { + re_ui + .small_icon_button(ui, &re_ui::icons::REMOVE) + .on_hover_text(tooltip) +} + fn visibility_button_ui( re_ui: &re_ui::ReUi, ui: &mut egui::Ui,