From 9ad0edd2a3f296336a2614ca4fedfd54d91bee13 Mon Sep 17 00:00:00 2001 From: Victor Berger Date: Wed, 9 Jun 2021 22:46:09 +0200 Subject: [PATCH] Fix several clippy lints --- anvil/src/input_handler.rs | 4 +-- anvil/src/winit.rs | 2 +- src/backend/allocator/dmabuf.rs | 14 +++++------ src/backend/allocator/gbm.rs | 2 +- src/backend/drm/render.rs | 14 ++++++----- src/backend/drm/surface/mod.rs | 2 +- src/backend/egl/native.rs | 6 ++--- src/backend/renderer/gles2/mod.rs | 5 ++-- src/utils/mod.rs | 12 +++++++++ src/wayland/compositor/handlers.rs | 10 ++++---- src/wayland/compositor/mod.rs | 4 +-- src/wayland/compositor/roles.rs | 28 +++++++++++++++++++-- src/wayland/compositor/tree.rs | 6 ++--- src/wayland/data_device/data_source.rs | 4 +-- src/wayland/data_device/dnd_grab.rs | 2 +- src/wayland/dmabuf/mod.rs | 9 ++++--- src/wayland/explicit_synchronization/mod.rs | 20 ++++++++++++--- src/xwayland/xserver.rs | 2 +- 18 files changed, 99 insertions(+), 47 deletions(-) diff --git a/anvil/src/input_handler.rs b/anvil/src/input_handler.rs index 1a665c9..1163fc4 100644 --- a/anvil/src/input_handler.rs +++ b/anvil/src/input_handler.rs @@ -46,7 +46,7 @@ impl AnvilState { // only process special actions on key press, not release return KeyAction::None; } - return action; + action } fn on_pointer_button(&mut self, evt: B::PointerButtonEvent) { @@ -223,7 +223,7 @@ impl AnvilState { } fn clamp_coords(&self, pos: (f64, f64)) -> (f64, f64) { - if self.backend_data.output_map.len() == 0 { + if self.backend_data.output_map.is_empty() { return pos; } diff --git a/anvil/src/winit.rs b/anvil/src/winit.rs index a8f8e2f..02f4b14 100644 --- a/anvil/src/winit.rs +++ b/anvil/src/winit.rs @@ -189,7 +189,7 @@ pub fn run_winit( Ok(()) }) .map_err(Into::::into) - .and_then(|x| x.into()); + .and_then(|x| x); renderer.window().set_cursor_visible(cursor_visible); diff --git a/src/backend/allocator/dmabuf.rs b/src/backend/allocator/dmabuf.rs index 7e939e4..cbd180a 100644 --- a/src/backend/allocator/dmabuf.rs +++ b/src/backend/allocator/dmabuf.rs @@ -152,7 +152,7 @@ impl DmabufBuilder { /// /// Returns `None` if the builder has no planes attached. pub fn build(mut self) -> Option { - if self.internal.planes.len() == 0 { + if self.internal.planes.is_empty() { return None; } @@ -167,7 +167,7 @@ impl Dmabuf { // Note: the `src` Buffer is only used a reference for size and format. // The contents are determined by the provided file descriptors, which // do not need to refer to the same buffer `src` does. - pub fn new_from_buffer(src: &impl Buffer, flags: DmabufFlags) -> DmabufBuilder { + pub fn builder_from_buffer(src: &impl Buffer, flags: DmabufFlags) -> DmabufBuilder { DmabufBuilder { internal: DmabufInternal { planes: Vec::with_capacity(MAX_PLANES), @@ -179,8 +179,8 @@ impl Dmabuf { } } - /// Create a new Dmabuf - pub fn new(width: u32, height: u32, format: Fourcc, flags: DmabufFlags) -> DmabufBuilder { + /// Create a new Dmabuf builder + pub fn builder(width: u32, height: u32, format: Fourcc, flags: DmabufFlags) -> DmabufBuilder { DmabufBuilder { internal: DmabufInternal { planes: Vec::with_capacity(MAX_PLANES), @@ -198,17 +198,17 @@ impl Dmabuf { } /// Returns raw handles of the planes of this buffer - pub fn handles<'a>(&'a self) -> impl Iterator + 'a { + pub fn handles(&self) -> impl Iterator + '_ { self.0.planes.iter().map(|p| *p.fd.as_ref().unwrap()) } /// Returns offsets for the planes of this buffer - pub fn offsets<'a>(&'a self) -> impl Iterator + 'a { + pub fn offsets(&self) -> impl Iterator + '_ { self.0.planes.iter().map(|p| p.offset) } /// Returns strides for the planes of this buffer - pub fn strides<'a>(&'a self) -> impl Iterator + 'a { + pub fn strides(&self) -> impl Iterator + '_ { self.0.planes.iter().map(|p| p.stride) } diff --git a/src/backend/allocator/gbm.rs b/src/backend/allocator/gbm.rs index 28d307d..45b6ca2 100644 --- a/src/backend/allocator/gbm.rs +++ b/src/backend/allocator/gbm.rs @@ -99,7 +99,7 @@ impl AsDmabuf for GbmBuffer { return Err(GbmConvertError::InvalidFD); } - let mut builder = Dmabuf::new_from_buffer(self, DmabufFlags::empty()); + let mut builder = Dmabuf::builder_from_buffer(self, DmabufFlags::empty()); for idx in 0..planes { builder.add_plane( self.fd()?, diff --git a/src/backend/drm/render.rs b/src/backend/drm/render.rs index fc67119..3c9ca7b 100644 --- a/src/backend/drm/render.rs +++ b/src/backend/drm/render.rs @@ -172,10 +172,10 @@ where debug!(logger, "Success, choosen format: {:?}", format); let buffers = Buffers::new(drm.clone(), gbm, buffer); Ok(DrmRenderSurface { - drm, - renderer, - swapchain, buffers, + swapchain, + renderer, + drm, }) } Err(err) => { @@ -337,12 +337,14 @@ impl Drop for FbHandle { } } +type DmabufSlot = Slot>)>; + struct Buffers { gbm: GbmDevice, drm: Arc>, - _current_fb: Slot>)>, - pending_fb: Option>)>>, - queued_fb: Option>)>>, + _current_fb: DmabufSlot, + pending_fb: Option>, + queued_fb: Option>, } impl Buffers diff --git a/src/backend/drm/surface/mod.rs b/src/backend/drm/surface/mod.rs index 2c47f87..79d2a5f 100644 --- a/src/backend/drm/surface/mod.rs +++ b/src/backend/drm/surface/mod.rs @@ -280,7 +280,7 @@ impl DrmSurface { .iter() .find(|handle| { // get information of that property - if let Some(info) = self.get_property(**handle).ok() { + if let Ok(info) = self.get_property(**handle) { // to find out, if we got the handle of the "IN_FORMATS" property ... if info.name().to_str().map(|x| x == "IN_FORMATS").unwrap_or(false) { // so we can use that to get formats diff --git a/src/backend/egl/native.rs b/src/backend/egl/native.rs index 98e7271..00cb950 100644 --- a/src/backend/egl/native.rs +++ b/src/backend/egl/native.rs @@ -120,7 +120,7 @@ impl<'a> Debug for EGLPlatform<'a> { /// Trait describing platform specific functionality to create a valid `EGLDisplay` using the `EGL_EXT_platform_base`(https://www.khronos.org/registry/EGL/extensions/EXT/EGL_EXT_platform_base.txt) extension. pub trait EGLNativeDisplay: Send { /// List of supported platforms that can be used to create a display using [`eglGetPlatformDisplayEXT`](https://www.khronos.org/registry/EGL/extensions/EXT/EGL_EXT_platform_base.txt) - fn supported_platforms<'a>(&'a self) -> Vec>; + fn supported_platforms(&self) -> Vec>; /// Type of surfaces created fn surface_type(&self) -> ffi::EGLint { @@ -130,7 +130,7 @@ pub trait EGLNativeDisplay: Send { #[cfg(feature = "backend_gbm")] impl EGLNativeDisplay for GbmDevice { - fn supported_platforms<'a>(&'a self) -> Vec> { + fn supported_platforms(&self) -> Vec> { vec![ // see: https://www.khronos.org/registry/EGL/extensions/KHR/EGL_KHR_platform_gbm.txt egl_platform!(PLATFORM_GBM_KHR, self.as_raw(), &["EGL_KHR_platform_gbm"]), @@ -142,7 +142,7 @@ impl EGLNativeDisplay for GbmDevice { #[cfg(feature = "backend_winit")] impl EGLNativeDisplay for WinitWindow { - fn supported_platforms<'a>(&'a self) -> Vec> { + fn supported_platforms(&self) -> Vec> { if let Some(display) = self.wayland_display() { vec![ // see: https://www.khronos.org/registry/EGL/extensions/KHR/EGL_KHR_platform_wayland.txt diff --git a/src/backend/renderer/gles2/mod.rs b/src/backend/renderer/gles2/mod.rs index 806db7b..0915fbc 100644 --- a/src/backend/renderer/gles2/mod.rs +++ b/src/backend/renderer/gles2/mod.rs @@ -537,7 +537,7 @@ impl ImportShm for Gles2Renderer { unsafe { self.gl.GenTextures(1, &mut tex) }; // new texture, upload in full upload_full = true; - let texture = Rc::new(Gles2TextureInternal { + Rc::new(Gles2TextureInternal { texture: tex, texture_kind: shader_idx, is_external: false, @@ -546,8 +546,7 @@ impl ImportShm for Gles2Renderer { height: height as u32, egl_images: None, destruction_callback_sender: self.destruction_callback_sender.clone(), - }); - texture + }) }), ); diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 5e1bc70..ae96d5f 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -3,3 +3,15 @@ mod rectangle; pub use self::rectangle::Rectangle; + +/// This resource is not managed by Smithay +#[derive(Debug)] +pub struct UnmanagedResource; + +impl std::fmt::Display for UnmanagedResource { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("This resource is not managed by Smithay.") + } +} + +impl std::error::Error for UnmanagedResource {} diff --git a/src/wayland/compositor/handlers.rs b/src/wayland/compositor/handlers.rs index 06241cf..2f891d1 100644 --- a/src/wayland/compositor/handlers.rs +++ b/src/wayland/compositor/handlers.rs @@ -7,8 +7,8 @@ use wayland_server::{ use super::{ tree::{Location, SurfaceData}, - BufferAssignment, CompositorToken, Damage, Rectangle, RectangleKind, RegionAttributes, Role, RoleType, - SubsurfaceRole, SurfaceEvent, + AlreadyHasRole, BufferAssignment, CompositorToken, Damage, Rectangle, RectangleKind, RegionAttributes, + Role, RoleType, SubsurfaceRole, SurfaceEvent, }; /* @@ -65,11 +65,11 @@ impl SurfaceImplem where R: 'static, { - fn receive_surface_request<'a>( + fn receive_surface_request( &mut self, req: wl_surface::Request, surface: wl_surface::WlSurface, - ddata: DispatchData<'a>, + ddata: DispatchData<'_>, ) { match req { wl_surface::Request::Attach { buffer, x, y } => { @@ -196,7 +196,7 @@ where { subcompositor.quick_assign(move |subcompositor, request, _| match request { wl_subcompositor::Request::GetSubsurface { id, surface, parent } => { - if let Err(()) = SurfaceData::::set_parent(&surface, &parent) { + if let Err(AlreadyHasRole) = SurfaceData::::set_parent(&surface, &parent) { subcompositor.as_ref().post_error( wl_subcompositor::Error::BadSurface as u32, "Surface already has a role.".into(), diff --git a/src/wayland/compositor/mod.rs b/src/wayland/compositor/mod.rs index 9d6a3ee..d666850 100644 --- a/src/wayland/compositor/mod.rs +++ b/src/wayland/compositor/mod.rs @@ -76,7 +76,7 @@ mod tree; pub use self::tree::TraversalAction; use self::{ - roles::{Role, RoleType, WrongRole}, + roles::{AlreadyHasRole, Role, RoleType, WrongRole}, tree::SurfaceData, }; use crate::utils::Rectangle; @@ -433,7 +433,7 @@ impl CompositorToken { /// /// If the surface is not managed by the `CompositorGlobal` that provided this token, this /// will panic (having more than one compositor is not supported). - pub fn give_role(self, surface: &WlSurface) -> Result<(), ()> + pub fn give_role(self, surface: &WlSurface) -> Result<(), AlreadyHasRole> where R: Role, RoleData: Default, diff --git a/src/wayland/compositor/roles.rs b/src/wayland/compositor/roles.rs index 81d2e90..990be47 100644 --- a/src/wayland/compositor/roles.rs +++ b/src/wayland/compositor/roles.rs @@ -92,6 +92,30 @@ #[derive(Debug)] pub struct WrongRole; +impl std::fmt::Display for WrongRole { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("Wrong role for surface.") + } +} + +impl std::error::Error for WrongRole {} + +/// An error type signifying that the surface already has a role and +/// cannot be assigned an other +/// +/// Generated if you attempt a role operation on a surface that does +/// not have the role you asked for. +#[derive(Debug)] +pub struct AlreadyHasRole; + +impl std::fmt::Display for AlreadyHasRole { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("Surface already has a role.") + } +} + +impl std::error::Error for AlreadyHasRole {} + /// A trait representing a type that can manage surface roles pub trait RoleType { /// Check if the associated surface has a role @@ -126,11 +150,11 @@ pub trait Role: RoleType { /// Set the role for the associated surface with default associated data /// /// Fails if the surface already has a role - fn set(&mut self) -> Result<(), ()> + fn set(&mut self) -> Result<(), AlreadyHasRole> where R: Default, { - self.set_with(Default::default()).map_err(|_| ()) + self.set_with(Default::default()).map_err(|_| AlreadyHasRole) } /// Set the role for the associated surface with given data diff --git a/src/wayland/compositor/tree.rs b/src/wayland/compositor/tree.rs index 7b20878..ad2b1db 100644 --- a/src/wayland/compositor/tree.rs +++ b/src/wayland/compositor/tree.rs @@ -127,7 +127,7 @@ impl SurfaceData { } /// Register that this surface has a role, fails if it already has one - pub fn give_role(surface: &WlSurface) -> Result<(), ()> + pub fn give_role(surface: &WlSurface) -> Result<(), AlreadyHasRole> where R: Role, RoleData: Default, @@ -215,12 +215,12 @@ impl + 'static> SurfaceData { /// /// if this surface already has a role, does nothing and fails, otherwise /// its role is now to be a subsurface - pub fn set_parent(child: &WlSurface, parent: &WlSurface) -> Result<(), ()> { + pub fn set_parent(child: &WlSurface, parent: &WlSurface) -> Result<(), AlreadyHasRole> { debug_assert!(child.as_ref().is_alive()); debug_assert!(parent.as_ref().is_alive()); // ensure the child is not already a parent of the parent if Self::is_ancestor(child, parent) { - return Err(()); + return Err(AlreadyHasRole); } // change child's parent diff --git a/src/wayland/data_device/data_source.rs b/src/wayland/data_device/data_source.rs index 3d4518c..446a59f 100644 --- a/src/wayland/data_device/data_source.rs +++ b/src/wayland/data_device/data_source.rs @@ -44,9 +44,9 @@ pub(crate) fn implement_data_source(src: Main) -> WlDataSource { pub fn with_source_metadata T>( source: &WlDataSource, f: F, -) -> Result { +) -> Result { match source.as_ref().user_data().get::>() { Some(data) => Ok(f(&data.borrow())), - None => Err(()), + None => Err(crate::utils::UnmanagedResource), } } diff --git a/src/wayland/data_device/dnd_grab.rs b/src/wayland/data_device/dnd_grab.rs index ac245bf..9a39bf1 100644 --- a/src/wayland/data_device/dnd_grab.rs +++ b/src/wayland/data_device/dnd_grab.rs @@ -251,7 +251,7 @@ fn implement_dnd_data_offer( match req { Request::Accept { mime_type, .. } => { if let Some(mtype) = mime_type { - if let Err(()) = with_source_metadata(&source, |meta| { + if let Err(crate::utils::UnmanagedResource) = with_source_metadata(&source, |meta| { data.accepted = meta.mime_types.contains(&mtype); }) { data.accepted = false; diff --git a/src/wayland/dmabuf/mod.rs b/src/wayland/dmabuf/mod.rs index 725c30d..9892a30 100644 --- a/src/wayland/dmabuf/mod.rs +++ b/src/wayland/dmabuf/mod.rs @@ -259,7 +259,7 @@ where return; } - let mut buf = Dmabuf::new( + let mut buf = Dmabuf::builder( width as u32, height as u32, format, @@ -277,7 +277,7 @@ where None => { params.as_ref().post_error( ParamError::Incomplete as u32, - format!("Provided buffer is incomplete, it has zero planes"), + "Provided buffer is incomplete, it has zero planes".to_string(), ); return; } @@ -305,6 +305,7 @@ where } } + #[allow(clippy::clippy::too_many_arguments)] fn create_immed<'a>( &mut self, params: &BufferParams, @@ -348,7 +349,7 @@ where return; } - let mut buf = Dmabuf::new( + let mut buf = Dmabuf::builder( width as u32, height as u32, format, @@ -366,7 +367,7 @@ where None => { params.as_ref().post_error( ParamError::Incomplete as u32, - format!("Provided buffer is incomplete, it has zero planes"), + "Provided buffer is incomplete, it has zero planes".to_string(), ); return; } diff --git a/src/wayland/explicit_synchronization/mod.rs b/src/wayland/explicit_synchronization/mod.rs index bf826b5..dc09fa2 100644 --- a/src/wayland/explicit_synchronization/mod.rs +++ b/src/wayland/explicit_synchronization/mod.rs @@ -61,7 +61,7 @@ //! the contents of sync_state //! */ //! }, -//! Err(()) => { +//! Err(NoExplicitSync) => { //! /* This surface is not explicitly synchronized, nothing more to do //! */ //! } @@ -155,6 +155,18 @@ pub enum ExplicitSyncError { NoBuffer, } +/// This surface is not explicitly synchronized +#[derive(Debug)] +pub struct NoExplicitSync; + +impl std::fmt::Display for NoExplicitSync { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("The surface is not explicitly synchronized.") + } +} + +impl std::error::Error for NoExplicitSync {} + /// Retrieve the explicit synchronization state commited by the client /// /// This state can contain an acquire fence and a release object, for synchronization (see module-level docs). @@ -163,12 +175,14 @@ pub enum ExplicitSyncError { /// should always call it on surface commit to avoid getting out-of-sync with the client. /// /// This function returns an error if the client has not setup explicit synchronization for this surface. -pub fn get_explicit_synchronization_state(attrs: &mut SurfaceAttributes) -> Result { +pub fn get_explicit_synchronization_state( + attrs: &mut SurfaceAttributes, +) -> Result { attrs .user_data .get::() .and_then(|s| s.take_state()) - .ok_or(()) + .ok_or(NoExplicitSync) } /// Send a synchronization error to a client diff --git a/src/xwayland/xserver.rs b/src/xwayland/xserver.rs index 9014e88..cb21546 100644 --- a/src/xwayland/xserver.rs +++ b/src/xwayland/xserver.rs @@ -192,7 +192,7 @@ fn launch(inner: &Rc>>) -> std::io::Result<()> { client.data_map().insert_if_missing(|| idle_inner.clone()); client.add_destructor(Filter::new(|e: Arc<_>, _, _| client_destroy::(&e))); - instance.wayland_client = Some(client.clone()); + instance.wayland_client = Some(client); } });