From 0e52672f4aea376c3ef8a06b64aedca46a40e9b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Art=C3=BAr=20Kov=C3=A1cs?= Date: Fri, 4 Feb 2022 12:13:04 +0100 Subject: [PATCH] On X11, Fix for repeated event loop iteration when `ControlFlow` was `Wait` (#2155) * On X11, Fix for repeated event loop iteration when `ControlFlow` was `Wait` * ControlFlow::Poll now runs continously as should --- CHANGELOG.md | 1 + src/platform_impl/linux/x11/mod.rs | 169 +++++++++++++++++++++-------- 2 files changed, 126 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c013879..6eb55550 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ And please only add new entries to the top of this list, right below the `# Unre # Unreleased +- On X11, fix for repeated event loop iteration when `ControlFlow` was `Wait` - On Wayland, report unaccelerated mouse deltas in `DeviceEvent::MouseMotion`. - **Breaking:** Bump `ndk` version to 0.6, ndk-sys to `v0.3`, `ndk-glue` to `0.6`. - Remove no longer needed `WINIT_LINK_COLORSYNC` environment variable. diff --git a/src/platform_impl/linux/x11/mod.rs b/src/platform_impl/linux/x11/mod.rs index 38180862..e0a2995e 100644 --- a/src/platform_impl/linux/x11/mod.rs +++ b/src/platform_impl/linux/x11/mod.rs @@ -32,7 +32,7 @@ use std::{ ptr, rc::Rc, slice, - sync::mpsc::{Receiver, Sender}, + sync::mpsc::{Receiver, Sender, TryRecvError}, sync::{mpsc, Arc, Weak}, time::{Duration, Instant}, }; @@ -63,6 +63,39 @@ struct WakeSender { waker: Arc, } +struct PeekableReceiver { + recv: Receiver, + first: Option, +} + +impl PeekableReceiver { + pub fn from_recv(recv: Receiver) -> Self { + Self { recv, first: None } + } + pub fn has_incoming(&mut self) -> bool { + if self.first.is_some() { + return true; + } + match self.recv.try_recv() { + Ok(v) => { + self.first = Some(v); + return true; + } + Err(TryRecvError::Empty) => return false, + Err(TryRecvError::Disconnected) => { + warn!("Channel was disconnected when checking incoming"); + return false; + } + } + } + pub fn try_recv(&mut self) -> Result { + if let Some(first) = self.first.take() { + return Ok(first); + } + self.recv.try_recv() + } +} + pub struct EventLoopWindowTarget { xconn: Arc, wm_delete_window: ffi::Atom, @@ -79,8 +112,8 @@ pub struct EventLoop { poll: Poll, waker: Arc, event_processor: EventProcessor, - redraw_channel: Receiver, - user_channel: Receiver, //waker.wake needs to be called whenever something gets sent + redraw_receiver: PeekableReceiver, + user_receiver: PeekableReceiver, //waker.wake needs to be called whenever something gets sent user_sender: Sender, target: Rc>, } @@ -240,8 +273,8 @@ impl EventLoop { poll, waker, event_processor, - redraw_channel, - user_channel, + redraw_receiver: PeekableReceiver::from_recv(redraw_channel), + user_receiver: PeekableReceiver::from_recv(user_channel), user_sender, target, } @@ -262,29 +295,38 @@ impl EventLoop { where F: FnMut(Event<'_, T>, &RootELW, &mut ControlFlow), { - let mut control_flow = ControlFlow::default(); - let mut events = Events::with_capacity(8); - let mut cause = StartCause::Init; - - let exit_code = loop { + struct IterationResult { + deadline: Option, + timeout: Option, + wait_start: Instant, + } + fn single_iteration( + this: &mut EventLoop, + control_flow: &mut ControlFlow, + cause: &mut StartCause, + callback: &mut F, + ) -> IterationResult + where + F: FnMut(Event<'_, T>, &RootELW, &mut ControlFlow), + { sticky_exit_callback( - crate::event::Event::NewEvents(cause), - &self.target, - &mut control_flow, - &mut callback, + crate::event::Event::NewEvents(*cause), + &this.target, + control_flow, + callback, ); // Process all pending events - self.drain_events(&mut callback, &mut control_flow); + this.drain_events(callback, control_flow); // Empty the user event buffer { - while let Ok(event) = self.user_channel.try_recv() { + while let Ok(event) = this.user_receiver.try_recv() { sticky_exit_callback( crate::event::Event::UserEvent(event), - &self.target, - &mut control_flow, - &mut callback, + &this.target, + control_flow, + callback, ); } } @@ -292,16 +334,16 @@ impl EventLoop { { sticky_exit_callback( crate::event::Event::MainEventsCleared, - &self.target, - &mut control_flow, - &mut callback, + &this.target, + control_flow, + callback, ); } // Empty the redraw requests { let mut windows = HashSet::new(); - while let Ok(window_id) = self.redraw_channel.try_recv() { + while let Ok(window_id) = this.redraw_receiver.try_recv() { windows.insert(window_id); } @@ -309,9 +351,9 @@ impl EventLoop { let window_id = crate::window::WindowId(super::WindowId::X(window_id)); sticky_exit_callback( Event::RedrawRequested(window_id), - &self.target, - &mut control_flow, - &mut callback, + &this.target, + control_flow, + callback, ); } } @@ -319,9 +361,9 @@ impl EventLoop { { sticky_exit_callback( crate::event::Event::RedrawEventsCleared, - &self.target, - &mut control_flow, - &mut callback, + &this.target, + control_flow, + callback, ); } @@ -329,14 +371,20 @@ impl EventLoop { let (deadline, timeout); match control_flow { - ControlFlow::ExitWithCode(code) => break code, + ControlFlow::ExitWithCode(_) => { + return IterationResult { + wait_start: start, + deadline: None, + timeout: None, + }; + } ControlFlow::Poll => { - cause = StartCause::Poll; + *cause = StartCause::Poll; deadline = None; timeout = Some(Duration::from_millis(0)); } ControlFlow::Wait => { - cause = StartCause::WaitCancelled { + *cause = StartCause::WaitCancelled { start, requested_resume: None, }; @@ -344,38 +392,71 @@ impl EventLoop { timeout = None; } ControlFlow::WaitUntil(wait_deadline) => { - cause = StartCause::ResumeTimeReached { + *cause = StartCause::ResumeTimeReached { start, - requested_resume: wait_deadline, + requested_resume: *wait_deadline, }; - timeout = if wait_deadline > start { - Some(wait_deadline - start) + timeout = if *wait_deadline > start { + Some(*wait_deadline - start) } else { Some(Duration::from_millis(0)) }; - deadline = Some(wait_deadline); + deadline = Some(*wait_deadline); } } + return IterationResult { + wait_start: start, + deadline, + timeout, + }; + } - // If the XConnection already contains buffered events, we don't - // need to wait for data on the socket. - if !self.event_processor.poll() { - if let Err(e) = self.poll.poll(&mut events, timeout) { + let mut control_flow = ControlFlow::default(); + let mut events = Events::with_capacity(8); + let mut cause = StartCause::Init; + + // run the initial loop iteration + let mut iter_result = single_iteration(self, &mut control_flow, &mut cause, &mut callback); + + let exit_code = loop { + if let ControlFlow::ExitWithCode(code) = control_flow { + break code; + } + let has_pending = self.event_processor.poll() + || self.user_receiver.has_incoming() + || self.redraw_receiver.has_incoming(); + if !has_pending { + // Wait until + if let Err(e) = self.poll.poll(&mut events, iter_result.timeout) { if e.raw_os_error() != Some(libc::EINTR) { panic!("epoll returned an error: {:?}", e); } } events.clear(); + + if control_flow == ControlFlow::Wait { + // We don't go straight into executing the event loop iteration, we instead go + // to the start of this loop and check again if there's any pending event. We + // must do this because during the execution of the iteration we sometimes wake + // the mio waker, and if the waker is already awaken before we call poll(), + // then poll doesn't block, but it returns immediately. This caused the event + // loop to run continously even if the control_flow was `Wait` + continue; + } } - let wait_cancelled = deadline.map_or(false, |deadline| Instant::now() < deadline); + let wait_cancelled = iter_result + .deadline + .map_or(false, |deadline| Instant::now() < deadline); if wait_cancelled { cause = StartCause::WaitCancelled { - start, - requested_resume: deadline, + start: iter_result.wait_start, + requested_resume: iter_result.deadline, }; } + + iter_result = single_iteration(self, &mut control_flow, &mut cause, &mut callback); }; callback(