From a031540c254685a17de36847b7905b751b9ecf35 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Tue, 14 May 2024 18:02:44 +0200 Subject: [PATCH 1/8] Add support for exact width to `PropertyContent` --- .../re_ui/src/list_item2/property_content.rs | 46 +++++++++++++++++-- crates/re_ui/src/list_item2/scope.rs | 24 +++++++++- 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/crates/re_ui/src/list_item2/property_content.rs b/crates/re_ui/src/list_item2/property_content.rs index e91996eb3ff5..1a00082a3f3a 100644 --- a/crates/re_ui/src/list_item2/property_content.rs +++ b/crates/re_ui/src/list_item2/property_content.rs @@ -1,6 +1,6 @@ use egui::{text::TextWrapping, Align, Align2, NumExt as _, Ui}; -use super::{ContentContext, DesiredWidth, ListItemContent}; +use super::{ContentContext, DesiredWidth, LayoutInfoStack, ListItemContent}; use crate::{Icon, ReUi}; /// Closure to draw an icon left of the label. @@ -20,6 +20,8 @@ struct PropertyActionButton<'a> { pub struct PropertyContent<'a> { label: egui::WidgetText, min_desired_width: f32, + exact_width: bool, + icon_fn: Option>>, show_only_when_collapsed: bool, value_fn: Option>>, @@ -37,6 +39,7 @@ impl<'a> PropertyContent<'a> { Self { label: label.into(), min_desired_width: 200.0, + exact_width: false, icon_fn: None, show_only_when_collapsed: true, value_fn: None, @@ -54,6 +57,17 @@ impl<'a> PropertyContent<'a> { self } + /// Allocate the exact width required for the entire content. + /// + /// Note: this is done by tracking the maximum width in the current [`super::list_item_scope`] + /// during the previous frame, so this is effective on the second frame only. If the first frame + /// is actually rendered, this can lead to a flicker. + #[inline] + pub fn exact_width(mut self, exact_width: bool) -> Self { + self.exact_width = exact_width; + self + } + /// Provide an [`Icon`] to be displayed on the left of the label. #[inline] pub fn with_icon(self, icon: &'a Icon) -> Self { @@ -188,6 +202,7 @@ impl ListItemContent for PropertyContent<'_> { show_only_when_collapsed, value_fn, action_buttons, + exact_width: _, } = *self; // │ │ @@ -313,6 +328,11 @@ impl ListItemContent for PropertyContent<'_> { let mut child_ui = ui.child_ui(value_rect, egui::Layout::left_to_right(egui::Align::Center)); value_fn(re_ui, &mut child_ui, visuals); + + context.layout_info.register_property_content_max_width( + child_ui.ctx(), + child_ui.min_rect().right() - context.layout_info.left_x, + ); } } @@ -336,7 +356,27 @@ impl ListItemContent for PropertyContent<'_> { } } - fn desired_width(&self, _re_ui: &ReUi, _ui: &Ui) -> DesiredWidth { - DesiredWidth::AtLeast(self.min_desired_width) + fn desired_width(&self, _re_ui: &ReUi, ui: &Ui) -> DesiredWidth { + let layout_info = LayoutInfoStack::top(ui.ctx()); + if self.exact_width { + if let Some(max_width) = layout_info.property_content_max_width { + let mut desired_width = max_width + layout_info.left_x - ui.max_rect().left(); + + // TODO(ab): ideally there wouldn't be as much code duplication with `Self::ui` + let action_button_dimension = + ReUi::small_icon_size().x + 2.0 * ui.spacing().button_padding.x; + let reserve_action_button_space = + self.action_buttons.is_some() || layout_info.reserve_action_button_space; + if reserve_action_button_space { + desired_width += action_button_dimension + ReUi::text_to_icon_padding(); + } + + DesiredWidth::Exact(desired_width.ceil()) + } else { + DesiredWidth::AtLeast(self.min_desired_width) + } + } else { + DesiredWidth::AtLeast(self.min_desired_width) + } } } diff --git a/crates/re_ui/src/list_item2/scope.rs b/crates/re_ui/src/list_item2/scope.rs index 0bc8ac2e6ad2..52d951436e87 100644 --- a/crates/re_ui/src/list_item2/scope.rs +++ b/crates/re_ui/src/list_item2/scope.rs @@ -51,6 +51,11 @@ struct LayoutStatistics { /// /// The width is calculated from [`LayoutInfo::left_x`] to the right edge of the item. max_item_width: Option, + + /// `PropertyContent` only — max content width in the current scope. + /// + /// This value is measured from `left_x`. + property_content_max_width: Option, } impl LayoutStatistics { @@ -116,6 +121,11 @@ pub struct LayoutInfo { /// Scope id, used to retrieve the corresponding [`LayoutStatistics`]. scope_id: egui::Id, + + /// `PropertyContent` only — last frame's max content width, to be used in `desired_width()` + /// + /// This value is measured from `left_x`. + pub(crate) property_content_max_width: Option, } impl Default for LayoutInfo { @@ -125,6 +135,7 @@ impl Default for LayoutInfo { left_column_width: None, reserve_action_button_space: true, scope_id: egui::Id::NULL, + property_content_max_width: None, } } } @@ -158,6 +169,16 @@ impl LayoutInfo { stats.max_item_width = stats.max_item_width.map(|v| v.max(width)).or(Some(width)); }); } + + /// `PropertyContent` only — register max content width in the current scope + pub(super) fn register_property_content_max_width(&self, ctx: &egui::Context, width: f32) { + LayoutStatistics::update(ctx, self.scope_id, |stats| { + stats.property_content_max_width = stats + .property_content_max_width + .map(|v| v.max(width)) + .or(Some(width)); + }); + } } /// Stack of [`LayoutInfo`]s. @@ -255,10 +276,11 @@ pub fn list_item_scope( left_column_width, reserve_action_button_space: layout_stats.is_action_button_used, scope_id, + property_content_max_width: layout_stats.property_content_max_width, }; // push, run, pop - LayoutInfoStack::push(ui.ctx(), state); + LayoutInfoStack::push(ui.ctx(), state.clone()); let result = ui .scope(|ui| { ui.spacing_mut().item_spacing.y = 0.0; From e5e21c4555f42057d7201bbe6e248ae0ccd76ea3 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Tue, 30 Apr 2024 18:29:29 +0200 Subject: [PATCH 2/8] Use `list_item2` for the component list in `InstancePath::data_ui` --- crates/re_data_ui/src/instance_path.rs | 163 ++++++++++++++++--------- 1 file changed, 102 insertions(+), 61 deletions(-) diff --git a/crates/re_data_ui/src/instance_path.rs b/crates/re_data_ui/src/instance_path.rs index 389dc936df3e..0d575c86eca7 100644 --- a/crates/re_data_ui/src/instance_path.rs +++ b/crates/re_data_ui/src/instance_path.rs @@ -1,11 +1,8 @@ -use std::sync::Arc; - use re_entity_db::InstancePath; use re_log_types::ComponentPath; -use re_viewer_context::{UiLayout, ViewerContext}; +use re_viewer_context::{HoverHighlight, Item, UiLayout, ViewerContext}; -use super::DataUi; -use crate::item_ui; +use super::{label_for_ui_layout, DataUi}; impl DataUi for InstancePath { fn data_ui( @@ -27,12 +24,18 @@ impl DataUi for InstancePath { else { if ctx.recording().is_known_entity(entity_path) { // This is fine - e.g. we're looking at `/world` and the user has only logged to `/world/car`. - ui.label(format!( - "No components logged on timeline {:?}", - query.timeline().name() - )); + label_for_ui_layout( + ui, + ui_layout, + format!( + "No components logged on timeline {:?}", + query.timeline().name() + ), + ); } else { - ui.label( + label_for_ui_layout( + ui, + ui_layout, ctx.re_ui .error_text(format!("Unknown entity: {entity_path:?}")), ); @@ -40,39 +43,69 @@ impl DataUi for InstancePath { return; }; - let mut components = crate::component_list_for_ui(&components); + let components = crate::component_list_for_ui(&components); + let indicator_count = components + .iter() + .filter(|c| c.is_indicator_component()) + .count(); - let split = components.partition_point(|c| c.is_indicator_component()); - let normal_components = components.split_off(split); - let indicator_components = components; + if ui_layout == UiLayout::List { + label_for_ui_layout( + ui, + ui_layout, + format!( + "{} component{} (including {} indicator component{})", + components.len(), + if components.len() > 1 { "s" } else { "" }, + indicator_count, + if indicator_count > 1 { "s" } else { "" } + ), + ); + return; + } let show_indicator_comps = match ui_layout { - UiLayout::List | UiLayout::Tooltip => { + UiLayout::Tooltip => { // Skip indicator components in hover ui (unless there are no other // types of components). - !normal_components.is_empty() + indicator_count == components.len() } UiLayout::SelectionPanelLimitHeight | UiLayout::SelectionPanelFull => true, + UiLayout::List => false, // unreachable }; - // First show indicator components, outside the grid: - if show_indicator_comps { - for component_name in indicator_components { - item_ui::component_path_button( - ctx, - ui, - &ComponentPath::new(entity_path.clone(), component_name), - db, - ); - } - } + let interactive = ui_layout != UiLayout::Tooltip; - // Now show the rest of the components: - egui::Grid::new("components") - .spacing(ui.spacing().item_spacing) - .num_columns(2) - .show(ui, |ui| { - for component_name in normal_components { + re_ui::list_item2::list_item_scope(ui, "component list", |ui| { + for component_name in components { + if !show_indicator_comps && component_name.is_indicator_component() { + continue; + } + + let component_path = ComponentPath::new(entity_path.clone(), component_name); + let is_static = db.is_component_static(&component_path).unwrap_or_default(); + let icon = if is_static { + &re_ui::icons::COMPONENT_STATIC + } else { + &re_ui::icons::COMPONENT + }; + let item = Item::ComponentPath(component_path); + + let mut list_item = ctx.re_ui.list_item2().interactive(interactive); + + if interactive { + let is_hovered = ctx.selection_state().highlight_for_ui_element(&item) + == HoverHighlight::Hovered; + list_item = list_item.force_hovered(is_hovered); + } + + let response = if component_name.is_indicator_component() { + list_item.show_flat( + ui, + re_ui::list_item2::LabelContent::new(component_name.short_name()) + .with_icon(icon), + ) + } else { let results = db.query_caches().latest_at( db.store(), query, @@ -83,36 +116,44 @@ impl DataUi for InstancePath { continue; // no need to show components that are unset at this point in time }; - item_ui::component_path_button( - ctx, + list_item.show_flat( ui, - &ComponentPath::new(entity_path.clone(), component_name), - db, - ); + re_ui::list_item2::PropertyContent::new(component_name.short_name()) + .with_icon(icon) + .value_fn(|_, ui, _| { + if instance.is_all() { + crate::EntityLatestAtResults { + entity_path: entity_path.clone(), + component_name, + results: std::sync::Arc::clone(results), + } + .data_ui( + ctx, + ui, + UiLayout::List, + query, + db, + ); + } else { + ctx.component_ui_registry.ui( + ctx, + ui, + UiLayout::List, + query, + db, + entity_path, + results, + instance, + ); + } + }), + ) + }; - if instance.is_all() { - crate::EntityLatestAtResults { - entity_path: entity_path.clone(), - component_name, - results: Arc::clone(results), - } - .data_ui(ctx, ui, UiLayout::List, query, db); - } else { - ctx.component_ui_registry.ui( - ctx, - ui, - UiLayout::List, - query, - db, - entity_path, - results, - instance, - ); - } - - ui.end_row(); + if interactive { + ctx.select_hovered_on_click(&response, item); } - Some(()) - }); + } + }); } } From 6fb4017f2c40ec8bd05d7f4f9c5eb71c5fadc9e6 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Tue, 14 May 2024 18:05:41 +0200 Subject: [PATCH 3/8] Don't max the tooltip width --- crates/re_data_ui/src/instance_path.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/crates/re_data_ui/src/instance_path.rs b/crates/re_data_ui/src/instance_path.rs index 0d575c86eca7..640be0e4391c 100644 --- a/crates/re_data_ui/src/instance_path.rs +++ b/crates/re_data_ui/src/instance_path.rs @@ -116,8 +116,7 @@ impl DataUi for InstancePath { continue; // no need to show components that are unset at this point in time }; - list_item.show_flat( - ui, + let mut content = re_ui::list_item2::PropertyContent::new(component_name.short_name()) .with_icon(icon) .value_fn(|_, ui, _| { @@ -146,8 +145,14 @@ impl DataUi for InstancePath { instance, ); } - }), - ) + }); + + // avoid the list item to max the tooltip width + if ui_layout == UiLayout::Tooltip { + content = content.exact_width(true); + } + + list_item.show_flat(ui, content) }; if interactive { From d934dfe9b7aff6ec731a68db5e26f68956fb9034 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Mon, 20 May 2024 18:34:12 +0200 Subject: [PATCH 4/8] Moved `[data_]label_for_ui_layout` and `table_for_ui_layout` to `UiLayout` methods --- Cargo.lock | 1 + crates/re_data_ui/src/annotation_context.rs | 18 ++-- crates/re_data_ui/src/component.rs | 5 +- .../re_data_ui/src/component_ui_registry.rs | 10 +- crates/re_data_ui/src/data.rs | 33 ++++--- crates/re_data_ui/src/image.rs | 6 +- crates/re_data_ui/src/instance_path.rs | 11 +-- crates/re_data_ui/src/lib.rs | 95 ------------------- crates/re_data_ui/src/pinhole.rs | 12 +-- crates/re_data_ui/src/rotation3d.rs | 6 +- crates/re_data_ui/src/transform3d.rs | 8 +- crates/re_viewer_context/Cargo.toml | 1 + .../src/component_ui_registry.rs | 88 +++++++++++++++++ 13 files changed, 144 insertions(+), 150 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b5e20855802c..66656d99e8a1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5066,6 +5066,7 @@ dependencies = [ "bytemuck", "egui", "egui-wgpu", + "egui_extras", "egui_tiles", "glam", "half 2.3.1", diff --git a/crates/re_data_ui/src/annotation_context.rs b/crates/re_data_ui/src/annotation_context.rs index 516ee67e4b1b..cec096ab82d2 100644 --- a/crates/re_data_ui/src/annotation_context.rs +++ b/crates/re_data_ui/src/annotation_context.rs @@ -7,7 +7,7 @@ use re_types::datatypes::{ }; use re_viewer_context::{auto_color, UiLayout, ViewerContext}; -use super::{data_label_for_ui_layout, label_for_ui_layout, table_for_ui_layout, DataUi}; +use super::DataUi; impl crate::EntityDataUi for re_types::components::ClassId { fn entity_data_ui( @@ -32,7 +32,7 @@ impl crate::EntityDataUi for re_types::components::ClassId { text.push(' '); text.push_str(label.as_str()); } - label_for_ui_layout(ui, ui_layout, text); + ui_layout.label(ui, text); }); let id = self.0; @@ -54,7 +54,7 @@ impl crate::EntityDataUi for re_types::components::ClassId { } } } else { - label_for_ui_layout(ui, ui_layout, format!("{}", self.0)); + ui_layout.label(ui, format!("{}", self.0)); } } } @@ -79,10 +79,10 @@ impl crate::EntityDataUi for re_types::components::KeypointId { text.push_str(label.as_str()); } - data_label_for_ui_layout(ui, ui_layout, text); + ui_layout.data_label(ui, text); }); } else { - data_label_for_ui_layout(ui, ui_layout, format!("{}", self.0)); + ui_layout.data_label(ui, format!("{}", self.0)); } } } @@ -128,7 +128,7 @@ impl DataUi for AnnotationContext { } else { format!("{} classes", self.0.len()) }; - label_for_ui_layout(ui, ui_layout, text); + ui_layout.label(ui, text); } UiLayout::SelectionPanelLimitHeight | UiLayout::SelectionPanelFull => { ui.vertical(|ui| { @@ -208,7 +208,8 @@ fn class_description_ui( ui.push_id(format!("keypoints_connections_{}", id.0), |ui| { use egui_extras::Column; - let table = table_for_ui_layout(ui_layout, ui) + let table = ui_layout + .table(ui) .cell_layout(egui::Layout::left_to_right(egui::Align::Center)) .column(Column::auto().clip(true).at_least(40.0)) .column(Column::auto().clip(true).at_least(40.0)); @@ -276,7 +277,8 @@ fn annotation_info_table_ui( use egui_extras::Column; - let table = table_for_ui_layout(ui_layout, ui) + let table = ui_layout + .table(ui) .cell_layout(egui::Layout::left_to_right(egui::Align::Center)) .column(Column::auto()) // id .column(Column::auto().clip(true).at_least(40.0)) // label diff --git a/crates/re_data_ui/src/component.rs b/crates/re_data_ui/src/component.rs index 13a34b5859c3..d90c71494c5f 100644 --- a/crates/re_data_ui/src/component.rs +++ b/crates/re_data_ui/src/component.rs @@ -8,7 +8,7 @@ use re_types::ComponentName; use re_ui::SyntaxHighlighting as _; use re_viewer_context::{UiLayout, ViewerContext}; -use super::{table_for_ui_layout, DataUi}; +use super::DataUi; use crate::item_ui; /// All the values of a specific [`re_log_types::ComponentPath`]. @@ -145,7 +145,8 @@ impl DataUi for EntityLatestAtResults { } else if one_line { ui.label(format!("{} values", re_format::format_uint(num_instances))); } else { - table_for_ui_layout(ui_layout, ui) + ui_layout + .table(ui) .resizable(false) .cell_layout(egui::Layout::left_to_right(egui::Align::Center)) .column(egui_extras::Column::auto()) diff --git a/crates/re_data_ui/src/component_ui_registry.rs b/crates/re_data_ui/src/component_ui_registry.rs index 5154ec45337e..7e9a76ace068 100644 --- a/crates/re_data_ui/src/component_ui_registry.rs +++ b/crates/re_data_ui/src/component_ui_registry.rs @@ -6,7 +6,7 @@ use re_viewer_context::{ComponentUiRegistry, UiLayout, ViewerContext}; use crate::editors::register_editors; -use super::{data_label_for_ui_layout, EntityDataUi}; +use super::EntityDataUi; pub fn create_component_ui_registry() -> ComponentUiRegistry { re_tracing::profile_function!(); @@ -90,14 +90,14 @@ fn arrow_ui(ui: &mut egui::Ui, ui_layout: UiLayout, array: &dyn arrow2::array::A if let Some(utf8) = array.as_any().downcast_ref::>() { if utf8.len() == 1 { let string = utf8.value(0); - data_label_for_ui_layout(ui, ui_layout, string); + ui_layout.data_label(ui, string); return; } } if let Some(utf8) = array.as_any().downcast_ref::>() { if utf8.len() == 1 { let string = utf8.value(0); - data_label_for_ui_layout(ui, ui_layout, string); + ui_layout.data_label(ui, string); return; } } @@ -108,7 +108,7 @@ fn arrow_ui(ui: &mut egui::Ui, ui_layout: UiLayout, array: &dyn arrow2::array::A let mut string = String::new(); let display = arrow2::array::get_display(array, "null"); if display(&mut string, 0).is_ok() { - data_label_for_ui_layout(ui, ui_layout, &string); + ui_layout.data_label(ui, &string); return; } } @@ -121,7 +121,7 @@ fn arrow_ui(ui: &mut egui::Ui, ui_layout: UiLayout, array: &dyn arrow2::array::A if data_type_formatted.len() < 20 { // e.g. "4.2 KiB of Float32" - data_label_for_ui_layout(ui, ui_layout, &format!("{bytes} of {data_type_formatted}")); + ui_layout.data_label(ui, &format!("{bytes} of {data_type_formatted}")); } else { // Huge datatype, probably a union horror show ui.label(format!("{bytes} of data")); diff --git a/crates/re_data_ui/src/data.rs b/crates/re_data_ui/src/data.rs index e186b5a8f308..eda2036cf02d 100644 --- a/crates/re_data_ui/src/data.rs +++ b/crates/re_data_ui/src/data.rs @@ -7,7 +7,7 @@ use re_types::blueprint::components::VisualBounds2D; use re_types::components::{Color, LineStrip2D, LineStrip3D, Range1D, ViewCoordinates}; use re_viewer_context::{UiLayout, ViewerContext}; -use super::{data_label_for_ui_layout, label_for_ui_layout, table_for_ui_layout, DataUi}; +use super::DataUi; /// Default number of ui points to show a number. const DEFAULT_NUMBER_WIDTH: f32 = 52.0; @@ -65,13 +65,14 @@ impl DataUi for ViewCoordinates { ) { match ui_layout { UiLayout::List => { - label_for_ui_layout(ui, ui_layout, self.describe_short()) + ui_layout + .label(ui, self.describe_short()) .on_hover_text(self.describe()); } UiLayout::SelectionPanelFull | UiLayout::SelectionPanelLimitHeight | UiLayout::Tooltip => { - label_for_ui_layout(ui, ui_layout, self.describe()); + ui_layout.label(ui, self.describe()); } } } @@ -114,7 +115,7 @@ impl DataUi for re_types::datatypes::Vec2D { _query: &re_data_store::LatestAtQuery, _db: &re_entity_db::EntityDb, ) { - data_label_for_ui_layout(ui, ui_layout, self.to_string()); + ui_layout.data_label(ui, self.to_string()); } } @@ -127,7 +128,7 @@ impl DataUi for re_types::datatypes::Vec3D { _query: &re_data_store::LatestAtQuery, _db: &re_entity_db::EntityDb, ) { - data_label_for_ui_layout(ui, ui_layout, self.to_string()); + ui_layout.data_label(ui, self.to_string()); } } @@ -140,7 +141,7 @@ impl DataUi for re_types::datatypes::Vec4D { _query: &re_data_store::LatestAtQuery, _db: &re_entity_db::EntityDb, ) { - data_label_for_ui_layout(ui, ui_layout, self.to_string()); + ui_layout.data_label(ui, self.to_string()); } } @@ -153,7 +154,7 @@ impl DataUi for re_types::datatypes::UVec2D { _query: &re_data_store::LatestAtQuery, _db: &re_entity_db::EntityDb, ) { - data_label_for_ui_layout(ui, ui_layout, self.to_string()); + ui_layout.data_label(ui, self.to_string()); } } @@ -166,7 +167,7 @@ impl DataUi for re_types::datatypes::UVec3D { _query: &re_data_store::LatestAtQuery, _db: &re_entity_db::EntityDb, ) { - data_label_for_ui_layout(ui, ui_layout, self.to_string()); + ui_layout.data_label(ui, self.to_string()); } } @@ -179,7 +180,7 @@ impl DataUi for re_types::datatypes::UVec4D { _query: &re_data_store::LatestAtQuery, _db: &re_entity_db::EntityDb, ) { - data_label_for_ui_layout(ui, ui_layout, self.to_string()); + ui_layout.data_label(ui, self.to_string()); } } @@ -192,7 +193,7 @@ impl DataUi for Range1D { _query: &LatestAtQuery, _db: &EntityDb, ) { - data_label_for_ui_layout(ui, ui_layout, self.to_string()); + ui_layout.data_label(ui, self.to_string()); } } @@ -205,7 +206,7 @@ impl DataUi for VisualBounds2D { _query: &LatestAtQuery, _db: &EntityDb, ) { - data_label_for_ui_layout(ui, ui_layout, self.to_string()); + ui_layout.data_label(ui, self.to_string()); } } @@ -220,11 +221,12 @@ impl DataUi for LineStrip2D { ) { match ui_layout { UiLayout::List | UiLayout::Tooltip => { - label_for_ui_layout(ui, ui_layout, format!("{} positions", self.0.len())); + ui_layout.label(ui, format!("{} positions", self.0.len())); } UiLayout::SelectionPanelLimitHeight | UiLayout::SelectionPanelFull => { use egui_extras::Column; - table_for_ui_layout(ui_layout, ui) + ui_layout + .table(ui) .resizable(true) .cell_layout(egui::Layout::left_to_right(egui::Align::Center)) .columns(Column::initial(DEFAULT_NUMBER_WIDTH).clip(true), 2) @@ -267,11 +269,12 @@ impl DataUi for LineStrip3D { ) { match ui_layout { UiLayout::List | UiLayout::Tooltip => { - label_for_ui_layout(ui, ui_layout, format!("{} positions", self.0.len())); + ui_layout.label(ui, format!("{} positions", self.0.len())); } UiLayout::SelectionPanelFull | UiLayout::SelectionPanelLimitHeight => { use egui_extras::Column; - table_for_ui_layout(ui_layout, ui) + ui_layout + .table(ui) .resizable(true) .cell_layout(egui::Layout::left_to_right(egui::Align::Center)) .columns(Column::initial(DEFAULT_NUMBER_WIDTH).clip(true), 3) diff --git a/crates/re_data_ui/src/image.rs b/crates/re_data_ui/src/image.rs index f5e810ad5391..753cade67648 100644 --- a/crates/re_data_ui/src/image.rs +++ b/crates/re_data_ui/src/image.rs @@ -12,7 +12,7 @@ use re_viewer_context::{ ViewerContext, }; -use crate::{image_meaning_for_entity, label_for_ui_layout}; +use crate::image_meaning_for_entity; use super::EntityDataUi; @@ -85,7 +85,7 @@ impl EntityDataUi for re_types::components::TensorData { ); } Err(err) => { - label_for_ui_layout(ui, ui_layout, ctx.re_ui.error_text(err.to_string())); + ui_layout.label(ui, ctx.re_ui.error_text(err.to_string())); } } } @@ -183,7 +183,7 @@ pub fn tensor_ui( original_tensor.dtype(), format_tensor_shape_single_line(&shape) ); - label_for_ui_layout(ui, ui_layout, text).on_hover_ui(|ui| { + ui_layout.label(ui, text).on_hover_ui(|ui| { tensor_summary_ui( ctx.re_ui, ui, diff --git a/crates/re_data_ui/src/instance_path.rs b/crates/re_data_ui/src/instance_path.rs index 640be0e4391c..adf1c241f46d 100644 --- a/crates/re_data_ui/src/instance_path.rs +++ b/crates/re_data_ui/src/instance_path.rs @@ -2,7 +2,7 @@ use re_entity_db::InstancePath; use re_log_types::ComponentPath; use re_viewer_context::{HoverHighlight, Item, UiLayout, ViewerContext}; -use super::{label_for_ui_layout, DataUi}; +use super::DataUi; impl DataUi for InstancePath { fn data_ui( @@ -24,18 +24,16 @@ impl DataUi for InstancePath { else { if ctx.recording().is_known_entity(entity_path) { // This is fine - e.g. we're looking at `/world` and the user has only logged to `/world/car`. - label_for_ui_layout( + ui_layout.label( ui, - ui_layout, format!( "No components logged on timeline {:?}", query.timeline().name() ), ); } else { - label_for_ui_layout( + ui_layout.label( ui, - ui_layout, ctx.re_ui .error_text(format!("Unknown entity: {entity_path:?}")), ); @@ -50,9 +48,8 @@ impl DataUi for InstancePath { .count(); if ui_layout == UiLayout::List { - label_for_ui_layout( + ui_layout.label( ui, - ui_layout, format!( "{} component{} (including {} indicator component{})", components.len(), diff --git a/crates/re_data_ui/src/lib.rs b/crates/re_data_ui/src/lib.rs index 7e628df3c4e2..58bd1b98502f 100644 --- a/crates/re_data_ui/src/lib.rs +++ b/crates/re_data_ui/src/lib.rs @@ -177,98 +177,3 @@ pub fn annotations( annotation_map.load(ctx, query, std::iter::once(entity_path)); annotation_map.find(entity_path) } - -// --------------------------------------------------------------------------- - -/// Build an egui table and configure it for the given UI context. -/// -/// Note that the caller is responsible for strictly limiting the number of displayed rows for -/// [`UiLayout::List`] and [`UiLayout::Tooltip`], as the table will not scroll. -pub fn table_for_ui_layout( - ui_layout: UiLayout, - ui: &mut egui::Ui, -) -> egui_extras::TableBuilder<'_> { - let table = egui_extras::TableBuilder::new(ui); - match ui_layout { - UiLayout::List | UiLayout::Tooltip => { - // Be as small as possible in the hover tooltips. No scrolling related configuration, as - // the content itself must be limited (scrolling is not possible in tooltips). - table.auto_shrink([true, true]) - } - UiLayout::SelectionPanelLimitHeight => { - // Don't take too much vertical space to leave room for other selected items. - table - .auto_shrink([false, true]) - .vscroll(true) - .max_scroll_height(100.0) - } - UiLayout::SelectionPanelFull => { - // We're alone in the selection panel. Let the outer ScrollArea do the work. - table.auto_shrink([false, true]).vscroll(false) - } - } -} - -/// Show a label while respecting the given UI layout. -/// -/// Important: for label only, data should use [`crate::data_label_for_ui_layout`] instead. -// TODO(#6315): must be merged with `data_label_for_ui_layout` and have an improved API -pub fn label_for_ui_layout( - ui: &mut egui::Ui, - ui_layout: UiLayout, - text: impl Into, -) -> egui::Response { - let mut label = egui::Label::new(text); - - match ui_layout { - UiLayout::List => label = label.truncate(true), - UiLayout::Tooltip | UiLayout::SelectionPanelLimitHeight | UiLayout::SelectionPanelFull => { - label = label.wrap(true); - } - } - - ui.add(label) -} - -/// Show data while respecting the given UI layout. -/// -/// Import: for data only, labels should use [`crate::label_for_ui_layout`] instead. -// TODO(#6315): must be merged with `label_for_ui_layout` and have an improved API -pub fn data_label_for_ui_layout(ui: &mut egui::Ui, ui_layout: UiLayout, string: impl AsRef) { - let string = string.as_ref(); - let font_id = egui::TextStyle::Monospace.resolve(ui.style()); - let color = ui.visuals().text_color(); - let wrap_width = ui.available_width(); - let mut layout_job = - egui::text::LayoutJob::simple(string.to_owned(), font_id, color, wrap_width); - - let mut needs_scroll_area = false; - - match ui_layout { - UiLayout::List => { - // Elide - layout_job.wrap.max_rows = 1; - layout_job.wrap.break_anywhere = true; - } - UiLayout::Tooltip => { - layout_job.wrap.max_rows = 3; - } - UiLayout::SelectionPanelLimitHeight => { - let num_newlines = string.chars().filter(|&c| c == '\n').count(); - needs_scroll_area = 10 < num_newlines || 300 < string.len(); - } - UiLayout::SelectionPanelFull => { - needs_scroll_area = false; - } - } - - let galley = ui.fonts(|f| f.layout_job(layout_job)); // We control the text layout; not the label - - if needs_scroll_area { - egui::ScrollArea::vertical().show(ui, |ui| { - ui.label(galley); - }); - } else { - ui.label(galley); - } -} diff --git a/crates/re_data_ui/src/pinhole.rs b/crates/re_data_ui/src/pinhole.rs index 3003c3d0897b..edc923455eb2 100644 --- a/crates/re_data_ui/src/pinhole.rs +++ b/crates/re_data_ui/src/pinhole.rs @@ -1,7 +1,7 @@ use re_types::components::{PinholeProjection, Resolution}; use re_viewer_context::{UiLayout, ViewerContext}; -use crate::{data_label_for_ui_layout, label_for_ui_layout, DataUi}; +use crate::DataUi; impl DataUi for PinholeProjection { fn data_ui( @@ -24,13 +24,9 @@ impl DataUi for PinholeProjection { fl.to_string() }; - label_for_ui_layout( - ui, - ui_layout, - format!("Focal length: {fl}, principal point: {pp}"), - ) + ui_layout.label(ui, format!("Focal length: {fl}, principal point: {pp}")) } else { - label_for_ui_layout(ui, ui_layout, "3×3 projection matrix") + ui_layout.label(ui, "3×3 projection matrix") } .on_hover_ui(|ui| self.data_ui(ctx, ui, UiLayout::Tooltip, query, db)); } @@ -51,6 +47,6 @@ impl DataUi for Resolution { _db: &re_entity_db::EntityDb, ) { let [x, y] = self.0 .0; - data_label_for_ui_layout(ui, ui_layout, format!("{x}×{y}")); + ui_layout.data_label(ui, format!("{x}×{y}")); } } diff --git a/crates/re_data_ui/src/rotation3d.rs b/crates/re_data_ui/src/rotation3d.rs index 938c37b88b77..1e736826f795 100644 --- a/crates/re_data_ui/src/rotation3d.rs +++ b/crates/re_data_ui/src/rotation3d.rs @@ -4,7 +4,7 @@ use re_types::{ }; use re_viewer_context::{UiLayout, ViewerContext}; -use crate::{data_label_for_ui_layout, label_for_ui_layout, DataUi}; +use crate::DataUi; impl DataUi for components::Rotation3D { fn data_ui( @@ -31,13 +31,13 @@ impl DataUi for datatypes::Rotation3D { match self { Self::Quaternion(q) => { // TODO(andreas): Better formatting for quaternions. - data_label_for_ui_layout(ui, ui_layout, format!("{q:?}")); + ui_layout.data_label(ui, format!("{q:?}")); } Self::AxisAngle(RotationAxisAngle { axis, angle }) => { match ui_layout { UiLayout::List => { // TODO(#6315): should be mixed label/data formatting - label_for_ui_layout(ui, ui_layout, format!("angle: {angle}, axis: {axis}")); + ui_layout.label(ui, format!("angle: {angle}, axis: {axis}")); } _ => { egui::Grid::new("axis_angle").num_columns(2).show(ui, |ui| { diff --git a/crates/re_data_ui/src/transform3d.rs b/crates/re_data_ui/src/transform3d.rs index 0560f58489a2..de2a17ef84d8 100644 --- a/crates/re_data_ui/src/transform3d.rs +++ b/crates/re_data_ui/src/transform3d.rs @@ -1,7 +1,7 @@ use re_types::datatypes::{Scale3D, Transform3D, TranslationAndMat3x3, TranslationRotationScale3D}; use re_viewer_context::{UiLayout, ViewerContext}; -use crate::{label_for_ui_layout, DataUi}; +use crate::DataUi; impl DataUi for re_types::components::Transform3D { #[allow(clippy::only_used_in_recursion)] @@ -16,7 +16,7 @@ impl DataUi for re_types::components::Transform3D { match ui_layout { UiLayout::List => { // TODO(andreas): Preview some information instead of just a label with hover ui. - label_for_ui_layout(ui, ui_layout, "3D transform").on_hover_ui(|ui| { + ui_layout.label(ui, "3D transform").on_hover_ui(|ui| { self.data_ui(ctx, ui, UiLayout::Tooltip, query, db); }); } @@ -35,9 +35,9 @@ impl DataUi for re_types::components::Transform3D { }; ui.vertical(|ui| { - label_for_ui_layout(ui, ui_layout, "3D transform"); + ui_layout.label(ui, "3D transform"); ui.indent("transform_repr", |ui| { - label_for_ui_layout(ui, ui_layout, dir_string); + ui_layout.label(ui, dir_string); self.0.data_ui(ctx, ui, ui_layout, query, db); }); }); diff --git a/crates/re_viewer_context/Cargo.toml b/crates/re_viewer_context/Cargo.toml index f90f05f6eb63..f55bbef8c1fc 100644 --- a/crates/re_viewer_context/Cargo.toml +++ b/crates/re_viewer_context/Cargo.toml @@ -38,6 +38,7 @@ bit-vec.workspace = true bytemuck.workspace = true egui-wgpu.workspace = true egui.workspace = true +egui_extras.workspace = true egui_tiles.workspace = true glam = { workspace = true, features = ["serde"] } half.workspace = true diff --git a/crates/re_viewer_context/src/component_ui_registry.rs b/crates/re_viewer_context/src/component_ui_registry.rs index 4ef9d9ea912e..d9fc71f17084 100644 --- a/crates/re_viewer_context/src/component_ui_registry.rs +++ b/crates/re_viewer_context/src/component_ui_registry.rs @@ -38,6 +38,94 @@ pub enum UiLayout { SelectionPanelFull, } +impl UiLayout { + /// Build an egui table and configure it for the given UI layout. + /// + /// Note that the caller is responsible for strictly limiting the number of displayed rows for + /// [`Self::List`] and [`Self::Tooltip`], as the table will not scroll. + pub fn table(self, ui: &mut egui::Ui) -> egui_extras::TableBuilder<'_> { + let table = egui_extras::TableBuilder::new(ui); + match self { + Self::List | Self::Tooltip => { + // Be as small as possible in the hover tooltips. No scrolling related configuration, as + // the content itself must be limited (scrolling is not possible in tooltips). + table.auto_shrink([true, true]) + } + Self::SelectionPanelLimitHeight => { + // Don't take too much vertical space to leave room for other selected items. + table + .auto_shrink([false, true]) + .vscroll(true) + .max_scroll_height(100.0) + } + Self::SelectionPanelFull => { + // We're alone in the selection panel. Let the outer ScrollArea do the work. + table.auto_shrink([false, true]).vscroll(false) + } + } + } + + /// Show a label while respecting the given UI layout. + /// + /// Important: for label only, data should use [`crate::data_label_for_ui_layout`] instead. + // TODO(#6315): must be merged with `Self::data_label` and have an improved API + pub fn label(self, ui: &mut egui::Ui, text: impl Into) -> egui::Response { + let mut label = egui::Label::new(text); + + match self { + Self::List => label = label.truncate(true), + Self::Tooltip | Self::SelectionPanelLimitHeight | Self::SelectionPanelFull => { + label = label.wrap(true); + } + } + + ui.add(label) + } + + /// Show data while respecting the given UI layout. + /// + /// Import: for data only, labels should use [`crate::label_for_ui_layout`] instead. + // TODO(#6315): must be merged with `Self::label` and have an improved API + pub fn data_label(self, ui: &mut egui::Ui, string: impl AsRef) { + let string = string.as_ref(); + let font_id = egui::TextStyle::Monospace.resolve(ui.style()); + let color = ui.visuals().text_color(); + let wrap_width = ui.available_width(); + let mut layout_job = + egui::text::LayoutJob::simple(string.to_owned(), font_id, color, wrap_width); + + let mut needs_scroll_area = false; + + match self { + Self::List => { + // Elide + layout_job.wrap.max_rows = 1; + layout_job.wrap.break_anywhere = true; + } + Self::Tooltip => { + layout_job.wrap.max_rows = 3; + } + Self::SelectionPanelLimitHeight => { + let num_newlines = string.chars().filter(|&c| c == '\n').count(); + needs_scroll_area = 10 < num_newlines || 300 < string.len(); + } + Self::SelectionPanelFull => { + needs_scroll_area = false; + } + } + + let galley = ui.fonts(|f| f.layout_job(layout_job)); // We control the text layout; not the label + + if needs_scroll_area { + egui::ScrollArea::vertical().show(ui, |ui| { + ui.label(galley); + }); + } else { + ui.label(galley); + } + } +} + type ComponentUiCallback = Box< dyn Fn( &ViewerContext<'_>, From d2e86d1e524ef2bd880a9003adc3f5207c5e9fbb Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Mon, 20 May 2024 18:44:45 +0200 Subject: [PATCH 5/8] Post rebase fixes --- crates/re_data_ui/src/instance_path.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/re_data_ui/src/instance_path.rs b/crates/re_data_ui/src/instance_path.rs index adf1c241f46d..f39678e87f30 100644 --- a/crates/re_data_ui/src/instance_path.rs +++ b/crates/re_data_ui/src/instance_path.rs @@ -84,7 +84,7 @@ impl DataUi for InstancePath { let icon = if is_static { &re_ui::icons::COMPONENT_STATIC } else { - &re_ui::icons::COMPONENT + &re_ui::icons::COMPONENT_TEMPORAL }; let item = Item::ComponentPath(component_path); From 1e11660eca4750782aabd82be88fb0199184b37f Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Wed, 29 May 2024 19:29:00 +0200 Subject: [PATCH 6/8] post merge fixes --- crates/re_data_ui/src/instance_path.rs | 8 ++++---- crates/re_viewer_context/src/component_ui_registry.rs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/re_data_ui/src/instance_path.rs b/crates/re_data_ui/src/instance_path.rs index f39678e87f30..f791ec9af252 100644 --- a/crates/re_data_ui/src/instance_path.rs +++ b/crates/re_data_ui/src/instance_path.rs @@ -73,7 +73,7 @@ impl DataUi for InstancePath { let interactive = ui_layout != UiLayout::Tooltip; - re_ui::list_item2::list_item_scope(ui, "component list", |ui| { + re_ui::list_item::list_item_scope(ui, "component list", |ui| { for component_name in components { if !show_indicator_comps && component_name.is_indicator_component() { continue; @@ -88,7 +88,7 @@ impl DataUi for InstancePath { }; let item = Item::ComponentPath(component_path); - let mut list_item = ctx.re_ui.list_item2().interactive(interactive); + let mut list_item = ctx.re_ui.list_item().interactive(interactive); if interactive { let is_hovered = ctx.selection_state().highlight_for_ui_element(&item) @@ -99,7 +99,7 @@ impl DataUi for InstancePath { let response = if component_name.is_indicator_component() { list_item.show_flat( ui, - re_ui::list_item2::LabelContent::new(component_name.short_name()) + re_ui::list_item::LabelContent::new(component_name.short_name()) .with_icon(icon), ) } else { @@ -114,7 +114,7 @@ impl DataUi for InstancePath { }; let mut content = - re_ui::list_item2::PropertyContent::new(component_name.short_name()) + re_ui::list_item::PropertyContent::new(component_name.short_name()) .with_icon(icon) .value_fn(|_, ui, _| { if instance.is_all() { diff --git a/crates/re_viewer_context/src/component_ui_registry.rs b/crates/re_viewer_context/src/component_ui_registry.rs index 173372deb78e..e7c0232cc160 100644 --- a/crates/re_viewer_context/src/component_ui_registry.rs +++ b/crates/re_viewer_context/src/component_ui_registry.rs @@ -73,9 +73,9 @@ impl UiLayout { let mut label = egui::Label::new(text); match self { - Self::List => label = label.truncate(true), + Self::List => label = label.truncate(), Self::Tooltip | Self::SelectionPanelLimitHeight | Self::SelectionPanelFull => { - label = label.wrap(true); + label = label.wrap(); } } From 9165768ddb101ca9f6e5ef4a02a881d3ce750c61 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Tue, 4 Jun 2024 17:54:17 +0200 Subject: [PATCH 7/8] Fix lint + do not silence `cargo fmt` in CI --- crates/re_data_ui/src/component_ui_registry.rs | 1 - scripts/ci/rust_checks.py | 5 +++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/re_data_ui/src/component_ui_registry.rs b/crates/re_data_ui/src/component_ui_registry.rs index 1691473455a4..175c6d80ca16 100644 --- a/crates/re_data_ui/src/component_ui_registry.rs +++ b/crates/re_data_ui/src/component_ui_registry.rs @@ -4,7 +4,6 @@ use re_log_types::{external::arrow2, EntityPath, Instance}; use re_types::external::arrow2::array::Utf8Array; use re_viewer_context::{ComponentUiRegistry, UiLayout, ViewerContext}; - use super::EntityDataUi; pub fn create_component_ui_registry() -> ComponentUiRegistry { diff --git a/scripts/ci/rust_checks.py b/scripts/ci/rust_checks.py index c19894e9d9a8..584d9feb95a0 100755 --- a/scripts/ci/rust_checks.py +++ b/scripts/ci/rust_checks.py @@ -39,7 +39,7 @@ def __init__(self, command: str, duration: float) -> None: def run_cargo(cargo_cmd: str, cargo_args: str, clippy_conf: str | None = None) -> Timing: args = ["cargo", cargo_cmd] - if cargo_cmd != "deny": + if cargo_cmd not in ["deny", "fmt", "format"]: args.append("--quiet") args += cargo_args.split(" ") @@ -52,7 +52,8 @@ def run_cargo(cargo_cmd: str, cargo_args: str, clippy_conf: str | None = None) - additional_env_vars["RUSTDOCFLAGS"] = "--deny warnings" if clippy_conf is not None: additional_env_vars["CLIPPY_CONF_DIR"] = ( - f"{os.getcwd()}/{clippy_conf}" # Clippy has issues finding this directory on CI when we're not using an absolute path here. + # Clippy has issues finding this directory on CI when we're not using an absolute path here. + f"{os.getcwd()}/{clippy_conf}" ) env = os.environ.copy() From e47d6cd93aa14efd81925d02532390c3e4b843d8 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Tue, 4 Jun 2024 18:09:45 +0200 Subject: [PATCH 8/8] doc fix --- crates/re_viewer_context/src/component_ui_registry.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/re_viewer_context/src/component_ui_registry.rs b/crates/re_viewer_context/src/component_ui_registry.rs index f558f0924ba2..db42ce079079 100644 --- a/crates/re_viewer_context/src/component_ui_registry.rs +++ b/crates/re_viewer_context/src/component_ui_registry.rs @@ -71,7 +71,7 @@ impl UiLayout { /// Show a label while respecting the given UI layout. /// - /// Important: for label only, data should use [`crate::data_label_for_ui_layout`] instead. + /// Important: for label only, data should use [`UiLayout::data_label`] instead. // TODO(#6315): must be merged with `Self::data_label` and have an improved API pub fn label(self, ui: &mut egui::Ui, text: impl Into) -> egui::Response { let mut label = egui::Label::new(text); @@ -88,7 +88,7 @@ impl UiLayout { /// Show data while respecting the given UI layout. /// - /// Import: for data only, labels should use [`crate::label_for_ui_layout`] instead. + /// Import: for data only, labels should use [`UiLayout::data_label`] instead. // TODO(#6315): must be merged with `Self::label` and have an improved API pub fn data_label(self, ui: &mut egui::Ui, string: impl AsRef) { let string = string.as_ref();