Skip to content

Commit

Permalink
Add Context::request_discard (emilk#5059)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
emilk authored and hacknus committed Oct 30, 2024
1 parent 0faa407 commit e2a4195
Show file tree
Hide file tree
Showing 36 changed files with 656 additions and 296 deletions.
7 changes: 3 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,7 @@ On Fedora Rawhide you need to run:
* Portable: the same code works on the web and as a native app
* Easy to integrate into any environment
* A simple 2D graphics API for custom painting ([`epaint`](https://docs.rs/epaint)).
* No callbacks
* Pure immediate mode
* Pure immediate mode: no callbacks
* Extensible: [easy to write your own widgets for egui](https://github.com/emilk/egui/blob/master/crates/egui_demo_lib/src/demo/toggle_switch.rs)
* Modular: You should be able to use small parts of egui and combine them in new ways
* Safe: there is no `unsafe` code in egui
Expand All @@ -113,7 +112,6 @@ egui is *not* a framework. egui is a library you call into, not an environment y

* Become the most powerful GUI library
* Native looking interface
* Advanced and flexible layouts (that's fundamentally incompatible with immediate mode)

## State

Expand Down Expand Up @@ -250,7 +248,8 @@ This is a fundamental shortcoming of immediate mode GUIs, and any attempt to res

One workaround is to store the size and use it the next frame. This produces a frame-delay for the correct layout, producing occasional flickering the first frame something shows up. `egui` does this for some things such as windows and grid layouts.

You can also call the layout code twice (once to get the size, once to do the interaction), but that is not only more expensive, it's also complex to implement, and in some cases twice is not enough. `egui` never does this.
The "first-frame jitter" can be covered up with an extra _pass_, which egui supports via `Context::request_discard`.
The downside of this is the added CPU cost of a second pass, so egui only does this in very rare circumstances (the majority of frames are single-pass).

For "atomic" widgets (e.g. a button) `egui` knows the size before showing it, so centering buttons, labels etc is possible in `egui` without any special workarounds.

Expand Down
14 changes: 6 additions & 8 deletions crates/eframe/src/native/glow_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,13 +251,13 @@ impl<'app> GlowWinitApp<'app> {
.set_request_repaint_callback(move |info| {
log::trace!("request_repaint_callback: {info:?}");
let when = Instant::now() + info.delay;
let frame_nr = info.current_frame_nr;
let cumulative_pass_nr = info.current_cumulative_pass_nr;
event_loop_proxy
.lock()
.send_event(UserEvent::RequestRepaint {
viewport_id: info.viewport_id,
when,
frame_nr,
cumulative_pass_nr,
})
.ok();
});
Expand Down Expand Up @@ -346,10 +346,8 @@ impl<'app> GlowWinitApp<'app> {
}

impl<'app> WinitApp for GlowWinitApp<'app> {
fn frame_nr(&self, viewport_id: ViewportId) -> u64 {
self.running
.as_ref()
.map_or(0, |r| r.integration.egui_ctx.frame_nr_for(viewport_id))
fn egui_ctx(&self) -> Option<&egui::Context> {
self.running.as_ref().map(|r| &r.integration.egui_ctx)
}

fn window(&self, window_id: WindowId) -> Option<Arc<Window>> {
Expand Down Expand Up @@ -712,7 +710,7 @@ impl<'app> GlowWinitRunning<'app> {

// give it time to settle:
#[cfg(feature = "__screenshot")]
if integration.egui_ctx.frame_nr() == 2 {
if integration.egui_ctx.cumulative_pass_nr() == 2 {
if let Ok(path) = std::env::var("EFRAME_SCREENSHOT_TO") {
save_screenshot_and_exit(&path, &painter, screen_size_in_pixels);
}
Expand Down Expand Up @@ -1397,7 +1395,7 @@ fn render_immediate_viewport(
let ImmediateViewport {
ids,
builder,
viewport_ui_cb,
mut viewport_ui_cb,
} = immediate_viewport;

let viewport_id = ids.this;
Expand Down
11 changes: 8 additions & 3 deletions crates/eframe/src/native/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,11 +228,16 @@ impl<T: WinitApp> ApplicationHandler<UserEvent> for WinitAppWrapper<T> {
let event_result = match event {
UserEvent::RequestRepaint {
when,
frame_nr,
cumulative_pass_nr,
viewport_id,
} => {
let current_frame_nr = self.winit_app.frame_nr(viewport_id);
if current_frame_nr == frame_nr || current_frame_nr == frame_nr + 1 {
let current_pass_nr = self
.winit_app
.egui_ctx()
.map_or(0, |ctx| ctx.cumulative_pass_nr_for(viewport_id));
if current_pass_nr == cumulative_pass_nr
|| current_pass_nr == cumulative_pass_nr + 1
{
log::trace!("UserEvent::RequestRepaint scheduling repaint at {when:?}");
if let Some(window_id) =
self.winit_app.window_id_from_viewport_id(viewport_id)
Expand Down
12 changes: 5 additions & 7 deletions crates/eframe/src/native/wgpu_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,13 +223,13 @@ impl<'app> WgpuWinitApp<'app> {
egui_ctx.set_request_repaint_callback(move |info| {
log::trace!("request_repaint_callback: {info:?}");
let when = Instant::now() + info.delay;
let frame_nr = info.current_frame_nr;
let cumulative_pass_nr = info.current_cumulative_pass_nr;

event_loop_proxy
.lock()
.send_event(UserEvent::RequestRepaint {
when,
frame_nr,
cumulative_pass_nr,
viewport_id: info.viewport_id,
})
.ok();
Expand Down Expand Up @@ -324,10 +324,8 @@ impl<'app> WgpuWinitApp<'app> {
}

impl<'app> WinitApp for WgpuWinitApp<'app> {
fn frame_nr(&self, viewport_id: ViewportId) -> u64 {
self.running
.as_ref()
.map_or(0, |r| r.integration.egui_ctx.frame_nr_for(viewport_id))
fn egui_ctx(&self) -> Option<&egui::Context> {
self.running.as_ref().map(|r| &r.integration.egui_ctx)
}

fn window(&self, window_id: WindowId) -> Option<Arc<Window>> {
Expand Down Expand Up @@ -916,7 +914,7 @@ fn render_immediate_viewport(
let ImmediateViewport {
ids,
builder,
viewport_ui_cb,
mut viewport_ui_cb,
} = immediate_viewport;

let input = {
Expand Down
12 changes: 8 additions & 4 deletions crates/eframe/src/native/winit_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ pub fn create_egui_context(storage: Option<&dyn crate::Storage>) -> egui::Contex

egui_ctx.set_embed_viewports(!IS_DESKTOP);

egui_ctx.options_mut(|o| {
// eframe supports multi-pass (Context::request_discard).
o.max_passes = 2.try_into().unwrap();
});

let memory = crate::native::epi_integration::load_egui_memory(storage).unwrap_or_default();
egui_ctx.memory_mut(|mem| *mem = memory);

Expand All @@ -42,8 +47,8 @@ pub enum UserEvent {
/// When to repaint.
when: Instant,

/// What the frame number was when the repaint was _requested_.
frame_nr: u64,
/// What the cumulative pass number was when the repaint was _requested_.
cumulative_pass_nr: u64,
},

/// A request related to [`accesskit`](https://accesskit.dev/).
Expand All @@ -59,8 +64,7 @@ impl From<accesskit_winit::Event> for UserEvent {
}

pub trait WinitApp {
/// The current frame number, as reported by egui.
fn frame_nr(&self, viewport_id: ViewportId) -> u64;
fn egui_ctx(&self) -> Option<&egui::Context>;

fn window(&self, window_id: WindowId) -> Option<Arc<Window>>;

Expand Down
2 changes: 2 additions & 0 deletions crates/eframe/src/web/app_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,8 @@ impl AppRunner {
ime,
#[cfg(feature = "accesskit")]
accesskit_update: _, // not currently implemented
num_completed_passes: _, // handled by `Context::run`
requested_discard: _, // handled by `Context::run`
} = platform_output;

super::set_cursor_icon(cursor_icon);
Expand Down
2 changes: 2 additions & 0 deletions crates/egui-winit/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,8 @@ impl State {
ime,
#[cfg(feature = "accesskit")]
accesskit_update,
num_completed_passes: _, // `egui::Context::run` handles this
requested_discard: _, // `egui::Context::run` handles this
} = platform_output;

self.set_cursor_icon(window, cursor_icon);
Expand Down
2 changes: 1 addition & 1 deletion crates/egui/src/containers/combo_box.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ pub type IconPainter = Box<dyn FnOnce(&Ui, Rect, &WidgetVisuals, bool, AboveOrBe
/// A drop-down selection menu with a descriptive label.
///
/// ```
/// # egui::__run_test_ui(|ui| {
/// # #[derive(Debug, PartialEq)]
/// # enum Enum { First, Second, Third }
/// # let mut selected = Enum::First;
/// # egui::__run_test_ui(|ui| {
/// egui::ComboBox::from_label("Select one!")
/// .selected_text(format!("{:?}", selected))
/// .show_ui(ui, |ui| {
Expand Down
10 changes: 5 additions & 5 deletions crates/egui/src/containers/panel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,10 +390,10 @@ impl SidePanel {
let rect = inner_response.response.rect;

match side {
Side::Left => ctx.frame_state_mut(|state| {
Side::Left => ctx.pass_state_mut(|state| {
state.allocate_left_panel(Rect::from_min_max(available_rect.min, rect.max));
}),
Side::Right => ctx.frame_state_mut(|state| {
Side::Right => ctx.pass_state_mut(|state| {
state.allocate_right_panel(Rect::from_min_max(rect.min, available_rect.max));
}),
}
Expand Down Expand Up @@ -885,12 +885,12 @@ impl TopBottomPanel {

match side {
TopBottomSide::Top => {
ctx.frame_state_mut(|state| {
ctx.pass_state_mut(|state| {
state.allocate_top_panel(Rect::from_min_max(available_rect.min, rect.max));
});
}
TopBottomSide::Bottom => {
ctx.frame_state_mut(|state| {
ctx.pass_state_mut(|state| {
state.allocate_bottom_panel(Rect::from_min_max(rect.min, available_rect.max));
});
}
Expand Down Expand Up @@ -1149,7 +1149,7 @@ impl CentralPanel {
let inner_response = self.show_inside_dyn(&mut panel_ui, add_contents);

// Only inform ctx about what we actually used, so we can shrink the native window to fit.
ctx.frame_state_mut(|state| state.allocate_central_panel(inner_response.response.rect));
ctx.pass_state_mut(|state| state.allocate_central_panel(inner_response.response.rect));

inner_response
}
Expand Down
12 changes: 6 additions & 6 deletions crates/egui/src/containers/popup.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
//! Show popup windows, tooltips, context menus etc.
use frame_state::PerWidgetTooltipState;
use pass_state::PerWidgetTooltipState;

use crate::{
frame_state, vec2, AboveOrBelow, Align, Align2, Area, AreaState, Context, Frame, Id,
pass_state, vec2, AboveOrBelow, Align, Align2, Area, AreaState, Context, Frame, Id,
InnerResponse, Key, LayerId, Layout, Order, Pos2, Rect, Response, Sense, Ui, UiKind, Vec2,
Widget, WidgetText,
};
Expand Down Expand Up @@ -162,7 +162,7 @@ fn show_tooltip_at_dyn<'c, R>(

remember_that_tooltip_was_shown(ctx);

let mut state = ctx.frame_state_mut(|fs| {
let mut state = ctx.pass_state_mut(|fs| {
// Remember that this is the widget showing the tooltip:
fs.layers
.entry(parent_layer)
Expand Down Expand Up @@ -213,14 +213,14 @@ fn show_tooltip_at_dyn<'c, R>(

state.tooltip_count += 1;
state.bounding_rect = state.bounding_rect.union(response.rect);
ctx.frame_state_mut(|fs| fs.tooltips.widget_tooltips.insert(widget_id, state));
ctx.pass_state_mut(|fs| fs.tooltips.widget_tooltips.insert(widget_id, state));

inner
}

/// What is the id of the next tooltip for this widget?
pub fn next_tooltip_id(ctx: &Context, widget_id: Id) -> Id {
let tooltip_count = ctx.frame_state(|fs| {
let tooltip_count = ctx.pass_state(|fs| {
fs.tooltips
.widget_tooltips
.get(&widget_id)
Expand Down Expand Up @@ -409,7 +409,7 @@ pub fn popup_above_or_below_widget<R>(
let frame_margin = frame.total_margin();
let inner_width = widget_response.rect.width() - frame_margin.sum().x;

parent_ui.ctx().frame_state_mut(|fs| {
parent_ui.ctx().pass_state_mut(|fs| {
fs.layers
.entry(parent_ui.layer_id())
.or_default()
Expand Down
10 changes: 5 additions & 5 deletions crates/egui/src/containers/scroll_area.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#![allow(clippy::needless_range_loop)]

use crate::{
emath, epaint, frame_state, lerp, pos2, remap, remap_clamp, vec2, Context, Id, NumExt, Pos2,
emath, epaint, lerp, pass_state, pos2, remap, remap_clamp, vec2, Context, Id, NumExt, Pos2,
Rangef, Rect, Sense, Ui, UiBuilder, UiKind, UiStackInfo, Vec2, Vec2b,
};

Expand Down Expand Up @@ -819,22 +819,22 @@ impl Prepared {

let scroll_delta = content_ui
.ctx()
.frame_state_mut(|state| std::mem::take(&mut state.scroll_delta));
.pass_state_mut(|state| std::mem::take(&mut state.scroll_delta));

for d in 0..2 {
// FrameState::scroll_delta is inverted from the way we apply the delta, so we need to negate it.
// PassState::scroll_delta is inverted from the way we apply the delta, so we need to negate it.
let mut delta = -scroll_delta.0[d];
let mut animation = scroll_delta.1;

// We always take both scroll targets regardless of which scroll axes are enabled. This
// is to avoid them leaking to other scroll areas.
let scroll_target = content_ui
.ctx()
.frame_state_mut(|state| state.scroll_target[d].take());
.pass_state_mut(|state| state.scroll_target[d].take());

if scroll_enabled[d] {
if let Some(target) = scroll_target {
let frame_state::ScrollTarget {
let pass_state::ScrollTarget {
range,
align,
animation: animation_update,
Expand Down
Loading

0 comments on commit e2a4195

Please sign in to comment.