From 5cce3208e21158270a19667753479dd85f5d64f1 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Wed, 13 Sep 2023 19:16:11 +0100 Subject: [PATCH 01/13] Update calloop and wayland-backend --- Cargo.toml | 6 +-- src/lib.rs | 110 ++++++++++++++++++++++++++++++----------------------- 2 files changed, 66 insertions(+), 50 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 43df72c..a0e8d53 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,8 +11,8 @@ keywords = ["wayland", "windowing"] rust-version = "1.65.0" [dependencies] -wayland-client = "0.30.2" -wayland-backend = "0.1.0" -calloop = "0.10.0" +wayland-client = "0.31.0" +wayland-backend = "0.3.0" +calloop = "0.12.0" log = { version = "0.4.19", optional = true } rustix = { version = "0.38.4", default-features = false, features = ["std"] } diff --git a/src/lib.rs b/src/lib.rs index 6737e2c..da25c66 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -31,7 +31,7 @@ use std::io; use std::os::unix::io::{AsRawFd, RawFd}; -use calloop::generic::Generic; +use calloop::generic::{FdWrapper, Generic}; use calloop::{ EventSource, InsertError, Interest, LoopHandle, Mode, Poll, PostAction, Readiness, RegistrationToken, Token, TokenFactory, @@ -58,18 +58,35 @@ use std::eprintln as log_error; #[derive(Debug)] pub struct WaylandSource { queue: EventQueue, - fd: Generic, + fd: Generic>, read_guard: Option, + fake_token: Option, + stored_error: Option, } impl WaylandSource { /// Wrap an [`EventQueue`] as a [`WaylandSource`]. - pub fn new(queue: EventQueue) -> Result, WaylandError> { + /// + /// If this returns None, that means that acquiring a read guard failed. + /// See [EventQueue::prepare_read] for details + /// This guard is only used to get the wayland file descriptor + pub fn new(queue: EventQueue) -> Option> { let guard = queue.prepare_read()?; - let fd = Generic::new(guard.connection_fd().as_raw_fd(), Interest::READ, Mode::Level); + let fd = Generic::new( + // Safety: `connection_fd` returns the wayland socket fd, + // and EventQueue (eventually) owns this socket + // fd is only used in calloop, which guarantees + // that the source is unregistered before dropping it, so the + // fd cannot be used after being freed + // Wayland-backend should probably document some guarantees here to make this sound, + // but this is unlikely to change + unsafe { FdWrapper::new(guard.connection_fd().as_raw_fd()) }, + Interest::READ, + Mode::Level, + ); drop(guard); - Ok(WaylandSource { queue, fd, read_guard: None }) + Some(WaylandSource { queue, fd, read_guard: None, fake_token: None, stored_error: None }) } /// Access the underlying event queue @@ -114,24 +131,15 @@ impl EventSource for WaylandSource { F: FnMut(Self::Event, &mut Self::Metadata) -> Self::Ret, { let queue = &mut self.queue; - let read_guard = &mut self.read_guard; - - let action = self.fd.process_events(readiness, token, |_, _| { - // 1. read events from the socket if any are available - if let Some(guard) = read_guard.take() { - // might be None if some other thread read events before us, concurrently - if let Err(WaylandError::Io(err)) = guard.read() { - if err.kind() != io::ErrorKind::WouldBlock { - return Err(err); - } - } - } + if let Some(err) = self.stored_error.take() { + return Err(err)?; + } + let mut callback = || { // 2. dispatch any pending events in the queue // This is done to ensure we are not waiting for messages that are already in // the buffer. Self::loop_callback_pending(queue, &mut callback)?; - *read_guard = Some(Self::prepare_read(queue)?); // 3. Once dispatching is finished, flush the responses to the compositor if let Err(WaylandError::Io(e)) = queue.flush() { @@ -146,7 +154,11 @@ impl EventSource for WaylandSource { } Ok(PostAction::Continue) - })?; + }; + if Some(token) == self.fake_token { + callback()?; + } + let action = self.fd.process_events(readiness, token, |_, _| callback())?; Ok(action) } @@ -156,6 +168,7 @@ impl EventSource for WaylandSource { poll: &mut Poll, token_factory: &mut TokenFactory, ) -> calloop::Result<()> { + self.fake_token = Some(token_factory.token()); self.fd.register(poll, token_factory) } @@ -171,10 +184,10 @@ impl EventSource for WaylandSource { self.fd.unregister(poll) } - fn pre_run(&mut self, mut callback: F) -> calloop::Result<()> - where - F: FnMut((), &mut Self::Metadata) -> Self::Ret, - { + // This is quite subtle + const NEEDS_EXTRA_LIFECYCLE_EVENTS: bool = true; + + fn before_sleep(&mut self) -> calloop::Result> { debug_assert!(self.read_guard.is_none()); // flush the display before starting to poll @@ -187,20 +200,35 @@ impl EventSource for WaylandSource { } } - // ensure we are not waiting for messages that are already in the buffer. - Self::loop_callback_pending(&mut self.queue, &mut callback)?; - self.read_guard = Some(Self::prepare_read(&mut self.queue)?); - - Ok(()) + // TODO: ensure we are not waiting for messages that are already in the buffer. + // is this needed? + // Self::loop_callback_pending(&mut self.queue, &mut callback)?; + self.read_guard = self.queue.prepare_read(); + match self.read_guard { + // If getting the guard failed + Some(_) => Ok(None), + // The readiness value is never used, we just need some marker + None => Ok(Some((Readiness::EMPTY, self.fake_token.unwrap()))), + } } - fn post_run(&mut self, _: F) -> calloop::Result<()> - where - F: FnMut((), &mut Self::Metadata) -> Self::Ret, - { - // Drop implementation of ReadEventsGuard will do cleanup - self.read_guard.take(); - Ok(()) + fn before_handle_events(&mut self, events: calloop::EventIterator<'_>) { + let guard = self.read_guard.take(); + if events.count() > 0 { + // 1. read events from the socket if any are available + // If none are available, that means + if let Some(guard) = guard { + // might be None if some other thread read events before us, concurrently + if let Err(WaylandError::Io(err)) = guard.read() { + if err.kind() != io::ErrorKind::WouldBlock { + // before_handle_events doesn't allow returning errors + // For now, cache it in self until process_events is called + self.stored_error = Some(err); + } + } + } + } + // Otherwise, drop the guard, as we don't want to do any reading when the fd is not polled } } @@ -237,16 +265,4 @@ impl WaylandSource { } } } - - fn prepare_read(queue: &mut EventQueue) -> io::Result { - queue.prepare_read().map_err(|err| match err { - WaylandError::Io(err) => err, - - WaylandError::Protocol(err) => { - log_error!("Protocol error received on display: {}", err); - - Errno::PROTO.into() - }, - }) - } } From 1e339436cc1f6ccd4b4ad8564d2ff5f3155b5520 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Wed, 13 Sep 2023 19:18:37 +0100 Subject: [PATCH 02/13] Update some docs --- src/lib.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index da25c66..af9afcc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -184,7 +184,6 @@ impl EventSource for WaylandSource { self.fd.unregister(poll) } - // This is quite subtle const NEEDS_EXTRA_LIFECYCLE_EVENTS: bool = true; fn before_sleep(&mut self) -> calloop::Result> { @@ -201,13 +200,16 @@ impl EventSource for WaylandSource { } // TODO: ensure we are not waiting for messages that are already in the buffer. - // is this needed? + // is this needed? I think in the case that this would occur, getting the guard + // would fail anyway // Self::loop_callback_pending(&mut self.queue, &mut callback)?; self.read_guard = self.queue.prepare_read(); match self.read_guard { // If getting the guard failed Some(_) => Ok(None), // The readiness value is never used, we just need some marker + // If getting the guard failed, we need to process the events 'instantly' + // tell calloop this None => Ok(Some((Readiness::EMPTY, self.fake_token.unwrap()))), } } @@ -228,7 +230,8 @@ impl EventSource for WaylandSource { } } } - // Otherwise, drop the guard, as we don't want to do any reading when the fd is not polled + // Otherwise, drop the guard if we have it, as we don't want to do any reading when we didn't + // get any events } } From 8e1db027df2ea398aac5ceae54a1e819493aff8a Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Wed, 13 Sep 2023 20:17:00 +0100 Subject: [PATCH 03/13] Run nightly formatting --- src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index af9afcc..6a71983 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -121,6 +121,8 @@ impl EventSource for WaylandSource { type Metadata = EventQueue; type Ret = Result; + const NEEDS_EXTRA_LIFECYCLE_EVENTS: bool = true; + fn process_events( &mut self, readiness: Readiness, @@ -184,8 +186,6 @@ impl EventSource for WaylandSource { self.fd.unregister(poll) } - const NEEDS_EXTRA_LIFECYCLE_EVENTS: bool = true; - fn before_sleep(&mut self) -> calloop::Result> { debug_assert!(self.read_guard.is_none()); @@ -230,8 +230,8 @@ impl EventSource for WaylandSource { } } } - // Otherwise, drop the guard if we have it, as we don't want to do any reading when we didn't - // get any events + // Otherwise, drop the guard if we have it, as we don't want to do any + // reading when we didn't get any events } } From ebeb3444e958582e466211cbd1f3e0fe06172d16 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Thu, 14 Sep 2023 16:01:59 +0100 Subject: [PATCH 04/13] Address review comments --- src/lib.rs | 54 +++++++++++++++++++++++------------------------------- 1 file changed, 23 insertions(+), 31 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 6a71983..500282e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -125,8 +125,8 @@ impl EventSource for WaylandSource { fn process_events( &mut self, - readiness: Readiness, - token: Token, + _: Readiness, + _: Token, mut callback: F, ) -> Result where @@ -137,32 +137,27 @@ impl EventSource for WaylandSource { return Err(err)?; } - let mut callback = || { - // 2. dispatch any pending events in the queue - // This is done to ensure we are not waiting for messages that are already in - // the buffer. - Self::loop_callback_pending(queue, &mut callback)?; + // In theory, we would need to call the process_events handler on fd. However, + // we know that Generic's process events's call is a no-op, so we can just handle the event ourselves - // 3. Once dispatching is finished, flush the responses to the compositor - if let Err(WaylandError::Io(e)) = queue.flush() { - if e.kind() != io::ErrorKind::WouldBlock { - // in case of error, forward it and fast-exit - return Err(e); - } - // WouldBlock error means the compositor could not process all - // our messages quickly. Either it is slowed - // down or we are a spammer. Should not really - // happen, if it does we do nothing and will flush again later - } + // 2. dispatch any pending events in the queue + // This is done to ensure we are not waiting for messages that are already in + // the buffer. + Self::loop_callback_pending(queue, &mut callback)?; - Ok(PostAction::Continue) - }; - if Some(token) == self.fake_token { - callback()?; + // 3. Once dispatching is finished, flush the responses to the compositor + if let Err(WaylandError::Io(e)) = queue.flush() { + if e.kind() != io::ErrorKind::WouldBlock { + // in case of error, forward it and fast-exit + return Err(e.into()); + } + // WouldBlock error means the compositor could not process all + // our messages quickly. Either it is slowed + // down or we are a spammer. Should not really + // happen, if it does we do nothing and will flush again later } - let action = self.fd.process_events(readiness, token, |_, _| callback())?; - Ok(action) + Ok(PostAction::Continue) } fn register( @@ -199,14 +194,11 @@ impl EventSource for WaylandSource { } } - // TODO: ensure we are not waiting for messages that are already in the buffer. - // is this needed? I think in the case that this would occur, getting the guard - // would fail anyway - // Self::loop_callback_pending(&mut self.queue, &mut callback)?; self.read_guard = self.queue.prepare_read(); match self.read_guard { - // If getting the guard failed Some(_) => Ok(None), + // If getting the guard failed, that means that there are + // events in the queue, and // The readiness value is never used, we just need some marker // If getting the guard failed, we need to process the events 'instantly' // tell calloop this @@ -218,10 +210,10 @@ impl EventSource for WaylandSource { let guard = self.read_guard.take(); if events.count() > 0 { // 1. read events from the socket if any are available - // If none are available, that means if let Some(guard) = guard { - // might be None if some other thread read events before us, concurrently if let Err(WaylandError::Io(err)) = guard.read() { + // If some other thread read events before us, concurrently, that's an expected + // case, so this error isn't an issue. Other error kinds do need to be returned, however if err.kind() != io::ErrorKind::WouldBlock { // before_handle_events doesn't allow returning errors // For now, cache it in self until process_events is called From 8cdd7dda9fb857e1813349b3569a52439cc2e865 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Thu, 14 Sep 2023 20:08:01 +0100 Subject: [PATCH 05/13] Address review comments --- src/lib.rs | 43 +++++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 500282e..07f6f13 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -60,8 +60,13 @@ pub struct WaylandSource { queue: EventQueue, fd: Generic>, read_guard: Option, + /// Calloop's before_will_sleep method allows + /// skipping the sleeping by returning a `Token`. + /// We cannot produce this on the fly, so store it here instead fake_token: Option, - stored_error: Option, + // Some calloop event handlers don't support error handling, so we have to store the error + // for a short time until we reach a method which allows it + stored_error: Result<(), io::Error>, } impl WaylandSource { @@ -70,6 +75,7 @@ impl WaylandSource { /// If this returns None, that means that acquiring a read guard failed. /// See [EventQueue::prepare_read] for details /// This guard is only used to get the wayland file descriptor + #[must_use] pub fn new(queue: EventQueue) -> Option> { let guard = queue.prepare_read()?; let fd = Generic::new( @@ -79,14 +85,15 @@ impl WaylandSource { // that the source is unregistered before dropping it, so the // fd cannot be used after being freed // Wayland-backend should probably document some guarantees here to make this sound, - // but this is unlikely to change + // but we know that they are unlikely to make the queue have a different socket/fd + // - there is no public API to do so unsafe { FdWrapper::new(guard.connection_fd().as_raw_fd()) }, Interest::READ, Mode::Level, ); drop(guard); - Some(WaylandSource { queue, fd, read_guard: None, fake_token: None, stored_error: None }) + Some(WaylandSource { queue, fd, read_guard: None, fake_token: None, stored_error: Ok(()) }) } /// Access the underlying event queue @@ -133,16 +140,15 @@ impl EventSource for WaylandSource { F: FnMut(Self::Event, &mut Self::Metadata) -> Self::Ret, { let queue = &mut self.queue; - if let Some(err) = self.stored_error.take() { - return Err(err)?; - } + // Take the stored error + std::mem::replace(&mut self.stored_error, Ok(()))?; - // In theory, we would need to call the process_events handler on fd. However, - // we know that Generic's process events's call is a no-op, so we can just handle the event ourselves + // Because our polling is based on a `Generic` source, in theory we might want + // to to call the process_events handler on fd. However, + // we know that Generic's `process_events` call is a no-op, so we can just + // handle the event ourselves - // 2. dispatch any pending events in the queue - // This is done to ensure we are not waiting for messages that are already in - // the buffer. + // Dispatch any pending events in the queue Self::loop_callback_pending(queue, &mut callback)?; // 3. Once dispatching is finished, flush the responses to the compositor @@ -198,10 +204,10 @@ impl EventSource for WaylandSource { match self.read_guard { Some(_) => Ok(None), // If getting the guard failed, that means that there are - // events in the queue, and - // The readiness value is never used, we just need some marker - // If getting the guard failed, we need to process the events 'instantly' - // tell calloop this + // events in the queue, and so we need to handle the events instantly + // rather than waiting on an event in polling. We tell calloop this + // by returning Some here. Note that the readiness value is + // never used, so we just need some marker None => Ok(Some((Readiness::EMPTY, self.fake_token.unwrap()))), } } @@ -209,15 +215,16 @@ impl EventSource for WaylandSource { fn before_handle_events(&mut self, events: calloop::EventIterator<'_>) { let guard = self.read_guard.take(); if events.count() > 0 { - // 1. read events from the socket if any are available + // Read events from the socket if any are available if let Some(guard) = guard { if let Err(WaylandError::Io(err)) = guard.read() { // If some other thread read events before us, concurrently, that's an expected - // case, so this error isn't an issue. Other error kinds do need to be returned, however + // case, so this error isn't an issue. Other error kinds do need to be returned, + // however if err.kind() != io::ErrorKind::WouldBlock { // before_handle_events doesn't allow returning errors // For now, cache it in self until process_events is called - self.stored_error = Some(err); + self.stored_error = Err(err); } } } From 1bd59099ed7b6d0cd9aed563f2b160d38adf2571 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Thu, 14 Sep 2023 20:09:33 +0100 Subject: [PATCH 06/13] Remove dangling 3. --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 07f6f13..cc56cb9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -151,7 +151,7 @@ impl EventSource for WaylandSource { // Dispatch any pending events in the queue Self::loop_callback_pending(queue, &mut callback)?; - // 3. Once dispatching is finished, flush the responses to the compositor + // Once dispatching is finished, flush the responses to the compositor if let Err(WaylandError::Io(e)) = queue.flush() { if e.kind() != io::ErrorKind::WouldBlock { // in case of error, forward it and fast-exit From 2f796560f82ef43d0a559249cfb55938745086b5 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Thu, 14 Sep 2023 20:14:42 +0100 Subject: [PATCH 07/13] Move flush into its own function --- src/lib.rs | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index cc56cb9..a702201 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -152,16 +152,7 @@ impl EventSource for WaylandSource { Self::loop_callback_pending(queue, &mut callback)?; // Once dispatching is finished, flush the responses to the compositor - if let Err(WaylandError::Io(e)) = queue.flush() { - if e.kind() != io::ErrorKind::WouldBlock { - // in case of error, forward it and fast-exit - return Err(e.into()); - } - // WouldBlock error means the compositor could not process all - // our messages quickly. Either it is slowed - // down or we are a spammer. Should not really - // happen, if it does we do nothing and will flush again later - } + flush_queue(queue)?; Ok(PostAction::Continue) } @@ -190,15 +181,7 @@ impl EventSource for WaylandSource { fn before_sleep(&mut self) -> calloop::Result> { debug_assert!(self.read_guard.is_none()); - // flush the display before starting to poll - if let Err(WaylandError::Io(err)) = self.queue.flush() { - if err.kind() != io::ErrorKind::WouldBlock { - // in case of error, don't prepare a read, if the error is persistent, it'll - // trigger in other wayland methods anyway - log_error!("Error trying to flush the wayland display: {}", err); - return Err(err.into()); - } - } + flush_queue(&mut self.queue)?; self.read_guard = self.queue.prepare_read(); match self.read_guard { @@ -234,6 +217,21 @@ impl EventSource for WaylandSource { } } +fn flush_queue(queue: &mut EventQueue) -> Result<(), calloop::Error> { + if let Err(WaylandError::Io(err)) = queue.flush() { + if err.kind() != io::ErrorKind::WouldBlock { + // in case of error, forward it and fast-exit + log_error!("Error trying to flush the wayland display: {}", err); + return Err(err.into()); + } + // WouldBlock error means the compositor could not process all + // our messages quickly. Either it is slowed + // down or we are a spammer. Should not really + // happen, if it does we do nothing and will flush again later + } + Ok(()) +} + impl WaylandSource { /// Loop over the callback until all pending messages have been dispatched. fn loop_callback_pending(queue: &mut EventQueue, callback: &mut F) -> io::Result<()> From 54f4d0dfe686173aaf3b17a6a3f75dee41b484d4 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Fri, 15 Sep 2023 18:26:20 +0100 Subject: [PATCH 08/13] Move the explanatory comment --- src/lib.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index a702201..b656a5a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -139,6 +139,8 @@ impl EventSource for WaylandSource { where F: FnMut(Self::Event, &mut Self::Metadata) -> Self::Ret, { + debug_assert!(self.read_guard.is_none()); + let queue = &mut self.queue; // Take the stored error std::mem::replace(&mut self.stored_error, Ok(()))?; @@ -196,6 +198,9 @@ impl EventSource for WaylandSource { } fn before_handle_events(&mut self, events: calloop::EventIterator<'_>) { + // It's important that the guard isn't held whilst process_events calls occur + // This can use arbitrary user-provided code, which may want to use the wayland socket + // For example, creating a Vulkan surface needs access to the connection let guard = self.read_guard.take(); if events.count() > 0 { // Read events from the socket if any are available @@ -212,8 +217,6 @@ impl EventSource for WaylandSource { } } } - // Otherwise, drop the guard if we have it, as we don't want to do any - // reading when we didn't get any events } } From ee6b60564f18c9d2265f061294d60b7b1dd45cef Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Fri, 15 Sep 2023 18:27:08 +0100 Subject: [PATCH 09/13] Do nightly formatting --- src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index b656a5a..8527e18 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -199,8 +199,9 @@ impl EventSource for WaylandSource { fn before_handle_events(&mut self, events: calloop::EventIterator<'_>) { // It's important that the guard isn't held whilst process_events calls occur - // This can use arbitrary user-provided code, which may want to use the wayland socket - // For example, creating a Vulkan surface needs access to the connection + // This can use arbitrary user-provided code, which may want to use the wayland + // socket For example, creating a Vulkan surface needs access to the + // connection let guard = self.read_guard.take(); if events.count() > 0 { // Read events from the socket if any are available From b73f07df409ff2b7955a9346d0e9b4a04f7a13e0 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Sat, 16 Sep 2023 18:53:18 +0100 Subject: [PATCH 10/13] Add an example (reproduced mostly from wayland-client) --- Cargo.toml | 6 +- examples/simplest-window.rs | 242 ++++++++++++++++++++++++++++++++++++ 2 files changed, 247 insertions(+), 1 deletion(-) create mode 100644 examples/simplest-window.rs diff --git a/Cargo.toml b/Cargo.toml index a0e8d53..d25dcdd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,6 +13,10 @@ rust-version = "1.65.0" [dependencies] wayland-client = "0.31.0" wayland-backend = "0.3.0" -calloop = "0.12.0" +calloop = { git = "https://github.com/DJMcNab/calloop/", rev = "9df1f5c949c3d7f23817b2bcbd5c481ff0ec7e65" } log = { version = "0.4.19", optional = true } rustix = { version = "0.38.4", default-features = false, features = ["std"] } + +[dev-dependencies] +tempfile = "3.8.0" +wayland-protocols = { version = "0.31.0", features = ["client"] } diff --git a/examples/simplest-window.rs b/examples/simplest-window.rs new file mode 100644 index 0000000..fd47b4e --- /dev/null +++ b/examples/simplest-window.rs @@ -0,0 +1,242 @@ +use std::{fs::File, os::unix::prelude::AsFd}; + +use calloop::EventLoop; +use calloop_wayland_source::WaylandSource; +use wayland_client::{ + delegate_noop, + protocol::{ + wl_buffer, wl_compositor, wl_keyboard, wl_registry, wl_seat, wl_shm, wl_shm_pool, + wl_surface, + }, + Connection, Dispatch, QueueHandle, WEnum, +}; + +use wayland_protocols::xdg::shell::client::{xdg_surface, xdg_toplevel, xdg_wm_base}; + +fn main() { + let conn = Connection::connect_to_env().unwrap(); + + let event_queue = conn.new_event_queue(); + let qhandle = event_queue.handle(); + + let display = conn.display(); + display.get_registry(&qhandle, ()); + + let mut event_loop: EventLoop = + EventLoop::try_new().expect("Failed to initialize the event loop!"); + let loop_handle = event_loop.handle(); + WaylandSource::new(event_queue).unwrap().insert(loop_handle).unwrap(); + let stop_handle = event_loop.get_signal(); + + let mut state = State { + running: true, + base_surface: None, + buffer: None, + wm_base: None, + xdg_surface: None, + configured: false, + stop_handle, + }; + + println!("Starting the example window app, press to quit."); + + event_loop.run(None, &mut state, |_| {}).unwrap(); +} + +struct State { + running: bool, + base_surface: Option, + buffer: Option, + wm_base: Option, + xdg_surface: Option<(xdg_surface::XdgSurface, xdg_toplevel::XdgToplevel)>, + configured: bool, + stop_handle: calloop::LoopSignal, +} + +impl Dispatch for State { + fn event( + state: &mut Self, + registry: &wl_registry::WlRegistry, + event: wl_registry::Event, + _: &(), + _: &Connection, + qh: &QueueHandle, + ) { + if let wl_registry::Event::Global { name, interface, .. } = event { + match &interface[..] { + "wl_compositor" => { + let compositor = + registry.bind::(name, 1, qh, ()); + let surface = compositor.create_surface(qh, ()); + state.base_surface = Some(surface); + + if state.wm_base.is_some() && state.xdg_surface.is_none() { + state.init_xdg_surface(qh); + } + }, + "wl_shm" => { + let shm = registry.bind::(name, 1, qh, ()); + + let (init_w, init_h) = (320, 240); + + let mut file = tempfile::tempfile().unwrap(); + draw(&mut file, (init_w, init_h)); + let pool = shm.create_pool(file.as_fd(), (init_w * init_h * 4) as i32, qh, ()); + let buffer = pool.create_buffer( + 0, + init_w as i32, + init_h as i32, + (init_w * 4) as i32, + wl_shm::Format::Argb8888, + qh, + (), + ); + state.buffer = Some(buffer.clone()); + + if state.configured { + let surface = state.base_surface.as_ref().unwrap(); + surface.attach(Some(&buffer), 0, 0); + surface.commit(); + } + }, + "wl_seat" => { + registry.bind::(name, 1, qh, ()); + }, + "xdg_wm_base" => { + let wm_base = registry.bind::(name, 1, qh, ()); + state.wm_base = Some(wm_base); + + if state.base_surface.is_some() && state.xdg_surface.is_none() { + state.init_xdg_surface(qh); + } + }, + _ => {}, + } + } + } +} + +// Ignore events from these object types in this example. +delegate_noop!(State: ignore wl_compositor::WlCompositor); +delegate_noop!(State: ignore wl_surface::WlSurface); +delegate_noop!(State: ignore wl_shm::WlShm); +delegate_noop!(State: ignore wl_shm_pool::WlShmPool); +delegate_noop!(State: ignore wl_buffer::WlBuffer); + +fn draw(tmp: &mut File, (buf_x, buf_y): (u32, u32)) { + use std::{cmp::min, io::Write}; + let mut buf = std::io::BufWriter::new(tmp); + for y in 0..buf_y { + for x in 0..buf_x { + let a = 0xFF; + let r = min(((buf_x - x) * 0xFF) / buf_x, ((buf_y - y) * 0xFF) / buf_y); + let g = min((x * 0xFF) / buf_x, ((buf_y - y) * 0xFF) / buf_y); + let b = min(((buf_x - x) * 0xFF) / buf_x, (y * 0xFF) / buf_y); + + let color = (a << 24) + (r << 16) + (g << 8) + b; + buf.write_all(&color.to_ne_bytes()).unwrap(); + } + } + buf.flush().unwrap(); +} + +impl State { + fn init_xdg_surface(&mut self, qh: &QueueHandle) { + let wm_base = self.wm_base.as_ref().unwrap(); + let base_surface = self.base_surface.as_ref().unwrap(); + + let xdg_surface = wm_base.get_xdg_surface(base_surface, qh, ()); + let toplevel = xdg_surface.get_toplevel(qh, ()); + toplevel.set_title("A fantastic window!".into()); + + base_surface.commit(); + + self.xdg_surface = Some((xdg_surface, toplevel)); + } +} + +impl Dispatch for State { + fn event( + _: &mut Self, + wm_base: &xdg_wm_base::XdgWmBase, + event: xdg_wm_base::Event, + _: &(), + _: &Connection, + _: &QueueHandle, + ) { + if let xdg_wm_base::Event::Ping { serial } = event { + wm_base.pong(serial); + } + } +} + +impl Dispatch for State { + fn event( + state: &mut Self, + xdg_surface: &xdg_surface::XdgSurface, + event: xdg_surface::Event, + _: &(), + _: &Connection, + _: &QueueHandle, + ) { + if let xdg_surface::Event::Configure { serial, .. } = event { + xdg_surface.ack_configure(serial); + state.configured = true; + let surface = state.base_surface.as_ref().unwrap(); + if let Some(ref buffer) = state.buffer { + surface.attach(Some(buffer), 0, 0); + surface.commit(); + } + } + } +} + +impl Dispatch for State { + fn event( + state: &mut Self, + _: &xdg_toplevel::XdgToplevel, + event: xdg_toplevel::Event, + _: &(), + _: &Connection, + _: &QueueHandle, + ) { + if let xdg_toplevel::Event::Close {} = event { + state.running = false; + } + } +} + +impl Dispatch for State { + fn event( + _: &mut Self, + seat: &wl_seat::WlSeat, + event: wl_seat::Event, + _: &(), + _: &Connection, + qh: &QueueHandle, + ) { + if let wl_seat::Event::Capabilities { capabilities: WEnum::Value(capabilities) } = event { + if capabilities.contains(wl_seat::Capability::Keyboard) { + seat.get_keyboard(qh, ()); + } + } + } +} + +impl Dispatch for State { + fn event( + state: &mut Self, + _: &wl_keyboard::WlKeyboard, + event: wl_keyboard::Event, + _: &(), + _: &Connection, + _: &QueueHandle, + ) { + if let wl_keyboard::Event::Key { key, .. } = event { + if key == 1 { + // ESC key + state.stop_handle.stop(); + } + } + } +} From 8a4034e478d816c7bf84269c74d8fc568408c948 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Tue, 19 Sep 2023 22:08:09 +0100 Subject: [PATCH 11/13] Update to latest calloop and wayland-client --- Cargo.toml | 4 +- examples/simplest-window.rs | 2 +- src/lib.rs | 73 +++++++++++++++++++------------------ 3 files changed, 40 insertions(+), 39 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d25dcdd..a12bf95 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,9 +11,9 @@ keywords = ["wayland", "windowing"] rust-version = "1.65.0" [dependencies] -wayland-client = "0.31.0" +wayland-client = "0.31.1" wayland-backend = "0.3.0" -calloop = { git = "https://github.com/DJMcNab/calloop/", rev = "9df1f5c949c3d7f23817b2bcbd5c481ff0ec7e65" } +calloop = "0.12.1" log = { version = "0.4.19", optional = true } rustix = { version = "0.38.4", default-features = false, features = ["std"] } diff --git a/examples/simplest-window.rs b/examples/simplest-window.rs index fd47b4e..4be36a6 100644 --- a/examples/simplest-window.rs +++ b/examples/simplest-window.rs @@ -25,7 +25,7 @@ fn main() { let mut event_loop: EventLoop = EventLoop::try_new().expect("Failed to initialize the event loop!"); let loop_handle = event_loop.handle(); - WaylandSource::new(event_queue).unwrap().insert(loop_handle).unwrap(); + WaylandSource::new(conn, event_queue).insert(loop_handle).unwrap(); let stop_handle = event_loop.get_signal(); let mut state = State { diff --git a/src/lib.rs b/src/lib.rs index 8527e18..ba538c7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -28,17 +28,17 @@ //! } //! ``` +#![forbid(unsafe_op_in_unsafe_fn)] use std::io; -use std::os::unix::io::{AsRawFd, RawFd}; -use calloop::generic::{FdWrapper, Generic}; +use calloop::generic::Generic; use calloop::{ EventSource, InsertError, Interest, LoopHandle, Mode, Poll, PostAction, Readiness, RegistrationToken, Token, TokenFactory, }; use rustix::io::Errno; use wayland_backend::client::{ReadEventsGuard, WaylandError}; -use wayland_client::{DispatchError, EventQueue}; +use wayland_client::{Connection, DispatchError, EventQueue}; #[cfg(feature = "log")] use log::error as log_error; @@ -57,8 +57,12 @@ use std::eprintln as log_error; /// loop and automatically dispatch pending events on the event queue. #[derive(Debug)] pub struct WaylandSource { + // In theory, we could use the same event queue inside `connection_source` + // However, calloop's safety requirements mean that we cannot then give + // mutable access to the queue, which is incompatible with our current interface + // Additionally, `Connection` is cheaply cloneable, so it's not a huge burden queue: EventQueue, - fd: Generic>, + connection_source: Generic, read_guard: Option, /// Calloop's before_will_sleep method allows /// skipping the sleeping by returning a `Token`. @@ -72,40 +76,34 @@ pub struct WaylandSource { impl WaylandSource { /// Wrap an [`EventQueue`] as a [`WaylandSource`]. /// - /// If this returns None, that means that acquiring a read guard failed. - /// See [EventQueue::prepare_read] for details - /// This guard is only used to get the wayland file descriptor + /// `queue` must be from the connection `Connection`. + /// This is not a safety invariant, but not following this may cause + /// freezes or hangs #[must_use] - pub fn new(queue: EventQueue) -> Option> { - let guard = queue.prepare_read()?; - let fd = Generic::new( - // Safety: `connection_fd` returns the wayland socket fd, - // and EventQueue (eventually) owns this socket - // fd is only used in calloop, which guarantees - // that the source is unregistered before dropping it, so the - // fd cannot be used after being freed - // Wayland-backend should probably document some guarantees here to make this sound, - // but we know that they are unlikely to make the queue have a different socket/fd - // - there is no public API to do so - unsafe { FdWrapper::new(guard.connection_fd().as_raw_fd()) }, - Interest::READ, - Mode::Level, - ); - drop(guard); + pub fn new(connection: Connection, queue: EventQueue) -> WaylandSource { + let connection_source = Generic::new(connection, Interest::READ, Mode::Level); - Some(WaylandSource { queue, fd, read_guard: None, fake_token: None, stored_error: Ok(()) }) + WaylandSource { + queue, + connection_source, + read_guard: None, + fake_token: None, + stored_error: Ok(()), + } } /// Access the underlying event queue /// - /// Note that you should be careful when interacting with it if you invoke - /// methods that interact with the wayland socket (such as `dispatch()` - /// or `prepare_read()`). These may interfere with the proper waking up - /// of this event source in the event loop. + /// Note that you should not replace this queue with a queue from a different + /// `Connection`, as that may cause freezes or other hangs. pub fn queue(&mut self) -> &mut EventQueue { &mut self.queue } + /// Access the connection to the Wayland server + pub fn connection(&self) -> &Connection { + self.connection_source.get_ref() + } /// Insert this source into the given event loop. /// /// This adapter will pass the event loop's shared data as the `D` type for @@ -141,15 +139,18 @@ impl EventSource for WaylandSource { { debug_assert!(self.read_guard.is_none()); - let queue = &mut self.queue; // Take the stored error std::mem::replace(&mut self.stored_error, Ok(()))?; - // Because our polling is based on a `Generic` source, in theory we might want - // to to call the process_events handler on fd. However, - // we know that Generic's `process_events` call is a no-op, so we can just - // handle the event ourselves + // We know that the event will either be a fake event + // produced in `before_will_sleep`, or a "real" event from the underlying + // source (self.queue_events). Our behaviour in both cases is the same. + // In theory we might want to call the process_events handler on the underlying + // event source. However, we know that Generic's `process_events` call is a no-op, + // so we just handle the event ourselves. + // Safety: We don't replace the + let queue = &mut self.queue; // Dispatch any pending events in the queue Self::loop_callback_pending(queue, &mut callback)?; @@ -165,7 +166,7 @@ impl EventSource for WaylandSource { token_factory: &mut TokenFactory, ) -> calloop::Result<()> { self.fake_token = Some(token_factory.token()); - self.fd.register(poll, token_factory) + self.connection_source.register(poll, token_factory) } fn reregister( @@ -173,11 +174,11 @@ impl EventSource for WaylandSource { poll: &mut Poll, token_factory: &mut TokenFactory, ) -> calloop::Result<()> { - self.fd.reregister(poll, token_factory) + self.connection_source.reregister(poll, token_factory) } fn unregister(&mut self, poll: &mut Poll) -> calloop::Result<()> { - self.fd.unregister(poll) + self.connection_source.unregister(poll) } fn before_sleep(&mut self) -> calloop::Result> { From ab4b6cc834ea08eb838d60735e458ab05949dac3 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Wed, 20 Sep 2023 13:04:26 +0100 Subject: [PATCH 12/13] Apply suggested changes from review --- src/lib.rs | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index ba538c7..ef1ee87 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -28,7 +28,7 @@ //! } //! ``` -#![forbid(unsafe_op_in_unsafe_fn)] +#![deny(unsafe_op_in_unsafe_fn)] use std::io; use calloop::generic::Generic; @@ -79,7 +79,6 @@ impl WaylandSource { /// `queue` must be from the connection `Connection`. /// This is not a safety invariant, but not following this may cause /// freezes or hangs - #[must_use] pub fn new(connection: Connection, queue: EventQueue) -> WaylandSource { let connection_source = Generic::new(connection, Interest::READ, Mode::Level); @@ -149,7 +148,6 @@ impl EventSource for WaylandSource { // event source. However, we know that Generic's `process_events` call is a no-op, // so we just handle the event ourselves. - // Safety: We don't replace the let queue = &mut self.queue; // Dispatch any pending events in the queue Self::loop_callback_pending(queue, &mut callback)?; @@ -206,16 +204,14 @@ impl EventSource for WaylandSource { let guard = self.read_guard.take(); if events.count() > 0 { // Read events from the socket if any are available - if let Some(guard) = guard { - if let Err(WaylandError::Io(err)) = guard.read() { - // If some other thread read events before us, concurrently, that's an expected - // case, so this error isn't an issue. Other error kinds do need to be returned, - // however - if err.kind() != io::ErrorKind::WouldBlock { - // before_handle_events doesn't allow returning errors - // For now, cache it in self until process_events is called - self.stored_error = Err(err); - } + if let Some(Err(WaylandError::Io(err))) = guard.map(ReadEventsGuard::read) { + // If some other thread read events before us, concurrently, that's an expected + // case, so this error isn't an issue. Other error kinds do need to be returned, + // however + if err.kind() != io::ErrorKind::WouldBlock { + // before_handle_events doesn't allow returning errors + // For now, cache it in self until process_events is called + self.stored_error = Err(err); } } } @@ -224,15 +220,15 @@ impl EventSource for WaylandSource { fn flush_queue(queue: &mut EventQueue) -> Result<(), calloop::Error> { if let Err(WaylandError::Io(err)) = queue.flush() { + // WouldBlock error means the compositor could not process all + // our messages quickly. Either it is slowed + // down or we are a spammer. Should not really + // happen, if it does we do nothing and will flush again later if err.kind() != io::ErrorKind::WouldBlock { // in case of error, forward it and fast-exit log_error!("Error trying to flush the wayland display: {}", err); return Err(err.into()); } - // WouldBlock error means the compositor could not process all - // our messages quickly. Either it is slowed - // down or we are a spammer. Should not really - // happen, if it does we do nothing and will flush again later } Ok(()) } From d3d1cf6fd0c3e741319c10676ab9e60951353bf8 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Wed, 20 Sep 2023 13:05:55 +0100 Subject: [PATCH 13/13] Apply nightly formatting and fix docs --- examples/simplest-window.rs | 16 +++++++--------- src/lib.rs | 11 ++++++----- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/examples/simplest-window.rs b/examples/simplest-window.rs index 4be36a6..424f481 100644 --- a/examples/simplest-window.rs +++ b/examples/simplest-window.rs @@ -1,15 +1,12 @@ -use std::{fs::File, os::unix::prelude::AsFd}; +use std::fs::File; +use std::os::unix::prelude::AsFd; use calloop::EventLoop; use calloop_wayland_source::WaylandSource; -use wayland_client::{ - delegate_noop, - protocol::{ - wl_buffer, wl_compositor, wl_keyboard, wl_registry, wl_seat, wl_shm, wl_shm_pool, - wl_surface, - }, - Connection, Dispatch, QueueHandle, WEnum, +use wayland_client::protocol::{ + wl_buffer, wl_compositor, wl_keyboard, wl_registry, wl_seat, wl_shm, wl_shm_pool, wl_surface, }; +use wayland_client::{delegate_noop, Connection, Dispatch, QueueHandle, WEnum}; use wayland_protocols::xdg::shell::client::{xdg_surface, xdg_toplevel, xdg_wm_base}; @@ -124,7 +121,8 @@ delegate_noop!(State: ignore wl_shm_pool::WlShmPool); delegate_noop!(State: ignore wl_buffer::WlBuffer); fn draw(tmp: &mut File, (buf_x, buf_y): (u32, u32)) { - use std::{cmp::min, io::Write}; + use std::cmp::min; + use std::io::Write; let mut buf = std::io::BufWriter::new(tmp); for y in 0..buf_y { for x in 0..buf_x { diff --git a/src/lib.rs b/src/lib.rs index ef1ee87..c0998b4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -20,7 +20,7 @@ //! let loop_handle = event_loop.handle(); //! //! // Insert the wayland source into the calloop's event loop. -//! WaylandSource::new(event_queue).unwrap().insert(loop_handle).unwrap(); +//! WaylandSource::new(connection, event_queue).insert(loop_handle).unwrap(); //! //! // This will start dispatching the event loop and processing pending wayland requests. //! while let Ok(_) = event_loop.dispatch(None, &mut ()) { @@ -93,8 +93,8 @@ impl WaylandSource { /// Access the underlying event queue /// - /// Note that you should not replace this queue with a queue from a different - /// `Connection`, as that may cause freezes or other hangs. + /// Note that you should not replace this queue with a queue from a + /// different `Connection`, as that may cause freezes or other hangs. pub fn queue(&mut self) -> &mut EventQueue { &mut self.queue } @@ -103,6 +103,7 @@ impl WaylandSource { pub fn connection(&self) -> &Connection { self.connection_source.get_ref() } + /// Insert this source into the given event loop. /// /// This adapter will pass the event loop's shared data as the `D` type for @@ -145,8 +146,8 @@ impl EventSource for WaylandSource { // produced in `before_will_sleep`, or a "real" event from the underlying // source (self.queue_events). Our behaviour in both cases is the same. // In theory we might want to call the process_events handler on the underlying - // event source. However, we know that Generic's `process_events` call is a no-op, - // so we just handle the event ourselves. + // event source. However, we know that Generic's `process_events` call is a + // no-op, so we just handle the event ourselves. let queue = &mut self.queue; // Dispatch any pending events in the queue