From 8e94a28f68188497492a6e79aaaf4f9e70add9d3 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 17 Jan 2024 11:45:51 +0100 Subject: [PATCH 1/2] Fix `stable_dt` This broke in https://github.com/emilk/egui/pull/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 https://github.com/emilk/egui/issues/3830 --- crates/egui/src/context.rs | 81 +++++++++++++++++++++------------- crates/egui/src/input_state.rs | 4 +- 2 files changed, 53 insertions(+), 32 deletions(-) diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index 939198058be..4c03891978b 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -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); } @@ -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, @@ -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 { + self.viewports.get(viewport_id).map_or(false, |v| { + v.repaint.requested_immediated_repaint_previous_frame() + }) } #[must_use] @@ -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 { @@ -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 { + self.prev_frame_paint_delay == Duration::ZERO + } +} + // ---------------------------------------------------------------------------- #[derive(Default)] @@ -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() { @@ -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(), pixels_per_point, ); @@ -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)) } /// Has a repaint been requested for the current viewport? diff --git a/crates/egui/src/input_state.rs b/crates/egui/src/input_state.rs index d97bc88ef24..7625b8b3ce7 100644 --- a/crates/egui/src/input_state.rs +++ b/crates/egui/src/input_state.rs @@ -147,7 +147,7 @@ impl InputState { pub fn begin_frame( mut self, mut new: RawInput, - requested_repaint_last_frame: bool, + requested_immediated_repaint_previous_frame: bool, pixels_per_point: f32, ) -> Self { crate::profile_function!(); @@ -155,7 +155,7 @@ impl InputState { 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 { // we should have had a repaint straight away, // so this should be trustable. unstable_dt From 693f02608ebca6721f279bdedb87edb1477036fc Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 17 Jan 2024 11:47:45 +0100 Subject: [PATCH 2/2] typo --- crates/egui/src/context.rs | 12 +++++------- crates/egui/src/input_state.rs | 4 ++-- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index 4c03891978b..f2f9614200c 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -118,9 +118,9 @@ impl ContextImpl { } #[must_use] - fn requested_immediated_repaint_previous_frame(&self, viewport_id: &ViewportId) -> bool { + fn requested_immediate_repaint_prev_frame(&self, viewport_id: &ViewportId) -> bool { self.viewports.get(viewport_id).map_or(false, |v| { - v.repaint.requested_immediated_repaint_previous_frame() + v.repaint.requested_immediate_repaint_prev_frame() }) } @@ -224,7 +224,7 @@ impl Default for ViewportRepaintInfo { } impl ViewportRepaintInfo { - pub fn requested_immediated_repaint_previous_frame(&self) -> bool { + pub fn requested_immediate_repaint_prev_frame(&self) -> bool { self.prev_frame_paint_delay == Duration::ZERO } } @@ -329,9 +329,7 @@ impl ContextImpl { viewport.input = std::mem::take(&mut viewport.input).begin_frame( new_raw_input, - viewport - .repaint - .requested_immediated_repaint_previous_frame(), + viewport.repaint.requested_immediate_repaint_prev_frame(), pixels_per_point, ); @@ -1333,7 +1331,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_immediated_repaint_previous_frame(viewport_id)) + self.read(|ctx| ctx.requested_immediate_repaint_prev_frame(viewport_id)) } /// Has a repaint been requested for the current viewport? diff --git a/crates/egui/src/input_state.rs b/crates/egui/src/input_state.rs index 7625b8b3ce7..260d6354b47 100644 --- a/crates/egui/src/input_state.rs +++ b/crates/egui/src/input_state.rs @@ -147,7 +147,7 @@ impl InputState { pub fn begin_frame( mut self, mut new: RawInput, - requested_immediated_repaint_previous_frame: bool, + requested_immediate_repaint_prev_frame: bool, pixels_per_point: f32, ) -> Self { crate::profile_function!(); @@ -155,7 +155,7 @@ impl InputState { 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_immediated_repaint_previous_frame { + let stable_dt = if requested_immediate_repaint_prev_frame { // we should have had a repaint straight away, // so this should be trustable. unstable_dt