From d46cb3520d40bd067ba4cc1f5f48da8f17c06974 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 29 May 2024 10:27:04 +0200 Subject: [PATCH] Add `Ui::is_sizing_pass` for better size estimation of `Area`s, and menus in particular (#4557) * Part of https://github.com/emilk/egui/issues/4535 * Closes https://github.com/emilk/egui/issues/3974 This adds a special `sizing_pass` mode to `Ui`, in which we have no centered or justified layouts, and everything is hidden. This is used by `Area` to use the first frame to measure the size of its contents so that it can then set the perfectly correct size the subsequent frames. For menus, where buttons are justified (span the full width), this finally the problem of auto-sizing. Before you would have to pick a width manually, and all buttons would expand to that width. If it was too wide, it looked weird. If it was too narrow, text would wrap. Now all menus are exactly the width they need to be. By default menus will wrap at `Spacing::menu_width`. This affects all situations when you have something that should be as small as possible, but still span the full width/height of the parent. For instance: the `egui::Separator` widget now checks the `ui.is_sizing_pass` flag before deciding on a size. In the sizing pass a horizontal separator is always 0 wide, and only in subsequent passes will it span the full width. --- Cargo.lock | 8 ++ crates/egui/src/containers/area.rs | 86 +++++++++++++++---- crates/egui/src/containers/frame.rs | 3 +- crates/egui/src/containers/popup.rs | 8 +- crates/egui/src/menu.rs | 20 ++--- crates/egui/src/style.rs | 21 ++++- crates/egui/src/ui.rs | 37 +++++++- crates/egui/src/widgets/separator.rs | 6 +- crates/egui_demo_lib/src/demo/context_menu.rs | 4 +- .../src/demo/demo_app_windows.rs | 1 - crates/emath/src/vec2.rs | 1 + tests/test_size_pass/Cargo.toml | 24 ++++++ tests/test_size_pass/src/main.rs | 25 ++++++ 13 files changed, 199 insertions(+), 45 deletions(-) create mode 100644 tests/test_size_pass/Cargo.toml create mode 100644 tests/test_size_pass/src/main.rs diff --git a/Cargo.lock b/Cargo.lock index 7f726392e33..c536538b39f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3628,6 +3628,14 @@ dependencies = [ "env_logger", ] +[[package]] +name = "test_size_pass" +version = "0.1.0" +dependencies = [ + "eframe", + "env_logger", +] + [[package]] name = "test_viewports" version = "0.1.0" diff --git a/crates/egui/src/containers/area.rs b/crates/egui/src/containers/area.rs index 857569da3ac..985d3af8c16 100644 --- a/crates/egui/src/containers/area.rs +++ b/crates/egui/src/containers/area.rs @@ -69,6 +69,7 @@ pub struct Area { constrain_rect: Option, order: Order, default_pos: Option, + default_size: Vec2, pivot: Align2, anchor: Option<(Align2, Vec2)>, new_pos: Option, @@ -87,6 +88,7 @@ impl Area { enabled: true, order: Order::Middle, default_pos: None, + default_size: Vec2::NAN, new_pos: None, pivot: Align2::LEFT_TOP, anchor: None, @@ -163,6 +165,35 @@ impl Area { self } + /// The size used for the [`Ui::max_rect`] the first frame. + /// + /// Text will wrap at this width, and images that expand to fill the available space + /// will expand to this size. + /// + /// If the contents are smaller than this size, the area will shrink to fit the contents. + /// If the contents overflow, the area will grow. + /// + /// If not set, [`style::Spacing::default_area_size`] will be used. + #[inline] + pub fn default_size(mut self, default_size: impl Into) -> Self { + self.default_size = default_size.into(); + self + } + + /// See [`Self::default_size`]. + #[inline] + pub fn default_width(mut self, default_width: f32) -> Self { + self.default_size.x = default_width; + self + } + + /// See [`Self::default_size`]. + #[inline] + pub fn default_height(mut self, default_height: f32) -> Self { + self.default_size.y = default_height; + self + } + /// Positions the window and prevents it from being moved #[inline] pub fn fixed_pos(mut self, fixed_pos: impl Into) -> Self { @@ -247,7 +278,7 @@ pub(crate) struct Prepared { /// This is so that we use the first frame to calculate the window size, /// and then can correctly position the window and its contents the next frame, /// without having one frame where the window is wrongly positioned or sized. - temporarily_invisible: bool, + sizing_pass: bool, } impl Area { @@ -272,6 +303,7 @@ impl Area { interactable, enabled, default_pos, + default_size, new_pos, pivot, anchor, @@ -292,11 +324,29 @@ impl Area { if is_new { ctx.request_repaint(); // if we don't know the previous size we are likely drawing the area in the wrong place } - let mut state = state.unwrap_or_else(|| State { - pivot_pos: default_pos.unwrap_or_else(|| automatic_area_position(ctx)), - pivot, - size: Vec2::ZERO, - interactable, + let mut state = state.unwrap_or_else(|| { + // during the sizing pass we will use this as the max size + let mut size = default_size; + + let default_area_size = ctx.style().spacing.default_area_size; + if size.x.is_nan() { + size.x = default_area_size.x; + } + if size.y.is_nan() { + size.y = default_area_size.y; + } + + if constrain { + let constrain_rect = constrain_rect.unwrap_or_else(|| ctx.screen_rect()); + size = size.at_most(constrain_rect.size()); + } + + State { + pivot_pos: default_pos.unwrap_or_else(|| automatic_area_position(ctx)), + pivot, + size, + interactable, + } }); state.pivot_pos = new_pos.unwrap_or(state.pivot_pos); state.interactable = interactable; @@ -365,7 +415,7 @@ impl Area { enabled, constrain, constrain_rect, - temporarily_invisible: is_new, + sizing_pass: is_new, } } @@ -431,12 +481,7 @@ impl Prepared { } }; - let max_rect = Rect::from_min_max( - self.state.left_top_pos(), - constrain_rect - .max - .at_least(self.state.left_top_pos() + Vec2::splat(32.0)), - ); + let max_rect = Rect::from_min_size(self.state.left_top_pos(), self.state.size); let clip_rect = constrain_rect; // Don't paint outside our bounds @@ -448,7 +493,9 @@ impl Prepared { clip_rect, ); ui.set_enabled(self.enabled); - ui.set_visible(!self.temporarily_invisible); + if self.sizing_pass { + ui.set_sizing_pass(); + } ui } @@ -461,11 +508,20 @@ impl Prepared { enabled: _, constrain: _, constrain_rect: _, - temporarily_invisible: _, + sizing_pass, } = self; state.size = content_ui.min_size(); + if sizing_pass { + // If during the sizing pass we measure our width to `123.45` and + // then try to wrap to exactly that next frame, + // we may accidentally wrap the last letter of some text. + // We only do this after the initial sizing pass though; + // otherwise we could end up with for-ever expanding areas. + state.size = state.size.ceil(); + } + ctx.memory_mut(|m| m.areas_mut().set_state(layer_id, state)); move_response diff --git a/crates/egui/src/containers/frame.rs b/crates/egui/src/containers/frame.rs index 40bd17d3c4d..8568a8f0b9b 100644 --- a/crates/egui/src/containers/frame.rs +++ b/crates/egui/src/containers/frame.rs @@ -266,7 +266,8 @@ impl Frame { self.show_dyn(ui, Box::new(add_contents)) } - fn show_dyn<'c, R>( + /// Show using dynamic dispatch. + pub fn show_dyn<'c, R>( self, ui: &mut Ui, add_contents: Box R + 'c>, diff --git a/crates/egui/src/containers/popup.rs b/crates/egui/src/containers/popup.rs index 9643a8eee31..0aca413a9ed 100644 --- a/crates/egui/src/containers/popup.rs +++ b/crates/egui/src/containers/popup.rs @@ -260,15 +260,11 @@ fn show_tooltip_area_dyn<'c, R>( Area::new(area_id) .order(Order::Tooltip) .fixed_pos(window_pos) + .default_width(ctx.style().spacing.tooltip_width) .constrain_to(ctx.screen_rect()) .interactable(false) .show(ctx, |ui| { - Frame::popup(&ctx.style()) - .show(ui, |ui| { - ui.set_max_width(ui.spacing().tooltip_width); - add_contents(ui) - }) - .inner + Frame::popup(&ctx.style()).show_dyn(ui, add_contents).inner }) } diff --git a/crates/egui/src/menu.rs b/crates/egui/src/menu.rs index 8b374ce7ad9..d23d5d20f31 100644 --- a/crates/egui/src/menu.rs +++ b/crates/egui/src/menu.rs @@ -133,10 +133,10 @@ pub(crate) fn submenu_button( } /// wrapper for the contents of every menu. -pub(crate) fn menu_ui<'c, R>( +fn menu_popup<'c, R>( ctx: &Context, - menu_id: Id, menu_state_arc: &Arc>, + menu_id: Id, add_contents: impl FnOnce(&mut Ui) -> R + 'c, ) -> InnerResponse { let pos = { @@ -150,6 +150,7 @@ pub(crate) fn menu_ui<'c, R>( .fixed_pos(pos) .constrain_to(ctx.screen_rect()) .interactable(true) + .default_width(ctx.style().spacing.menu_width) .sense(Sense::hover()); let area_response = area.show(ctx, |ui| { @@ -157,7 +158,6 @@ pub(crate) fn menu_ui<'c, R>( Frame::menu(ui.style()) .show(ui, |ui| { - ui.set_max_width(ui.spacing().menu_width); ui.set_menu_state(Some(menu_state_arc.clone())); ui.with_layout(Layout::top_down_justified(Align::LEFT), add_contents) .inner @@ -306,8 +306,7 @@ impl MenuRoot { add_contents: impl FnOnce(&mut Ui) -> R, ) -> (MenuResponse, Option>) { if self.id == button.id { - let inner_response = - MenuState::show(&button.ctx, &self.menu_state, self.id, add_contents); + let inner_response = menu_popup(&button.ctx, &self.menu_state, self.id, add_contents); let menu_state = self.menu_state.read(); if menu_state.response.is_close() { @@ -593,15 +592,6 @@ impl MenuState { self.response = MenuResponse::Close; } - pub fn show( - ctx: &Context, - menu_state: &Arc>, - id: Id, - add_contents: impl FnOnce(&mut Ui) -> R, - ) -> InnerResponse { - crate::menu::menu_ui(ctx, id, menu_state, add_contents) - } - fn show_submenu( &mut self, ctx: &Context, @@ -609,7 +599,7 @@ impl MenuState { add_contents: impl FnOnce(&mut Ui) -> R, ) -> Option { let (sub_response, response) = self.submenu(id).map(|sub| { - let inner_response = Self::show(ctx, sub, id, add_contents); + let inner_response = menu_popup(ctx, sub, id, add_contents); (sub.read().response, inner_response.inner) })?; self.cascade_close_response(sub_response); diff --git a/crates/egui/src/style.rs b/crates/egui/src/style.rs index f2410747dfb..9e34498eb70 100644 --- a/crates/egui/src/style.rs +++ b/crates/egui/src/style.rs @@ -316,10 +316,21 @@ pub struct Spacing { /// This is the spacing between the icon and the text pub icon_spacing: f32, + /// The size used for the [`Ui::max_rect`] the first frame. + /// + /// Text will wrap at this width, and images that expand to fill the available space + /// will expand to this size. + /// + /// If the contents are smaller than this size, the area will shrink to fit the contents. + /// If the contents overflow, the area will grow. + pub default_area_size: Vec2, + /// Width of a tooltip (`on_hover_ui`, `on_hover_text` etc). pub tooltip_width: f32, - /// The default width of a menu. + /// The default wrapping width of a menu. + /// + /// Items longer than this will wrap to a new line. pub menu_width: f32, /// Horizontal distance between a menu and a submenu. @@ -1073,8 +1084,9 @@ impl Default for Spacing { icon_width: 14.0, icon_width_inner: 8.0, icon_spacing: 4.0, + default_area_size: vec2(600.0, 400.0), tooltip_width: 600.0, - menu_width: 150.0, + menu_width: 400.0, menu_spacing: 2.0, combo_height: 200.0, scroll: Default::default(), @@ -1459,6 +1471,7 @@ impl Spacing { icon_width, icon_width_inner, icon_spacing, + default_area_size, tooltip_width, menu_width, menu_spacing, @@ -1509,6 +1522,10 @@ impl Spacing { ui.add(DragValue::new(combo_width).clamp_range(0.0..=1000.0)); ui.end_row(); + ui.label("Default area size"); + ui.add(two_drag_values(default_area_size, 0.0..=1000.0)); + ui.end_row(); + ui.label("TextEdit width"); ui.add(DragValue::new(text_edit_width).clamp_range(0.0..=1000.0)); ui.end_row(); diff --git a/crates/egui/src/ui.rs b/crates/egui/src/ui.rs index f3227109e36..40ffea8314e 100644 --- a/crates/egui/src/ui.rs +++ b/crates/egui/src/ui.rs @@ -61,6 +61,10 @@ pub struct Ui { /// and all widgets will assume a gray style. enabled: bool, + /// Set to true in special cases where we do one frame + /// where we size up the contents of the Ui, without actually showing it. + sizing_pass: bool, + /// Indicates whether this Ui belongs to a Menu. menu_state: Option>>, } @@ -82,6 +86,7 @@ impl Ui { style, placer: Placer::new(max_rect, Layout::default()), enabled: true, + sizing_pass: false, menu_state: None, }; @@ -108,9 +113,18 @@ impl Ui { pub fn child_ui_with_id_source( &mut self, max_rect: Rect, - layout: Layout, + mut layout: Layout, id_source: impl Hash, ) -> Self { + if self.sizing_pass { + // During the sizing pass we want widgets to use up as little space as possible, + // so that we measure the only the space we _need_. + layout.cross_justify = false; + if layout.cross_align == Align::Center { + layout.cross_align = Align::Min; + } + } + debug_assert!(!max_rect.any_nan()); let next_auto_id_source = Id::new(self.next_auto_id_source).with("child").value(); self.next_auto_id_source = self.next_auto_id_source.wrapping_add(1); @@ -121,6 +135,7 @@ impl Ui { style: self.style.clone(), placer: Placer::new(max_rect, layout), enabled: self.enabled, + sizing_pass: self.sizing_pass, menu_state: self.menu_state.clone(), }; @@ -140,6 +155,26 @@ impl Ui { // ------------------------------------------------- + /// Set to true in special cases where we do one frame + /// where we size up the contents of the Ui, without actually showing it. + /// + /// This will also turn the Ui invisible. + /// Should be called right after [`Self::new`], if at all. + #[inline] + pub fn set_sizing_pass(&mut self) { + self.sizing_pass = true; + self.set_visible(false); + } + + /// Set to true in special cases where we do one frame + /// where we size up the contents of the Ui, without actually showing it. + #[inline] + pub fn is_sizing_pass(&self) -> bool { + self.sizing_pass + } + + // ------------------------------------------------- + /// A unique identity of this [`Ui`]. #[inline] pub fn id(&self) -> Id { diff --git a/crates/egui/src/widgets/separator.rs b/crates/egui/src/widgets/separator.rs index 83c01f31200..f408c6fb0c5 100644 --- a/crates/egui/src/widgets/separator.rs +++ b/crates/egui/src/widgets/separator.rs @@ -96,7 +96,11 @@ impl Widget for Separator { let is_horizontal_line = is_horizontal_line .unwrap_or_else(|| ui.is_grid() || !ui.layout().main_dir().is_horizontal()); - let available_space = ui.available_size_before_wrap(); + let available_space = if ui.is_sizing_pass() { + Vec2::ZERO + } else { + ui.available_size_before_wrap() + }; let size = if is_horizontal_line { vec2(available_space.x, spacing) diff --git a/crates/egui_demo_lib/src/demo/context_menu.rs b/crates/egui_demo_lib/src/demo/context_menu.rs index 5195a2b0609..00d1431ef14 100644 --- a/crates/egui_demo_lib/src/demo/context_menu.rs +++ b/crates/egui_demo_lib/src/demo/context_menu.rs @@ -80,8 +80,6 @@ impl super::View for ContextMenus { ui.label("Right-click plot to edit it!"); ui.horizontal(|ui| { self.example_plot(ui).context_menu(|ui| { - ui.set_min_width(220.0); - ui.menu_button("Plot", |ui| { if ui.radio_value(&mut self.plot, Plot::Sin, "Sin").clicked() || ui @@ -188,6 +186,6 @@ impl ContextMenus { ui.close_menu(); } }); - let _ = ui.button("Very long text for this item"); + let _ = ui.button("Very long text for this item that should be wrapped"); } } diff --git a/crates/egui_demo_lib/src/demo/demo_app_windows.rs b/crates/egui_demo_lib/src/demo/demo_app_windows.rs index 1e34c464759..dc0e91f1b47 100644 --- a/crates/egui_demo_lib/src/demo/demo_app_windows.rs +++ b/crates/egui_demo_lib/src/demo/demo_app_windows.rs @@ -334,7 +334,6 @@ fn file_menu_button(ui: &mut Ui) { } ui.menu_button("File", |ui| { - ui.set_min_width(220.0); ui.style_mut().wrap_mode = Some(egui::TextWrapMode::Extend); // On the web the browser controls the zoom diff --git a/crates/emath/src/vec2.rs b/crates/emath/src/vec2.rs index 99f7d0c05c7..29050935fda 100644 --- a/crates/emath/src/vec2.rs +++ b/crates/emath/src/vec2.rs @@ -131,6 +131,7 @@ impl Vec2 { pub const ZERO: Self = Self { x: 0.0, y: 0.0 }; pub const INFINITY: Self = Self::splat(f32::INFINITY); + pub const NAN: Self = Self::splat(f32::NAN); #[inline(always)] pub const fn new(x: f32, y: f32) -> Self { diff --git a/tests/test_size_pass/Cargo.toml b/tests/test_size_pass/Cargo.toml new file mode 100644 index 00000000000..6886c7af2a1 --- /dev/null +++ b/tests/test_size_pass/Cargo.toml @@ -0,0 +1,24 @@ +[package] +name = "test_size_pass" +version = "0.1.0" +authors = ["Emil Ernerfeldt "] +license = "MIT OR Apache-2.0" +edition = "2021" +rust-version = "1.76" +publish = false + +[lints] +workspace = true + +[features] +wgpu = ["eframe/wgpu"] + +[dependencies] +eframe = { workspace = true, features = [ + "default", + "__screenshot", # __screenshot is so we can dump a screenshot using EFRAME_SCREENSHOT_TO +] } +env_logger = { version = "0.10", default-features = false, features = [ + "auto-color", + "humantime", +] } diff --git a/tests/test_size_pass/src/main.rs b/tests/test_size_pass/src/main.rs new file mode 100644 index 00000000000..13b49911ffb --- /dev/null +++ b/tests/test_size_pass/src/main.rs @@ -0,0 +1,25 @@ +#![cfg_attr(not(debug_assertions), windows_subsystem = "windows")] // hide console window on Windows in release +#![allow(rustdoc::missing_crate_level_docs)] // it's a test + +use eframe::egui; + +fn main() -> eframe::Result<()> { + env_logger::init(); // Use `RUST_LOG=debug` to see logs. + + let options = eframe::NativeOptions::default(); + eframe::run_simple_native("My egui App", options, move |ctx, _frame| { + egui::CentralPanel::default().show(ctx, |ui| { + ui.label("The menu should be as wide as the widest button"); + ui.menu_button("Click for menu", |ui| { + let _ = ui.button("Narrow").clicked(); + let _ = ui.button("Very wide text").clicked(); + let _ = ui.button("Narrow").clicked(); + }); + + ui.label("Hover for tooltip").on_hover_ui(|ui| { + ui.label("A separator:"); + ui.separator(); + }); + }); + }) +}