Skip to content

Commit

Permalink
Fix: request_repaint_after works even when called from background t…
Browse files Browse the repository at this point in the history 
…hread (emilk#2939)

* Refactor repaint logic

* request_repaint_after also fires the request_repaint callback

* Bug fixes

* Add test to egui_demo_app

* build_demo_web: build debug unless --release is specified

* Fix the web backend too

* Run special clippy for wasm, forbidding some types/methods

* Remove wasm_bindgen_check.sh

* Fix typos

* Revert "Remove wasm_bindgen_check.sh"

This reverts commit 92dde25.

* Only run cranky/clippy once
  • Loading branch information
emilk authored Apr 20, 2023
1 parent d46cf06 commit 834e2e9
Show file tree
Hide file tree
Showing 14 changed files with 370 additions and 188 deletions.
5 changes: 1 addition & 4 deletions .github/workflows/rust.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,7 @@ jobs:
- run: ./scripts/wasm_bindgen_check.sh --skip-setup

- name: Cranky wasm32
uses: actions-rs/cargo@v1
with:
command: cranky
args: --target wasm32-unknown-unknown --all-features -p egui_demo_app --lib -- -D warnings
run: ./scripts/clippy_wasm.sh

# ---------------------------------------------------------------------------

Expand Down
2 changes: 2 additions & 0 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 10 additions & 1 deletion clippy.toml
View file
Original file line number Diff line number Diff line change
@@ -1 +1,10 @@
doc-valid-idents = ["AccessKit", ".."]
# There is also a scripts/clippy_wasm/clippy.toml which forbids some mthods that are not available in wasm.

msrv = "1.65"

# Allow-list of words for markdown in dosctrings https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown
doc-valid-idents = [
# You must also update the same list in the root `clippy.toml`!
"AccessKit",
"..",
]
182 changes: 112 additions & 70 deletions crates/eframe/src/native/run.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Note that this file contains two similar paths - one for [`glow`], one for [`wgpu`].
//! When making changes to one you often also want to apply it to the other.
use std::time::{Duration, Instant};
use std::time::Instant;

use winit::event_loop::{
ControlFlow, EventLoop, EventLoopBuilder, EventLoopProxy, EventLoopWindowTarget,
Expand All @@ -19,7 +19,12 @@ use super::epi_integration::{self, EpiIntegration};

#[derive(Debug)]
pub enum UserEvent {
RequestRepaint,
RequestRepaint {
when: Instant,
/// What the frame number was when the repaint was _requested_.
frame_nr: u64,
},

#[cfg(feature = "accesskit")]
AccessKitActionRequest(accesskit_winit::ActionRequestEvent),
}
Expand Down Expand Up @@ -58,6 +63,9 @@ enum EventResult {
}

trait WinitApp {
/// The current frame number, as reported by egui.
fn frame_nr(&self) -> u64;

fn is_focused(&self) -> bool;

fn integration(&self) -> Option<&EpiIntegration>;
Expand All @@ -66,7 +74,7 @@ trait WinitApp {

fn save_and_destroy(&mut self);

fn paint(&mut self) -> EventResult;
fn run_ui_and_paint(&mut self) -> EventResult;

fn on_event(
&mut self,
Expand Down Expand Up @@ -136,18 +144,30 @@ fn run_and_return(
// See: https://github.com/rust-windowing/winit/issues/987
// See: https://github.com/rust-windowing/winit/issues/1619
winit::event::Event::RedrawEventsCleared if cfg!(windows) => {
next_repaint_time = Instant::now() + Duration::from_secs(1_000_000_000);
winit_app.paint()
next_repaint_time = extremely_far_future();
winit_app.run_ui_and_paint()
}
winit::event::Event::RedrawRequested(_) if !cfg!(windows) => {
next_repaint_time = Instant::now() + Duration::from_secs(1_000_000_000);
winit_app.paint()
next_repaint_time = extremely_far_future();
winit_app.run_ui_and_paint()
}

winit::event::Event::UserEvent(UserEvent::RequestRepaint { when, frame_nr }) => {
if winit_app.frame_nr() == *frame_nr {
log::trace!("UserEvent::RequestRepaint scheduling repaint at {when:?}");
EventResult::RepaintAt(*when)
} else {
log::trace!("Got outdated UserEvent::RequestRepaint");
EventResult::Wait // old request - we've already repainted
}
}

winit::event::Event::UserEvent(UserEvent::RequestRepaint)
| winit::event::Event::NewEvents(winit::event::StartCause::ResumeTimeReached {
winit::event::Event::NewEvents(winit::event::StartCause::ResumeTimeReached {
..
}) => EventResult::RepaintNext,
}) => {
log::trace!("Woke up to check next_repaint_time");
EventResult::Wait
}

winit::event::Event::WindowEvent { window_id, .. }
if winit_app.window().is_none()
Expand All @@ -174,8 +194,8 @@ fn run_and_return(
log::trace!("Repaint caused by winit::Event: {:?}", event);
if cfg!(windows) {
// Fix flickering on Windows, see https://github.com/emilk/egui/pull/2280
next_repaint_time = Instant::now() + Duration::from_secs(1_000_000_000);
winit_app.paint();
next_repaint_time = extremely_far_future();
winit_app.run_ui_and_paint();
} else {
// Fix for https://github.com/emilk/egui/issues/2425
next_repaint_time = Instant::now();
Expand All @@ -196,18 +216,20 @@ fn run_and_return(
}
}

*control_flow = match next_repaint_time.checked_duration_since(Instant::now()) {
None => {
if let Some(window) = winit_app.window() {
window.request_redraw();
}
next_repaint_time = Instant::now() + Duration::from_secs(1_000_000_000);
ControlFlow::Poll
*control_flow = if next_repaint_time <= Instant::now() {
if let Some(window) = winit_app.window() {
log::trace!("request_redraw");
window.request_redraw();
}
Some(time_until_next_repaint) => {
ControlFlow::WaitUntil(Instant::now() + time_until_next_repaint)
next_repaint_time = extremely_far_future();
ControlFlow::Poll
} else {
let time_until_next = next_repaint_time.saturating_duration_since(Instant::now());
if time_until_next < std::time::Duration::from_secs(10_000) {
log::trace!("WaitUntil {time_until_next:?}");
}
}
ControlFlow::WaitUntil(next_repaint_time)
};
});

log::debug!("eframe window closed");
Expand Down Expand Up @@ -239,18 +261,25 @@ fn run_and_exit(event_loop: EventLoop<UserEvent>, mut winit_app: impl WinitApp +
// See: https://github.com/rust-windowing/winit/issues/987
// See: https://github.com/rust-windowing/winit/issues/1619
winit::event::Event::RedrawEventsCleared if cfg!(windows) => {
next_repaint_time = Instant::now() + Duration::from_secs(1_000_000_000);
winit_app.paint()
next_repaint_time = extremely_far_future();
winit_app.run_ui_and_paint()
}
winit::event::Event::RedrawRequested(_) if !cfg!(windows) => {
next_repaint_time = Instant::now() + Duration::from_secs(1_000_000_000);
winit_app.paint()
next_repaint_time = extremely_far_future();
winit_app.run_ui_and_paint()
}

winit::event::Event::UserEvent(UserEvent::RequestRepaint)
| winit::event::Event::NewEvents(winit::event::StartCause::ResumeTimeReached {
winit::event::Event::UserEvent(UserEvent::RequestRepaint { when, frame_nr }) => {
if winit_app.frame_nr() == frame_nr {
EventResult::RepaintAt(when)
} else {
EventResult::Wait // old request - we've already repainted
}
}

winit::event::Event::NewEvents(winit::event::StartCause::ResumeTimeReached {
..
}) => EventResult::RepaintNext,
}) => EventResult::Wait, // We just woke up to check next_repaint_time

event => match winit_app.on_event(event_loop, &event) {
Ok(event_result) => event_result,
Expand All @@ -265,8 +294,8 @@ fn run_and_exit(event_loop: EventLoop<UserEvent>, mut winit_app: impl WinitApp +
EventResult::RepaintNow => {
if cfg!(windows) {
// Fix flickering on Windows, see https://github.com/emilk/egui/pull/2280
next_repaint_time = Instant::now() + Duration::from_secs(1_000_000_000);
winit_app.paint();
next_repaint_time = extremely_far_future();
winit_app.run_ui_and_paint();
} else {
// Fix for https://github.com/emilk/egui/issues/2425
next_repaint_time = Instant::now();
Expand All @@ -286,17 +315,15 @@ fn run_and_exit(event_loop: EventLoop<UserEvent>, mut winit_app: impl WinitApp +
}
}

*control_flow = match next_repaint_time.checked_duration_since(Instant::now()) {
None => {
if let Some(window) = winit_app.window() {
window.request_redraw();
}
ControlFlow::Poll
}
Some(time_until_next_repaint) => {
ControlFlow::WaitUntil(Instant::now() + time_until_next_repaint)
*control_flow = if next_repaint_time <= Instant::now() {
if let Some(window) = winit_app.window() {
window.request_redraw();
}
}
next_repaint_time = extremely_far_future();
ControlFlow::Poll
} else {
ControlFlow::WaitUntil(next_repaint_time)
};
})
}

Expand Down Expand Up @@ -601,8 +628,6 @@ mod glow_integration {
// suspends and resumes.
app_creator: Option<epi::AppCreator>,
is_focused: bool,

frame_nr: u64,
}

impl GlowWinitApp {
Expand All @@ -619,7 +644,6 @@ mod glow_integration {
running: None,
app_creator: Some(app_creator),
is_focused: true,
frame_nr: 0,
}
}

Expand Down Expand Up @@ -698,12 +722,17 @@ mod glow_integration {

{
let event_loop_proxy = self.repaint_proxy.clone();
integration.egui_ctx.set_request_repaint_callback(move || {
event_loop_proxy
.lock()
.send_event(UserEvent::RequestRepaint)
.ok();
});
integration
.egui_ctx
.set_request_repaint_callback(move |info| {
log::trace!("request_repaint_callback: {info:?}");
let when = Instant::now() + info.after;
let frame_nr = info.current_frame_nr;
event_loop_proxy
.lock()
.send_event(UserEvent::RequestRepaint { when, frame_nr })
.ok();
});
}

let app_creator = std::mem::take(&mut self.app_creator)
Expand Down Expand Up @@ -734,6 +763,12 @@ mod glow_integration {
}

impl WinitApp for GlowWinitApp {
fn frame_nr(&self) -> u64 {
self.running
.as_ref()
.map_or(0, |r| r.integration.egui_ctx.frame_nr())
}

fn is_focused(&self) -> bool {
self.is_focused
}
Expand All @@ -756,7 +791,7 @@ mod glow_integration {
}
}

fn paint(&mut self) -> EventResult {
fn run_ui_and_paint(&mut self) -> EventResult {
if let Some(running) = &mut self.running {
#[cfg(feature = "puffin")]
puffin::GlobalProfiler::lock().new_frame();
Expand Down Expand Up @@ -820,7 +855,7 @@ mod glow_integration {

#[cfg(feature = "__screenshot")]
// give it time to settle:
if self.frame_nr == 2 {
if integration.egui_ctx.frame_nr() == 2 {
if let Ok(path) = std::env::var("EFRAME_SCREENSHOT_TO") {
assert!(
path.ends_with(".png"),
Expand Down Expand Up @@ -871,8 +906,6 @@ mod glow_integration {
std::thread::sleep(std::time::Duration::from_millis(10));
}

self.frame_nr += 1;

control_flow
} else {
EventResult::Wait
Expand Down Expand Up @@ -1150,13 +1183,18 @@ mod wgpu_integration {

{
let event_loop_proxy = self.repaint_proxy.clone();
integration.egui_ctx.set_request_repaint_callback(move || {
event_loop_proxy
.lock()
.unwrap()
.send_event(UserEvent::RequestRepaint)
.ok();
});
integration
.egui_ctx
.set_request_repaint_callback(move |info| {
log::trace!("request_repaint_callback: {info:?}");
let when = Instant::now() + info.after;
let frame_nr = info.current_frame_nr;
event_loop_proxy
.lock()
.unwrap()
.send_event(UserEvent::RequestRepaint { when, frame_nr })
.ok();
});
}

let app_creator = std::mem::take(&mut self.app_creator)
Expand Down Expand Up @@ -1186,6 +1224,12 @@ mod wgpu_integration {
}

impl WinitApp for WgpuWinitApp {
fn frame_nr(&self) -> u64 {
self.running
.as_ref()
.map_or(0, |r| r.integration.egui_ctx.frame_nr())
}

fn is_focused(&self) -> bool {
self.is_focused
}
Expand Down Expand Up @@ -1214,7 +1258,7 @@ mod wgpu_integration {
}
}

fn paint(&mut self) -> EventResult {
fn run_ui_and_paint(&mut self) -> EventResult {
if let (Some(running), Some(window)) = (&mut self.running, &self.window) {
#[cfg(feature = "puffin")]
puffin::GlobalProfiler::lock().new_frame();
Expand Down Expand Up @@ -1433,12 +1477,11 @@ mod wgpu_integration {
}
}

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

#[cfg(feature = "wgpu")]
pub use wgpu_integration::run_wgpu;

#[cfg(any(target_os = "windows", target_os = "macos"))]
// ----------------------------------------------------------------------------

fn system_theme(window: &winit::window::Window, options: &NativeOptions) -> Option<crate::Theme> {
if options.follow_system_theme {
window
Expand All @@ -1449,9 +1492,8 @@ fn system_theme(window: &winit::window::Window, options: &NativeOptions) -> Opti
}
}

// Winit only reads the system theme on macOS and Windows.
// See: https://github.com/rust-windowing/winit/issues/1549
#[cfg(not(any(target_os = "windows", target_os = "macos")))]
fn system_theme(_window: &winit::window::Window, _options: &NativeOptions) -> Option<crate::Theme> {
None
// ----------------------------------------------------------------------------

fn extremely_far_future() -> std::time::Instant {
std::time::Instant::now() + std::time::Duration::from_secs(10_000_000_000)
}
4 changes: 2 additions & 2 deletions crates/eframe/src/web/backend.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,8 +259,8 @@ impl AppRunner {
let needs_repaint: std::sync::Arc<NeedRepaint> = Default::default();
{
let needs_repaint = needs_repaint.clone();
egui_ctx.set_request_repaint_callback(move || {
needs_repaint.repaint_asap();
egui_ctx.set_request_repaint_callback(move |info| {
needs_repaint.repaint_after(info.after.as_secs_f64());
});
}

Expand Down
Loading

0 comments on commit 834e2e9

Please sign in to comment.