From 43860aba84a7303eed47dfafefcee6dde952afc4 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 7 Feb 2022 23:10:53 +0100 Subject: [PATCH] Fix a whackton of warnings and clippy lints The things remaining are all of the cursor things in the X11 implementation (there's _a lot_ of it, so there's probably a reason why it's all still there but unused), and the super unsound immutable reference to mutable reference cast in the macOS implementation that Clippy automatically errors out on. --- src/gl/mod.rs | 6 +- src/gl/win.rs | 2 +- src/keyboard.rs | 1 + src/macos/window.rs | 2 +- src/win/keyboard.rs | 124 +++----------------------------------- src/win/window.rs | 11 ++-- src/x11/cursor.rs | 2 +- src/x11/window.rs | 37 ++++++------ src/x11/xcb_connection.rs | 9 ++- 9 files changed, 46 insertions(+), 148 deletions(-) diff --git a/src/gl/mod.rs b/src/gl/mod.rs index f3e786f..a7922b7 100644 --- a/src/gl/mod.rs +++ b/src/gl/mod.rs @@ -1,8 +1,10 @@ -use raw_window_handle::HasRawWindowHandle; - use std::ffi::c_void; use std::marker::PhantomData; +// On X11 creating the context is a two step process +#[cfg(not(target_os = "linux"))] +use raw_window_handle::HasRawWindowHandle; + #[cfg(target_os = "windows")] mod win; #[cfg(target_os = "windows")] diff --git a/src/gl/win.rs b/src/gl/win.rs index 57982c9..f98734f 100644 --- a/src/gl/win.rs +++ b/src/gl/win.rs @@ -255,7 +255,7 @@ impl GlContext { let hglrc = wglCreateContextAttribsARB.unwrap()(hdc, std::ptr::null_mut(), ctx_attribs.as_ptr()); - if hglrc == std::ptr::null_mut() { + if hglrc.is_null() { return Err(GlError::CreationFailed(())); } diff --git a/src/keyboard.rs b/src/keyboard.rs index c2af463..c56abd3 100644 --- a/src/keyboard.rs +++ b/src/keyboard.rs @@ -17,6 +17,7 @@ //! Keyboard types. +#[cfg(any(target_os = "linux", target_os = "macos"))] use keyboard_types::{Code, Location}; #[cfg(any(target_os = "linux", target_os = "macos"))] diff --git a/src/macos/window.rs b/src/macos/window.rs index a8520ed..0875377 100644 --- a/src/macos/window.rs +++ b/src/macos/window.rs @@ -44,7 +44,7 @@ pub struct WindowHandle { impl WindowHandle { pub fn close(&mut self) { - if let Some(_) = self.raw_window_handle.take() { + if self.raw_window_handle.take().is_some() { self.close_requested.store(true, Ordering::Relaxed); } } diff --git a/src/win/keyboard.rs b/src/win/keyboard.rs index f04d2cc..e4045d1 100644 --- a/src/win/keyboard.rs +++ b/src/win/keyboard.rs @@ -19,7 +19,6 @@ use std::cmp::Ordering; use std::collections::{HashMap, HashSet}; -use std::convert::TryInto; use std::mem; use std::ops::RangeInclusive; @@ -29,15 +28,15 @@ use winapi::shared::minwindef::{HKL, INT, LPARAM, UINT, WPARAM}; use winapi::shared::ntdef::SHORT; use winapi::shared::windef::HWND; use winapi::um::winuser::{ - GetKeyState, GetKeyboardLayout, MapVirtualKeyExW, PeekMessageW, ToUnicodeEx, VkKeyScanW, - MAPVK_VK_TO_CHAR, MAPVK_VSC_TO_VK_EX, PM_NOREMOVE, VK_ACCEPT, VK_ADD, VK_APPS, VK_ATTN, - VK_BACK, VK_BROWSER_BACK, VK_BROWSER_FAVORITES, VK_BROWSER_FORWARD, VK_BROWSER_HOME, - VK_BROWSER_REFRESH, VK_BROWSER_SEARCH, VK_BROWSER_STOP, VK_CANCEL, VK_CAPITAL, VK_CLEAR, - VK_CONTROL, VK_CONVERT, VK_CRSEL, VK_DECIMAL, VK_DELETE, VK_DIVIDE, VK_DOWN, VK_END, VK_EREOF, - VK_ESCAPE, VK_EXECUTE, VK_EXSEL, VK_F1, VK_F10, VK_F11, VK_F12, VK_F2, VK_F3, VK_F4, VK_F5, - VK_F6, VK_F7, VK_F8, VK_F9, VK_FINAL, VK_HELP, VK_HOME, VK_INSERT, VK_JUNJA, VK_KANA, VK_KANJI, - VK_LAUNCH_APP1, VK_LAUNCH_APP2, VK_LAUNCH_MAIL, VK_LAUNCH_MEDIA_SELECT, VK_LCONTROL, VK_LEFT, - VK_LMENU, VK_LSHIFT, VK_LWIN, VK_MEDIA_NEXT_TRACK, VK_MEDIA_PLAY_PAUSE, VK_MEDIA_PREV_TRACK, + GetKeyState, GetKeyboardLayout, MapVirtualKeyExW, PeekMessageW, ToUnicodeEx, MAPVK_VK_TO_CHAR, + MAPVK_VSC_TO_VK_EX, PM_NOREMOVE, VK_ACCEPT, VK_ADD, VK_APPS, VK_ATTN, VK_BACK, VK_BROWSER_BACK, + VK_BROWSER_FAVORITES, VK_BROWSER_FORWARD, VK_BROWSER_HOME, VK_BROWSER_REFRESH, + VK_BROWSER_SEARCH, VK_BROWSER_STOP, VK_CANCEL, VK_CAPITAL, VK_CLEAR, VK_CONTROL, VK_CONVERT, + VK_CRSEL, VK_DECIMAL, VK_DELETE, VK_DIVIDE, VK_DOWN, VK_END, VK_EREOF, VK_ESCAPE, VK_EXECUTE, + VK_EXSEL, VK_F1, VK_F10, VK_F11, VK_F12, VK_F2, VK_F3, VK_F4, VK_F5, VK_F6, VK_F7, VK_F8, + VK_F9, VK_FINAL, VK_HELP, VK_HOME, VK_INSERT, VK_JUNJA, VK_KANA, VK_KANJI, VK_LAUNCH_APP1, + VK_LAUNCH_APP2, VK_LAUNCH_MAIL, VK_LAUNCH_MEDIA_SELECT, VK_LCONTROL, VK_LEFT, VK_LMENU, + VK_LSHIFT, VK_LWIN, VK_MEDIA_NEXT_TRACK, VK_MEDIA_PLAY_PAUSE, VK_MEDIA_PREV_TRACK, VK_MEDIA_STOP, VK_MENU, VK_MODECHANGE, VK_MULTIPLY, VK_NEXT, VK_NONCONVERT, VK_NUMLOCK, VK_NUMPAD0, VK_NUMPAD1, VK_NUMPAD2, VK_NUMPAD3, VK_NUMPAD4, VK_NUMPAD5, VK_NUMPAD6, VK_NUMPAD7, VK_NUMPAD8, VK_NUMPAD9, VK_OEM_ATTN, VK_OEM_CLEAR, VK_PAUSE, VK_PLAY, VK_PRINT, VK_PRIOR, @@ -344,110 +343,6 @@ fn vk_to_key(vk: VkCode) -> Option { }) } -/// Convert a key to a virtual key code. -/// -/// The virtual key code is needed in various winapi interfaces, including -/// accelerators. This provides the virtual key code in the current keyboard -/// map. -/// -/// The virtual key code can have modifiers in the higher order byte when the -/// argument is a `Character` variant. See: -/// https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-vkkeyscanw -pub(crate) fn key_to_vk(key: &Key) -> Option { - Some(match key { - Key::Character(s) => { - if let Some(code_point) = s.chars().next() { - if let Ok(wchar) = (code_point as u32).try_into() { - unsafe { VkKeyScanW(wchar) as i32 } - } else { - return None; - } - } else { - return None; - } - } - Key::Cancel => VK_CANCEL, - Key::Backspace => VK_BACK, - Key::Tab => VK_TAB, - Key::Clear => VK_CLEAR, - Key::Enter => VK_RETURN, - Key::Shift => VK_SHIFT, - Key::Control => VK_CONTROL, - Key::Alt => VK_MENU, - Key::Pause => VK_PAUSE, - Key::CapsLock => VK_CAPITAL, - // TODO: disambiguate kana and hangul? same vk - Key::KanaMode => VK_KANA, - Key::JunjaMode => VK_JUNJA, - Key::FinalMode => VK_FINAL, - Key::KanjiMode => VK_KANJI, - Key::Escape => VK_ESCAPE, - Key::NonConvert => VK_NONCONVERT, - Key::Accept => VK_ACCEPT, - Key::PageUp => VK_PRIOR, - Key::PageDown => VK_NEXT, - Key::End => VK_END, - Key::Home => VK_HOME, - Key::ArrowLeft => VK_LEFT, - Key::ArrowUp => VK_UP, - Key::ArrowRight => VK_RIGHT, - Key::ArrowDown => VK_DOWN, - Key::Select => VK_SELECT, - Key::Print => VK_PRINT, - Key::Execute => VK_EXECUTE, - Key::PrintScreen => VK_SNAPSHOT, - Key::Insert => VK_INSERT, - Key::Delete => VK_DELETE, - Key::Help => VK_HELP, - Key::Meta => VK_LWIN, - Key::ContextMenu => VK_APPS, - Key::Standby => VK_SLEEP, - Key::F1 => VK_F1, - Key::F2 => VK_F2, - Key::F3 => VK_F3, - Key::F4 => VK_F4, - Key::F5 => VK_F5, - Key::F6 => VK_F6, - Key::F7 => VK_F7, - Key::F8 => VK_F8, - Key::F9 => VK_F9, - Key::F10 => VK_F10, - Key::F11 => VK_F11, - Key::F12 => VK_F12, - Key::NumLock => VK_NUMLOCK, - Key::ScrollLock => VK_SCROLL, - Key::BrowserBack => VK_BROWSER_BACK, - Key::BrowserForward => VK_BROWSER_FORWARD, - Key::BrowserRefresh => VK_BROWSER_REFRESH, - Key::BrowserStop => VK_BROWSER_STOP, - Key::BrowserSearch => VK_BROWSER_SEARCH, - Key::BrowserFavorites => VK_BROWSER_FAVORITES, - Key::BrowserHome => VK_BROWSER_HOME, - Key::AudioVolumeMute => VK_VOLUME_MUTE, - Key::AudioVolumeDown => VK_VOLUME_DOWN, - Key::AudioVolumeUp => VK_VOLUME_UP, - Key::MediaTrackNext => VK_MEDIA_NEXT_TRACK, - Key::MediaTrackPrevious => VK_MEDIA_PREV_TRACK, - Key::MediaStop => VK_MEDIA_STOP, - Key::MediaPlayPause => VK_MEDIA_PLAY_PAUSE, - Key::LaunchMail => VK_LAUNCH_MAIL, - Key::LaunchMediaPlayer => VK_LAUNCH_MEDIA_SELECT, - Key::LaunchApplication1 => VK_LAUNCH_APP1, - Key::LaunchApplication2 => VK_LAUNCH_APP2, - Key::Alphanumeric => VK_OEM_ATTN, - Key::Convert => VK_CONVERT, - Key::ModeChange => VK_MODECHANGE, - Key::Process => VK_PROCESSKEY, - Key::Attn => VK_ATTN, - Key::CrSel => VK_CRSEL, - Key::ExSel => VK_EXSEL, - Key::EraseEof => VK_EREOF, - Key::Play => VK_PLAY, - Key::ZoomToggle => VK_ZOOM, - _ => return None, - }) -} - fn code_unit_to_key(code_unit: u32) -> Key { match code_unit { 0x8 | 0x7F => Key::Backspace, @@ -692,6 +587,7 @@ impl KeyboardState { key_state[VK_LCONTROL as usize] = if has_altgr { 0x80 } else { 0 }; key_state[VK_MENU as usize] = if has_altgr { 0x80 } else { 0 }; key_state[VK_RMENU as usize] = if has_altgr { 0x80 } else { 0 }; + #[allow(clippy::iter_overeager_cloned)] for vk in PRINTABLE_VKS.iter().cloned().flatten() { let ret = ToUnicodeEx( vk as UINT, diff --git a/src/win/window.rs b/src/win/window.rs index db35523..42caafa 100644 --- a/src/win/window.rs +++ b/src/win/window.rs @@ -205,11 +205,8 @@ unsafe extern "system" fn wnd_proc( } } WM_TIMER => { - match wparam { - WIN_FRAME_TIMER => { - window_state.handler.on_frame(&mut window); - } - _ => (), + if wparam == WIN_FRAME_TIMER { + window_state.handler.on_frame(&mut window); } return 0; } @@ -414,8 +411,8 @@ impl Window { break; } - TranslateMessage(&mut msg); - DispatchMessageW(&mut msg); + TranslateMessage(&msg); + DispatchMessageW(&msg); } } } diff --git a/src/x11/cursor.rs b/src/x11/cursor.rs index 6f3851e..8e8fd88 100644 --- a/src/x11/cursor.rs +++ b/src/x11/cursor.rs @@ -101,5 +101,5 @@ pub(super) fn get_xcursor(display: *mut x11::xlib::Display, cursor: MouseCursor) MouseCursor::RowResize => loadn(&[b"split_v\0", b"v_double_arrow\0"]), }; - cursor.or(load(b"left_ptr\0")).unwrap_or(0) + cursor.or_else(|| load(b"left_ptr\0")).unwrap_or(0) } diff --git a/src/x11/window.rs b/src/x11/window.rs index 8e378df..bfe9567 100644 --- a/src/x11/window.rs +++ b/src/x11/window.rs @@ -35,7 +35,7 @@ pub struct WindowHandle { impl WindowHandle { pub fn close(&mut self) { - if let Some(_) = self.raw_window_handle.take() { + if self.raw_window_handle.take().is_some() { // FIXME: This will need to be changed from just setting an atomic to somehow // synchronizing with the window being closed (using a synchronous channel, or // by joining on the event loop thread). @@ -96,6 +96,7 @@ pub struct Window { xcb_connection: XcbConnection, window_id: u32, window_info: WindowInfo, + // FIXME: There's all this mouse cursor logic but it's never actually used, is this correct? mouse_cursor: MouseCursor, frame_interval: Duration, @@ -135,7 +136,7 @@ impl Window { let (parent_handle, mut window_handle) = ParentHandle::new(); - let thread = thread::spawn(move || { + thread::spawn(move || { Self::window_thread(Some(parent_id), options, build, tx.clone(), Some(parent_handle)); }); @@ -155,7 +156,7 @@ impl Window { let (parent_handle, mut window_handle) = ParentHandle::new(); - let thread = thread::spawn(move || { + thread::spawn(move || { Self::window_thread(None, options, build, tx.clone(), Some(parent_handle)); }); @@ -174,12 +175,14 @@ impl Window { let (tx, rx) = mpsc::sync_channel::(1); let thread = thread::spawn(move || { - Self::window_thread(None, options, build, tx.clone(), None); + Self::window_thread(None, options, build, tx, None); }); let _ = rx.recv().unwrap().unwrap(); - thread.join(); + thread.join().unwrap_or_else(|err| { + eprintln!("Window thread panicked: {:#?}", err); + }); } fn window_thread( @@ -302,16 +305,16 @@ impl Window { title.as_bytes(), ); - xcb_connection.atoms.wm_protocols.zip(xcb_connection.atoms.wm_delete_window).map( - |(wm_protocols, wm_delete_window)| { - xcb_util::icccm::set_wm_protocols( - &xcb_connection.conn, - window_id, - wm_protocols, - &[wm_delete_window], - ); - }, - ); + if let Some((wm_protocols, wm_delete_window)) = + xcb_connection.atoms.wm_protocols.zip(xcb_connection.atoms.wm_delete_window) + { + xcb_util::icccm::set_wm_protocols( + &xcb_connection.conn, + window_id, + wm_protocols, + &[wm_delete_window], + ); + } xcb_connection.conn.flush(); @@ -636,7 +639,7 @@ impl Window { handler.on_event( &mut crate::Window::new(self), - Event::Keyboard(convert_key_press_event(&event)), + Event::Keyboard(convert_key_press_event(event)), ); } @@ -645,7 +648,7 @@ impl Window { handler.on_event( &mut crate::Window::new(self), - Event::Keyboard(convert_key_release_event(&event)), + Event::Keyboard(convert_key_release_event(event)), ); } diff --git a/src/x11/xcb_connection.rs b/src/x11/xcb_connection.rs index 083249b..179e4c7 100644 --- a/src/x11/xcb_connection.rs +++ b/src/x11/xcb_connection.rs @@ -11,7 +11,6 @@ use super::cursor; pub(crate) struct Atoms { pub wm_protocols: Option, pub wm_delete_window: Option, - pub wm_normal_hints: Option, } pub struct XcbConnection { @@ -20,6 +19,7 @@ pub struct XcbConnection { pub(crate) atoms: Atoms, + // FIXME: Same here, there's a ton of unused cursor machinery in here pub(super) cursor_cache: HashMap, } @@ -46,14 +46,13 @@ impl XcbConnection { conn.set_event_queue_owner(xcb::base::EventQueueOwner::Xcb); - let (wm_protocols, wm_delete_window, wm_normal_hints) = - intern_atoms!(&conn, WM_PROTOCOLS, WM_DELETE_WINDOW, WM_NORMAL_HINTS); + let (wm_protocols, wm_delete_window) = intern_atoms!(&conn, WM_PROTOCOLS, WM_DELETE_WINDOW); Ok(Self { conn, xlib_display, - atoms: Atoms { wm_protocols, wm_delete_window, wm_normal_hints }, + atoms: Atoms { wm_protocols, wm_delete_window }, cursor_cache: HashMap::new(), }) @@ -136,7 +135,7 @@ impl XcbConnection { #[inline] pub fn get_scaling(&self) -> Option { - self.get_scaling_xft().or(self.get_scaling_screen_dimensions()) + self.get_scaling_xft().or_else(|| self.get_scaling_screen_dimensions()) } #[inline]