From aaf9d5e39d23af2df4c10d806c18dfdab506f00f Mon Sep 17 00:00:00 2001 From: Antoine Beyeler <49431240+abey79@users.noreply.github.com> Date: Tue, 4 Jun 2024 18:44:53 +0200 Subject: [PATCH] Use `list_item` for the component list in `InstancePath::data_ui` (#6309) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### What - Fixes #4161 - Follow up to #6297 - Follow up to #6325 This PR uses `list_item` for the component list in the entity path selection panel and tooltip. ~~Blocked by https://github.com/emilk/egui/issues/4471 to resolve the tooltip "first frame flicker"~~ TODO: - [x] ⚠️ full_span_scope panic on hover on spatial space view: https://github.com/rerun-io/rerun/issues/6246 #### selection panel before: image after: image *Note*: the hover behaviour is the biggest change here: now full span, previously just on the left-column button #### tooltip before: image after: 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 examples from latest `main` build: [rerun.io/viewer](https://rerun.io/viewer/pr/6309?manifest_url=https://app.rerun.io/version/main/examples_manifest.json) * Using full set of examples from `nightly` build: [rerun.io/viewer](https://rerun.io/viewer/pr/6309?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 * [x] If applicable, add a new check to the [release checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)! - [PR Build Summary](https://build.rerun.io/pr/6309) - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) To run all checks from `main`, comment on the PR with `@rerun-bot full-check`. --- 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 | 161 +++++++++++------- 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_ui/src/list_item/scope.rs | 2 +- crates/re_viewer_context/Cargo.toml | 1 + .../src/component_ui_registry.rs | 88 ++++++++++ scripts/ci/rust_checks.py | 5 +- 15 files changed, 246 insertions(+), 205 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8ec08fb31765..0f36a62b0cd0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5274,6 +5274,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 1a28068bcbe3..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,7 @@ use re_log_types::{external::arrow2, EntityPath, Instance}; use re_types::external::arrow2::array::Utf8Array; use re_viewer_context::{ComponentUiRegistry, UiLayout, ViewerContext}; -use super::{data_label_for_ui_layout, EntityDataUi}; +use super::EntityDataUi; pub fn create_component_ui_registry() -> ComponentUiRegistry { re_tracing::profile_function!(); @@ -86,14 +86,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; } } @@ -104,7 +104,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; } } @@ -117,7 +117,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 86abfcf06300..4f78b2a99af8 100644 --- a/crates/re_data_ui/src/image.rs +++ b/crates/re_data_ui/src/image.rs @@ -11,7 +11,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; @@ -84,7 +84,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())); } } } @@ -187,7 +187,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 389dc936df3e..f791ec9af252 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; impl DataUi for InstancePath { fn data_ui( @@ -27,12 +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`. - ui.label(format!( - "No components logged on timeline {:?}", - query.timeline().name() - )); + ui_layout.label( + ui, + format!( + "No components logged on timeline {:?}", + query.timeline().name() + ), + ); } else { - ui.label( + ui_layout.label( + ui, ctx.re_ui .error_text(format!("Unknown entity: {entity_path:?}")), ); @@ -40,39 +41,68 @@ 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 { + ui_layout.label( + ui, + 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_item::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_TEMPORAL + }; + let item = Item::ComponentPath(component_path); + + let mut list_item = ctx.re_ui.list_item().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_item::LabelContent::new(component_name.short_name()) + .with_icon(icon), + ) + } else { let results = db.query_caches().latest_at( db.store(), query, @@ -83,36 +113,49 @@ impl DataUi for InstancePath { continue; // no need to show components that are unset at this point in time }; - item_ui::component_path_button( - ctx, - ui, - &ComponentPath::new(entity_path.clone(), component_name), - db, - ); + let mut content = + re_ui::list_item::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, - ); + // avoid the list item to max the tooltip width + if ui_layout == UiLayout::Tooltip { + content = content.exact_width(true); } - ui.end_row(); + list_item.show_flat(ui, content) + }; + + if interactive { + ctx.select_hovered_on_click(&response, item); } - Some(()) - }); + } + }); } } diff --git a/crates/re_data_ui/src/lib.rs b/crates/re_data_ui/src/lib.rs index b97e05ba024c..4a411ec175e1 100644 --- a/crates/re_data_ui/src/lib.rs +++ b/crates/re_data_ui/src/lib.rs @@ -176,98 +176,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(), - UiLayout::Tooltip | UiLayout::SelectionPanelLimitHeight | UiLayout::SelectionPanelFull => { - label = label.wrap(); - } - } - - 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_ui/src/list_item/scope.rs b/crates/re_ui/src/list_item/scope.rs index ac4798a709c0..3c3eb713bfec 100644 --- a/crates/re_ui/src/list_item/scope.rs +++ b/crates/re_ui/src/list_item/scope.rs @@ -280,7 +280,7 @@ pub fn list_item_scope( }; // 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; diff --git a/crates/re_viewer_context/Cargo.toml b/crates/re_viewer_context/Cargo.toml index 18503e45de91..08a7ac9ed7ef 100644 --- a/crates/re_viewer_context/Cargo.toml +++ b/crates/re_viewer_context/Cargo.toml @@ -39,6 +39,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 a7a001f30303..db42ce079079 100644 --- a/crates/re_viewer_context/src/component_ui_registry.rs +++ b/crates/re_viewer_context/src/component_ui_registry.rs @@ -42,6 +42,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 [`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); + + match self { + Self::List => label = label.truncate(), + Self::Tooltip | Self::SelectionPanelLimitHeight | Self::SelectionPanelFull => { + label = label.wrap(); + } + } + + ui.add(label) + } + + /// Show data while respecting the given UI layout. + /// + /// 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(); + 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<'_>, 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()