From 3a79ef1a2c94a4a564fadc40286873ffe54bcd49 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Jun 2024 16:43:46 +0200 Subject: [PATCH 01/12] Make `ListItem` widgets as narrow as possible during the sizing pass This ensures tooltips with `ListItem`s don't get unnecessarily wide --- crates/re_data_ui/src/item_ui.rs | 3 +-- crates/re_ui/src/list_item/list_item.rs | 15 ++++++++------- crates/re_ui/src/list_item/mod.rs | 4 +--- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/crates/re_data_ui/src/item_ui.rs b/crates/re_data_ui/src/item_ui.rs index 9324cac3969c..db312ddc1b6e 100644 --- a/crates/re_data_ui/src/item_ui.rs +++ b/crates/re_data_ui/src/item_ui.rs @@ -421,8 +421,7 @@ pub fn component_path_button_to( } else { "Temporal component" }) - .with_icon(icon) - .exact_width(true), + .with_icon(icon), ); let component_name = component_path.component_name; diff --git a/crates/re_ui/src/list_item/list_item.rs b/crates/re_ui/src/list_item/list_item.rs index 3beae3de19a5..9127f363dfca 100644 --- a/crates/re_ui/src/list_item/list_item.rs +++ b/crates/re_ui/src/list_item/list_item.rs @@ -220,14 +220,15 @@ impl<'a> ListItem<'a> { }; let desired_width = match content.desired_width(re_ui, ui) { - // // content will use all available width - // None => ui.available_width().at_least(extra_indent + collapse_extra), - // // content will use the required width - // Some(desired_width) => extra_indent + collapse_extra + desired_width, DesiredWidth::Exact(width) => extra_indent + collapse_extra + width, - DesiredWidth::AtLeast(width) => ui - .available_width() - .at_least(extra_indent + collapse_extra + width), + DesiredWidth::AtLeast(width) => { + let total_width = extra_indent + collapse_extra + width; + if ui.is_sizing_pass() { + total_width + } else { + ui.available_width().at_least(total_width) + } + } }; let desired_size = egui::vec2(desired_width, height); diff --git a/crates/re_ui/src/list_item/mod.rs b/crates/re_ui/src/list_item/mod.rs index 708ca5d993ce..f5f92e2dd057 100644 --- a/crates/re_ui/src/list_item/mod.rs +++ b/crates/re_ui/src/list_item/mod.rs @@ -87,9 +87,7 @@ pub fn hyperlink_to_ui( let response = ListItem::new(re_ui) .show_flat( ui, - LabelContent::new(text) - .with_icon(&crate::icons::EXTERNAL_LINK) - .exact_width(true), + LabelContent::new(text).with_icon(&crate::icons::EXTERNAL_LINK), ) .on_hover_cursor(egui::CursorIcon::PointingHand); if response.clicked() { From 59069e245251c1dc9eeda860323cdb1c6e647a73 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Jun 2024 16:53:56 +0200 Subject: [PATCH 02/12] Automatic min-width of ListItem --- crates/re_ui/src/list_item/label_content.rs | 23 +++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/crates/re_ui/src/list_item/label_content.rs b/crates/re_ui/src/list_item/label_content.rs index 32a9ae5e0f11..0dc6e4df8ec5 100644 --- a/crates/re_ui/src/list_item/label_content.rs +++ b/crates/re_ui/src/list_item/label_content.rs @@ -18,7 +18,7 @@ pub struct LabelContent<'a> { buttons_fn: Option egui::Response + 'a>>, always_show_buttons: bool, - min_desired_width: f32, + min_desired_width: Option, exact_width: bool, } @@ -33,7 +33,7 @@ impl<'a> LabelContent<'a> { icon_fn: None, buttons_fn: None, always_show_buttons: false, - min_desired_width: 0.0, + min_desired_width: None, exact_width: false, } } @@ -97,7 +97,7 @@ impl<'a> LabelContent<'a> { /// This defaults to zero. #[inline] pub fn min_desired_width(mut self, min_desired_width: f32) -> Self { - self.min_desired_width = min_desired_width; + self.min_desired_width = Some(min_desired_width); self } @@ -256,7 +256,7 @@ impl ListItemContent for LabelContent<'_> { } fn desired_width(&self, _re_ui: &ReUi, ui: &Ui) -> DesiredWidth { - if self.exact_width { + let measured_width = { //TODO(ab): ideally there wouldn't be as much code duplication with `Self::ui` let mut text = self.text.clone(); if self.italics || self.label_style == LabelStyle::Unnamed { @@ -276,9 +276,20 @@ impl ListItemContent for LabelContent<'_> { // The `ceil()` is needed to avoid some rounding errors which leads to text being // truncated even though we allocated enough space. - DesiredWidth::Exact(desired_width.ceil().at_least(self.min_desired_width)) + desired_width.ceil() + }; + + // TODO(emilk): use the egui `UiStack` to check if we're somewhere resizable, + // and only then turn on text truncation. + // For instance: in a tooltip we must not truncate the text, because + // the user can't resize the tooltip to read the full text. + // But if we're in a panel, the user can resize the panel to read the full text. + let min_desired_width = self.min_desired_width.unwrap_or(measured_width); + + if self.exact_width { + DesiredWidth::Exact(measured_width.at_least(min_desired_width)) } else { - DesiredWidth::AtLeast(self.min_desired_width) + DesiredWidth::AtLeast(min_desired_width) } } } From 805811f2a571ef534df8dbb0af19f170d5c15b35 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 5 Jun 2024 16:42:56 +0200 Subject: [PATCH 03/12] Use `UiStack` to figure out the default wrap-mode for ListItem labels --- crates/re_time_panel/src/lib.rs | 6 +- .../examples/re_ui_example/right_panel.rs | 7 +- crates/re_ui/src/lib.rs | 4 +- crates/re_ui/src/list_item/label_content.rs | 75 +++++++++++++------ 4 files changed, 60 insertions(+), 32 deletions(-) diff --git a/crates/re_time_panel/src/lib.rs b/crates/re_time_panel/src/lib.rs index 36da28e88205..3cfbad0153b0 100644 --- a/crates/re_time_panel/src/lib.rs +++ b/crates/re_time_panel/src/lib.rs @@ -635,7 +635,7 @@ impl TimePanel { ctx, &InstancePath::from(tree.path.clone()), )) - .exact_width(true), + .truncate(false), |_, ui| { self.show_children( ctx, @@ -780,7 +780,7 @@ impl TimePanel { } else { &re_ui::icons::COMPONENT_TEMPORAL }) - .exact_width(true), + .truncate(false), ); context_menu_ui_for_item( @@ -826,7 +826,7 @@ impl TimePanel { ) }, )) - .exact_width(true) + .truncate(false) .with_icon(if is_static { &re_ui::icons::COMPONENT_STATIC } else { diff --git a/crates/re_ui/examples/re_ui_example/right_panel.rs b/crates/re_ui/examples/re_ui_example/right_panel.rs index e9bf12da0dc8..84415eba474e 100644 --- a/crates/re_ui/examples/re_ui_example/right_panel.rs +++ b/crates/re_ui/examples/re_ui_example/right_panel.rs @@ -94,16 +94,13 @@ impl RightPanel { format!("Some item {i}") }; - // Note: we use `exact_width(true)` here to force the item to allocate + // Note: we use `truncate(false)` here to force the item to allocate // as much as needed for the label, which in turn will trigger the // scroll area. if re_ui .list_item() .selected(Some(i) == self.selected_list_item) - .show_flat( - ui, - list_item::LabelContent::new(&label).exact_width(true), - ) + .show_flat(ui, list_item::LabelContent::new(&label).truncate(false)) .clicked() { self.selected_list_item = Some(i); diff --git a/crates/re_ui/src/lib.rs b/crates/re_ui/src/lib.rs index 1135814f3509..e8b6b59e8c6f 100644 --- a/crates/re_ui/src/lib.rs +++ b/crates/re_ui/src/lib.rs @@ -1327,9 +1327,9 @@ pub trait UiExt { if node.has_visible_frame() || node.is_panel_ui() || node.is_root_ui() - || node.kind == Some(egui::UiKind::TableCell) + || node.kind() == Some(egui::UiKind::TableCell) { - return (node.max_rect + node.frame.inner_margin).x_range(); + return (node.max_rect + node.frame().inner_margin).x_range(); } } diff --git a/crates/re_ui/src/list_item/label_content.rs b/crates/re_ui/src/list_item/label_content.rs index 0dc6e4df8ec5..afbe2a4e9f01 100644 --- a/crates/re_ui/src/list_item/label_content.rs +++ b/crates/re_ui/src/list_item/label_content.rs @@ -18,23 +18,26 @@ pub struct LabelContent<'a> { buttons_fn: Option egui::Response + 'a>>, always_show_buttons: bool, + text_wrap_mode: Option, min_desired_width: Option, - exact_width: bool, } impl<'a> LabelContent<'a> { pub fn new(text: impl Into) -> Self { Self { text: text.into(), + subdued: false, weak: false, italics: false, + label_style: Default::default(), icon_fn: None, buttons_fn: None, always_show_buttons: false, + + text_wrap_mode: None, min_desired_width: None, - exact_width: false, } } @@ -79,16 +82,13 @@ impl<'a> LabelContent<'a> { self } - /// Allocate the exact width required for the label. - /// - /// By default, [`LabelContent`] uses the available width. By setting `exact_width` to true, - /// the exact width required by the label (and the icon if any) is allocated instead. See - /// [`super::DesiredWidth::Exact`]. - /// - /// Note that if [`Self::min_desired_width`] is set, it is used as a minimum value. - #[inline] - pub fn exact_width(mut self, exact_width: bool) -> Self { - self.exact_width = exact_width; + /// Should we truncate text if it is too long? + pub fn truncate(mut self, truncate: bool) -> Self { + self.text_wrap_mode = Some(if truncate { + egui::TextWrapMode::Truncate + } else { + egui::TextWrapMode::Extend + }); self } @@ -148,10 +148,39 @@ impl<'a> LabelContent<'a> { self.always_show_buttons = always_show_buttons; self } + + fn get_text_wrap_mode(&self, ui: &egui::Ui) -> egui::TextWrapMode { + if let Some(text_wrap_mode) = self.text_wrap_mode { + text_wrap_mode + } else { + let mut is_in_side_panel = false; + for frame in ui.stack().iter() { + if let Some(kind) = frame.kind() { + if kind.is_area() { + // Our popups (tooltips etc) aren't resizable, so show all of the text + return egui::TextWrapMode::Extend; + } + if matches!(kind, egui::UiKind::LeftPanel | egui::UiKind::RightPanel) { + is_in_side_panel = true; + } + } + } + + if is_in_side_panel { + // Our side-panels are resizable, so truncate the text if we don't fit. + egui::TextWrapMode::Truncate + } else { + // Safe fallback + egui::TextWrapMode::Extend + } + } + } } impl ListItemContent for LabelContent<'_> { fn ui(self: Box, re_ui: &ReUi, ui: &mut Ui, context: &ContentContext<'_>) { + let text_wrap_mode = self.get_text_wrap_mode(ui); + let Self { mut text, subdued, @@ -161,8 +190,8 @@ impl ListItemContent for LabelContent<'_> { icon_fn, buttons_fn, always_show_buttons, + text_wrap_mode: _, min_desired_width: _, - exact_width: _, } = *self; let icon_rect = egui::Rect::from_center_size( @@ -235,7 +264,7 @@ impl ListItemContent for LabelContent<'_> { let mut layout_job = text.into_layout_job(ui.style(), egui::FontSelection::Default, Align::LEFT); - layout_job.wrap = TextWrapping::truncate_at_width(text_rect.width()); + layout_job.wrap = TextWrapping::from_wrap_mode_and_width(text_wrap_mode, text_rect.width()); let galley = ui.fonts(|fonts| fonts.layout_job(layout_job)); @@ -256,6 +285,8 @@ impl ListItemContent for LabelContent<'_> { } fn desired_width(&self, _re_ui: &ReUi, ui: &Ui) -> DesiredWidth { + let text_wrap_mode = self.get_text_wrap_mode(ui); + let measured_width = { //TODO(ab): ideally there wouldn't be as much code duplication with `Self::ui` let mut text = self.text.clone(); @@ -279,16 +310,16 @@ impl ListItemContent for LabelContent<'_> { desired_width.ceil() }; - // TODO(emilk): use the egui `UiStack` to check if we're somewhere resizable, - // and only then turn on text truncation. - // For instance: in a tooltip we must not truncate the text, because - // the user can't resize the tooltip to read the full text. - // But if we're in a panel, the user can resize the panel to read the full text. - let min_desired_width = self.min_desired_width.unwrap_or(measured_width); - - if self.exact_width { + if text_wrap_mode == egui::TextWrapMode::Extend { + let min_desired_width = self.min_desired_width.unwrap_or(0.0); DesiredWidth::Exact(measured_width.at_least(min_desired_width)) } else { + // If the user set an explicit min-width, use it. + // Otherwise, show at least `default_min_width`, unless the text is even short. + let default_min_width = 64.0; + let min_desired_width = self + .min_desired_width + .unwrap_or_else(|| measured_width.min(default_min_width)); DesiredWidth::AtLeast(min_desired_width) } } From 84553b702c9d4858dd5eab5707347a4e9f1f3076 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Thu, 6 Jun 2024 10:09:34 +0200 Subject: [PATCH 04/12] Post-merge fixes --- crates/re_data_ui/src/item_ui.rs | 20 +++++++++----------- crates/re_time_panel/src/lib.rs | 2 +- crates/re_ui/src/ui_ext.rs | 4 +--- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/crates/re_data_ui/src/item_ui.rs b/crates/re_data_ui/src/item_ui.rs index 648a442512d0..6e209876dc21 100644 --- a/crates/re_data_ui/src/item_ui.rs +++ b/crates/re_data_ui/src/item_ui.rs @@ -410,17 +410,15 @@ pub fn component_path_button_to( ui.style_mut().wrap_mode = Some(egui::TextWrapMode::Extend); // Make tooltip as wide as needed list_item::list_item_scope(ui, "component_path_tooltip", |ui| { - ui.list_item() - .interactive(false) - .show_flat( - ui, - list_item::LabelContent::new(if is_static { - "Static component" - } else { - "Temporal component" - }) - .with_icon(icon), - ); + ui.list_item().interactive(false).show_flat( + ui, + list_item::LabelContent::new(if is_static { + "Static component" + } else { + "Temporal component" + }) + .with_icon(icon), + ); let component_name = component_path.component_name; diff --git a/crates/re_time_panel/src/lib.rs b/crates/re_time_panel/src/lib.rs index 372ae98c2795..026c79fddad3 100644 --- a/crates/re_time_panel/src/lib.rs +++ b/crates/re_time_panel/src/lib.rs @@ -816,7 +816,7 @@ impl TimePanel { ) }, )) - .truncate(false) + .truncate(false) .with_icon(if is_static { &re_ui::icons::COMPONENT_STATIC } else { diff --git a/crates/re_ui/src/ui_ext.rs b/crates/re_ui/src/ui_ext.rs index 6112a63f08fa..a2591938742c 100644 --- a/crates/re_ui/src/ui_ext.rs +++ b/crates/re_ui/src/ui_ext.rs @@ -905,9 +905,7 @@ pub trait UiExt { let response = ListItem::new() .show_flat( ui, - LabelContent::new(text) - .with_icon(&crate::icons::EXTERNAL_LINK) - .exact_width(true), + LabelContent::new(text).with_icon(&crate::icons::EXTERNAL_LINK), ) .on_hover_cursor(egui::CursorIcon::PointingHand); if response.clicked() { From b1aaa72fa772bc3bf7cdf4be274bdd96bcd43bac Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 10 Jun 2024 20:20:52 +0200 Subject: [PATCH 05/12] Create helper function `is_in_resizable_area` --- Cargo.lock | 1 + crates/re_ui/Cargo.toml | 1 + crates/re_ui/src/lib.rs | 26 +++++++++++++++++++++ crates/re_ui/src/list_item/label_content.rs | 23 +++--------------- 4 files changed, 31 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d10a85d82f82..c5265d815d61 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5187,6 +5187,7 @@ dependencies = [ "re_format", "re_log", "re_log_types", + "re_tracing", "serde", "serde_json", "strum", diff --git a/crates/re_ui/Cargo.toml b/crates/re_ui/Cargo.toml index 748d75ae9429..be5bdc81393b 100644 --- a/crates/re_ui/Cargo.toml +++ b/crates/re_ui/Cargo.toml @@ -33,6 +33,7 @@ re_entity_db.workspace = true # syntax-highlighting for InstancePath. TODO(emilk re_format.workspace = true re_log.workspace = true re_log_types.workspace = true # syntax-highlighting for EntityPath +re_tracing.workspace = true egui.workspace = true egui_commonmark = { workspace = true, features = ["pulldown_cmark"] } diff --git a/crates/re_ui/src/lib.rs b/crates/re_ui/src/lib.rs index 3ee6a6b78d59..3d3b21dc0f1d 100644 --- a/crates/re_ui/src/lib.rs +++ b/crates/re_ui/src/lib.rs @@ -88,3 +88,29 @@ pub fn apply_style_and_install_loaders(egui_ctx: &egui::Context) { design_tokens().apply(egui_ctx); } + +/// Used as a heuristic to figure out if it is safe to truncate text. +/// +/// If this returns false, we should never truncate. +fn is_in_resizable_area(ui: &egui::Ui) -> bool { + re_tracing::profile_function!(); + + let mut is_in_side_panel = false; + + for frame in ui.stack().iter() { + if let Some(kind) = frame.kind() { + if kind.is_area() { + return false; // Our popups (tooltips etc) aren't resizable + } + if matches!(kind, egui::UiKind::LeftPanel | egui::UiKind::RightPanel) { + is_in_side_panel = true; + } + } + } + + if is_in_side_panel { + true // Our side-panels are resizable + } else { + false // Safe fallback + } +} diff --git a/crates/re_ui/src/list_item/label_content.rs b/crates/re_ui/src/list_item/label_content.rs index 8570b459a029..7726061afb18 100644 --- a/crates/re_ui/src/list_item/label_content.rs +++ b/crates/re_ui/src/list_item/label_content.rs @@ -152,27 +152,10 @@ impl<'a> LabelContent<'a> { fn get_text_wrap_mode(&self, ui: &egui::Ui) -> egui::TextWrapMode { if let Some(text_wrap_mode) = self.text_wrap_mode { text_wrap_mode + } else if crate::is_in_resizable_area(ui) { + egui::TextWrapMode::Truncate } else { - let mut is_in_side_panel = false; - for frame in ui.stack().iter() { - if let Some(kind) = frame.kind() { - if kind.is_area() { - // Our popups (tooltips etc) aren't resizable, so show all of the text - return egui::TextWrapMode::Extend; - } - if matches!(kind, egui::UiKind::LeftPanel | egui::UiKind::RightPanel) { - is_in_side_panel = true; - } - } - } - - if is_in_side_panel { - // Our side-panels are resizable, so truncate the text if we don't fit. - egui::TextWrapMode::Truncate - } else { - // Safe fallback - egui::TextWrapMode::Extend - } + egui::TextWrapMode::Extend } } } From 231ed7a4e2e810586cb04933ca1735e5c687f6bb Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 10 Jun 2024 20:21:12 +0200 Subject: [PATCH 06/12] =?UTF-8?q?Remove=20the=20`.exact=5Fwidth(=E2=80=A6)?= =?UTF-8?q?`=20hack?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/re_data_ui/src/instance_path.rs | 7 +----- .../re_ui/src/list_item/property_content.rs | 23 ++++--------------- 2 files changed, 6 insertions(+), 24 deletions(-) diff --git a/crates/re_data_ui/src/instance_path.rs b/crates/re_data_ui/src/instance_path.rs index 304aaccee447..788cdc2d3f8b 100644 --- a/crates/re_data_ui/src/instance_path.rs +++ b/crates/re_data_ui/src/instance_path.rs @@ -114,7 +114,7 @@ impl DataUi for InstancePath { continue; // no need to show components that are unset at this point in time }; - let mut content = + let content = re_ui::list_item::PropertyContent::new(component_name.short_name()) .with_icon(icon) .value_fn(|ui, _| { @@ -145,11 +145,6 @@ impl DataUi for InstancePath { } }); - // 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) }; diff --git a/crates/re_ui/src/list_item/property_content.rs b/crates/re_ui/src/list_item/property_content.rs index 1a9143657be5..1ef50d0994c5 100644 --- a/crates/re_ui/src/list_item/property_content.rs +++ b/crates/re_ui/src/list_item/property_content.rs @@ -21,7 +21,6 @@ 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, @@ -40,7 +39,6 @@ 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, @@ -58,17 +56,6 @@ 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 { @@ -218,7 +205,6 @@ impl ListItemContent for PropertyContent<'_> { show_only_when_collapsed, value_fn, action_buttons, - exact_width: _, } = *self; // │ │ @@ -382,7 +368,10 @@ impl ListItemContent for PropertyContent<'_> { fn desired_width(&self, ui: &Ui) -> DesiredWidth { let layout_info = LayoutInfoStack::top(ui.ctx()); - if self.exact_width { + + if crate::is_in_resizable_area(ui) { + DesiredWidth::AtLeast(self.min_desired_width) + } else { 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(); @@ -395,12 +384,10 @@ impl ListItemContent for PropertyContent<'_> { desired_width += action_button_dimension + DesignTokens::text_to_icon_padding(); } - DesiredWidth::Exact(desired_width.ceil()) + DesiredWidth::AtLeast(desired_width.ceil()) } else { DesiredWidth::AtLeast(self.min_desired_width) } - } else { - DesiredWidth::AtLeast(self.min_desired_width) } } } From 53be85431660726c839e014af5d5b3de5e58a4a7 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 10 Jun 2024 20:25:49 +0200 Subject: [PATCH 07/12] Show debug-log when debugging --- .vscode/launch.json | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 11ee57e12d59..3cdc81be79f2 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -60,7 +60,10 @@ } }, "args": [], - "cwd": "${workspaceFolder}" + "cwd": "${workspaceFolder}", + "env": { + "RUST_LOG": "debug" + } }, { "name": "Debug 'rerun ../data.rrd'", @@ -81,7 +84,10 @@ "args": [ "../data.rrd" ], - "cwd": "${workspaceFolder}" + "cwd": "${workspaceFolder}", + "env": { + "RUST_LOG": "debug" + } }, { "name": "Debug 'rerun' colmap.rrd from url", From 3213d0b19e02f4d8d5dfa8d76a3a023496d2d747 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 10 Jun 2024 20:31:23 +0200 Subject: [PATCH 08/12] Add `#[inline]` --- crates/re_ui/src/list_item/label_content.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/re_ui/src/list_item/label_content.rs b/crates/re_ui/src/list_item/label_content.rs index 7726061afb18..af2b1d2ffcbd 100644 --- a/crates/re_ui/src/list_item/label_content.rs +++ b/crates/re_ui/src/list_item/label_content.rs @@ -83,6 +83,7 @@ impl<'a> LabelContent<'a> { } /// Should we truncate text if it is too long? + #[inline] pub fn truncate(mut self, truncate: bool) -> Self { self.text_wrap_mode = Some(if truncate { egui::TextWrapMode::Truncate From eb6a13cdfc2d75cf74e0b422396593dcdd31e499 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 10 Jun 2024 20:31:28 +0200 Subject: [PATCH 09/12] Clippy lint --- .../re_ui/src/list_item/property_content.rs | 30 +++++++++---------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/crates/re_ui/src/list_item/property_content.rs b/crates/re_ui/src/list_item/property_content.rs index 1ef50d0994c5..7520b982e848 100644 --- a/crates/re_ui/src/list_item/property_content.rs +++ b/crates/re_ui/src/list_item/property_content.rs @@ -371,23 +371,21 @@ impl ListItemContent for PropertyContent<'_> { if crate::is_in_resizable_area(ui) { DesiredWidth::AtLeast(self.min_desired_width) - } else { - 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 = - DesignTokens::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 + DesignTokens::text_to_icon_padding(); - } - - DesiredWidth::AtLeast(desired_width.ceil()) - } else { - DesiredWidth::AtLeast(self.min_desired_width) + } else 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 = + DesignTokens::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 + DesignTokens::text_to_icon_padding(); } + + DesiredWidth::AtLeast(desired_width.ceil()) + } else { + DesiredWidth::AtLeast(self.min_desired_width) } } } From 65b181fb11ff01f0a6dc76041918dd7af641a40c Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 10 Jun 2024 20:40:37 +0200 Subject: [PATCH 10/12] Better names and comments --- crates/re_ui/src/lib.rs | 2 +- crates/re_ui/src/list_item/label_content.rs | 6 +++--- crates/re_ui/src/list_item/list_item.rs | 4 ++++ crates/re_ui/src/list_item/property_content.rs | 2 +- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/crates/re_ui/src/lib.rs b/crates/re_ui/src/lib.rs index 3d3b21dc0f1d..abf6ca9e5f14 100644 --- a/crates/re_ui/src/lib.rs +++ b/crates/re_ui/src/lib.rs @@ -92,7 +92,7 @@ pub fn apply_style_and_install_loaders(egui_ctx: &egui::Context) { /// Used as a heuristic to figure out if it is safe to truncate text. /// /// If this returns false, we should never truncate. -fn is_in_resizable_area(ui: &egui::Ui) -> bool { +fn is_in_resizable_panel(ui: &egui::Ui) -> bool { re_tracing::profile_function!(); let mut is_in_side_panel = false; diff --git a/crates/re_ui/src/list_item/label_content.rs b/crates/re_ui/src/list_item/label_content.rs index af2b1d2ffcbd..92956eefb7bb 100644 --- a/crates/re_ui/src/list_item/label_content.rs +++ b/crates/re_ui/src/list_item/label_content.rs @@ -153,10 +153,10 @@ impl<'a> LabelContent<'a> { fn get_text_wrap_mode(&self, ui: &egui::Ui) -> egui::TextWrapMode { if let Some(text_wrap_mode) = self.text_wrap_mode { text_wrap_mode - } else if crate::is_in_resizable_area(ui) { - egui::TextWrapMode::Truncate + } else if crate::is_in_resizable_panel(ui) { + egui::TextWrapMode::Truncate // The user can resize the panl to see the full text } else { - egui::TextWrapMode::Extend + egui::TextWrapMode::Extend // Show everything } } } diff --git a/crates/re_ui/src/list_item/list_item.rs b/crates/re_ui/src/list_item/list_item.rs index 26c1f6ac88bb..748d2b59d88e 100644 --- a/crates/re_ui/src/list_item/list_item.rs +++ b/crates/re_ui/src/list_item/list_item.rs @@ -230,8 +230,12 @@ impl ListItem { DesiredWidth::AtLeast(width) => { let total_width = extra_indent + collapse_extra + width; if ui.is_sizing_pass() { + // In the sizing pass we try to be as small as possible. + // egui will then use the maxiumum width from the sizing pass + // as the max width in all following frames. total_width } else { + // Use as much space as we are given (i.e. fill up the full width of the ui). ui.available_width().at_least(total_width) } } diff --git a/crates/re_ui/src/list_item/property_content.rs b/crates/re_ui/src/list_item/property_content.rs index 7520b982e848..b6f556b81b96 100644 --- a/crates/re_ui/src/list_item/property_content.rs +++ b/crates/re_ui/src/list_item/property_content.rs @@ -369,7 +369,7 @@ impl ListItemContent for PropertyContent<'_> { fn desired_width(&self, ui: &Ui) -> DesiredWidth { let layout_info = LayoutInfoStack::top(ui.ctx()); - if crate::is_in_resizable_area(ui) { + if crate::is_in_resizable_panel(ui) { DesiredWidth::AtLeast(self.min_desired_width) } else 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(); From 16d78c653b120b518064ae5a9ef036c7853b5cd3 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 10 Jun 2024 20:42:21 +0200 Subject: [PATCH 11/12] Fix typo --- crates/re_ui/src/list_item/list_item.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/re_ui/src/list_item/list_item.rs b/crates/re_ui/src/list_item/list_item.rs index 748d2b59d88e..ce6c0d6fa616 100644 --- a/crates/re_ui/src/list_item/list_item.rs +++ b/crates/re_ui/src/list_item/list_item.rs @@ -231,7 +231,7 @@ impl ListItem { let total_width = extra_indent + collapse_extra + width; if ui.is_sizing_pass() { // In the sizing pass we try to be as small as possible. - // egui will then use the maxiumum width from the sizing pass + // egui will then use the maximum width from the sizing pass // as the max width in all following frames. total_width } else { From 972751d21ffcc16626262552fa44f35af257935d Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 11 Jun 2024 10:03:24 +0200 Subject: [PATCH 12/12] Expand docstring --- crates/re_ui/src/lib.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/re_ui/src/lib.rs b/crates/re_ui/src/lib.rs index abf6ca9e5f14..0f1b629b7c7a 100644 --- a/crates/re_ui/src/lib.rs +++ b/crates/re_ui/src/lib.rs @@ -89,9 +89,16 @@ pub fn apply_style_and_install_loaders(egui_ctx: &egui::Context) { design_tokens().apply(egui_ctx); } +/// Is this Ui in a resizable panel? +/// /// Used as a heuristic to figure out if it is safe to truncate text. /// -/// If this returns false, we should never truncate. +/// In a resizable panel, it is safe to truncate text if it doesn't fit, +/// because the user can just make the panel wider to see the full text. +/// +/// In other places, we should never truncate text, because then the user +/// cannot read it all. In those places (when this functions returns `false`) +/// you should either wrap the text or let it grow the Ui it is in. fn is_in_resizable_panel(ui: &egui::Ui) -> bool { re_tracing::profile_function!();