Fix most clippy warnings (#185)

* Add missing documentation

* Fix most clippy warnings

anvil/src/buffer_utils.rs

@@ -3,7 +3,7 @@ use std::{cell::RefCell, rc::Rc};
 use slog::Logger;
 
 #[cfg(feature = "egl")]
-use smithay::backend::egl::{BufferAccessError, EGLDisplay};
+use smithay::backend::egl::EGLDisplay;
 use smithay::{
     reexports::wayland_server::protocol::wl_buffer::WlBuffer,
     wayland::shm::with_buffer_contents as shm_buffer_contents,

anvil/src/shell.rs

@@ -17,7 +17,7 @@ use smithay::{
     },
     utils::Rectangle,
     wayland::{
-        compositor::{compositor_init, CompositorToken, RegionAttributes, SurfaceAttributes, SurfaceEvent},
+        compositor::{compositor_init, BufferAssignment, CompositorToken, RegionAttributes, SurfaceEvent},
         data_device::DnDIconRole,
         seat::{AxisFrame, CursorImageRole, GrabStartData, PointerGrab, PointerInnerHandle, Seat},
         shell::{
@@ -29,7 +29,6 @@ use smithay::{
                 XdgSurfacePendingState, XdgSurfaceRole,
             },
         },
-        SERIAL_COUNTER as SCOUNTER,
     },
 };
 
@@ -734,7 +733,7 @@ fn surface_commit(
 
         // we retrieve the contents of the associated buffer and copy it
         match attributes.buffer.take() {
-            Some(Some((buffer, (_x, _y)))) => {
+            Some(BufferAssignment::NewBuffer { buffer, .. }) => {
                 // new contents
                 // TODO: handle hotspot coordinates
                 if let Some(old_buffer) = data.buffer.replace(buffer) {
@@ -744,7 +743,7 @@ fn surface_commit(
                 // If this fails, the buffer will be discarded later by the drawing code.
                 data.dimensions = buffer_utils.dimensions(data.buffer.as_ref().unwrap());
             }
-            Some(None) => {
+            Some(BufferAssignment::Removed) => {
                 // erase the contents
                 if let Some(old_buffer) = data.buffer.take() {
                     old_buffer.release();

anvil/src/udev.rs

@@ -1,6 +1,6 @@
 use std::{
     cell::RefCell,
-    collections::HashMap,
+    collections::hash_map::{Entry, HashMap},
     io::Error as IoError,
     os::unix::io::{AsRawFd, RawFd},
     path::PathBuf,
@@ -264,7 +264,8 @@ pub fn run_udev(mut display: Display, mut event_loop: EventLoop<AnvilState>, log
             if state.need_wayland_dispatch {
                 display
                     .borrow_mut()
-                    .dispatch(std::time::Duration::from_millis(0), &mut state);
+                    .dispatch(std::time::Duration::from_millis(0), &mut state)
+                    .unwrap();
             }
             display.borrow_mut().flush_clients(&mut state);
             window_map.borrow_mut().refresh();
@@ -284,19 +285,18 @@ pub fn run_udev(mut display: Display, mut event_loop: EventLoop<AnvilState>, log
     Ok(())
 }
 
+struct BackendData<S: SessionNotifier> {
+    id: S::Id,
+    event_source: Source<Generic<SourceFd<RenderDevice>>>,
+    surfaces: Rc<RefCell<HashMap<crtc::Handle, GliumDrawer<RenderSurface>>>>,
+}
+
 struct UdevHandlerImpl<S: SessionNotifier, Data: 'static> {
     compositor_token: CompositorToken<Roles>,
     #[cfg(feature = "egl")]
     active_egl_context: Rc<RefCell<Option<EGLDisplay>>>,
     session: AutoSession,
-    backends: HashMap<
+    backends: HashMap<dev_t, BackendData<S>>,
-        dev_t,
-        (
-            S::Id,
-            Source<Generic<SourceFd<RenderDevice>>>,
-            Rc<RefCell<HashMap<crtc::Handle, GliumDrawer<RenderSurface>>>>,
-        ),
-    >,
     display: Rc<RefCell<Display>>,
     primary_gpu: Option<PathBuf>,
     window_map: Rc<RefCell<MyWindowMap>>,
@@ -340,14 +340,14 @@ impl<S: SessionNotifier, Data: 'static> UdevHandlerImpl<S, Data> {
                 .collect::<Vec<EncoderInfo>>();
             for encoder_info in encoder_infos {
                 for crtc in res_handles.filter_crtcs(encoder_info.possible_crtcs()) {
-                    if !backends.contains_key(&crtc) {
+                    if let Entry::Vacant(entry) = backends.entry(crtc) {
                         let renderer = GliumDrawer::init(
                             device.create_surface(crtc).unwrap(),
                             egl_display.clone(),
                             logger.clone(),
                         );
 
-                        backends.insert(crtc, renderer);
+                        entry.insert(renderer);
                         break;
                     }
                 }
@@ -471,17 +471,23 @@ impl<S: SessionNotifier, Data: 'static> UdevHandler for UdevHandlerImpl<S, Data>
                 }
             }
 
-            self.backends
+            self.backends.insert(
-                .insert(dev_id, (device_session_id, event_source, backends));
+                dev_id,
+                BackendData {
+                    id: device_session_id,
+                    event_source,
+                    surfaces: backends,
+                },
+            );
         }
     }
 
     fn device_changed(&mut self, device: dev_t) {
         //quick and dirty, just re-init all backends
-        if let Some((_, ref mut evt_source, ref backends)) = self.backends.get_mut(&device) {
+        if let Some(ref mut backend_data) = self.backends.get_mut(&device) {
-            let source = evt_source.clone_inner();
+            let source = backend_data.event_source.clone_inner();
             let mut evented = source.borrow_mut();
-            let mut backends = backends.borrow_mut();
+            let mut backends = backend_data.surfaces.borrow_mut();
             #[cfg(feature = "egl")]
             let new_backends = UdevHandlerImpl::<S, Data>::scan_connectors(
                 &mut (*evented).0,
@@ -511,12 +517,12 @@ impl<S: SessionNotifier, Data: 'static> UdevHandler for UdevHandlerImpl<S, Data>
 
     fn device_removed(&mut self, device: dev_t) {
         // drop the backends on this side
-        if let Some((id, evt_source, renderers)) = self.backends.remove(&device) {
+        if let Some(backend_data) = self.backends.remove(&device) {
             // drop surfaces
-            renderers.borrow_mut().clear();
+            backend_data.surfaces.borrow_mut().clear();
             debug!(self.logger, "Surfaces dropped");
 
-            let device = Rc::try_unwrap(evt_source.remove().unwrap())
+            let device = Rc::try_unwrap(backend_data.event_source.remove().unwrap())
                 .map_err(|_| "This should not happend")
                 .unwrap()
                 .into_inner()
@@ -530,7 +536,7 @@ impl<S: SessionNotifier, Data: 'static> UdevHandler for UdevHandlerImpl<S, Data>
                 }
             }
 
-            self.notifier.unregister(id);
+            self.notifier.unregister(backend_data.id);
             debug!(self.logger, "Dropping device");
         }
     }

anvil/src/winit.rs

@@ -201,7 +201,9 @@ pub fn run_winit(
             running.store(false, Ordering::SeqCst);
         } else {
             if state.need_wayland_dispatch {
-                display.dispatch(std::time::Duration::from_millis(0), &mut state);
+                display
+                    .dispatch(std::time::Duration::from_millis(0), &mut state)
+                    .unwrap();
             }
             display.flush_clients(&mut state);
             window_map.borrow_mut().refresh();

src/backend/drm/gbm/surface.rs

@@ -287,6 +287,11 @@ impl<D: RawDevice + 'static> GbmSurface<D> {
     /// *Note*: This might trigger a full modeset on the underlying device,
     /// potentially causing some flickering. In that case this operation is
     /// blocking until the crtc is in the desired state.
+    ///
+    /// # Safety
+    ///
+    /// When used in conjunction with an EGL context, this must be called exactly once
+    /// after page-flipping the associated context.
     pub unsafe fn page_flip(&self) -> ::std::result::Result<(), SwapBuffersError> {
         self.0.page_flip()
     }

src/backend/drm/legacy/mod.rs

@@ -66,8 +66,8 @@ pub enum Error {
     /// No encoder was found for a given connector on the set crtc
     #[error("No encoder found for the given connector '{connector:?}' on crtc `{crtc:?}`")]
     NoSuitableEncoder {
-        /// Connector info
+        /// Connector
-        connector: connector::Info,
+        connector: connector::Handle,
         /// CRTC
         crtc: crtc::Handle,
     },

src/backend/drm/legacy/surface.rs

@@ -150,7 +150,7 @@ impl<A: AsRawFd + 'static> Surface for LegacyDrmSurfaceInternal<A> {
                 .all(|crtc_list| resource_handles.filter_crtcs(crtc_list).contains(&self.crtc))
             {
                 return Err(Error::NoSuitableEncoder {
-                    connector: info,
+                    connector: info.handle(),
                     crtc: self.crtc,
                 });
             }
@@ -158,7 +158,7 @@ impl<A: AsRawFd + 'static> Surface for LegacyDrmSurfaceInternal<A> {
             pending.connectors.insert(conn);
             Ok(())
         } else {
-            return Err(Error::ModeNotSuitable(pending.mode.unwrap()));
+            Err(Error::ModeNotSuitable(pending.mode.unwrap()))
         }
     }
 

src/backend/drm/mod.rs

@@ -114,10 +114,19 @@ pub trait Device: AsRawFd + DevPath {
     /// [`ResourceHandle`](drm::control::ResourceHandle)
     fn resource_handles(&self) -> Result<ResourceHandles, <Self::Surface as Surface>::Error>;
 
+    /// Retrieve the information for a connector
     fn get_connector_info(&self, conn: connector::Handle) -> Result<connector::Info, DrmError>;
+
+    /// Retrieve the information for a crtc
     fn get_crtc_info(&self, crtc: crtc::Handle) -> Result<crtc::Info, DrmError>;
+
+    /// Retrieve the information for an encoder
     fn get_encoder_info(&self, enc: encoder::Handle) -> Result<encoder::Info, DrmError>;
+
+    /// Retrieve the information for a framebuffer
     fn get_framebuffer_info(&self, fb: framebuffer::Handle) -> Result<framebuffer::Info, DrmError>;
+
+    /// Retrieve the information for a plane
     fn get_plane_info(&self, plane: plane::Handle) -> Result<plane::Info, DrmError>;
 }
 
@@ -221,6 +230,9 @@ impl<A: AsRawFd> DevPath for A {
     }
 }
 
+/// calloop source associated with a Device
+pub type DrmSource<D> = Generic<SourceFd<D>>;
+
 /// Bind a `Device` to an [`EventLoop`](calloop::EventLoop),
 ///
 /// This will cause it to recieve events and feed them into a previously
@@ -228,7 +240,7 @@ impl<A: AsRawFd> DevPath for A {
 pub fn device_bind<D: Device + 'static, Data>(
     handle: &LoopHandle<Data>,
     device: D,
-) -> ::std::result::Result<Source<Generic<SourceFd<D>>>, InsertError<Generic<SourceFd<D>>>>
+) -> ::std::result::Result<Source<DrmSource<D>>, InsertError<DrmSource<D>>>
 where
     D: Device,
     Data: 'static,

src/backend/egl/context.rs

@@ -446,7 +446,9 @@ impl<B: native::Backend, N: native::NativeDisplay<B>> EGLContext<B, N> {
 
     /// Returns the address of an OpenGL function.
     ///
-    /// Supposes that the context has been made current before this function is called.
+    /// # Safety
+    ///
+    /// The context must have been made current before this function is called.
     pub unsafe fn get_proc_address(&self, symbol: &str) -> *const c_void {
         let addr = CString::new(symbol.as_bytes()).unwrap();
         let addr = addr.as_ptr();

src/backend/egl/mod.rs

@@ -183,7 +183,7 @@ impl EGLImages {
     ///
     /// This does only temporarily modify the OpenGL state any changes are reverted before returning.
     ///
-    /// # Unsafety
+    /// # Safety
     ///
     /// The given `tex_id` needs to be a valid GL texture otherwise undefined behavior might occur.
     #[cfg(feature = "renderer_gl")]

src/backend/egl/native.rs

@@ -20,7 +20,7 @@ pub trait Backend {
 
     /// Return an [`EGLDisplay`](ffi::egl::types::EGLDisplay) based on this backend
     ///
-    /// # Unsafety
+    /// # Safety
     ///
     /// The returned [`EGLDisplay`](ffi::egl::types::EGLDisplay) needs to be a valid pointer for EGL,
     /// but there is no way to test that.
@@ -154,7 +154,7 @@ unsafe impl NativeDisplay<Wayland> for WinitWindow {
                 wegl::WlEglSurface::new_from_raw(surface as *mut _, size.width as i32, size.height as i32)
             })
         } else {
-            return Err(Error::NonMatchingBackend("Wayland"));
+            Err(Error::NonMatchingBackend("Wayland"))
         }
     }
 }

src/backend/egl/surface.rs

@@ -100,7 +100,7 @@ impl<N: native::NativeSurface> EGLSurface<N> {
 
     /// Makes the OpenGL context the current context in the current thread.
     ///
-    /// # Unsafety
+    /// # Safety
     ///
     /// This function is marked unsafe, because the context cannot be made current
     /// on multiple threads.

src/backend/graphics/errors.rs

@@ -1,44 +0,0 @@
-use std::error::Error;
-use std::fmt;
-
-/// Error that can happen when swapping buffers.
-#[derive(Debug, Clone, PartialEq)]
-pub enum SwapBuffersError {
-    /// The corresponding context has been lost and needs to be recreated.
-    ///
-    /// All the objects associated to it (textures, buffers, programs, etc.)
-    /// need to be recreated from scratch.
-    ///
-    /// Operations will have no effect. Functions that read textures, buffers, etc.
-    /// will return uninitialized data instead.
-    ContextLost,
-    /// The buffers have already been swapped.
-    ///
-    /// This error can be returned when `swap_buffers` has been called multiple times
-    /// without any modification in between.
-    AlreadySwapped,
-    /// Unknown error
-    Unknown(u32),
-}
-
-impl fmt::Display for SwapBuffersError {
-    fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
-        write!(formatter, "{}", self.description())
-    }
-}
-
-impl Error for SwapBuffersError {
-    fn description(&self) -> &str {
-        match *self {
-            SwapBuffersError::ContextLost => "The context has been lost, it needs to be recreated",
-            SwapBuffersError::AlreadySwapped => {
-                "Buffers are already swapped, swap_buffers was called too many times"
-            }
-            SwapBuffersError::Unknown(_) => "Unknown error occurred",
-        }
-    }
-
-    fn cause(&self) -> Option<&dyn Error> {
-        None
-    }
-}

src/backend/graphics/gl.rs

@@ -19,7 +19,9 @@ pub trait GLGraphicsBackend {
 
     /// Returns the address of an OpenGL function.
     ///
-    /// Supposes that the context has been made current before this function is called.
+    /// # Safety
+    ///
+    /// The context must have been made current before this function is called.
     unsafe fn get_proc_address(&self, symbol: &str) -> *const c_void;
 
     /// Returns the dimensions of the window, or screen, etc in points.
@@ -34,10 +36,9 @@ pub trait GLGraphicsBackend {
 
     /// Makes the OpenGL context the current context in the current thread.
     ///
-    /// # Unsafety
+    /// # Safety
     ///
-    /// This function is marked unsafe, because the context cannot be made current
+    /// The context cannot be made current on multiple threads.
-    /// on multiple threads.
     unsafe fn make_current(&self) -> Result<(), SwapBuffersError>;
 
     /// Returns the pixel format of the main framebuffer of the context.

src/backend/graphics/mod.rs

@@ -2,9 +2,6 @@
 //!
 //! Note: Not every API may be supported by every backend
 
-mod errors;
-pub use self::errors::*;
-
 mod cursor;
 pub use self::cursor::*;
 
@@ -17,3 +14,26 @@ pub mod gl;
 pub mod glium;
 #[cfg(feature = "renderer_software")]
 pub mod software;
+
+/// Error that can happen when swapping buffers.
+#[derive(Debug, Clone, PartialEq, thiserror::Error)]
+pub enum SwapBuffersError {
+    /// The corresponding context has been lost and needs to be recreated.
+    ///
+    /// All the objects associated to it (textures, buffers, programs, etc.)
+    /// need to be recreated from scratch.
+    ///
+    /// Operations will have no effect. Functions that read textures, buffers, etc.
+    /// will return uninitialized data instead.
+    #[error("The context has been lost, it needs to be recreated")]
+    ContextLost,
+    /// The buffers have already been swapped.
+    ///
+    /// This error can be returned when `swap_buffers` has been called multiple times
+    /// without any modification in between.
+    #[error("Buffers are already swapped, swap_buffers was called too many times")]
+    AlreadySwapped,
+    /// Unknown error
+    #[error("Unknown error: {0:x}")]
+    Unknown(u32),
+}

src/backend/libinput.rs

@@ -614,6 +614,9 @@ impl AsRawFd for LibinputInputBackend {
     }
 }
 
+/// calloop source associated with the libinput backend
+pub type LibinputSource = Generic<SourceFd<LibinputInputBackend>>;
+
 /// Binds a [`LibinputInputBackend`] to a given [`LoopHandle`].
 ///
 /// Automatically feeds the backend with incoming events without any manual calls to
@@ -621,10 +624,7 @@ impl AsRawFd for LibinputInputBackend {
 pub fn libinput_bind<Data: 'static>(
     backend: LibinputInputBackend,
     handle: LoopHandle<Data>,
-) -> ::std::result::Result<
+) -> Result<Source<LibinputSource>, InsertError<LibinputSource>> {
-    Source<Generic<SourceFd<LibinputInputBackend>>>,
-    InsertError<Generic<SourceFd<LibinputInputBackend>>>,
-> {
     let mut source = Generic::from_fd_source(backend);
     source.set_interest(Interest::READABLE);
 

src/backend/session/dbus/logind.rs

@@ -268,7 +268,7 @@ impl LogindSessionImpl {
                 //This session will never live again, but the user maybe has other sessions open
                 //So lets just put it to sleep.. forever
                 for signal in &mut *self.signals.borrow_mut() {
-                    if let &mut Some(ref mut signal) = signal {
+                    if let Some(ref mut signal) = signal {
                         signal.pause(None);
                     }
                 }
@@ -285,7 +285,7 @@ impl LogindSessionImpl {
                         "Request of type \"{}\" to close device ({},{})", pause_type, major, minor
                     );
                     for signal in &mut *self.signals.borrow_mut() {
-                        if let &mut Some(ref mut signal) = signal {
+                        if let Some(ref mut signal) = signal {
                             signal.pause(Some((major, minor)));
                         }
                     }
@@ -310,7 +310,7 @@ impl LogindSessionImpl {
                     let fd = fd.ok_or(Error::UnexpectedMethodReturn)?.into_fd();
                     debug!(self.logger, "Reactivating device ({},{})", major, minor);
                     for signal in &mut *self.signals.borrow_mut() {
-                        if let &mut Some(ref mut signal) = signal {
+                        if let Some(ref mut signal) = signal {
                             signal.activate(Some((major, minor, Some(fd))));
                         }
                     }
@@ -356,7 +356,7 @@ impl Session for LogindSession {
             let fd = fd.ok_or(Error::UnexpectedMethodReturn)?.into_fd();
             Ok(fd)
         } else {
-            return Err(Error::SessionLost);
+            Err(Error::SessionLost)
         }
     }
 
@@ -376,7 +376,7 @@ impl Session for LogindSession {
             )
             .map(|_| ())
         } else {
-            return Err(Error::SessionLost);
+            Err(Error::SessionLost)
         }
     }
 
@@ -404,7 +404,7 @@ impl Session for LogindSession {
             )
             .map(|_| ())
         } else {
-            return Err(Error::SessionLost);
+            Err(Error::SessionLost)
         }
     }
 }
@@ -548,20 +548,30 @@ pub enum Error {
     /// Failed call to a dbus method
     #[error("Failed to call dbus method for service: {bus:?}, path: {path:?}, interface: {interface:?}, member: {member:?}")]
     FailedToSendDbusCall {
+        /// Name of the service
         bus: BusName<'static>,
+        /// Object path
         path: DbusPath<'static>,
+        /// Interface
         interface: Interface<'static>,
+        /// Method called
         member: Member<'static>,
+        /// DBus error
         #[source]
         source: dbus::Error,
     },
     /// DBus method call failed
     #[error("Dbus message call failed for service: {bus:?}, path: {path:?}, interface: {interface:?}, member: {member:?}")]
     DbusCallFailed {
+        /// Name of the service
         bus: BusName<'static>,
+        /// Object path
         path: DbusPath<'static>,
+        /// Interface
         interface: Interface<'static>,
+        /// Method called
         member: Member<'static>,
+        /// DBus error
         #[source]
         source: dbus::Error,
     },

src/backend/session/direct.rs

@@ -403,10 +403,9 @@ impl BoundDirectSession {
     pub fn unbind(self) -> DirectSessionNotifier {
         let BoundDirectSession { source, notifier } = self;
         source.remove();
-        match Rc::try_unwrap(notifier) {
+        Rc::try_unwrap(notifier)
-            Ok(notifier) => notifier.into_inner(),
+            .map(RefCell::into_inner)
-            Err(_) => panic!("Notifier should have been freed from the event loop!"),
+            .unwrap_or_else(|_| panic!("Notifier should have been freed from the event loop!"))
-        }
     }
 }
 

src/backend/udev.rs

@@ -90,6 +90,9 @@ impl<T: UdevHandler + 'static> Drop for UdevBackend<T> {
     }
 }
 
+/// calloop event source associated with the Udev backend
+pub type UdevSource<T> = Generic<SourceFd<UdevBackend<T>>>;
+
 /// Binds a [`UdevBackend`] to a given [`EventLoop`](calloop::EventLoop).
 ///
 /// Allows the backend to receive kernel events and thus to drive the [`UdevHandler`].
@@ -97,7 +100,7 @@ impl<T: UdevHandler + 'static> Drop for UdevBackend<T> {
 pub fn udev_backend_bind<T: UdevHandler + 'static, Data: 'static>(
     udev: UdevBackend<T>,
     handle: &LoopHandle<Data>,
-) -> Result<Source<Generic<SourceFd<UdevBackend<T>>>>, InsertError<Generic<SourceFd<UdevBackend<T>>>>> {
+) -> Result<Source<UdevSource<T>>, InsertError<UdevSource<T>>> {
     let mut source = Generic::from_fd_source(udev);
     source.set_interest(Interest::READABLE);
 

src/wayland/compositor/handlers.rs

@@ -7,8 +7,8 @@ use wayland_server::{
 
 use super::{
     tree::{Location, SurfaceData},
-    CompositorToken, Damage, Rectangle, RectangleKind, RegionAttributes, Role, RoleType, SubsurfaceRole,
+    BufferAssignment, CompositorToken, Damage, Rectangle, RectangleKind, RegionAttributes, Role, RoleType,
-    SurfaceEvent,
+    SubsurfaceRole, SurfaceEvent,
 };
 
 /*
@@ -42,10 +42,12 @@ where
  * wl_surface
  */
 
+type SurfaceImplemFn<R> = dyn FnMut(SurfaceEvent, wl_surface::WlSurface, CompositorToken<R>);
+
 // Internal implementation data of surfaces
 pub(crate) struct SurfaceImplem<R> {
     log: ::slog::Logger,
-    implem: Rc<RefCell<dyn FnMut(SurfaceEvent, wl_surface::WlSurface, CompositorToken<R>)>>,
+    implem: Rc<RefCell<SurfaceImplemFn<R>>>,
 }
 
 impl<R> SurfaceImplem<R> {
@@ -64,7 +66,15 @@ where
     fn receive_surface_request(&mut self, req: wl_surface::Request, surface: wl_surface::WlSurface) {
         match req {
             wl_surface::Request::Attach { buffer, x, y } => {
-                SurfaceData::<R>::with_data(&surface, |d| d.buffer = Some(buffer.map(|b| (b, (x, y)))));
+                SurfaceData::<R>::with_data(&surface, |d| {
+                    d.buffer = Some(match buffer {
+                        Some(buffer) => BufferAssignment::NewBuffer {
+                            buffer,
+                            delta: (x, y),
+                        },
+                        None => BufferAssignment::Removed,
+                    })
+                });
             }
             wl_surface::Request::Damage { x, y, width, height } => {
                 SurfaceData::<R>::with_data(&surface, |d| {
@@ -128,10 +138,7 @@ where
         move |surface, req, _| implem.receive_surface_request(req, surface.deref().clone())
     });
     surface.assign_destructor(Filter::new(|surface, _, _| SurfaceData::<R>::cleanup(&surface)));
-    surface
+    surface.as_ref().user_data().set_threadsafe(SurfaceData::<R>::new);
-        .as_ref()
-        .user_data()
-        .set_threadsafe(|| SurfaceData::<R>::new());
     SurfaceData::<R>::init(&surface);
     surface.deref().clone()
 }

src/wayland/compositor/mod.rs

@@ -105,6 +105,19 @@ struct Marker<R> {
     _r: ::std::marker::PhantomData<R>,
 }
 
+/// New buffer assignation for a surface
+pub enum BufferAssignment {
+    /// The surface no longer has a buffer attached to it
+    Removed,
+    /// A new buffer has been attached
+    NewBuffer {
+        /// The buffer object
+        buffer: wl_buffer::WlBuffer,
+        /// location of the new buffer relative to the previous one
+        delta: (i32, i32),
+    },
+}
+
 /// Data associated with a surface, aggregated by the handlers
 ///
 /// Most of the fields of this struct represent a double-buffered state, which
@@ -116,16 +129,10 @@ struct Marker<R> {
 pub struct SurfaceAttributes {
     /// Buffer defining the contents of the surface
     ///
-    /// The tuple represent the coordinates of this buffer
-    /// relative to the location of the current buffer.
-    ///
-    /// If set to `Some(None)`, it means the user specifically asked for the
-    /// surface to be unmapped.
-    ///
     /// You are free to set this field to `None` to avoid processing it several
     /// times. It'll be set to `Some(...)` if the user attaches a buffer (or `NULL`) to
-    /// the surface.
+    /// the surface, and be left to `None` if the user does not attach anything.
-    pub buffer: Option<Option<(wl_buffer::WlBuffer, (i32, i32))>>,
+    pub buffer: Option<BufferAssignment>,
     /// Scale of the contents of the buffer, for higher-resolution contents.
     ///
     /// If it matches the one of the output displaying this surface, no change

src/wayland/compositor/roles.rs

@@ -153,6 +153,10 @@ pub trait Role<R>: RoleType {
     fn unset(&mut self) -> Result<R, WrongRole>;
 }
 
+/// The roles defining macro
+///
+/// See the docs of the [`wayland::compositor::roles`](wayland/compositor/roles/index.html) module
+/// for an explanation of its use.
 #[macro_export]
 macro_rules! define_roles(
     ($enum_name: ident) => {

src/wayland/dmabuf/mod.rs

@@ -103,6 +103,7 @@ pub struct Plane {
 }
 
 bitflags! {
+    /// Possible flags for a DMA buffer
     pub struct BufferFlags: u32 {
         /// The buffer content is Y-inverted
         const Y_INVERT = 1;
@@ -219,9 +220,14 @@ where
                                 stride,
                                 modifier_hi,
                                 modifier_lo,
-                            } => {
+                            } => handler.add(
-                                handler.add(&*params, fd, plane_idx, offset, stride, modifier_hi, modifier_lo)
+                                &*params,
-                            }
+                                fd,
+                                plane_idx,
+                                offset,
+                                stride,
+                                ((modifier_hi as u64) << 32) + (modifier_lo as u64),
+                            ),
                             ParamsRequest::Create {
                                 width,
                                 height,
@@ -269,8 +275,7 @@ impl<H: DmabufHandler> ParamsHandler<H> {
         plane_idx: u32,
         offset: u32,
         stride: u32,
-        modifier_hi: u32,
+        modifier: u64,
-        modifier_lo: u32,
     ) {
         // protocol checks:
         // Cannot reuse a params:
@@ -304,7 +309,7 @@ impl<H: DmabufHandler> ParamsHandler<H> {
             plane_idx,
             offset,
             stride,
-            modifier: ((modifier_hi as u64) << 32) + (modifier_lo as u64),
+            modifier,
         });
     }
 

src/wayland/seat/mod.rs

@@ -361,7 +361,7 @@ where
                     // same as pointer, should error but cannot
                 }
             }
-            wl_seat::Request::GetTouch { id: _ } => {
+            wl_seat::Request::GetTouch { .. } => {
                 // TODO
             }
             wl_seat::Request::Release => {

src/wayland/shell/legacy/mod.rs

@@ -97,7 +97,7 @@ impl<R> Clone for ShellSurface<R> {
         Self {
             wl_surface: self.wl_surface.clone(),
             shell_surface: self.shell_surface.clone(),
-            token: self.token.clone(),
+            token: self.token,
         }
     }
 }

src/wayland/shell/mod.rs

@@ -1,2 +1,18 @@
+//! Handler utilities for the various shell protocols
+//!
+//! Wayland, via its different protocol extensions, supports different kind of
+//! shells. Here a shell represent the logic associated to displaying windows and
+//! arranging them on the screen.
+//!
+//! The shell protocols thus define what kind of interactions a client can have with
+//! the compositor to properly display its contents on the screen.
+//!
+//! Smithay currently provides two of them:
+//!
+//! - The [`xdg`](xdg/index.hmtl) module provides handlers for the `xdg_shell` protocol, which is
+//!   the current standard for desktop apps
+//! - The [`legacy`](legacy/index.html) module provides handlers for the `wl_shell` protocol, which
+//!   is now deprecated. You only need it if you want to support apps predating `xdg_shell`.
+
 pub mod legacy;
 pub mod xdg;

src/wayland/shell/xdg/mod.rs

@@ -499,7 +499,7 @@ impl<R> Clone for ToplevelSurface<R> {
         Self {
             wl_surface: self.wl_surface.clone(),
             shell_surface: self.shell_surface.clone(),
-            token: self.token.clone(),
+            token: self.token,
         }
     }
 }

src/xwayland/mod.rs

@@ -1,3 +1,18 @@
+//! XWayland utilities
+//!
+//! This module contains helpers to manage XWayland from your compositor, in order
+//! to support running X11 apps.
+//!
+//! The starting point is the [`XWayland`](struct.XWayland.html) struct, which represents the
+//! running `XWayland` instance. Dropping it will shutdown XWayland.
+//!
+//! You need to provide an implementation of the `XWindowManager` trait which gives you
+//! access to the X11 WM connection and the `Client` associated with XWayland. You'll need
+//! to treat XWayland (and all its X11 apps) as one special client, and play the role of
+//! an X11 Window Manager.
+//!
+//! Smithay does not provide any helper for doing that yet, but it is planned.
+
 mod x11_sockets;
 mod xserver;
 

src/xwayland/xserver.rs

@@ -126,10 +126,12 @@ struct XWaylandInstance {
     child_pid: Option<Pid>,
 }
 
+type SourceMaker<WM> = dyn FnMut(Rc<RefCell<Inner<WM>>>) -> Result<Source<Signals>, ()>;
+
 // Inner implementation of the XWayland manager
 struct Inner<WM: XWindowManager> {
     wm: WM,
-    source_maker: Box<dyn FnMut(Rc<RefCell<Inner<WM>>>) -> Result<Source<Signals>, ()>>,
+    source_maker: Box<SourceMaker<WM>>,
     wayland_display: Rc<RefCell<Display>>,
     instance: Option<XWaylandInstance>,
     log: ::slog::Logger,