From ac4d75e9b4fc3e0fa85828b1988150638488bcd6 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 29 Aug 2022 11:20:19 +0200 Subject: [PATCH] egui-winit: don't repaint when just moving window (#1980) * Bug fix: reset repaint countdown when we pass it * eframe: debug-print what winit event caused a repaint * egui-winit: don't repaint when only moving window * fix docstring --- crates/eframe/src/native/epi_integration.rs | 14 ++- crates/eframe/src/native/run.rs | 31 +++-- crates/egui-winit/src/lib.rs | 118 +++++++++++++++---- crates/egui_glium/examples/native_texture.rs | 6 +- crates/egui_glium/examples/pure_glium.rs | 6 +- crates/egui_glium/src/lib.rs | 10 +- crates/egui_glow/examples/pure_glow.rs | 6 +- crates/egui_glow/src/winit.rs | 10 +- 8 files changed, 139 insertions(+), 62 deletions(-) diff --git a/crates/eframe/src/native/epi_integration.rs b/crates/eframe/src/native/epi_integration.rs index 335683f4..d0f4ea91 100644 --- a/crates/eframe/src/native/epi_integration.rs +++ b/crates/eframe/src/native/epi_integration.rs @@ -1,7 +1,9 @@ -use crate::{epi, Theme, WindowInfo}; -use egui_winit::{native_pixels_per_point, WindowSettings}; use winit::event_loop::EventLoopWindowTarget; +use egui_winit::{native_pixels_per_point, EventResponse, WindowSettings}; + +use crate::{epi, Theme, WindowInfo}; + pub fn points_to_size(points: egui::Vec2) -> winit::dpi::LogicalSize { winit::dpi::LogicalSize { width: points.x as f64, @@ -248,7 +250,11 @@ impl EpiIntegration { self.close } - pub fn on_event(&mut self, app: &mut dyn epi::App, event: &winit::event::WindowEvent<'_>) { + pub fn on_event( + &mut self, + app: &mut dyn epi::App, + event: &winit::event::WindowEvent<'_>, + ) -> EventResponse { use winit::event::{ElementState, MouseButton, WindowEvent}; match event { @@ -262,7 +268,7 @@ impl EpiIntegration { _ => {} } - self.egui_winit.on_event(&self.egui_ctx, event); + self.egui_winit.on_event(&self.egui_ctx, event) } pub fn update( diff --git a/crates/eframe/src/native/run.rs b/crates/eframe/src/native/run.rs index 26dc17cc..d76a9e59 100644 --- a/crates/eframe/src/native/run.rs +++ b/crates/eframe/src/native/run.rs @@ -19,6 +19,7 @@ pub struct RequestRepaintEvent; pub use epi::NativeOptions; +#[derive(Debug)] enum EventResult { Wait, RepaintAsap, @@ -35,7 +36,7 @@ trait WinitApp { fn on_event( &mut self, event_loop: &EventLoopWindowTarget, - event: winit::event::Event<'_, RequestRepaintEvent>, + event: &winit::event::Event<'_, RequestRepaintEvent>, ) -> EventResult; } @@ -81,7 +82,7 @@ fn run_and_return(event_loop: &mut EventLoop, mut winit_app let mut next_repaint_time = Instant::now(); event_loop.run_return(|event, event_loop, control_flow| { - let event_result = match event { + let event_result = match &event { winit::event::Event::LoopDestroyed => EventResult::Exit, // Platform-dependent event handlers to workaround a winit bug @@ -103,7 +104,7 @@ fn run_and_return(event_loop: &mut EventLoop, mut winit_app winit::event::Event::WindowEvent { window_id, .. } if winit_app.window().is_none() - || window_id != winit_app.window().unwrap().id() => + || *window_id != winit_app.window().unwrap().id() => { // This can happen if we close a window, and then reopen a new one, // or if we have multiple windows open. @@ -116,6 +117,7 @@ fn run_and_return(event_loop: &mut EventLoop, mut winit_app match event_result { EventResult::Wait => {} EventResult::RepaintAsap => { + tracing::debug!("Repaint caused by winit::Event: {:?}", event); next_repaint_time = Instant::now(); } EventResult::RepaintAt(repaint_time) => { @@ -132,6 +134,7 @@ fn run_and_return(event_loop: &mut EventLoop, mut winit_app if let Some(window) = winit_app.window() { window.request_redraw(); } + next_repaint_time = Instant::now() + Duration::from_secs(1_000_000_000); ControlFlow::Poll } Some(time_until_next_repaint) => { @@ -181,7 +184,7 @@ fn run_and_exit( .. }) => EventResult::RepaintAsap, - event => winit_app.on_event(event_loop, event), + event => winit_app.on_event(event_loop, &event), }; match event_result { @@ -496,7 +499,7 @@ mod glow_integration { fn on_event( &mut self, event_loop: &EventLoopWindowTarget, - event: winit::event::Event<'_, RequestRepaintEvent>, + event: &winit::event::Event<'_, RequestRepaintEvent>, ) -> EventResult { match event { winit::event::Event::Resumed => { @@ -551,13 +554,15 @@ mod glow_integration { _ => {} } - running.integration.on_event(running.app.as_mut(), &event); + let event_response = + running.integration.on_event(running.app.as_mut(), event); if running.integration.should_close() { EventResult::Exit - } else { - // TODO(emilk): ask egui if the event warrants a repaint + } else if event_response.repaint { EventResult::RepaintAsap + } else { + EventResult::Wait } } else { EventResult::Wait @@ -847,7 +852,7 @@ mod wgpu_integration { fn on_event( &mut self, event_loop: &EventLoopWindowTarget, - event: winit::event::Event<'_, RequestRepaintEvent>, + event: &winit::event::Event<'_, RequestRepaintEvent>, ) -> EventResult { match event { winit::event::Event::Resumed => { @@ -912,12 +917,14 @@ mod wgpu_integration { _ => {} }; - running.integration.on_event(running.app.as_mut(), &event); + let event_response = + running.integration.on_event(running.app.as_mut(), event); if running.integration.should_close() { EventResult::Exit - } else { - // TODO(emilk): ask egui if the event warrants a repaint + } else if event_response.repaint { EventResult::RepaintAsap + } else { + EventResult::Wait } } else { EventResult::Wait diff --git a/crates/egui-winit/src/lib.rs b/crates/egui-winit/src/lib.rs index 7e5bca76..1f230a41 100644 --- a/crates/egui-winit/src/lib.rs +++ b/crates/egui-winit/src/lib.rs @@ -39,6 +39,25 @@ pub fn screen_size_in_pixels(window: &winit::window::Window) -> egui::Vec2 { egui::vec2(size.width as f32, size.height as f32) } +// ---------------------------------------------------------------------------- + +#[must_use] +pub struct EventResponse { + /// If true, egui consumed this event, i.e. wants exclusive use of this event + /// (e.g. a mouse click on an egui window, or entering text into a text field). + /// + /// For instance, if you use egui for a game, you should only + /// pass on the events to your game when [`Self::consumed`] is `false. + /// + /// Note that egui uses `tab` to move focus between elements, so this will always be `true` for tabs. + pub consumed: bool, + + /// Do we need an egui refresh because of this event? + pub repaint: bool, +} + +// ---------------------------------------------------------------------------- + /// Handles the integration between egui and winit. pub struct State { start_time: instant::Instant, @@ -152,51 +171,63 @@ impl State { /// Call this when there is a new event. /// /// The result can be found in [`Self::egui_input`] and be extracted with [`Self::take_egui_input`]. - /// - /// Returns `true` if egui wants exclusive use of this event - /// (e.g. a mouse click on an egui window, or entering text into a text field). - /// For instance, if you use egui for a game, you want to first call this - /// and only when this returns `false` pass on the events to your game. - /// - /// Note that egui uses `tab` to move focus between elements, so this will always return `true` for tabs. pub fn on_event( &mut self, egui_ctx: &egui::Context, event: &winit::event::WindowEvent<'_>, - ) -> bool { + ) -> EventResponse { use winit::event::WindowEvent; match event { WindowEvent::ScaleFactorChanged { scale_factor, .. } => { let pixels_per_point = *scale_factor as f32; self.egui_input.pixels_per_point = Some(pixels_per_point); self.current_pixels_per_point = pixels_per_point; - false + EventResponse { + repaint: true, + consumed: false, + } } WindowEvent::MouseInput { state, button, .. } => { self.on_mouse_button_input(*state, *button); - egui_ctx.wants_pointer_input() + EventResponse { + repaint: true, + consumed: egui_ctx.wants_pointer_input(), + } } WindowEvent::MouseWheel { delta, .. } => { self.on_mouse_wheel(*delta); - egui_ctx.wants_pointer_input() + EventResponse { + repaint: true, + consumed: egui_ctx.wants_pointer_input(), + } } WindowEvent::CursorMoved { position, .. } => { self.on_cursor_moved(*position); - egui_ctx.is_using_pointer() + EventResponse { + repaint: true, + consumed: egui_ctx.is_using_pointer(), + } } WindowEvent::CursorLeft { .. } => { self.pointer_pos_in_points = None; self.egui_input.events.push(egui::Event::PointerGone); - false + EventResponse { + repaint: true, + consumed: false, + } } // WindowEvent::TouchpadPressure {device_id, pressure, stage, .. } => {} // TODO WindowEvent::Touch(touch) => { self.on_touch(touch); - match touch.phase { + let consumed = match touch.phase { winit::event::TouchPhase::Started | winit::event::TouchPhase::Ended | winit::event::TouchPhase::Cancelled => egui_ctx.wants_pointer_input(), winit::event::TouchPhase::Moved => egui_ctx.is_using_pointer(), + }; + EventResponse { + repaint: true, + consumed, } } WindowEvent::ReceivedCharacter(ch) => { @@ -205,37 +236,54 @@ impl State { let is_mac_cmd = cfg!(target_os = "macos") && (self.egui_input.modifiers.ctrl || self.egui_input.modifiers.mac_cmd); - if is_printable_char(*ch) && !is_mac_cmd { + let consumed = if is_printable_char(*ch) && !is_mac_cmd { self.egui_input .events .push(egui::Event::Text(ch.to_string())); egui_ctx.wants_keyboard_input() } else { false + }; + EventResponse { + repaint: true, + consumed, } } WindowEvent::KeyboardInput { input, .. } => { self.on_keyboard_input(input); - egui_ctx.wants_keyboard_input() - || input.virtual_keycode == Some(winit::event::VirtualKeyCode::Tab) + let consumed = egui_ctx.wants_keyboard_input() + || input.virtual_keycode == Some(winit::event::VirtualKeyCode::Tab); + EventResponse { + repaint: true, + consumed, + } } WindowEvent::Focused(has_focus) => { self.egui_input.has_focus = *has_focus; // We will not be given a KeyboardInput event when the modifiers are released while // the window does not have focus. Unset all modifier state to be safe. self.egui_input.modifiers = egui::Modifiers::default(); - false + EventResponse { + repaint: true, + consumed: false, + } } WindowEvent::HoveredFile(path) => { self.egui_input.hovered_files.push(egui::HoveredFile { path: Some(path.clone()), ..Default::default() }); - false + EventResponse { + repaint: true, + consumed: false, + } } WindowEvent::HoveredFileCancelled => { self.egui_input.hovered_files.clear(); - false + EventResponse { + repaint: true, + consumed: false, + } } WindowEvent::DroppedFile(path) => { self.egui_input.hovered_files.clear(); @@ -243,7 +291,10 @@ impl State { path: Some(path.clone()), ..Default::default() }); - false + EventResponse { + repaint: true, + consumed: false, + } } WindowEvent::ModifiersChanged(state) => { self.egui_input.modifiers.alt = state.alt(); @@ -255,12 +306,27 @@ impl State { } else { state.ctrl() }; - false - } - _ => { - // dbg!(event); - false + EventResponse { + repaint: true, + consumed: false, + } } + WindowEvent::AxisMotion { .. } + | WindowEvent::CloseRequested + | WindowEvent::CursorEntered { .. } + | WindowEvent::Destroyed + | WindowEvent::Ime(_) + | WindowEvent::Occluded(_) + | WindowEvent::Resized(_) + | WindowEvent::ThemeChanged(_) + | WindowEvent::TouchpadPressure { .. } => EventResponse { + repaint: true, + consumed: false, + }, + WindowEvent::Moved(_) => EventResponse { + repaint: false, // moving a window doesn't warrant a repaint + consumed: false, + }, } } diff --git a/crates/egui_glium/examples/native_texture.rs b/crates/egui_glium/examples/native_texture.rs index 9521976e..53d6840d 100644 --- a/crates/egui_glium/examples/native_texture.rs +++ b/crates/egui_glium/examples/native_texture.rs @@ -87,9 +87,11 @@ fn main() { *control_flow = glutin::event_loop::ControlFlow::Exit; } - egui_glium.on_event(&event); + let event_response = egui_glium.on_event(&event); - display.gl_window().window().request_redraw(); // TODO(emilk): ask egui if the events warrants a repaint instead + if event_response.repaint { + display.gl_window().window().request_redraw(); + } } glutin::event::Event::NewEvents(glutin::event::StartCause::ResumeTimeReached { .. diff --git a/crates/egui_glium/examples/pure_glium.rs b/crates/egui_glium/examples/pure_glium.rs index 019642cf..ac069a93 100644 --- a/crates/egui_glium/examples/pure_glium.rs +++ b/crates/egui_glium/examples/pure_glium.rs @@ -66,9 +66,11 @@ fn main() { *control_flow = glutin::event_loop::ControlFlow::Exit; } - egui_glium.on_event(&event); + let event_response = egui_glium.on_event(&event); - display.gl_window().window().request_redraw(); // TODO(emilk): ask egui if the events warrants a repaint instead + if event_response.repaint { + display.gl_window().window().request_redraw(); + } } glutin::event::Event::NewEvents(glutin::event::StartCause::ResumeTimeReached { .. diff --git a/crates/egui_glium/src/lib.rs b/crates/egui_glium/src/lib.rs index 64f53413..1d42ef18 100644 --- a/crates/egui_glium/src/lib.rs +++ b/crates/egui_glium/src/lib.rs @@ -15,7 +15,9 @@ mod painter; pub use painter::Painter; pub use egui_winit; + use egui_winit::winit::event_loop::EventLoopWindowTarget; +pub use egui_winit::EventResponse; // ---------------------------------------------------------------------------- @@ -47,13 +49,7 @@ impl EguiGlium { } } - /// Returns `true` if egui wants exclusive use of this event - /// (e.g. a mouse click on an egui window, or entering text into a text field). - /// For instance, if you use egui for a game, you want to first call this - /// and only when this returns `false` pass on the events to your game. - /// - /// Note that egui uses `tab` to move focus between elements, so this will always return `true` for tabs. - pub fn on_event(&mut self, event: &glium::glutin::event::WindowEvent<'_>) -> bool { + pub fn on_event(&mut self, event: &glium::glutin::event::WindowEvent<'_>) -> EventResponse { self.egui_winit.on_event(&self.egui_ctx, event) } diff --git a/crates/egui_glow/examples/pure_glow.rs b/crates/egui_glow/examples/pure_glow.rs index 07d4791d..cda33902 100644 --- a/crates/egui_glow/examples/pure_glow.rs +++ b/crates/egui_glow/examples/pure_glow.rs @@ -79,9 +79,11 @@ fn main() { gl_window.resize(**new_inner_size); } - egui_glow.on_event(&event); + let event_response = egui_glow.on_event(&event); - gl_window.window().request_redraw(); // TODO(emilk): ask egui if the events warrants a repaint instead + if event_response.repaint { + gl_window.window().request_redraw(); + } } glutin::event::Event::LoopDestroyed => { egui_glow.destroy(); diff --git a/crates/egui_glow/src/winit.rs b/crates/egui_glow/src/winit.rs index 786cb18a..931b6957 100644 --- a/crates/egui_glow/src/winit.rs +++ b/crates/egui_glow/src/winit.rs @@ -1,4 +1,6 @@ pub use egui_winit; +pub use egui_winit::EventResponse; + use egui_winit::winit; /// Use [`egui`] from a [`glow`] app based on [`winit`]. @@ -31,13 +33,7 @@ impl EguiGlow { } } - /// Returns `true` if egui wants exclusive use of this event - /// (e.g. a mouse click on an egui window, or entering text into a text field). - /// For instance, if you use egui for a game, you want to first call this - /// and only when this returns `false` pass on the events to your game. - /// - /// Note that egui uses `tab` to move focus between elements, so this will always return `true` for tabs. - pub fn on_event(&mut self, event: &winit::event::WindowEvent<'_>) -> bool { + pub fn on_event(&mut self, event: &winit::event::WindowEvent<'_>) -> EventResponse { self.egui_winit.on_event(&self.egui_ctx, event) }