Skip to content

Commit

Permalink
Fix stable_dt
Browse files Browse the repository at this point in the history
This broke in #3172

Since 0.24.1, `stable_dt` has been fixed at 1/60s, which is a little
bit _too_ stable.

The code issue was the logic for asking "Is this the result of an
immeditate repaint?" was completely broken (always returning false).

* Closes #3830
  • Loading branch information
emilk committed Jan 17, 2024
1 parent 9e924ec commit 8e94a28
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 32 deletions.
81 changes: 51 additions & 30 deletions crates/egui/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,28 @@ impl Default for WrappedTextureManager {

/// Repaint-logic
impl ContextImpl {
/// This is where we update the repaint logic.
fn begin_frame_repaint_logic(&mut self, viewport_id: ViewportId) {
let viewport = self.viewports.entry(viewport_id).or_default();

viewport.repaint.prev_frame_paint_delay = viewport.repaint.repaint_delay;

if viewport.repaint.outstanding == 0 {
// We are repainting now, so we can wait a while for the next repaint.
viewport.repaint.repaint_delay = Duration::MAX;
} else {
viewport.repaint.repaint_delay = Duration::ZERO;
viewport.repaint.outstanding -= 1;
if let Some(callback) = &self.request_repaint_callback {
(callback)(RequestRepaintInfo {
viewport_id,
delay: Duration::ZERO,
current_frame_nr: viewport.repaint.frame_nr,
});
}
}
}

fn request_repaint(&mut self, viewport_id: ViewportId) {
self.request_repaint_after(Duration::ZERO, viewport_id);
}
Expand All @@ -79,13 +101,13 @@ impl ContextImpl {
// This solves some corner-cases of missing repaints on frame-delayed responses.
viewport.repaint.outstanding = 1;

if let Some(callback) = &self.request_repaint_callback {
// We save some CPU time by only calling the callback if we need to.
// If the new delay is greater or equal to the previous lowest,
// it means we have already called the callback, and don't need to do it again.
if delay < viewport.repaint.repaint_delay {
viewport.repaint.repaint_delay = delay;
// We save some CPU time by only calling the callback if we need to.
// If the new delay is greater or equal to the previous lowest,
// it means we have already called the callback, and don't need to do it again.
if delay < viewport.repaint.repaint_delay {
viewport.repaint.repaint_delay = delay;

if let Some(callback) = &self.request_repaint_callback {
(callback)(RequestRepaintInfo {
viewport_id,
delay,
Expand All @@ -96,10 +118,10 @@ impl ContextImpl {
}

#[must_use]
fn requested_repaint_last_frame(&self, viewport_id: &ViewportId) -> bool {
self.viewports
.get(viewport_id)
.map_or(false, |v| v.repaint.requested_last_frame)
fn requested_immediated_repaint_previous_frame(&self, viewport_id: &ViewportId) -> bool {

Check warning on line 121 in crates/egui/src/context.rs

View workflow job for this annotation

GitHub Actions / typos

"immediated" should be "immediate" or "immediately".
self.viewports.get(viewport_id).map_or(false, |v| {
v.repaint.requested_immediated_repaint_previous_frame()

Check warning on line 123 in crates/egui/src/context.rs

View workflow job for this annotation

GitHub Actions / typos

"immediated" should be "immediate" or "immediately".
})
}

#[must_use]
Expand Down Expand Up @@ -178,8 +200,11 @@ struct ViewportRepaintInfo {
/// While positive, keep requesting repaints. Decrement at the start of each frame.
outstanding: u8,

/// Did we?
requested_last_frame: bool,
/// What was the output of `repaint_delay` on the previous frame?
///
/// If this was zero, we are repaining as quickly as possible
/// (as far as we know).
prev_frame_paint_delay: Duration,
}

impl Default for ViewportRepaintInfo {
Expand All @@ -193,11 +218,17 @@ impl Default for ViewportRepaintInfo {
// Let's run a couple of frames at the start, because why not.
outstanding: 1,

requested_last_frame: false,
prev_frame_paint_delay: Duration::MAX,
}
}
}

impl ViewportRepaintInfo {
pub fn requested_immediated_repaint_previous_frame(&self) -> bool {

Check warning on line 227 in crates/egui/src/context.rs

View workflow job for this annotation

GitHub Actions / typos

"immediated" should be "immediate" or "immediately".
self.prev_frame_paint_delay == Duration::ZERO
}
}

// ----------------------------------------------------------------------------

#[derive(Default)]
Expand Down Expand Up @@ -261,22 +292,10 @@ impl ContextImpl {

let is_outermost_viewport = self.viewport_stack.is_empty(); // not necessarily root, just outermost immediate viewport
self.viewport_stack.push(ids);
let viewport = self.viewports.entry(viewport_id).or_default();

if viewport.repaint.outstanding == 0 {
// We are repainting now, so we can wait a while for the next repaint.
viewport.repaint.repaint_delay = Duration::MAX;
} else {
viewport.repaint.repaint_delay = Duration::ZERO;
viewport.repaint.outstanding -= 1;
if let Some(callback) = &self.request_repaint_callback {
(callback)(RequestRepaintInfo {
viewport_id,
delay: Duration::ZERO,
current_frame_nr: viewport.repaint.frame_nr,
});
}
}
self.begin_frame_repaint_logic(viewport_id);

let viewport = self.viewports.entry(viewport_id).or_default();

if is_outermost_viewport {
if let Some(new_zoom_factor) = self.new_zoom_factor.take() {
Expand Down Expand Up @@ -310,7 +329,9 @@ impl ContextImpl {

viewport.input = std::mem::take(&mut viewport.input).begin_frame(
new_raw_input,
viewport.repaint.requested_last_frame,
viewport
.repaint
.requested_immediated_repaint_previous_frame(),

Check warning on line 334 in crates/egui/src/context.rs

View workflow job for this annotation

GitHub Actions / typos

"immediated" should be "immediate" or "immediately".
pixels_per_point,
);

Expand Down Expand Up @@ -1312,7 +1333,7 @@ impl Context {
/// Was a repaint requested last frame for the given viewport?
#[must_use]
pub fn requested_repaint_last_frame_for(&self, viewport_id: &ViewportId) -> bool {
self.read(|ctx| ctx.requested_repaint_last_frame(viewport_id))
self.read(|ctx| ctx.requested_immediated_repaint_previous_frame(viewport_id))

Check warning on line 1336 in crates/egui/src/context.rs

View workflow job for this annotation

GitHub Actions / typos

"immediated" should be "immediate" or "immediately".
}

/// Has a repaint been requested for the current viewport?
Expand Down
4 changes: 2 additions & 2 deletions crates/egui/src/input_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,15 +147,15 @@ impl InputState {
pub fn begin_frame(
mut self,
mut new: RawInput,
requested_repaint_last_frame: bool,
requested_immediated_repaint_previous_frame: bool,

Check warning on line 150 in crates/egui/src/input_state.rs

View workflow job for this annotation

GitHub Actions / typos

"immediated" should be "immediate" or "immediately".
pixels_per_point: f32,
) -> Self {
crate::profile_function!();

let time = new.time.unwrap_or(self.time + new.predicted_dt as f64);
let unstable_dt = (time - self.time) as f32;

let stable_dt = if requested_repaint_last_frame {
let stable_dt = if requested_immediated_repaint_previous_frame {

Check warning on line 158 in crates/egui/src/input_state.rs

View workflow job for this annotation

GitHub Actions / typos

"immediated" should be "immediate" or "immediately".
// we should have had a repaint straight away,
// so this should be trustable.
unstable_dt
Expand Down

0 comments on commit 8e94a28

Please sign in to comment.