Ui.allocate_new_ui() adds extra padding not present when is_sizing_pass()

[barries]

Describe the bug

When is_sizing_pass(), allocate_new_ui() correctly does not add space before and after itself. When rendering "for real", it does. ui.horizontal() does not have this issue. _(I'm using sizing_pass to get the size of the popup so I can center it when it's rendered "for real"--this seems like a natural use case for sizingpass, but maybe there's a better way?)

In this image, I left the .invisible() off the sizing pass, the (correctly) laid out buttons in the upper left are how they are laid out while sizing. The popup is what's rendered, with !is_sizing_pass(), wherein you can see that the allocate_new_ui() allocates extra, unwanted padding, in addition to item_spacing.y.

To Reproduce

Run code like:

let modal = ModalPopup::new(c, &def);                                                   

let layer_id   = LayerId::new(Order::Foreground, Id::new("PopupLayer"));                
let window_id  = Id::new("ModalDialog");                                                
let ui_builder = UiBuilder::new().ui_stack_info(UiStackInfo::new(UiKind::Popup));       

let content_size = {                                                                    
    let sizing_builder = ui_builder.clone().sizing_pass()/*.invisible()*/;              
    modal.render_content(&mut Ui::new(ctx.clone(), layer_id, window_id, sizing_builder))
};                                                                                      

let popup_size = content_size + Vec2::splat(c.vh_to_px(DIALOG_BUTTON_HEIGHT_VH));

let popup_rect = Rect::from_pos(c.ctx.screen_rect().center())                           
    .expand2(popup_size / 2.0);                                                         

Ui::new(ctx.clone(), layer_id, window_id, ui_builder.max_rect(popup_rect))              
    .add(modal);

where render_content() looks like:

impl<'c, 'vuc> ModalPopup<'c, 'vuc> {                                                                 
    fn render_content(&self, ui: &mut Ui) -> Vec2 {
        ui.style_mut().spacing.item_spacing.y = self.get_margin_px();                                 

        ui.label(LayoutJob::simple(                                                                   
            self.modal.get_message().to_string(),                                                     
            egui::FontId::new(self.c.vh_to_px(DIALOG_TEXT_HEIGHT_VH), egui::FontFamily::Proportional),
            Color32::BLACK,                                                                           
            self.c.ctx.screen_rect().width() - 100.0                                                  
        ));                                                                                           

        ui.separator();                                                                               

        self.modal.update_controls(ui, self.c);                                                       

        ui.min_size()                                                                                 
    }                                                                                                 
}                                                                                                     

and ui() looks like:

impl<'c, 'vuc> Widget for ModalPopup<'c, 'vuc> {              
    fn ui(self, ui: &mut Ui) -> Response {                    
        let stroke_px = 1.0;                                  

        let shadow = Shadow {                                 
            offset: vec2(10.0, 20.0),                         
            blur:   20.0,                                     
            spread: 0.0,                                      
            color: Color32::from_black_alpha(96),             
        };                                                    

        let outer_margin = Margin  {                          
            left:   stroke_px,
            top:    stroke_px,                                
            right:  stroke_px + shadow.offset.x + shadow.blur,
            bottom: stroke_px + shadow.offset.y + shadow.blur,
        };                                                    

        let margin_px = self.get_margin_px();                 

        let frame = Frame {                                   
            inner_margin: Margin::same(margin_px),            
            outer_margin: outer_margin,
            fill:         Color32::WHITE,                     
            stroke:       Stroke::new(1.0, Color32::GRAY),    
            rounding:     Rounding::same(margin_px),          
            shadow:       shadow,                             
            ..Default::default()                              
        };                                                    

        frame.show(ui, |ui| self.render_content(ui)).response 
    }                                                         
}

and update_controls() looks like:

impl<'refs> Modal for SettingsModal<'refs> {
    fn get_message<'refs2>(&'refs2 self) -> &'refs2 str {                                                                        
        self.message
    }                                                                                                                            

    fn update_controls<'args, 'vuc>(&self, ui: &mut Ui, c: &'args ViewUpdateContext<'args, 'vuc>) {                              
        ui.allocate_new_ui(                                                                                                      
            UiBuilder::new()                                                                                                     
                .layout(egui::Layout::top_down(Align::Min)),                                                                     
            |ui| {                                                                                                               
                dialog_button(ui, c, "load_button", "Load", Some(SM::Event::LoadIconClicked));                                   
                dialog_button(ui, c, "load_button", "Load", Some(SM::Event::LoadIconClicked));                                   
                ui.allocate_new_ui(  // <==== This is the allocate_new_ui() in question
                    UiBuilder::new()                                                                                             
                        .layout(egui::Layout::left_to_right(Align::Center)),                                                      
                    |ui| {                                                                                                       
                        dialog_button(ui, c, "simulate_none_button",       "No Sim.",        Some(SM::Event::SimulateNone     ));
                        dialog_button(ui, c, "simulate_drive_plus_button", "Sim Drive Plus", Some(SM::Event::SimulateDrivePlus));
                        dialog_button(ui, c, "simulate_632_button",        "Sim 632",        Some(SM::Event::Simulate632      ));
                    }                                                                                                            
                );                                                                                                               
                dialog_button(ui, c, "cancel_button", "CANCEL", None);                                                           
            }                                                                                                                    
        );                                                                                                                       
    }                                                                                                                            
}

Expected behavior allocate_new_ui() does not add extra vertical spacing when !is_sizing_pass().

Desktop (please complete the following information):

• OS: Windows 11 Pro
• Browser: n/a
• Version: egui 1.29

[lucasmerlin]

Is there a reason you're no using a Area? You can use Area::anchor to anchor it on the center of the screen, then you don't need your extra sizing pass. (The area does this by remembering the sizes from the last frame, which is usually a better idea than rendering the ui twice).

For the extra space, just a wild guess, have you tried Align::Start instead of Align::Center? I think Align::Center takes up all available space in order to center items.

[barries]

Thanks! The reason is that there are no docs that I've found that guide learners down the recommended path, lol. Also, the persistence of size from frame to frame seems like it would come back to haunt me if the same dialog is shown again with smaller or larger content. I don't want to have a two-repaint flicker while it finds its size each time its displayed.

But, nonetheless, this seems like buggy behavior--sizing pass should not ever change how things are minimally sized, they need to be 100% identically sized when rendered again during the "for real this time" next pass. The second pass can have different positions, but if the size of text, child UIs, etc., change from pass to pass, it's going to manifest as layout defects sooner or later. For example, since I wrote this issue, I've also run into a text wrapping thing where, when in sizing pass, the text is not wrapped, then later in the display pass it is--leading to an incorrect window height.

I may try an area, but switched to using a Frame instead of a Window because the Window's stale cached frame size issue when redisplayed was still biting me. The issues with caching the size from frame to frame is that the app needs to have some way of invalidating them--cache invalidation being one of the truly hard problems, lol. Is there such a way?

I'd be happy with a "layout and sizing" phase every update, for example (it would be very fast, and the font and image rendering texture caches are a huge help in this). But, today, it seems like the sizing pass is implemented for a few use cases (menus, combo boxes, etc), and the other layout logic that's baked into the Ui monolith isn't really fully sizing-pass aware. But, I'm new and may be missing something crucial.

Thanks!

[lucasmerlin]

same dialog is shown again with smaller or larger content. I don't want to have a two-repaint flicker while it finds its size each time its displayed.

There now is Context::request_discard to prevent such flickers. (But I'm not sure if it's actually used by the area yet, it's quite new)

---

For the window size cache problem, there is an open issue: https://github.com/emilk/egui/issues/5138 For me this workaround is working (add it to the beginning of the area / window):

    let screen_size = ui.ctx().screen_rect().size();
    ui.set_max_size(screen_size);

it will allow your area to grow / shrink freely to fit the content but still properly centering the ui based on the Ui::min_rect.

---

But, nonetheless, this seems like buggy behavior--sizing pass should not ever change how things are minimally sized, they need to be 100% identically sized when rendered again during the "for real this time" next pass.

I think the code from your example is behaving as expected. You're using Align::Center, which means the ui will allocate the full available vertical height and then center the elements in there.

In your sizing pass the max rect of the ui was likely 0, meaning the your call to allocate_new_ui only took the minimal space needed to show the horizontal buttons. Then in the actual ui, where you set max_rect to the measured size, the allocate_new_ui will take all the remaining space and center the buttons in there, causing the extra white space.

So this is not related at all to the UiBuilder::sizing_pass flag. Using Align::Min or Ui::horizontal_top instead should fix this problem.

I hope this clarifies things a bit! It can take a bit to get used to egui's layout quirks 😄

[barries]

Thanks for the feedback and ideas. It still seems like a bug that a control takes a different size in "sizing pass" than when rendering, because it results in bugs like the one shown in the screencap. Maybe it's a design issue, but from an API design point of view, having a sizing pass that results in unreliable sizing seems like a bug. If "sizing pass" is meant to be a width-measurement pass, then changing the name would make that clear.

But, thanks for the workarounds, I switched to a different API shortly after posting this bug.

Thanks again, and I really appreciate your contributions, I see them all over the place, and they're always helpful and productive.