From c05eb69703f354d047d68545f2541db952e90429 Mon Sep 17 00:00:00 2001 From: Nicolas Avrutin Date: Wed, 6 Mar 2024 00:22:05 -0500 Subject: [PATCH] stop waking up every 1ms for polling and do regular frame scheduling Actually schedule frame callbacks the right way by scheduling the callback to be completed a frame interval after the current commit, less a couple ms for processing time. Also stop processing buffers for sync subsurfaces twice. --- src/bin/wprsc.rs | 55 ++++++------------ src/bin/wprsd.rs | 82 ++++++++++----------------- src/bin/xwayland-xdg-shell.rs | 84 +++++++++------------------- src/client/mod.rs | 4 -- src/compositor_utils.rs | 2 +- src/serialization/wayland.rs | 13 +---- src/server/mod.rs | 30 +++------- src/server/smithay_handlers.rs | 64 +++++++++++++++++++-- src/xwayland_xdg_shell/compositor.rs | 15 +++-- src/xwayland_xdg_shell/mod.rs | 11 +--- src/xwayland_xdg_shell/xwayland.rs | 52 ++++++++--------- 11 files changed, 173 insertions(+), 239 deletions(-) diff --git a/src/bin/wprsc.rs b/src/bin/wprsc.rs index ed25c6c5..5d3b5334 100644 --- a/src/bin/wprsc.rs +++ b/src/bin/wprsc.rs @@ -13,9 +13,7 @@ // limitations under the License. use std::fs; -use std::io::ErrorKind; use std::path::PathBuf; -use std::time::Duration; use bpaf::Parser; use optional_struct::optional_struct; @@ -24,7 +22,7 @@ use serde_derive::Deserialize; use serde_derive::Serialize; use smithay::reexports::calloop::channel::Event; use smithay::reexports::calloop::EventLoop; -use smithay_client_toolkit::reexports::client::backend::WaylandError; +use smithay_client_toolkit::reexports::calloop_wayland_source::WaylandSource; use smithay_client_toolkit::reexports::client::globals::registry_queue_init; use smithay_client_toolkit::reexports::client::ConnectError; use smithay_client_toolkit::reexports::client::Connection; @@ -33,7 +31,6 @@ use wprs::args; use wprs::args::Config; use wprs::args::OptionalConfig; use wprs::args::SerializableLevel; -use wprs::client::CalloopData; use wprs::client::ClientOptions; use wprs::client::WprsClientState; use wprs::control_server; @@ -136,7 +133,7 @@ fn main() -> Result<()> { _ => anyhow!(e), })?; - let (globals, mut event_queue) = registry_queue_init(&conn)?; + let (globals, event_queue) = registry_queue_init(&conn)?; fs::create_dir_all(config.socket.parent().location(loc!())?).location(loc!())?; let mut serializer = Serializer::new_client(&config.socket).with_context(loc!(), || { @@ -154,17 +151,22 @@ fn main() -> Result<()> { let options = ClientOptions { title_prefix: config.title_prefix, }; - let mut loop_data = CalloopData { - state: WprsClientState::new(event_queue.handle(), globals, conn, serializer, options) - .location(loc!())?, - }; + let mut state = WprsClientState::new( + event_queue.handle(), + globals, + conn.clone(), + serializer, + options, + ) + .location(loc!())?; let mut event_loop = EventLoop::try_new()?; + event_loop.handle().insert_source( reader, - |event, _metadata, loop_data: &mut CalloopData| { + |event, _metadata, state: &mut WprsClientState| { match event { - Event::Msg(msg) => loop_data.state.handle_request(msg), + Event::Msg(msg) => state.handle_request(msg), Event::Closed => { unreachable!("serialization::client_loop terminates the process when the server disconnects."); }, @@ -173,7 +175,7 @@ fn main() -> Result<()> { ).unwrap(); { - let capabilities = loop_data.state.capabilities.clone(); + let capabilities = state.capabilities.clone(); control_server::start(config.control_socket, move |input: &str| { Ok(match input { // TODO: make the input use json when we have more commands @@ -187,32 +189,11 @@ fn main() -> Result<()> { .location(loc!())?; } - event_loop - .run(Duration::from_millis(1), &mut loop_data, move |loop_data| { - // WouldBlock means that the compositor can't keep up with our messages. - // Seems rare, so we will retry again on next loop. - - match event_queue.flush() { - Ok(_) => {}, - Err(WaylandError::Io(err)) if err.kind() == ErrorKind::WouldBlock => { - warn!("{err:?}"); - }, - _ => { - panic!("Error writing to wayland connection."); - }, - }; - - match event_queue.prepare_read().unwrap().read() { - Ok(_) => { - event_queue.dispatch_pending(&mut loop_data.state).unwrap(); - }, - Err(WaylandError::Io(err)) if err.kind() == ErrorKind::WouldBlock => {}, - _ => { - panic!("Error reading from wayland connection."); - }, - }; - }) + WaylandSource::new(conn, event_queue) + .insert(event_loop.handle()) .location(loc!())?; + event_loop.run(None, &mut state, |_| {}).location(loc!())?; + Ok(()) } diff --git a/src/bin/wprsd.rs b/src/bin/wprsd.rs index d1c3a7d3..c2795d94 100644 --- a/src/bin/wprsd.rs +++ b/src/bin/wprsd.rs @@ -25,8 +25,6 @@ use serde_derive::Deserialize; use serde_derive::Serialize; use smithay::reexports::calloop::channel::Event; use smithay::reexports::calloop::generic::Generic; -use smithay::reexports::calloop::timer::TimeoutAction; -use smithay::reexports::calloop::timer::Timer; use smithay::reexports::calloop::EventLoop; use smithay::reexports::calloop::Interest; use smithay::reexports::calloop::Mode; @@ -173,26 +171,20 @@ impl OptionalConfig for OptionalWprsdConfig { } } -pub struct CalloopData { - pub state: WprsServerState, - pub display: Display, -} - fn init_wayland_listener( wayland_display: &str, - loop_data: &mut CalloopData, - event_loop: &EventLoop, + mut display: Display, + state: &mut WprsServerState, + event_loop: &EventLoop, ) -> Result<()> { let listening_socket = ListeningSocketSource::with_name(wayland_display).location(loc!())?; - let writer = loop_data.state.serializer.writer().into_inner(); + let writer = state.serializer.writer().into_inner(); + let mut dh = display.handle(); event_loop .handle() - .insert_source(listening_socket, move |stream, _, loop_data| { - loop_data - .display - .handle() - .insert_client(stream, Arc::new(ClientState::new(writer.clone()))) + .insert_source(listening_socket, move |stream, _, _| { + dh.insert_client(stream, Arc::new(ClientState::new(writer.clone()))) .unwrap(); }) .location(loc!())?; @@ -201,20 +193,12 @@ fn init_wayland_listener( .handle() .insert_source( Generic::new( - loop_data - .display - .backend() - .poll_fd() - .try_clone_to_owned() - .unwrap(), + display.backend().poll_fd().try_clone_to_owned().unwrap(), Interest::READ, Mode::Level, ), - |_, _, loop_data| { - loop_data - .display - .dispatch_clients(&mut loop_data.state) - .unwrap(); + move |_, _, state| { + display.dispatch_clients(state).unwrap(); Ok(PostAction::Continue) }, ) @@ -263,17 +247,19 @@ pub fn main() -> Result<()> { let mut event_loop = EventLoop::try_new().location(loc!())?; let display: Display = Display::new().location(loc!())?; - let mut loop_data = CalloopData { - state: WprsServerState::new( - display.handle(), - serializer, - config.enable_xwayland, - config.kde_server_side_decorations, - ), - display, - }; + let frame_interval = Duration::from_secs_f64(1.0 / (config.framerate as f64)); + + let mut state = WprsServerState::new( + display.handle(), + event_loop.handle(), + serializer, + config.enable_xwayland, + frame_interval, + config.kde_server_side_decorations, + ); - init_wayland_listener(&config.wayland_display, &mut loop_data, &event_loop).location(loc!())?; + init_wayland_listener(&config.wayland_display, display, &mut state, &event_loop) + .location(loc!())?; if config.enable_xwayland { start_xwayland_xdg_shell( @@ -285,38 +271,26 @@ pub fn main() -> Result<()> { } // TODO: do this in WprsServerState::new; - let _keyboard = loop_data - .state + let _keyboard = state .seat .add_keyboard(Default::default(), 200, 200) .location(loc!())?; - let _pointer = loop_data.state.seat.add_pointer(); + let _pointer = state.seat.add_pointer(); event_loop .handle() - .insert_source(reader, |event, _metadata, loop_data| { + .insert_source(reader, |event, _metadata, state| { match event { - Event::Msg(msg) => loop_data.state.handle_event(msg), + Event::Msg(msg) => state.handle_event(msg), Event::Closed => { unreachable!("reader is an in-memory channel whose write end has the same lifetime as serializer: the lifetime of the program.") }, } }).unwrap(); - let frame_interval = Duration::from_secs_f64(1.0 / (config.framerate as f64)); - event_loop - .handle() - .insert_source(Timer::immediate(), move |_, _, loop_data| { - if loop_data.state.serializer.other_end_connected() { - loop_data.state.send_callbacks(frame_interval); - } - TimeoutAction::ToDuration(Duration::from_millis(1)) - }) - .unwrap(); - event_loop - .run(Duration::from_millis(1), &mut loop_data, move |loop_data| { - loop_data.display.flush_clients().unwrap(); + .run(None, &mut state, move |state| { + state.dh.flush_clients().unwrap(); }) .location(loc!())?; diff --git a/src/bin/xwayland-xdg-shell.rs b/src/bin/xwayland-xdg-shell.rs index 627b3097..ccc8de2a 100644 --- a/src/bin/xwayland-xdg-shell.rs +++ b/src/bin/xwayland-xdg-shell.rs @@ -13,9 +13,7 @@ // limitations under the License. use std::ffi::OsString; -use std::io::ErrorKind; use std::path::PathBuf; -use std::time::Duration; use bpaf::Parser; use optional_struct::optional_struct; @@ -29,7 +27,7 @@ use smithay::reexports::calloop::Mode; use smithay::reexports::calloop::PostAction; use smithay::reexports::wayland_server::Display; use smithay::wayland::socket::ListeningSocketSource; -use smithay_client_toolkit::reexports::client::backend::WaylandError; +use smithay_client_toolkit::reexports::calloop_wayland_source::WaylandSource; use smithay_client_toolkit::reexports::client::globals::registry_queue_init; use smithay_client_toolkit::reexports::client::Connection; use tracing::Level; @@ -40,7 +38,6 @@ use wprs::args::SerializableLevel; use wprs::prelude::*; use wprs::utils; use wprs::xwayland_xdg_shell::compositor::DecorationBehavior; -use wprs::xwayland_xdg_shell::CalloopData; use wprs::xwayland_xdg_shell::WprsState; #[optional_struct] @@ -141,8 +138,8 @@ impl OptionalConfig for OptionalXwaylandXdgShellConfig { fn init_wayland_listener( wayland_display: &str, - display: &mut Display, - event_loop: &EventLoop, + mut display: Display, + event_loop: &EventLoop, ) -> Result { let listening_socket = ListeningSocketSource::with_name(wayland_display).location(loc!())?; let socket_name = listening_socket.socket_name().to_os_string(); @@ -155,11 +152,8 @@ fn init_wayland_listener( Interest::READ, Mode::Level, ), - |_, _, loop_data| { - loop_data - .display - .dispatch_clients(&mut loop_data.state) - .unwrap(); + move |_, _, state| { + display.dispatch_clients(state).unwrap(); Ok(PostAction::Continue) }, ) @@ -184,38 +178,32 @@ pub fn main() -> Result<()> { let display: Display = Display::new().location(loc!())?; let conn = Connection::connect_to_env().location(loc!())?; - let (globals, mut event_queue) = registry_queue_init(&conn).location(loc!())?; + let (globals, event_queue) = registry_queue_init(&conn).location(loc!())?; + + let mut state = WprsState::new( + display.handle(), + &globals, + event_queue.handle(), + conn.clone(), + event_loop.handle(), + config.decoration_behavior, + ) + .location(loc!())?; - let mut loop_data = CalloopData { - state: WprsState::new( - display.handle(), - &globals, - event_queue.handle(), - conn, - event_loop.handle(), - config.decoration_behavior, - ) - .location(loc!())?, - display, - globals, - }; + init_wayland_listener(&config.wayland_display, display, &event_loop).location(loc!())?; - init_wayland_listener(&config.wayland_display, &mut loop_data.display, &event_loop) - .location(loc!())?; - - let seat = &mut loop_data.state.compositor_state.seat; + let seat = &mut state.compositor_state.seat; // TODO: do this in WprsState::new; let _keyboard = seat .add_keyboard(Default::default(), 200, 200) .location(loc!())?; let _pointer = seat.add_pointer(); - loop_data - .state + state .compositor_state .xwayland .start( - loop_data.state.event_loop_handle.clone(), + state.event_loop_handle.clone(), config.display, vec![( "WAYLAND_DEBUG", @@ -230,33 +218,13 @@ pub fn main() -> Result<()> { ) .context(loc!(), "Error starting Xwayland.")?; - event_loop - .run(Duration::from_millis(1), &mut loop_data, move |loop_data| { - // WouldBlock means that the compositor can't keep up with our messages. - // Seems rare, so we will retry again on next loop. - - match event_queue.flush() { - Ok(_) => {}, - Err(WaylandError::Io(err)) if err.kind() == ErrorKind::WouldBlock => { - warn!("{err:?}"); - }, - _ => { - panic!("Error writing to wayland connection."); - }, - }; - - match event_queue.prepare_read().unwrap().read() { - Ok(_) => { - event_queue.dispatch_pending(&mut loop_data.state).unwrap(); - }, - Err(WaylandError::Io(err)) if err.kind() == ErrorKind::WouldBlock => {}, - _ => { - panic!("Error reading from wayland connection."); - }, - }; + WaylandSource::new(conn, event_queue) + .insert(event_loop.handle()) + .location(loc!())?; - // compositor stuff - loop_data.display.flush_clients().unwrap(); + event_loop + .run(None, &mut state, move |state| { + state.dh.flush_clients().unwrap(); }) .context(loc!(), "Error starting event loop.")?; Ok(()) diff --git a/src/client/mod.rs b/src/client/mod.rs index 5e1356b9..fed36229 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -93,10 +93,6 @@ impl ObjectBimapExt for ObjectBimap { } } -pub struct CalloopData { - pub state: WprsClientState, -} - pub struct ClientOptions { pub title_prefix: String, } diff --git a/src/compositor_utils.rs b/src/compositor_utils.rs index 1aa0b770..ddb00fb7 100644 --- a/src/compositor_utils.rs +++ b/src/compositor_utils.rs @@ -54,7 +54,7 @@ where // Based on https://github.com/Smithay/smithay/blob/b1c682742ac7b9fa08736476df3e651489709ac2/src/desktop/wayland/utils.rs. #[derive(Debug, Default)] -struct SurfaceFrameThrottlingState(Mutex>); +pub(crate) struct SurfaceFrameThrottlingState(Mutex>); impl SurfaceFrameThrottlingState { pub fn update(&self, time: Duration, throttle: Duration) -> bool { diff --git a/src/serialization/wayland.rs b/src/serialization/wayland.rs index d229540d..90fb4143 100644 --- a/src/serialization/wayland.rs +++ b/src/serialization/wayland.rs @@ -216,16 +216,9 @@ impl Buffer { let self_data = match Arc::get_mut(&mut self.data) { Some(self_data) => self_data, None => { - // TODO: this happens more frequently than we'd like. This seems - // to happen when a callback is sent shortly after a commit and - // then the client immediately commits again (as is its right). - // Sending callbacks is currently done on a timer, not related - // to commits. We used to send it right after a commit, but we - // didn't have a good way to throttle them there. Ideally we'd - // send them on a timer but delay sending them for specific - // surfaces if that surface's last commit is still being - // processed. Also change the log line to a warning after we fix - // this. + // TODO: this happens rarely but still more frequently than we'd + // like. Figure out why. Also change the log line to a warning + // after we fix this. debug!("Next commit received for surface before serialization of previous commit finished."); self.data = Arc::new(Vec4u8s::with_total_size(data.len())); // We just created the Arc, no one else can have a copy of it diff --git a/src/server/mod.rs b/src/server/mod.rs index c7455ba3..61db5d30 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -22,16 +22,15 @@ use std::time::Instant; use smithay::input::Seat; use smithay::input::SeatState; use smithay::output::Output; +use smithay::reexports::calloop::LoopHandle; use smithay::reexports::wayland_server::backend::GlobalId; use smithay::reexports::wayland_server::backend::ObjectId; use smithay::reexports::wayland_server::protocol::wl_data_source::WlDataSource; use smithay::reexports::wayland_server::protocol::wl_surface::WlSurface; -use smithay::reexports::wayland_server::Display; use smithay::reexports::wayland_server::DisplayHandle; use smithay::reexports::wayland_server::Resource; use smithay::wayland::compositor; use smithay::wayland::compositor::CompositorState; -use smithay::wayland::compositor::SurfaceAttributes; use smithay::wayland::compositor::SurfaceData; use smithay::wayland::compositor::TraversalAction; use smithay::wayland::selection::data_device::DataDeviceState; @@ -42,7 +41,6 @@ use smithay::wayland::shell::xdg::decoration::XdgDecorationState; use smithay::wayland::shm::ShmState; use smithay::reexports::wayland_protocols_misc::server_decoration::server::org_kde_kwin_server_decoration_manager::Mode as KdeDecorationMode; -use crate::compositor_utils; use crate::prelude::*; use crate::serialization::wayland::SurfaceRequest; use crate::serialization::wayland::SurfaceRequestPayload; @@ -81,15 +79,12 @@ fn surface_destruction_callback(state: &mut WprsServerState, surface: &WlSurface }); } -pub struct CalloopData { - pub state: WprsServerState, - pub display: Display, -} - pub struct WprsServerState { pub dh: DisplayHandle, + pub lh: LoopHandle<'static, Self>, pub compositor_state: CompositorState, pub start_time: Instant, + pub frame_interval: Duration, pub xwayland_enabled: bool, pub xdg_shell_state: XdgShellState, pub xdg_decoration_state: XdgDecorationState, @@ -128,8 +123,10 @@ pub struct WprsServerState { impl WprsServerState { pub fn new( dh: DisplayHandle, + lh: LoopHandle<'static, Self>, serializer: Serializer, xwayland_enabled: bool, + frame_interval: Duration, kde_server_side_decorations: bool, ) -> Self { let mut seat_state = SeatState::new(); @@ -142,9 +139,11 @@ impl WprsServerState { Self { dh: dh.clone(), + lh, compositor_state: CompositorState::new::(&dh), start_time: Instant::now(), xwayland_enabled, + frame_interval, xdg_shell_state: XdgShellState::new::(&dh), xdg_decoration_state: XdgDecorationState::new::(&dh), kde_decoration_state: KdeDecorationState::new::(&dh, kde_default_decoration_mode), @@ -208,19 +207,4 @@ impl WprsServerState { ) } } - - pub fn send_callbacks(&self, frame_interval: Duration) { - self.for_each_surface(|surface, surface_data| { - let mut surface_attributes = surface_data.cached_state.current::(); - - compositor_utils::send_frames( - surface, - &surface_data.data_map, - &mut surface_attributes, - self.start_time.elapsed(), - frame_interval, - ) - .log_and_ignore(loc!()); - }); - } } diff --git a/src/server/smithay_handlers.rs b/src/server/smithay_handlers.rs index 0a4f9e13..804ef03f 100644 --- a/src/server/smithay_handlers.rs +++ b/src/server/smithay_handlers.rs @@ -13,8 +13,10 @@ // limitations under the License. /// Handlers for events from Smithay. +use std::mem; use std::os::fd::OwnedFd; use std::sync::Arc; +use std::time::Duration; use crossbeam_channel::Sender; use smithay::backend::renderer::utils::on_commit_buffer_handler; @@ -38,6 +40,8 @@ use smithay::input::pointer::RelativeMotionEvent; use smithay::input::Seat; use smithay::input::SeatHandler; use smithay::input::SeatState; +use smithay::reexports::calloop::timer::TimeoutAction; +use smithay::reexports::calloop::timer::Timer; use smithay::reexports::wayland_protocols::xdg::decoration::zv1::server::zxdg_toplevel_decoration_v1::Mode as XdgDecorationMode; use smithay::reexports::wayland_protocols_misc::server_decoration::server::org_kde_kwin_server_decoration::Mode as KdeDecorationMode; use smithay::reexports::wayland_protocols_misc::server_decoration::server::org_kde_kwin_server_decoration::OrgKdeKwinServerDecoration; @@ -507,7 +511,7 @@ impl CompositorHandler for WprsServerState { // Send over the updated buffers from the children first so that the // client already has them when the parent is comitted. let children_dirty = commit_sync_children(self, surface, &commit).unwrap(); - commit(surface, self, children_dirty).log_and_ignore(loc!()); + commit(surface, self, children_dirty, false).log_and_ignore(loc!()); } } @@ -518,14 +522,14 @@ pub(crate) fn commit_sync_children( commit_fn: &F, ) -> Result where - F: Fn(&WlSurface, &mut T, bool) -> Result, + F: Fn(&WlSurface, &mut T, bool, bool) -> Result, { compositor::get_children(surface) .iter() .filter(|child| compositor::is_sync_subsurface(child)) .map(|child| { let children_dirty = commit_sync_children(state, child, commit_fn).location(loc!())?; - commit_fn(child, state, children_dirty).location(loc!()) + commit_fn(child, state, children_dirty, true).location(loc!()) }) .collect::>>() .location(loc!())? @@ -557,6 +561,7 @@ pub fn commit( surface: &WlSurface, state: &mut WprsServerState, children_dirty: bool, + skip_buffer: bool, ) -> Result { let surface_order = get_child_positions(surface); @@ -575,6 +580,7 @@ pub fn commit( parent, surface_order, children_dirty, + skip_buffer, ) }) .location(loc!())?; @@ -647,6 +653,7 @@ pub fn set_xdg_toplevel_attributes( Ok(()) } +#[allow(clippy::iter_with_drain)] #[instrument(skip(state), level = "debug")] pub fn commit_impl( surface: &WlSurface, @@ -656,6 +663,10 @@ pub fn commit_impl( parent: Option, surface_order: Vec, children_dirty: bool, + // TODO: This is a hack to stop sending the same buffer over twice. The + // subsurface logic needs another pass overall, we shouldn be able to avoid + // two back-to-back commits ina better way. + skip_buffer: bool, ) -> Result { let surface_state = &mut surface_data .data_map @@ -685,7 +696,48 @@ pub fn commit_impl( position: (0, 0).into(), }); - let surface_attributes = surface_data.cached_state.current::(); + let mut surface_attributes = surface_data.cached_state.current::(); + + let mut frame_callbacks = mem::take(&mut surface_attributes.frame_callbacks); + + if !frame_callbacks.is_empty() { + let surface = surface.clone(); + state + .lh + .insert_source( + Timer::from_duration( + state + .frame_interval + // "The server should give some time for the client to + // draw and commit after sending the frame callback + // events to let it hit the next output refresh." + .saturating_sub(Duration::from_millis(2)), + ), + move |_, _, state| { + if !surface.is_alive() { + return TimeoutAction::Drop; + } + + if state.serializer.other_end_connected() { + // We can't use into_iter() because we can't move + // frame_callbacks because this is a FnMut. However, this + // works because this branch will only ever be taken once. + for callback in frame_callbacks.drain(..) { + debug!( + "Sending callback for surface {:?}: {:?}", + surface.id(), + callback.id() + ); + callback.done(state.start_time.elapsed().as_millis() as u32); + } + TimeoutAction::Drop + } else { + TimeoutAction::ToDuration(state.frame_interval) + } + }, + ) + .expect("timer registration should never fail"); + } set_regions(&surface_attributes, surface_state); set_transformation(&surface_attributes, surface_state); @@ -710,7 +762,7 @@ pub fn commit_impl( // TODO: make a function and dedupe with compositor.rs. debug!("buffer assignment: {:?}", &surface_attributes.buffer); match &surface_attributes.buffer { - Some(SmithayBufferAssignment::NewBuffer(buffer)) => { + Some(SmithayBufferAssignment::NewBuffer(buffer)) if !skip_buffer => { compositor_utils::with_buffer_contents(buffer, |data, spec| { surface_state.set_buffer(&spec, data) }) @@ -746,7 +798,7 @@ pub fn commit_impl( surface_state.buffer = None; surface_state_to_send.buffer = Some(BufferAssignment::Removed); }, - None => { + Some(SmithayBufferAssignment::NewBuffer(_)) | None => { if (surface_state_to_send == prev_without_buffer) && !children_dirty { return Ok(false); } diff --git a/src/xwayland_xdg_shell/compositor.rs b/src/xwayland_xdg_shell/compositor.rs index cad2bde7..4b706dd3 100644 --- a/src/xwayland_xdg_shell/compositor.rs +++ b/src/xwayland_xdg_shell/compositor.rs @@ -77,7 +77,6 @@ use crate::serialization::wayland::OutputInfo; use crate::utils::SerialMap; use crate::xwayland_xdg_shell::client::Role; use crate::xwayland_xdg_shell::wmname; -use crate::xwayland_xdg_shell::CalloopData; use crate::xwayland_xdg_shell::WprsState; use crate::xwayland_xdg_shell::XWaylandSurface; @@ -118,7 +117,7 @@ impl WprsCompositorState { /// On failure launching xwayland. pub fn new( dh: DisplayHandle, - event_loop_handle: LoopHandle<'static, CalloopData>, + event_loop_handle: LoopHandle<'static, WprsState>, decoration_behavior: DecorationBehavior, ) -> Self { let mut seat_state = SeatState::new(); @@ -135,7 +134,7 @@ impl WprsCompositorState { display, } => { let wm = X11Wm::start_wm( - data.state.event_loop_handle.clone(), + data.event_loop_handle.clone(), dh.clone(), connection, client, @@ -146,10 +145,10 @@ impl WprsCompositorState { wmname::set_wmname(Some(&format!(":{}", display)), "LG3D") .expect("Failed to set WM name."); - data.state.compositor_state.xwm = Some(wm); + data.compositor_state.xwm = Some(wm); }, XWaylandEvent::Exited => { - let _ = data.state.compositor_state.xwm.take(); + let _ = data.compositor_state.xwm.take(); }, }); if let Err(e) = ret { @@ -253,9 +252,9 @@ fn execute_or_defer_commit(state: &mut WprsState, surface: WlSurface) -> Result< || is_cursor) { debug!("deferring commit"); - X11Wm::commit_hook::(&surface); - state.event_loop_handle.insert_idle(|loop_data| { - execute_or_defer_commit(&mut loop_data.state, surface).log_and_ignore(loc!()); + X11Wm::commit_hook::(&surface); + state.event_loop_handle.insert_idle(|state| { + execute_or_defer_commit(state, surface).log_and_ignore(loc!()); }); } Ok(()) diff --git a/src/xwayland_xdg_shell/mod.rs b/src/xwayland_xdg_shell/mod.rs index 64dd07d5..3432d544 100644 --- a/src/xwayland_xdg_shell/mod.rs +++ b/src/xwayland_xdg_shell/mod.rs @@ -23,7 +23,6 @@ use smithay::output::Output; use smithay::reexports::calloop::LoopHandle; use smithay::reexports::wayland_server::backend::ObjectId as CompositorObjectId; use smithay::reexports::wayland_server::protocol::wl_surface::WlSurface as CompositorWlSurface; -use smithay::reexports::wayland_server::Display; use smithay::reexports::wayland_server::DisplayHandle; use smithay::reexports::wayland_server::Resource; use smithay::utils::Serial; @@ -219,7 +218,7 @@ impl WaylandSurface for XWaylandSurface { #[derive(Debug)] pub struct WprsState { pub dh: DisplayHandle, - pub event_loop_handle: LoopHandle<'static, CalloopData>, + pub event_loop_handle: LoopHandle<'static, Self>, pub client_state: WprsClientState, pub compositor_state: WprsCompositorState, pub surface_bimap: BiMap, @@ -233,7 +232,7 @@ impl WprsState { globals: &GlobalList, qh: QueueHandle, conn: Connection, - event_loop_handle: LoopHandle<'static, CalloopData>, + event_loop_handle: LoopHandle<'static, Self>, decoration_behavior: DecorationBehavior, ) -> Result { Ok(Self { @@ -357,9 +356,3 @@ pub fn xsurface_from_x11_surface<'a>( .unwrap_or(false) }) } - -pub struct CalloopData { - pub state: WprsState, - pub display: Display, - pub globals: GlobalList, -} diff --git a/src/xwayland_xdg_shell/xwayland.rs b/src/xwayland_xdg_shell/xwayland.rs index d8627757..fdde3aef 100644 --- a/src/xwayland_xdg_shell/xwayland.rs +++ b/src/xwayland_xdg_shell/xwayland.rs @@ -32,11 +32,11 @@ use smithay::xwayland::XwmHandler; use crate::prelude::*; use crate::xwayland_xdg_shell::client::Role; use crate::xwayland_xdg_shell::xsurface_from_x11_surface; -use crate::xwayland_xdg_shell::CalloopData; +use crate::xwayland_xdg_shell::WprsState; -impl XwmHandler for CalloopData { +impl XwmHandler for WprsState { fn xwm_state(&mut self, _xwm: XwmId) -> &mut X11Wm { - self.state.compositor_state.xwm.as_mut().unwrap() + self.compositor_state.xwm.as_mut().unwrap() } fn new_window(&mut self, _xwm: XwmId, _window: X11Surface) {} @@ -48,11 +48,11 @@ impl XwmHandler for CalloopData { geo.loc.x = 0; geo.loc.y = 0; window.configure(geo).log_and_ignore(loc!()); - self.state.compositor_state.x11_surfaces.push(window); + self.compositor_state.x11_surfaces.push(window); } fn mapped_override_redirect_window(&mut self, _xwm: XwmId, window: X11Surface) { - self.state.compositor_state.x11_surfaces.push(window); + self.compositor_state.x11_surfaces.push(window); } #[instrument(skip(self, _xwm), level = "debug")] @@ -60,18 +60,18 @@ impl XwmHandler for CalloopData { if let Some(wl_surface) = window.wl_surface() { // TODO: verify that we don't end up with stale entries let surface_id = wl_surface.id(); - self.state.remove_surface(&surface_id); + self.remove_surface(&surface_id); // TODO: maybe do this on leave? // Without this, xwayland still thinks the key that triggered the // window close is still held down and sends key repeat events. - if let Some(keyboard) = self.state.compositor_state.seat.get_keyboard() { + if let Some(keyboard) = self.compositor_state.seat.get_keyboard() { if keyboard .current_focus() .map_or(false, |focus| focus == window) { let serial = SERIAL_COUNTER.next_serial(); - keyboard.set_focus(&mut self.state, None, serial); + keyboard.set_focus(self, None, serial); } } } @@ -143,8 +143,7 @@ impl XwmHandler for CalloopData { // need to worry about that saving the old geometry and restoring it here. fn maximize_request(&mut self, _xwm: XwmId, window: X11Surface) { - if let Some(xwayland_surface) = xsurface_from_x11_surface(&mut self.state.surfaces, &window) - { + if let Some(xwayland_surface) = xsurface_from_x11_surface(&mut self.surfaces, &window) { if let Some(Role::XdgToplevel(toplevel)) = &xwayland_surface.role { toplevel.local_window.set_maximized(); } else { @@ -156,8 +155,7 @@ impl XwmHandler for CalloopData { } fn unmaximize_request(&mut self, _xwm: XwmId, window: X11Surface) { - if let Some(xwayland_surface) = xsurface_from_x11_surface(&mut self.state.surfaces, &window) - { + if let Some(xwayland_surface) = xsurface_from_x11_surface(&mut self.surfaces, &window) { if let Some(Role::XdgToplevel(toplevel)) = &xwayland_surface.role { toplevel.local_window.unset_maximized(); } else { @@ -169,8 +167,7 @@ impl XwmHandler for CalloopData { } fn fullscreen_request(&mut self, _xwm: XwmId, window: X11Surface) { - if let Some(xwayland_surface) = xsurface_from_x11_surface(&mut self.state.surfaces, &window) - { + if let Some(xwayland_surface) = xsurface_from_x11_surface(&mut self.surfaces, &window) { if let Some(Role::XdgToplevel(toplevel)) = &mut xwayland_surface.role { toplevel.local_window.set_fullscreen(None); } else { @@ -182,8 +179,7 @@ impl XwmHandler for CalloopData { } fn unfullscreen_request(&mut self, _xwm: XwmId, window: X11Surface) { - if let Some(xwayland_surface) = xsurface_from_x11_surface(&mut self.state.surfaces, &window) - { + if let Some(xwayland_surface) = xsurface_from_x11_surface(&mut self.surfaces, &window) { if let Some(Role::XdgToplevel(toplevel)) = &mut xwayland_surface.role { toplevel.local_window.unset_fullscreen(); } else { @@ -212,7 +208,7 @@ impl XwmHandler for CalloopData { fn allow_selection_access(&mut self, _xwm: XwmId, selection: SelectionTarget) -> bool { true // TODO: the below should be correct but needs to be verified. - // !self.state.client_state.selection_offers.is_empty() + // !self.client_state.selection_offers.is_empty() } #[instrument(skip(self, _xwm), level = "debug")] @@ -225,8 +221,7 @@ impl XwmHandler for CalloopData { ) { let read_pipe = match selection { SelectionTarget::Primary => { - let Some(cur_offer) = self.state.client_state.primary_selection_offer.clone() - else { + let Some(cur_offer) = self.client_state.primary_selection_offer.clone() else { warn!("primary_selection_offer was empty"); return; }; @@ -234,7 +229,7 @@ impl XwmHandler for CalloopData { cur_offer.receive(mime_type.clone()).ok() }, SelectionTarget::Clipboard => { - let Some(cur_offer) = self.state.client_state.selection_offer.clone() else { + let Some(cur_offer) = self.client_state.selection_offer.clone() else { warn!("selection_offer was empty"); return; }; @@ -267,43 +262,42 @@ impl XwmHandler for CalloopData { selection: SelectionTarget, mut mime_types: Vec, ) { - if let Some(seat_obj) = self.state.client_state.seat_objects.last() { + if let Some(seat_obj) = self.client_state.seat_objects.last() { mime_types.push("_xwayland_xdg_shell_marker".to_owned()); match selection { SelectionTarget::Clipboard => { let source = self - .state .client_state .data_device_manager_state .create_copy_paste_source( - &self.state.client_state.qh, + &self.client_state.qh, mime_types.iter().map(String::as_str), ); source.set_selection( &seat_obj.data_device, - self.state.client_state.last_implicit_grab_serial, + self.client_state.last_implicit_grab_serial, ); - self.state.client_state.selection_source = Some(source); + self.client_state.selection_source = Some(source); }, SelectionTarget::Primary => { if let (Some(primary_selection_manager_state), Some(primary_selection_device)) = ( - &self.state.client_state.primary_selection_manager_state, + &self.client_state.primary_selection_manager_state, &seat_obj.primary_selection_device, ) { let source = primary_selection_manager_state.create_selection_source( - &self.state.client_state.qh, + &self.client_state.qh, mime_types.iter().map(String::as_str), ); source.set_selection( primary_selection_device, - self.state.client_state.last_implicit_grab_serial, + self.client_state.last_implicit_grab_serial, ); - self.state.client_state.primary_selection_source = Some(source); + self.client_state.primary_selection_source = Some(source); } }, };