Skip to content

Commit

Permalink
Remove lifetime from the Event
Browse files Browse the repository at this point in the history
Lifetimes don't work nicely when dealing with multithreaded environments
in the current design of the existing winit's event handling model, so
remove it in favor of `Weak` fences passed to client, so they could
try to update the size.

Fixes #1387.
  • Loading branch information
kchibisov committed Jul 28, 2023
1 parent 755c533 commit 57fc861
Show file tree
Hide file tree
Showing 29 changed files with 169 additions and 406 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ And please only add new entries to the top of this list, right below the `# Unre

# Unreleased

- **Breaking:** Remove `PartialEq` implementation from the event.
- **Breaking:** Remove `lifetime` from the event.
- On iOS, always wake the event loop when transitioning from `ControlFlow::Poll` to `ControlFlow::Poll`.
- **Breaking:** `ActivationTokenDone` event which could be requested with the new `startup_notify` module, see its docs for more.
- On Wayland, make double clicking and moving the CSD frame more reliable.
Expand Down
2 changes: 1 addition & 1 deletion examples/child_window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ fn main() -> Result<(), impl std::error::Error> {

println!("parent window: {parent_window:?})");

event_loop.run(move |event: Event<'_, ()>, event_loop, control_flow| {
event_loop.run(move |event: Event<()>, event_loop, control_flow| {
*control_flow = ControlFlow::Wait;

if let Event::WindowEvent { event, window_id } = event {
Expand Down
4 changes: 1 addition & 3 deletions examples/multithreaded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,7 @@ fn main() -> Result<(), impl std::error::Error> {
}
_ => {
if let Some(tx) = window_senders.get(&window_id) {
if let Some(event) = event.to_static() {
tx.send(event).unwrap();
}
tx.send(event).unwrap();
}
}
},
Expand Down
269 changes: 13 additions & 256 deletions src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,12 @@
//!
//! [`EventLoop::run(...)`]: crate::event_loop::EventLoop::run
//! [`ControlFlow::WaitUntil`]: crate::event_loop::ControlFlow::WaitUntil
use smol_str::SmolStr;
use std::path::PathBuf;
use std::sync::{Mutex, Weak};
#[cfg(not(wasm_platform))]
use std::time::Instant;

use smol_str::SmolStr;
#[cfg(wasm_platform)]
use web_time::Instant;

Expand All @@ -54,8 +56,8 @@ use crate::{
/// Describes a generic event.
///
/// See the module-level docs for more information on the event loop manages each event.
#[derive(Debug, PartialEq)]
pub enum Event<'a, T: 'static> {
#[derive(Debug)]
pub enum Event<T: 'static> {
/// Emitted when new events arrive from the OS to be processed.
///
/// This event type is useful as a place to put code that should be done before you start
Expand All @@ -67,7 +69,7 @@ pub enum Event<'a, T: 'static> {
/// Emitted when the OS sends an event to a winit window.
WindowEvent {
window_id: WindowId,
event: WindowEvent<'a>,
event: WindowEvent,
},

/// Emitted when the OS sends an event to a device.
Expand Down Expand Up @@ -261,33 +263,9 @@ pub enum Event<'a, T: 'static> {
LoopDestroyed,
}

impl<T: Clone> Clone for Event<'static, T> {
fn clone(&self) -> Self {
use self::Event::*;
match self {
WindowEvent { window_id, event } => WindowEvent {
window_id: *window_id,
event: event.clone(),
},
UserEvent(event) => UserEvent(event.clone()),
DeviceEvent { device_id, event } => DeviceEvent {
device_id: *device_id,
event: event.clone(),
},
NewEvents(cause) => NewEvents(*cause),
MainEventsCleared => MainEventsCleared,
RedrawRequested(wid) => RedrawRequested(*wid),
RedrawEventsCleared => RedrawEventsCleared,
LoopDestroyed => LoopDestroyed,
Suspended => Suspended,
Resumed => Resumed,
}
}
}

impl<'a, T> Event<'a, T> {
impl<T> Event<T> {
#[allow(clippy::result_large_err)]
pub fn map_nonuser_event<U>(self) -> Result<Event<'a, U>, Event<'a, T>> {
pub fn map_nonuser_event<U>(self) -> Result<Event<U>, Event<T>> {
use self::Event::*;
match self {
UserEvent(_) => Err(self),
Expand All @@ -302,26 +280,6 @@ impl<'a, T> Event<'a, T> {
Resumed => Ok(Resumed),
}
}

/// If the event doesn't contain a reference, turn it into an event with a `'static` lifetime.
/// Otherwise, return `None`.
pub fn to_static(self) -> Option<Event<'static, T>> {
use self::Event::*;
match self {
WindowEvent { window_id, event } => event
.to_static()
.map(|event| WindowEvent { window_id, event }),
UserEvent(event) => Some(UserEvent(event)),
DeviceEvent { device_id, event } => Some(DeviceEvent { device_id, event }),
NewEvents(cause) => Some(NewEvents(cause)),
MainEventsCleared => Some(MainEventsCleared),
RedrawRequested(wid) => Some(RedrawRequested(wid)),
RedrawEventsCleared => Some(RedrawEventsCleared),
LoopDestroyed => Some(LoopDestroyed),
Suspended => Some(Suspended),
Resumed => Some(Resumed),
}
}
}

/// Describes the reason the event loop is resuming.
Expand Down Expand Up @@ -355,8 +313,8 @@ pub enum StartCause {
}

/// Describes an event from a [`Window`].
#[derive(Debug, PartialEq)]
pub enum WindowEvent<'a> {
#[derive(Debug, Clone)]
pub enum WindowEvent {
/// The activation token was delivered back and now could be used.
///
#[cfg_attr(
Expand Down Expand Up @@ -590,7 +548,9 @@ pub enum WindowEvent<'a> {
/// For more information about DPI in general, see the [`dpi`](crate::dpi) module.
ScaleFactorChanged {
scale_factor: f64,
new_inner_size: &'a mut PhysicalSize<u32>,
/// The pointer could be upgraded successfully only from the callback for this event,
/// otherwise it'll always fail to do so.
new_inner_size: Weak<Mutex<PhysicalSize<u32>>>,
},

/// The system window theme has changed.
Expand Down Expand Up @@ -619,209 +579,6 @@ pub enum WindowEvent<'a> {
Occluded(bool),
}

impl Clone for WindowEvent<'static> {
fn clone(&self) -> Self {
use self::WindowEvent::*;
return match self {
ActivationTokenDone { serial, token } => ActivationTokenDone {
serial: *serial,
token: token.clone(),
},
Resized(size) => Resized(*size),
Moved(pos) => Moved(*pos),
CloseRequested => CloseRequested,
Destroyed => Destroyed,
DroppedFile(file) => DroppedFile(file.clone()),
HoveredFile(file) => HoveredFile(file.clone()),
HoveredFileCancelled => HoveredFileCancelled,
Focused(f) => Focused(*f),
KeyboardInput {
device_id,
event,
is_synthetic,
} => KeyboardInput {
device_id: *device_id,
event: event.clone(),
is_synthetic: *is_synthetic,
},
Ime(preedit_state) => Ime(preedit_state.clone()),
ModifiersChanged(modifiers) => ModifiersChanged(*modifiers),
CursorMoved {
device_id,
position,
} => CursorMoved {
device_id: *device_id,
position: *position,
},
CursorEntered { device_id } => CursorEntered {
device_id: *device_id,
},
CursorLeft { device_id } => CursorLeft {
device_id: *device_id,
},
MouseWheel {
device_id,
delta,
phase,
} => MouseWheel {
device_id: *device_id,
delta: *delta,
phase: *phase,
},
MouseInput {
device_id,
state,
button,
} => MouseInput {
device_id: *device_id,
state: *state,
button: *button,
},
TouchpadMagnify {
device_id,
delta,
phase,
} => TouchpadMagnify {
device_id: *device_id,
delta: *delta,
phase: *phase,
},
SmartMagnify { device_id } => SmartMagnify {
device_id: *device_id,
},
TouchpadRotate {
device_id,
delta,
phase,
} => TouchpadRotate {
device_id: *device_id,
delta: *delta,
phase: *phase,
},
TouchpadPressure {
device_id,
pressure,
stage,
} => TouchpadPressure {
device_id: *device_id,
pressure: *pressure,
stage: *stage,
},
AxisMotion {
device_id,
axis,
value,
} => AxisMotion {
device_id: *device_id,
axis: *axis,
value: *value,
},
Touch(touch) => Touch(*touch),
ThemeChanged(theme) => ThemeChanged(*theme),
ScaleFactorChanged { .. } => {
unreachable!("Static event can't be about scale factor changing")
}
Occluded(occluded) => Occluded(*occluded),
};
}
}

impl<'a> WindowEvent<'a> {
pub fn to_static(self) -> Option<WindowEvent<'static>> {
use self::WindowEvent::*;
match self {
ActivationTokenDone { serial, token } => Some(ActivationTokenDone { serial, token }),
Resized(size) => Some(Resized(size)),
Moved(position) => Some(Moved(position)),
CloseRequested => Some(CloseRequested),
Destroyed => Some(Destroyed),
DroppedFile(file) => Some(DroppedFile(file)),
HoveredFile(file) => Some(HoveredFile(file)),
HoveredFileCancelled => Some(HoveredFileCancelled),
Focused(focused) => Some(Focused(focused)),
KeyboardInput {
device_id,
event,
is_synthetic,
} => Some(KeyboardInput {
device_id,
event,
is_synthetic,
}),
ModifiersChanged(modifers) => Some(ModifiersChanged(modifers)),
Ime(event) => Some(Ime(event)),
CursorMoved {
device_id,
position,
} => Some(CursorMoved {
device_id,
position,
}),
CursorEntered { device_id } => Some(CursorEntered { device_id }),
CursorLeft { device_id } => Some(CursorLeft { device_id }),
MouseWheel {
device_id,
delta,
phase,
} => Some(MouseWheel {
device_id,
delta,
phase,
}),
MouseInput {
device_id,
state,
button,
} => Some(MouseInput {
device_id,
state,
button,
}),
TouchpadMagnify {
device_id,
delta,
phase,
} => Some(TouchpadMagnify {
device_id,
delta,
phase,
}),
SmartMagnify { device_id } => Some(SmartMagnify { device_id }),
TouchpadRotate {
device_id,
delta,
phase,
} => Some(TouchpadRotate {
device_id,
delta,
phase,
}),
TouchpadPressure {
device_id,
pressure,
stage,
} => Some(TouchpadPressure {
device_id,
pressure,
stage,
}),
AxisMotion {
device_id,
axis,
value,
} => Some(AxisMotion {
device_id,
axis,
value,
}),
Touch(touch) => Some(Touch(touch)),
ThemeChanged(theme) => Some(ThemeChanged(theme)),
ScaleFactorChanged { .. } => None,
Occluded(occluded) => Some(Occluded(occluded)),
}
}
}

/// Identifier of an input device.
///
/// Whenever you receive an event arising from a particular input device, this event contains a `DeviceId` which
Expand Down
2 changes: 1 addition & 1 deletion src/event_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ impl<T> EventLoop<T> {
#[inline]
pub fn run<F>(self, event_handler: F) -> Result<(), RunLoopError>
where
F: 'static + FnMut(Event<'_, T>, &EventLoopWindowTarget<T>, &mut ControlFlow),
F: 'static + FnMut(Event<T>, &EventLoopWindowTarget<T>, &mut ControlFlow),
{
self.event_loop.run(event_handler)
}
Expand Down
12 changes: 2 additions & 10 deletions src/platform/pump_events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,23 +174,15 @@ pub trait EventLoopExtPumpEvents {
/// callback.
fn pump_events<F>(&mut self, timeout: Option<Duration>, event_handler: F) -> PumpStatus
where
F: FnMut(
Event<'_, Self::UserEvent>,
&EventLoopWindowTarget<Self::UserEvent>,
&mut ControlFlow,
);
F: FnMut(Event<Self::UserEvent>, &EventLoopWindowTarget<Self::UserEvent>, &mut ControlFlow);
}

impl<T> EventLoopExtPumpEvents for EventLoop<T> {
type UserEvent = T;

fn pump_events<F>(&mut self, timeout: Option<Duration>, event_handler: F) -> PumpStatus
where
F: FnMut(
Event<'_, Self::UserEvent>,
&EventLoopWindowTarget<Self::UserEvent>,
&mut ControlFlow,
),
F: FnMut(Event<Self::UserEvent>, &EventLoopWindowTarget<Self::UserEvent>, &mut ControlFlow),
{
self.event_loop.pump_events(timeout, event_handler)
}
Expand Down
Loading

0 comments on commit 57fc861

Please sign in to comment.