Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improving layouting #4378

Open
emilk opened this issue Apr 18, 2024 · 9 comments
Open

Improving layouting #4378

emilk opened this issue Apr 18, 2024 · 9 comments
Labels
layout Related to widget layout rerun Desired for Rerun.io Tracking issue

Comments

@emilk
Copy link
Owner

emilk commented Apr 18, 2024

Understanding the problem

One of the main drawback of single-pass immediate mode GUIs is layouting. There is a cyclical dependency between position, size, and layout. This can be illustrated explained by an example:

Say you want to show a small dialog window in the center of the screen. To position the window we must first know the size of it. To know the size of it we must first layout the contents of the window. In immediate mode, the layout code also checks for interaction ("was the OK button clicked?"), thus we need to know where to place the OK button before doing the layout. So we need to know where to put the window before we can layout the contents of the window, but only after laying out the contents of the window can we know the size of the window, and thus where to place it so that it is centered on the screen.

The crux of the issue is really knowing the size of a thing before you can lay it out properly. So, how can we know the size of a thing before calling the layout code for it?

There are simple atomic widgets like egui::Button that calculates its size based on the text it contains and the border. This is where egui "bottoms out". These atomic widgets can be properly centered, because they only result in a single call to ui.allocate*.

Whenever you have a group of widgets though, you have a problem.

Figuring out sizes before layout

So how can we know the size of a thing before doing the layout?

There are a few different ways:

A) Using fixed size elements (e.g. decide that the dialog window is always exactly 300x200 no matter what it contains)
B) Remember sizes of elements from previous frames (first frame calculate sizes, all other frames: position things based on those sizes). egui uses this strategy for some stuff (like Window, Grid, Table, …)
C) An API for calculating the size of an element before adding it (#606 - has potential for exponential slowdowns)
D) Multi-pass immediate mode (rejected)

I think B) is the most interesting one to dig deeper into.

Whenever the B) strategy is used, one should be careful not to show the widget during the "layout" frame. For instance, a centered egui::Window is invisible for the first frame.

The B) strategy also fails if the thing changes size over time. Though it is self correcting, it has a frame-delay. So in the original example: if the dialog windows grows it will shift to the right for one frame, then re-center then next frame. This will look ugly.

Improving layout given sizes

Once you have the size you should be able to apply any advanced layouting technique. For instance using:

Here we have a lot of opportunity for improvement in egui. As a start, we should at least write good examples for all of these.

@emilk emilk added layout Related to widget layout rerun Desired for Rerun.io Tracking issue labels Apr 18, 2024
@YgorSouza
Copy link
Contributor

I think some issues like #1996 #2786 #2798 #3054 #4159 are related to this.

@MeGaGiGaGon
Copy link
Contributor

MeGaGiGaGon commented Apr 27, 2024

I recently also ran into this while trying to fix #3074. One other possible way, at least where it's an issue of translations, is to fake everything being fine by fixing it afterwards. This should work when it's happening while the mouse is already occupied doing, like dragging windows around, since there wouldn't be any way to notice the response being 1 frame delayed. Unfortunately it falls apart for anything more complex, or when it is possible to do two things at once like with touch controls, or when there are massive differences between the estimated and actual positions.

That does beg the question, how is correct interactivity vs visuals weighed? I think more people will notice when visuals are delayed by a frame, while noticing a lagging response is harder due to them being invisible. Is that an acceptable tradeoff, and what other methods like translations and hiding windows for an initial frame are available when it is?

Edit 2: Leaving some of this here but removing a lot of my rambling. The more I think on this, the more it's turning into a worse version of multi-pass immediate mode, so probably the wrong direction if not applied carefully.

@AlexanderSchuetz97
Copy link

Perhaps a mix of immedieate and pre layouted mode would be a good idea? Perhaps implement a widget that acts as a container for pre layouted widgets. Those widgets still get drawn on the screen every frame, but cannot change their bounding box during a frame. These pre layouted widgets could request a resize to be performed before the next redraw. You could still do things like change color on hover or highlight a button this way during draw.

For those pre layouted containers you could implement some more advanced layouts aswell as provide a trait users could implement to provide their own layouting.

A big con is however these pre layouted components would probably require a completely different api. Sizing information would probably need to be made available in a dedicated method. Id especially pay attention to text sizes as those are a bitch in every other framework that does pre layouting. The draw method would need to be told its bounding box and perhaps steps should even be taken to prevent drawing outside of the bounding box.

Adding/Useing immedeate widgets inside of pre layouted components/Containers should be possible without much issue if you assign a bounding box with a size known to the layouter to them.

I think many people will fine pre layouting for some use cases more intuitive than others. Allowing users to explicitly combine them with immedieate widgets would probably be ideal, at least in my opinion.

StanleyCHale added a commit to StanleyCHale/Capstone-Vehicle-Sim-Project-Team3 that referenced this issue May 22, 2024
Sadly we can't center sliders atm with egui.

References:
emilk/egui#1144
emilk/egui#4378
@emilk
Copy link
Owner Author

emilk commented Jun 6, 2024

I experimented a bit with a new Group container which uses the new sizing pass to calculate the size the first frame, and then store it for later. This lets you put a group of widgets in e.g. a centered layout

@emilk emilk pinned this issue Jun 6, 2024
@emilk emilk unpinned this issue Jun 25, 2024
@DanielHeath
Copy link

Item B is a very close match for how React useRef hooks work, and the problems solved by 'react hooks' in general are a close match for various issues found in immediate mode UI rendering.

@lucasmerlin
Copy link
Collaborator

lucasmerlin commented Sep 6, 2024

I've been experimenting with improving layout in egui for a while, some time back I made egui_taffy which works pretty well but it didn't feel nice to use with egui, since it had to borrow the ui closures for until the taffy pass was finished which caused some lifetime issues, so I've never finished or published it.

introducing egui_flex

I've been experimenting how much of flexbox I could implement in egui by just remembering the widget sizes from the previous frame and the results are quite impressive! I've published them here: egui_flex

EDITED: Check the egui_flex readme for limitations

Real world example

Finally I wanted to share a real world example how flex can improve the ui by a lot. In the hello_egui demo app I have a list of crates displayed as small tags in the sidebar. When just shown with ui.horizontal_wrapped it looks really weird:

image

When updated to use egui_flex it looks much nicer:

image

egui or separate crate?

I plan on publishing this as a separate crate but I think it could also make sense to add this to egui similar to the Grid layout?
Maybe it could even be the default layout in the future? I think it should be pretty compatible with egui's current horizontal / vertical / with_layout layouting while solving most problems it currently has. @emilk what are your thoughts on this?

@emilk
Copy link
Owner Author

emilk commented Sep 10, 2024

egui_flex looks very promising! 😍

Storing the sizes from previous frame should work great, but has the problem that the first frame will (usually) be wrong.
This should be solvable once #5059 is done (and I plan to get that in this week).

I suggest we start out with egui_flex as a separate crate first until I have more time to think about the whole layout story in egui.

emilk added a commit that referenced this issue Sep 13, 2024
* Closes #4976
* Part of #4378 
* Implements parts of #843

### Background
Some widgets (like `Grid` and `Table`) needs to know the width of future
elements in order to properly size themselves. For instance, the width
of the first column of a grid may not be known until all rows of the
grid has been added, at which point it is too late. Therefore these
widgets store sizes from the previous frame. This leads to "first-frame
jitter", were the content is placed in the wrong place for one frame,
before being accurately laid out in subsequent frames.

### What
This PR adds the function `ctx.request_discard` which discards the
visual output and does another _pass_, i.e. calls the whole app UI code
once again (in eframe this means calling `App::update` again). This will
thus discard the shapes produced by the wrongly placed widgets, and
replace it with new shapes. Note that only the visual output is
discarded - all other output events are accumulated.

Calling `ctx.request_discard` should only be done in very rare
circumstances, e.g. when a `Grid` is first shown. Calling it every frame
will mean the UI code will become unnecessarily slow.

Two safe-guards are in place:

* `Options::max_passes` is by default 2, meaning egui will never do more
than 2 passes even if `request_discard` is called on every pass
* If multiple passes is done for multiple frames in a row, a warning
will be printed on the screen in debug builds:


![image](https://github.com/user-attachments/assets/c2c1e4a4-b7c9-4d7a-b3ad-abdd74bf449f)

### Breaking changes
A bunch of things that had "frame" in the name now has "pass" in them
instead:

* Functions called `begin_frame` and `end_frame` are now called
`begin_pass` and `end_pass`
* `FrameState` is now `PassState`
* etc


### TODO
* [x] Figure out good names for everything (`ctx.request_discard`)
* [x] Add API to query if we're gonna repeat this frame (to early-out
from expensive rendering)
* [x] Clear up naming confusion (pass vs frame) e.g. for `FrameState`
* [x] Figure out when to call this
* [x] Show warning on screen when there are several frames in a row with
multiple passes
* [x] Document
* [x] Default on or off?
* [x] Change `Context::frame_nr` name/docs
* [x] Rename `Context::begin_frame/end_frame` and deprecate the old ones
* [x] Test with Rerun
* [x] Document breaking changes
emilk added a commit that referenced this issue Sep 19, 2024
…rates (#5082)

This adds a `intrinsic_size` field to the Response struct which allows
me to grow a egui button frame while still being able to know it's
intrinsic size in
[egui_flex](https://github.com/lucasmerlin/hello_egui/tree/main/crates/egui_flex)

* Related to
#4378 (comment)
* [X] I have followed the instructions in the PR template

---------

Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
@lucasmerlin
Copy link
Collaborator

I just released egui_flex v0.1.0!

Here is a simple example of how you use it:

Flex::horizontal().show(ui, |flex| {
    flex.add(item().grow(1.0), Button::new("Growing button"));
    flex.add(item(), Button::new("Non-growing button"));

    // Nested flex
    flex.add_flex(
        item().grow(1.0),
        Flex::vertical().align_content(FlexAlignContent::Stretch),
        |flex| {
            flex.add(item(), Button::new("Vertical button"));
            flex.add(item(), Button::new("Another Vertical button"));
        },
    );
});

I also added a small demo to the hello_egui demo app.

hacknus pushed a commit to hacknus/egui that referenced this issue Oct 30, 2024
* Closes emilk#4976
* Part of emilk#4378 
* Implements parts of emilk#843

### Background
Some widgets (like `Grid` and `Table`) needs to know the width of future
elements in order to properly size themselves. For instance, the width
of the first column of a grid may not be known until all rows of the
grid has been added, at which point it is too late. Therefore these
widgets store sizes from the previous frame. This leads to "first-frame
jitter", were the content is placed in the wrong place for one frame,
before being accurately laid out in subsequent frames.

### What
This PR adds the function `ctx.request_discard` which discards the
visual output and does another _pass_, i.e. calls the whole app UI code
once again (in eframe this means calling `App::update` again). This will
thus discard the shapes produced by the wrongly placed widgets, and
replace it with new shapes. Note that only the visual output is
discarded - all other output events are accumulated.

Calling `ctx.request_discard` should only be done in very rare
circumstances, e.g. when a `Grid` is first shown. Calling it every frame
will mean the UI code will become unnecessarily slow.

Two safe-guards are in place:

* `Options::max_passes` is by default 2, meaning egui will never do more
than 2 passes even if `request_discard` is called on every pass
* If multiple passes is done for multiple frames in a row, a warning
will be printed on the screen in debug builds:


![image](https://github.com/user-attachments/assets/c2c1e4a4-b7c9-4d7a-b3ad-abdd74bf449f)

### Breaking changes
A bunch of things that had "frame" in the name now has "pass" in them
instead:

* Functions called `begin_frame` and `end_frame` are now called
`begin_pass` and `end_pass`
* `FrameState` is now `PassState`
* etc


### TODO
* [x] Figure out good names for everything (`ctx.request_discard`)
* [x] Add API to query if we're gonna repeat this frame (to early-out
from expensive rendering)
* [x] Clear up naming confusion (pass vs frame) e.g. for `FrameState`
* [x] Figure out when to call this
* [x] Show warning on screen when there are several frames in a row with
multiple passes
* [x] Document
* [x] Default on or off?
* [x] Change `Context::frame_nr` name/docs
* [x] Rename `Context::begin_frame/end_frame` and deprecate the old ones
* [x] Test with Rerun
* [x] Document breaking changes
hacknus pushed a commit to hacknus/egui that referenced this issue Oct 30, 2024
…rates (emilk#5082)

This adds a `intrinsic_size` field to the Response struct which allows
me to grow a egui button frame while still being able to know it's
intrinsic size in
[egui_flex](https://github.com/lucasmerlin/hello_egui/tree/main/crates/egui_flex)

* Related to
emilk#4378 (comment)
* [X] I have followed the instructions in the PR template

---------

Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
@PPakalns
Copy link
Contributor

PPakalns commented Nov 10, 2024

Wanted to provide some feedback about possible approach to improve layouting.

Played around with integrating taffy into egui using the same approach as in @lucasmerlin egui_flex , which uses egui 0.29 new features (request_discard and intrinsic_size), but layouting logic instead is calculated using taffy (inspired by egui_taffy , but fixed the problem with closures that @lucasmerlin mentioned).

https://gist.github.com/PPakalns/337d0539f7b01bd8216c627c0f65a30c

Seems to work really nice. I can integrate with egui widgets or separate child ui using the same approach as in egui_flex. I am even starting to add scroll area support.

This approach feels more usable as egui_flex supports only couple of layouting options and we would not need to reimplement full layouting logic, but taffy could be used for that instead.

Thanks @lucasmerlin for previous exploration in this area with libraries (egui_taffy, egui_flex).

Example:
Top menu bar, game overlay panels, game overlay left panel contents are layed out using this approach. Buttons can grow, shrink, scroll areas work nicely.
image

Notes:
Looking into adding support to 9 slice backgrounds for taffy elements, probably will even add TaffyWidget trait where widgets are made from taffy containers and egui widgets are located in leaf nodes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
layout Related to widget layout rerun Desired for Rerun.io Tracking issue
Projects
None yet
Development

No branches or pull requests

7 participants