Skip to content

Commit

Permalink
On Wayland, fix Window::request_redraw being delayed
Browse files Browse the repository at this point in the history 
On Waylnad when asking for redraw before `MainEventsCleared`
would result for redraw being send on the next event loop tick,
which is not expectable given that it must be delivered on the same
event loop tick.
  • Loading branch information
@kchibisov
kchibisov authored Aug 12, 2022
1 parent fa83bac commit ec2888b
Show file tree
Hide file tree
Showing 5 changed files with 156 additions and 114 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ And please only add new entries to the top of this list, right below the `# Unre
- On Windows, respect min/max inner sizes when creating the window.
- For backwards compatibility, `Window` now (additionally) implements the old version (`0.4`) of the `HasRawWindowHandle` trait
- On Windows, added support for `EventLoopWindowTarget::set_device_event_filter`.
- On Wayland, fix user requested `WindowEvent::RedrawRequested` being delayed by a frame.

# 0.27.1 (2022-07-30)

Expand Down
115 changes: 73 additions & 42 deletions src/platform_impl/linux/wayland/event_loop/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::cell::RefCell;
use std::collections::HashMap;
use std::error::Error;
use std::io::Result as IOResult;
use std::mem;
use std::process;
use std::rc::Rc;
use std::time::{Duration, Instant};
Expand All @@ -26,7 +27,7 @@ use crate::platform_impl::EventLoopWindowTarget as PlatformEventLoopWindowTarget
use super::env::{WindowingFeatures, WinitEnv};
use super::output::OutputManager;
use super::seat::SeatManager;
use super::window::shim::{self, WindowUpdate};
use super::window::shim::{self, WindowCompositorUpdate, WindowUserRequest};
use super::{DeviceId, WindowId};

mod proxy;
Expand Down Expand Up @@ -82,6 +83,9 @@ impl<T> EventLoopWindowTarget<T> {
}

pub struct EventLoop<T: 'static> {
/// Dispatcher of Wayland events.
pub wayland_dispatcher: WinitDispatcher,

/// Event loop.
event_loop: calloop::EventLoop<'static, WinitState>,

Expand All @@ -94,9 +98,6 @@ pub struct EventLoop<T: 'static> {
/// Sender of user events.
user_events_sender: calloop::channel::Sender<T>,

/// Dispatcher of Wayland events.
pub wayland_dispatcher: WinitDispatcher,

/// Window target.
window_target: RootEventLoopWindowTarget<T>,

Expand Down Expand Up @@ -164,17 +165,19 @@ impl<T: 'static> EventLoop<T> {
let (event_loop_awakener, event_loop_awakener_source) = calloop::ping::make_ping()?;

// Handler of window requests.
event_loop.handle().insert_source(
event_loop_awakener_source,
move |_, _, winit_state| {
shim::handle_window_requests(winit_state);
},
)?;
event_loop
.handle()
.insert_source(event_loop_awakener_source, move |_, _, state| {
// Drain events here as well to account for application doing batch event processing
// on RedrawEventsCleared.
shim::handle_window_requests(state);
})?;

let event_loop_handle = event_loop.handle();
let window_map = HashMap::new();
let event_sink = EventSink::new();
let window_updates = HashMap::new();
let window_user_requests = HashMap::new();
let window_compositor_updates = HashMap::new();

// Create event loop window target.
let event_loop_window_target = EventLoopWindowTarget {
Expand All @@ -183,7 +186,8 @@ impl<T: 'static> EventLoop<T> {
state: RefCell::new(WinitState {
window_map,
event_sink,
window_updates,
window_user_requests,
window_compositor_updates,
}),
event_loop_handle,
output_manager,
Expand Down Expand Up @@ -236,7 +240,8 @@ impl<T: 'static> EventLoop<T> {
// applications don't themselves have a formal suspend/resume lifecycle.
callback(Event::Resumed, &self.window_target, &mut control_flow);

let mut window_updates: Vec<(WindowId, WindowUpdate)> = Vec::new();
let mut window_compositor_updates: Vec<(WindowId, WindowCompositorUpdate)> = Vec::new();
let mut window_user_requests: Vec<(WindowId, WindowUserRequest)> = Vec::new();
let mut event_sink_back_buffer = Vec::new();

// NOTE We break on errors from dispatches, since if we've got protocol error
Expand Down Expand Up @@ -357,25 +362,26 @@ impl<T: 'static> EventLoop<T> {
);
}

// Process 'new' pending updates.
// Process 'new' pending updates from compositor.
self.with_state(|state| {
window_updates.clear();
window_updates.extend(
window_compositor_updates.clear();
window_compositor_updates.extend(
state
.window_updates
.window_compositor_updates
.iter_mut()
.map(|(wid, window_update)| (*wid, window_update.take())),
.map(|(wid, window_update)| (*wid, mem::take(window_update))),
);
});

for (window_id, window_update) in window_updates.iter_mut() {
if let Some(scale_factor) = window_update.scale_factor.map(|f| f as f64) {
for (window_id, window_compositor_update) in window_compositor_updates.iter_mut() {
if let Some(scale_factor) = window_compositor_update.scale_factor.map(|f| f as f64)
{
let mut physical_size = self.with_state(|state| {
let window_handle = state.window_map.get(window_id).unwrap();
let mut size = window_handle.size.lock().unwrap();

// Update the new logical size if it was changed.
let window_size = window_update.size.unwrap_or(*size);
let window_size = window_compositor_update.size.unwrap_or(*size);
*size = window_size;

window_size.to_physical(scale_factor)
Expand All @@ -397,26 +403,27 @@ impl<T: 'static> EventLoop<T> {
// We don't update size on a window handle since we'll do that later
// when handling size update.
let new_logical_size = physical_size.to_logical(scale_factor);
window_update.size = Some(new_logical_size);
window_compositor_update.size = Some(new_logical_size);
}

if let Some(size) = window_update.size.take() {
if let Some(size) = window_compositor_update.size.take() {
let physical_size = self.with_state(|state| {
let window_handle = state.window_map.get_mut(window_id).unwrap();
let mut window_size = window_handle.size.lock().unwrap();

// Always issue resize event on scale factor change.
let physical_size =
if window_update.scale_factor.is_none() && *window_size == size {
// The size hasn't changed, don't inform downstream about that.
None
} else {
*window_size = size;
let scale_factor =
sctk::get_surface_scale_factor(window_handle.window.surface());
let physical_size = size.to_physical(scale_factor as f64);
Some(physical_size)
};
let physical_size = if window_compositor_update.scale_factor.is_none()
&& *window_size == size
{
// The size hasn't changed, don't inform downstream about that.
None
} else {
*window_size = size;
let scale_factor =
sctk::get_surface_scale_factor(window_handle.window.surface());
let physical_size = size.to_physical(scale_factor as f64);
Some(physical_size)
};

// We still perform all of those resize related logic even if the size
// hasn't changed, since GNOME relies on `set_geometry` calls after
Expand All @@ -425,7 +432,11 @@ impl<T: 'static> EventLoop<T> {
window_handle.window.refresh();

// Mark that refresh isn't required, since we've done it right now.
window_update.refresh_frame = false;
state
.window_user_requests
.get_mut(window_id)
.unwrap()
.refresh_frame = false;

physical_size
});
Expand All @@ -443,7 +454,8 @@ impl<T: 'static> EventLoop<T> {
}
}

if window_update.close_window {
// If the close is requested, send it here.
if window_compositor_update.close_window {
sticky_exit_callback(
Event::WindowEvent {
window_id: crate::window::WindowId(*window_id),
Expand Down Expand Up @@ -480,21 +492,40 @@ impl<T: 'static> EventLoop<T> {
&mut callback,
);

// Apply user requests, so every event required resize and latter surface commit will
// be applied right before drawing. This will also ensure that every `RedrawRequested`
// event will be delivered in time.
self.with_state(|state| {
shim::handle_window_requests(state);
});

// Process 'new' pending updates from compositor.
self.with_state(|state| {
window_user_requests.clear();
window_user_requests.extend(
state
.window_user_requests
.iter_mut()
.map(|(wid, window_request)| (*wid, mem::take(window_request))),
);
});

// Handle RedrawRequested events.
for (window_id, window_update) in window_updates.iter() {
for (window_id, mut window_request) in window_user_requests.iter() {
// Handle refresh of the frame.
if window_update.refresh_frame {
if window_request.refresh_frame {
self.with_state(|state| {
let window_handle = state.window_map.get_mut(window_id).unwrap();
window_handle.window.refresh();
if !window_update.redraw_requested {
window_handle.window.surface().commit();
}
});

// In general refreshing the frame requires surface commit, those force user
// to redraw.
window_request.redraw_requested = true;
}

// Handle redraw request.
if window_update.redraw_requested {
if window_request.redraw_requested {
sticky_exit_callback(
Event::RedrawRequested(crate::window::WindowId(*window_id)),
&self.window_target,
Expand Down
10 changes: 8 additions & 2 deletions src/platform_impl/linux/wayland/event_loop/state.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
use std::collections::HashMap;

use super::EventSink;
use crate::platform_impl::wayland::window::shim::{WindowHandle, WindowUpdate};
use crate::platform_impl::wayland::window::shim::{
WindowCompositorUpdate, WindowHandle, WindowUserRequest,
};
use crate::platform_impl::wayland::WindowId;

/// Wrapper to carry winit's state.
Expand All @@ -12,10 +14,14 @@ pub struct WinitState {
/// event loop and forwarded downstream afterwards.
pub event_sink: EventSink,

/// Window updates comming from the user requests. Those are separatelly dispatched right after
/// `MainEventsCleared`.
pub window_user_requests: HashMap<WindowId, WindowUserRequest>,

/// Window updates, which are coming from SCTK or the compositor, which require
/// calling back to the winit's downstream. They are handled right in the event loop,
/// unlike the ones coming from buffers on the `WindowHandle`'s.
pub window_updates: HashMap<WindowId, WindowUpdate>,
pub window_compositor_updates: HashMap<WindowId, WindowCompositorUpdate>,

/// Window map containing all SCTK windows. Since those windows aren't allowed
/// to be sent to other threads, they live on the event loop's thread
Expand Down
50 changes: 35 additions & 15 deletions src/platform_impl/linux/wayland/window/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use super::{EventLoopWindowTarget, WindowId};

pub mod shim;

use shim::{WindowHandle, WindowRequest, WindowUpdate};
use shim::{WindowCompositorUpdate, WindowHandle, WindowRequest, WindowUserRequest};

#[cfg(feature = "sctk-adwaita")]
pub type WinitFrame = sctk_adwaita::AdwaitaFrame;
Expand Down Expand Up @@ -94,11 +94,20 @@ impl Window {

// Get the window that received the event.
let window_id = super::make_wid(&surface);
let mut window_update = winit_state.window_updates.get_mut(&window_id).unwrap();
let mut window_compositor_update = winit_state
.window_compositor_updates
.get_mut(&window_id)
.unwrap();

// Mark that we need a frame refresh on the DPI change.
winit_state
.window_user_requests
.get_mut(&window_id)
.unwrap()
.refresh_frame = true;

// Set pending scale factor.
window_update.scale_factor = Some(scale);
window_update.redraw_requested = true;
window_compositor_update.scale_factor = Some(scale);

surface.set_buffer_scale(scale);
})
Expand Down Expand Up @@ -128,26 +137,33 @@ impl Window {
use sctk::window::{Event, State};

let winit_state = dispatch_data.get::<WinitState>().unwrap();
let mut window_update = winit_state.window_updates.get_mut(&window_id).unwrap();
let mut window_compositor_update = winit_state
.window_compositor_updates
.get_mut(&window_id)
.unwrap();

let mut window_user_requests = winit_state
.window_user_requests
.get_mut(&window_id)
.unwrap();

match event {
Event::Refresh => {
window_update.refresh_frame = true;
window_user_requests.refresh_frame = true;
}
Event::Configure { new_size, states } => {
let is_maximized = states.contains(&State::Maximized);
maximized_clone.store(is_maximized, Ordering::Relaxed);
let is_fullscreen = states.contains(&State::Fullscreen);
fullscreen_clone.store(is_fullscreen, Ordering::Relaxed);

window_update.refresh_frame = true;
window_update.redraw_requested = true;
window_user_requests.refresh_frame = true;
if let Some((w, h)) = new_size {
window_update.size = Some(LogicalSize::new(w, h));
window_compositor_update.size = Some(LogicalSize::new(w, h));
}
}
Event::Close => {
window_update.close_window = true;
window_compositor_update.close_window = true;
}
}
},
Expand Down Expand Up @@ -234,9 +250,9 @@ impl Window {
let size = Arc::new(Mutex::new(LogicalSize::new(width, height)));

// We should trigger redraw and commit the surface for the newly created window.
let mut window_update = WindowUpdate::new();
window_update.refresh_frame = true;
window_update.redraw_requested = true;
let mut window_user_request = WindowUserRequest::new();
window_user_request.refresh_frame = true;
window_user_request.redraw_requested = true;

let window_id = super::make_wid(&surface);
let window_requests = Arc::new(Mutex::new(Vec::with_capacity(64)));
Expand All @@ -262,9 +278,13 @@ impl Window {
.event_sink
.push_window_event(crate::event::WindowEvent::Focused(false), window_id);

// Add state for the window.
winit_state
.window_user_requests
.insert(window_id, window_user_request);
winit_state
.window_updates
.insert(window_id, WindowUpdate::new());
.window_compositor_updates
.insert(window_id, WindowCompositorUpdate::new());

let windowing_features = event_loop_window_target.windowing_features;

Expand Down
Loading

0 comments on commit ec2888b

Please sign in to comment.