diff --git a/CHANGELOG.md b/CHANGELOG.md index 65094f3a36..451c5ff12d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -114,6 +114,7 @@ You can find its changes [documented below](#060---2020-06-01). - Fix `widget::Either` using the wrong paint insets ([#1299] by [@andrewhickman]) - Various fixes to cross-platform menus ([#1306] by [@raphlinus]) - Improve Windows 7 DXGI compatibility ([#1311] by [@raphlinus]) +- Don't drop events while showing file dialogs ([#1302], [#1328] by [@jneem]) ### Visual @@ -511,10 +512,12 @@ Last release without a changelog :( [#1295]: https://github.com/linebender/druid/pull/1280 [#1298]: https://github.com/linebender/druid/pull/1298 [#1299]: https://github.com/linebender/druid/pull/1299 +[#1302]: https://github.com/linebender/druid/pull/1302 [#1306]: https://github.com/linebender/druid/pull/1306 [#1311]: https://github.com/linebender/druid/pull/1311 [#1320]: https://github.com/linebender/druid/pull/1320 [#1326]: https://github.com/linebender/druid/pull/1326 +[#1328]: https://github.com/linebender/druid/pull/1328 [#1346]: https://github.com/linebender/druid/pull/1346 [Unreleased]: https://github.com/linebender/druid/compare/v0.6.0...master diff --git a/druid-shell/examples/shello.rs b/druid-shell/examples/shello.rs index 2a0624aa76..2808a0926b 100644 --- a/druid-shell/examples/shello.rs +++ b/druid-shell/examples/shello.rs @@ -18,8 +18,8 @@ use druid_shell::kurbo::{Line, Size}; use druid_shell::piet::{Color, RenderContext}; use druid_shell::{ - Application, Cursor, FileDialogOptions, FileSpec, HotKey, KeyEvent, Menu, MouseEvent, Region, - SysMods, TimerToken, WinHandler, WindowBuilder, WindowHandle, + Application, Cursor, FileDialogOptions, FileDialogToken, FileInfo, FileSpec, HotKey, KeyEvent, + Menu, MouseEvent, Region, SysMods, TimerToken, WinHandler, WindowBuilder, WindowHandle, }; const BG_COLOR: Color = Color::rgb8(0x27, 0x28, 0x22); @@ -56,13 +56,16 @@ impl WinHandler for HelloState { FileSpec::TEXT, FileSpec::JPG, ]); - let filename = self.handle.open_file_sync(options); - println!("result: {:?}", filename); + self.handle.open_file(options); } _ => println!("unexpected id {}", id), } } + fn open_file(&mut self, _token: FileDialogToken, file_info: Option) { + println!("open file result: {:?}", file_info); + } + fn key_down(&mut self, event: KeyEvent) -> bool { println!("keydown: {:?}", event); false diff --git a/druid-shell/src/platform/gtk/window.rs b/druid-shell/src/platform/gtk/window.rs index 090d28f12c..25fa2520e8 100644 --- a/druid-shell/src/platform/gtk/window.rs +++ b/druid-shell/src/platform/gtk/window.rs @@ -366,8 +366,6 @@ impl WindowBuilder { // Note that we're borrowing the surface while calling the handler. This is ok, // because we don't return control to the system or re-borrow the surface from // any code that the client can call. - // (TODO: the above comment isn't true yet because of save_as_sync and - // open_sync, but it will be true soon) state.with_handler_and_dont_check_the_other_borrows(|handler| { let surface_context = cairo::Context::new(surface); @@ -753,6 +751,10 @@ impl WindowState { .map(|s| FileInfo { path: s.into() }); self.with_handler(|h| h.save_as(token, file_info)); } + e => { + // The other deferred ops shouldn't appear, because we don't defer them in GTK. + log::error!("unexpected deferred op {:?}", e); + } } } } @@ -986,16 +988,6 @@ impl WindowHandle { } } - pub fn save_as_sync(&mut self, _options: FileDialogOptions) -> Option { - log::error!("save as sync no longer supported on GTK"); - None - } - - pub fn open_file_sync(&mut self, _options: FileDialogOptions) -> Option { - log::error!("open file sync no longer supported on GTK"); - None - } - /// Get a handle that can be used to schedule an idle task. pub fn get_idle_handle(&self) -> Option { self.state.upgrade().map(|s| IdleHandle { diff --git a/druid-shell/src/platform/mac/window.rs b/druid-shell/src/platform/mac/window.rs index fc5f62e14a..080290ba93 100644 --- a/druid-shell/src/platform/mac/window.rs +++ b/druid-shell/src/platform/mac/window.rs @@ -934,20 +934,10 @@ impl WindowHandle { } } - pub fn open_file_sync(&mut self, _options: FileDialogOptions) -> Option { - log::warn!("open_file_sync should no longer be called on mac!"); - None - } - pub fn open_file(&mut self, options: FileDialogOptions) -> Option { self.open_save_impl(FileDialogType::Open, options) } - pub fn save_as_sync(&mut self, _options: FileDialogOptions) -> Option { - log::warn!("save_as_sync should no longer be called on mac!"); - None - } - pub fn save_as(&mut self, options: FileDialogOptions) -> Option { self.open_save_impl(FileDialogType::Save, options) } diff --git a/druid-shell/src/platform/web/window.rs b/druid-shell/src/platform/web/window.rs index a390aa47dc..2d4388c1fd 100644 --- a/druid-shell/src/platform/web/window.rs +++ b/druid-shell/src/platform/web/window.rs @@ -564,25 +564,13 @@ impl WindowHandle { None } - pub fn open_file_sync(&mut self, options: FileDialogOptions) -> Option { - log::warn!("open_file_sync is currently unimplemented for web."); - self.file_dialog(FileDialogType::Open, options) - .ok() - .map(|s| FileInfo { path: s.into() }) - } - pub fn open_file(&mut self, _options: FileDialogOptions) -> Option { + log::warn!("open_file is currently unimplemented for web."); None } - pub fn save_as_sync(&mut self, options: FileDialogOptions) -> Option { - log::warn!("save_as_sync is currently unimplemented for web."); - self.file_dialog(FileDialogType::Save, options) - .ok() - .map(|s| FileInfo { path: s.into() }) - } - pub fn save_as(&mut self, _options: FileDialogOptions) -> Option { + log::warn!("save_as is currently unimplemented for web."); None } diff --git a/druid-shell/src/platform/windows/application.rs b/druid-shell/src/platform/windows/application.rs index c88335b8f2..c4bbaefc55 100644 --- a/druid-shell/src/platform/windows/application.rs +++ b/druid-shell/src/platform/windows/application.rs @@ -28,8 +28,8 @@ use winapi::um::errhandlingapi::GetLastError; use winapi::um::shellscalingapi::PROCESS_PER_MONITOR_DPI_AWARE; use winapi::um::winuser::{ DispatchMessageW, GetAncestor, GetMessageW, LoadIconW, PeekMessageW, PostMessageW, - PostQuitMessage, RegisterClassW, SendMessageW, TranslateAcceleratorW, TranslateMessage, - GA_ROOT, IDI_APPLICATION, MSG, PM_NOREMOVE, WM_TIMER, WNDCLASSW, + PostQuitMessage, RegisterClassW, TranslateAcceleratorW, TranslateMessage, GA_ROOT, + IDI_APPLICATION, MSG, PM_NOREMOVE, WM_TIMER, WNDCLASSW, }; use crate::application::AppHandler; @@ -38,7 +38,7 @@ use super::accels; use super::clipboard::Clipboard; use super::error::Error; use super::util::{self, ToWide, CLASS_NAME, OPTIONAL_FUNCTIONS}; -use super::window::{self, DS_HANDLE_DEFERRED, DS_REQUEST_DESTROY}; +use super::window::{self, DS_REQUEST_DESTROY}; #[derive(Clone)] pub(crate) struct Application { @@ -112,11 +112,6 @@ impl Application { // NOTE: Code here will not run when we aren't in charge of the message loop. That // will include when moving or resizing the window, and when showing modal dialogs. loop { - if let Ok(state) = self.state.try_borrow() { - for hwnd in &state.windows { - SendMessageW(*hwnd, DS_HANDLE_DEFERRED, 0, 0); - } - } let mut msg = mem::MaybeUninit::uninit(); // Timer messages have a low priority and tend to get delayed. Peeking for them diff --git a/druid-shell/src/platform/windows/window.rs b/druid-shell/src/platform/windows/window.rs index f6283983df..c0dab5d2b8 100644 --- a/druid-shell/src/platform/windows/window.rs +++ b/druid-shell/src/platform/windows/window.rs @@ -19,6 +19,7 @@ use std::any::Any; use std::cell::{Cell, RefCell}; use std::mem; +use std::panic::Location; use std::ptr::{null, null_mut}; use std::rc::{Rc, Weak}; use std::sync::{Arc, Mutex}; @@ -66,7 +67,7 @@ use crate::mouse::{Cursor, CursorDesc, MouseButton, MouseButtons, MouseEvent}; use crate::region::Region; use crate::scale::{Scalable, Scale, ScaledArea}; use crate::window; -use crate::window::{FileDialogToken, IdleToken, TimerToken, WinHandler, WindowLevel}; +use crate::window::{DeferredOp, FileDialogToken, IdleToken, TimerToken, WinHandler, WindowLevel}; /// The platform target DPI. /// @@ -123,142 +124,6 @@ pub struct WindowHandle { state: Weak, } -#[derive(Clone)] -enum DeferredOp { - SetPosition(Point), - SetSize(Size), - DecorationChanged(), - // Needs a better name - SetWindowSizeState(window::WindowState), -} - -struct DeferredQueue { - queue: Vec, -} - -impl DeferredQueue { - pub fn new() -> Self { - DeferredQueue { queue: Vec::new() } - } - - /// Adds a DeferredOp message to the queue. - /// The message will be run at a later time. - pub fn add(state: Weak, message: DeferredOp) { - if let Some(w) = state.upgrade() { - if let Ok(mut q) = w.deferred_queue.try_borrow_mut() { - q.queue.push(message) - } else { - warn!("failed to borrow deferred queue"); - } - } - } - - /// Empties the current message queue, returning a queue with the messages. - pub fn empty(&mut self) -> Vec { - let ret = self.queue.clone(); - self.queue = Vec::new(); - ret - } - - // Here we handle messages generated by WindowHandle - // that needs to run after borrow is released - fn handle_deferred(proc: &MyWndProc, op: DeferredOp) { - if let Some(hwnd) = proc.handle.borrow().get_hwnd() { - match op { - DeferredOp::SetSize(size) => unsafe { - if SetWindowPos( - hwnd, - HWND_TOPMOST, - 0, - 0, - (size.width * proc.scale().x()) as i32, - (size.height * proc.scale().y()) as i32, - SWP_NOMOVE | SWP_NOZORDER, - ) == 0 - { - warn!( - "failed to resize window: {}", - Error::Hr(HRESULT_FROM_WIN32(GetLastError())) - ); - }; - }, - DeferredOp::SetPosition(position) => unsafe { - if SetWindowPos( - hwnd, - HWND_TOPMOST, - position.x as i32, - position.y as i32, - 0, - 0, - SWP_NOSIZE | SWP_NOZORDER, - ) == 0 - { - warn!( - "failed to move window: {}", - Error::Hr(HRESULT_FROM_WIN32(GetLastError())) - ); - }; - }, - DeferredOp::DecorationChanged() => unsafe { - let resizable = proc.resizable(); - let titlebar = proc.has_titlebar(); - - let mut style = GetWindowLongPtrW(hwnd, GWL_STYLE) as u32; - if style == 0 { - warn!( - "failed to get window style: {}", - Error::Hr(HRESULT_FROM_WIN32(GetLastError())) - ); - return; - } - - if !resizable { - style &= !(WS_THICKFRAME | WS_MAXIMIZEBOX); - } else { - style |= WS_THICKFRAME | WS_MAXIMIZEBOX; - } - if !titlebar { - style &= !(WS_MINIMIZEBOX | WS_SYSMENU | WS_OVERLAPPED); - } else { - style |= WS_MINIMIZEBOX | WS_SYSMENU | WS_OVERLAPPED; - } - if SetWindowLongPtrW(hwnd, GWL_STYLE, style as isize) == 0 { - warn!( - "failed to set the window style: {}", - Error::Hr(HRESULT_FROM_WIN32(GetLastError())) - ); - } - if SetWindowPos( - hwnd, - HWND_TOPMOST, - 0, - 0, - 0, - 0, - SWP_SHOWWINDOW | SWP_NOMOVE | SWP_NOZORDER | SWP_FRAMECHANGED | SWP_NOSIZE, - ) == 0 - { - warn!( - "failed to update window style: {}", - Error::Hr(HRESULT_FROM_WIN32(GetLastError())) - ); - }; - }, - DeferredOp::SetWindowSizeState(val) => unsafe { - let s = match val { - window::WindowState::MAXIMIZED => SW_MAXIMIZE, - window::WindowState::MINIMIZED => SW_MINIMIZE, - window::WindowState::RESTORED => SW_RESTORE, - }; - ShowWindow(hwnd, s); - }, - } - } else { - warn!("Could not get HWND"); - } - } -} - /// A handle that can get used to schedule an idle handler. Note that /// this handle is thread safe. If the handle is used after the hwnd /// has been destroyed, probably not much will go wrong (the DS_RUN_IDLE @@ -286,7 +151,7 @@ struct WindowState { wndproc: Box, idle_queue: Arc>>, timers: Arc>, - deferred_queue: RefCell, + deferred_queue: RefCell>, has_titlebar: Cell, // For resizable borders, window can still be resized with code. is_resizable: Cell, @@ -366,21 +231,6 @@ const DS_RUN_IDLE: UINT = WM_USER; /// time it is handled, we can successfully borrow the handler. pub(crate) const DS_REQUEST_DESTROY: UINT = WM_USER + 1; -/// A message which must be delivered deferred due to reentrancy. -/// -/// Due to the architecture of Windows, it sometimes delivers messages -/// reentrantly. This is problematic when the mutable state for the window -/// is borrowed, as it's not possible to safely dispatch the message. The two -/// choices are to drop the message, or put it in a queue for deferred -/// processing. -/// -/// This mechanism should be used very sparingly, as delivery of these -/// deferred messages can not be guaranteed in a timely and reliable -/// fashion. In particular, we do not run our own runloop when handling -/// live resize, when a modal dialog is open, or when the application is a -/// a guest (such as a VST plugin). -pub(crate) const DS_HANDLE_DEFERRED: UINT = WM_USER + 2; - impl Default for PresentStrategy { fn default() -> PresentStrategy { PresentStrategy::Sequential @@ -424,6 +274,51 @@ fn is_point_in_client_rect(hwnd: HWND, x: i32, y: i32) -> bool { } } +fn set_style(hwnd: HWND, resizable: bool, titlebar: bool) { + unsafe { + let mut style = GetWindowLongPtrW(hwnd, GWL_STYLE) as u32; + if style == 0 { + warn!( + "failed to get window style: {}", + Error::Hr(HRESULT_FROM_WIN32(GetLastError())) + ); + return; + } + + if !resizable { + style &= !(WS_THICKFRAME | WS_MAXIMIZEBOX); + } else { + style |= WS_THICKFRAME | WS_MAXIMIZEBOX; + } + if !titlebar { + style &= !(WS_MINIMIZEBOX | WS_SYSMENU | WS_OVERLAPPED); + } else { + style |= WS_MINIMIZEBOX | WS_SYSMENU | WS_OVERLAPPED; + } + if SetWindowLongPtrW(hwnd, GWL_STYLE, style as isize) == 0 { + warn!( + "failed to set the window style: {}", + Error::Hr(HRESULT_FROM_WIN32(GetLastError())) + ); + } + if SetWindowPos( + hwnd, + HWND_TOPMOST, + 0, + 0, + 0, + 0, + SWP_SHOWWINDOW | SWP_NOMOVE | SWP_NOZORDER | SWP_FRAMECHANGED | SWP_NOSIZE, + ) == 0 + { + warn!( + "failed to update window style: {}", + Error::Hr(HRESULT_FROM_WIN32(GetLastError())) + ); + }; + } +} + impl WndState { fn rebuild_render_target(&mut self, d2d: &D2DFactory, scale: Scale) { unsafe { @@ -471,31 +366,41 @@ impl WndState { } impl MyWndProc { - /// Create debugging output for dropped messages due to wndproc reentrancy. - /// - /// In the future, we choose to do something else other than logging and dropping, - /// such as queuing and replaying after the nested call returns. - fn log_dropped_msg(&self, hwnd: HWND, msg: UINT, wparam: WPARAM, lparam: LPARAM) { - error!( - "dropped message 0x{:x}, hwnd={:?}, wparam=0x{:x}, lparam=0x{:x}", - msg, hwnd, wparam, lparam - ); - } - fn with_window_state(&self, f: F) -> R where F: FnOnce(Rc) -> R, { f(self .handle - // Right now there aren't any mutable borrows to this. - // TODO: Attempt to guarantee this by making mutable handle borrows useless. + // There are no mutable borrows to this: we only use a mutable borrow during + // initialization. .borrow() .state .upgrade() .unwrap()) // WindowState drops after WM_NCDESTROY, so it's always here. } + #[track_caller] + fn with_wnd_state(&self, f: F) -> Option + where + F: FnOnce(&mut WndState) -> R, + { + let ret = if let Ok(mut s) = self.state.try_borrow_mut() { + if let Some(state) = &mut *s { + Some(f(state)) + } else { + None + } + } else { + log::error!("failed to borrow WndState at {}", Location::caller()); + None + }; + if ret.is_some() { + self.handle_deferred_queue(); + } + ret + } + fn scale(&self) -> Scale { self.with_window_state(|state| state.scale.get()) } @@ -532,9 +437,86 @@ impl MyWndProc { } fn handle_deferred_queue(&self) { - let q = self.with_window_state(move |state| state.deferred_queue.borrow_mut().empty()); + let q = self.with_window_state(move |state| state.deferred_queue.replace(Vec::new())); for op in q { - DeferredQueue::handle_deferred(self, op); + self.handle_deferred(op); + } + } + + fn handle_deferred(&self, op: DeferredOp) { + if let Some(hwnd) = self.handle.borrow().get_hwnd() { + match op { + DeferredOp::SetSize(size) => unsafe { + if SetWindowPos( + hwnd, + HWND_TOPMOST, + 0, + 0, + (size.width * self.scale().x()) as i32, + (size.height * self.scale().y()) as i32, + SWP_NOMOVE | SWP_NOZORDER, + ) == 0 + { + warn!( + "failed to resize window: {}", + Error::Hr(HRESULT_FROM_WIN32(GetLastError())) + ); + }; + }, + DeferredOp::SetPosition(position) => unsafe { + if SetWindowPos( + hwnd, + HWND_TOPMOST, + position.x as i32, + position.y as i32, + 0, + 0, + SWP_NOSIZE | SWP_NOZORDER, + ) == 0 + { + warn!( + "failed to move window: {}", + Error::Hr(HRESULT_FROM_WIN32(GetLastError())) + ); + }; + }, + DeferredOp::ShowTitlebar(titlebar) => { + self.with_window_state(|s| s.has_titlebar.set(titlebar)); + set_style(hwnd, self.resizable(), titlebar); + } + DeferredOp::SetResizable(resizable) => { + self.with_window_state(|s| s.is_resizable.set(resizable)); + set_style(hwnd, resizable, self.has_titlebar()); + } + DeferredOp::SetWindowState(val) => unsafe { + let s = match val { + window::WindowState::MAXIMIZED => SW_MAXIMIZE, + window::WindowState::MINIMIZED => SW_MINIMIZE, + window::WindowState::RESTORED => SW_RESTORE, + }; + ShowWindow(hwnd, s); + }, + DeferredOp::SaveAs(options, token) => { + let info = unsafe { + get_file_dialog_path(hwnd, FileDialogType::Save, options) + .ok() + .map(|os_str| FileInfo { + path: os_str.into(), + }) + }; + self.with_wnd_state(|s| s.handler.save_as(token, info)); + } + DeferredOp::Open(options, token) => { + let info = unsafe { + get_file_dialog_path(hwnd, FileDialogType::Open, options) + .ok() + .map(|s| FileInfo { path: s.into() }) + }; + self.with_wnd_state(|s| s.handler.open_file(token, info)); + } + } + } else { + warn!("Could not get HWND"); } } } @@ -630,17 +612,11 @@ impl WndProc for MyWndProc { } WM_ERASEBKGND => Some(0), WM_SETFOCUS => { - if let Ok(mut s) = self.state.try_borrow_mut() { - let s = s.as_mut().unwrap(); - s.handler.got_focus(); - } else { - self.log_dropped_msg(hwnd, msg, wparam, lparam); - } + self.with_wnd_state(|s| s.handler.got_focus()); Some(0) } WM_PAINT => unsafe { - if let Ok(mut s) = self.state.try_borrow_mut() { - let s = s.as_mut().unwrap(); + self.with_wnd_state(|s| { // We call prepare_paint before GetUpdateRect, so that anything invalidated during // prepare_paint will be reflected in GetUpdateRect. s.handler.prepare_paint(); @@ -668,9 +644,7 @@ impl WndProc for MyWndProc { (*ds.swap_chain).Present1(1, 0, ¶ms); } } - } else { - self.log_dropped_msg(hwnd, msg, wparam, lparam); - } + }); Some(0) }, WM_DPICHANGED => unsafe { @@ -780,8 +754,7 @@ impl WndProc for MyWndProc { if width == 0 || height == 0 { return Some(0); } - if let Ok(mut s) = self.state.try_borrow_mut() { - let s = s.as_mut().unwrap(); + self.with_wnd_state(|s| { let scale = self.scale(); let area = ScaledArea::from_px((width as f64, height as f64), scale); let size_dp = area.size_dp(); @@ -812,26 +785,18 @@ impl WndProc for MyWndProc { } else { error!("ResizeBuffers failed: 0x{:x}", res); } - } else { - self.log_dropped_msg(hwnd, msg, wparam, lparam); - } - Some(0) + }) + .map(|_| 0) }, WM_COMMAND => { - if let Ok(mut s) = self.state.try_borrow_mut() { - let s = s.as_mut().unwrap(); - s.handler.command(LOWORD(wparam as u32) as u32); - } else { - self.log_dropped_msg(hwnd, msg, wparam, lparam); - } + self.with_wnd_state(|s| s.handler.command(LOWORD(wparam as u32) as u32)); Some(0) } //TODO: WM_SYSCOMMAND WM_CHAR | WM_SYSCHAR | WM_KEYDOWN | WM_SYSKEYDOWN | WM_KEYUP | WM_SYSKEYUP | WM_INPUTLANGCHANGE => { unsafe { - if let Ok(mut s) = self.state.try_borrow_mut() { - let s = s.as_mut().unwrap(); + let handled = self.with_wnd_state(|s| { if let Some(event) = s.keyboard_state.process_message(hwnd, msg, wparam, lparam) { @@ -843,28 +808,30 @@ impl WndProc for MyWndProc { match event.state { KeyState::Down => { if s.handler.key_down(event) || handle_menu { - return Some(0); + return true; } } KeyState::Up => { s.handler.key_up(event); if handle_menu { - return Some(0); + return true; } } } } + false + }); + if handled == Some(true) { + Some(0) } else { - self.log_dropped_msg(hwnd, msg, wparam, lparam); + None } } - None } WM_MOUSEWHEEL | WM_MOUSEHWHEEL => { // TODO: apply mouse sensitivity based on // SPI_GETWHEELSCROLLLINES setting. - if let Ok(mut s) = self.state.try_borrow_mut() { - let s = s.as_mut().unwrap(); + let handled = self.with_wnd_state(|s| { let system_delta = HIWORD(wparam as u32) as i16 as f64; let down_state = LOWORD(wparam as u32) as usize; let mods = s.keyboard_state.get_modifiers(); @@ -886,7 +853,7 @@ impl WndProc for MyWndProc { "ScreenToClient failed: {}", Error::Hr(HRESULT_FROM_WIN32(GetLastError())) ); - return None; + return false; } } @@ -902,14 +869,16 @@ impl WndProc for MyWndProc { wheel_delta, }; s.handler.wheel(&event); + true + }); + if handled == Some(false) { + None } else { - self.log_dropped_msg(hwnd, msg, wparam, lparam); + Some(0) } - Some(0) } WM_MOUSEMOVE => { - if let Ok(mut s) = self.state.try_borrow_mut() { - let s = s.as_mut().unwrap(); + self.with_wnd_state(|s| { let x = LOWORD(lparam as u32) as i16 as i32; let y = HIWORD(lparam as u32) as i16 as i32; @@ -949,19 +918,14 @@ impl WndProc for MyWndProc { wheel_delta: Vec2::ZERO, }; s.handler.mouse_move(&event); - } else { - self.log_dropped_msg(hwnd, msg, wparam, lparam); - } + }); Some(0) } WM_MOUSELEAVE => { - if let Ok(mut s) = self.state.try_borrow_mut() { - let s = s.as_mut().unwrap(); + self.with_wnd_state(|s| { s.has_mouse_focus = false; s.handler.mouse_leave(); - } else { - self.log_dropped_msg(hwnd, msg, wparam, lparam); - } + }); Some(0) } // Note: we handle the double-click events out of caution here, but we don't expect @@ -989,8 +953,7 @@ impl WndProc for MyWndProc { } _ => unreachable!(), } { - if let Ok(mut s) = self.state.try_borrow_mut() { - let s = s.as_mut().unwrap(); + self.with_wnd_state(|s| { let down = matches!( msg, WM_LBUTTONDOWN @@ -1042,9 +1005,7 @@ impl WndProc for MyWndProc { s.handler.mouse_up(&event); should_release_capture = s.exit_mouse_capture(button); } - } else { - self.log_dropped_msg(hwnd, msg, wparam, lparam); - } + }); } // ReleaseCapture() is deferred: it needs to be called without having a mutable @@ -1063,15 +1024,9 @@ impl WndProc for MyWndProc { Some(0) } - WM_CLOSE => { - if let Ok(mut s) = self.state.try_borrow_mut() { - let s = s.as_mut().unwrap(); - s.handler.request_close(); - Some(0) - } else { - None - } - } + WM_CLOSE => self + .with_wnd_state(|s| s.handler.request_close()) + .map(|_| 0), DS_REQUEST_DESTROY => { unsafe { DestroyWindow(hwnd); @@ -1079,12 +1034,7 @@ impl WndProc for MyWndProc { Some(0) } WM_DESTROY => { - if let Ok(mut s) = self.state.try_borrow_mut() { - let s = s.as_mut().unwrap(); - s.handler.destroy(); - } else { - self.log_dropped_msg(hwnd, msg, wparam, lparam); - } + self.with_wnd_state(|s| s.handler.destroy()); Some(0) } WM_TIMER => { @@ -1094,39 +1044,27 @@ impl WndProc for MyWndProc { } let token = TimerToken::from_raw(id as u64); self.handle.borrow().free_timer_slot(token); - if let Ok(mut s) = self.state.try_borrow_mut() { - let s = s.as_mut().unwrap(); - s.handler.timer(token); - } + self.with_wnd_state(|s| s.handler.timer(token)); Some(1) } WM_CAPTURECHANGED => { - if let Ok(mut s) = self.state.try_borrow_mut() { - let s = s.as_mut().unwrap(); - s.captured_mouse_buttons.clear(); - } else { - self.log_dropped_msg(hwnd, msg, wparam, lparam); - } + self.with_wnd_state(|s| s.captured_mouse_buttons.clear()); Some(0) } WM_GETMINMAXINFO => { let min_max_info = unsafe { &mut *(lparam as *mut MINMAXINFO) }; - if let Ok(s) = self.state.try_borrow() { - let s = s.as_ref().unwrap(); + self.with_wnd_state(|s| { if let Some(min_size_dp) = s.min_size { let min_area = ScaledArea::from_dp(min_size_dp, self.scale()); let min_size_px = min_area.size_px(); min_max_info.ptMinTrackSize.x = min_size_px.width as i32; min_max_info.ptMinTrackSize.y = min_size_px.height as i32; } - } else { - self.log_dropped_msg(hwnd, msg, wparam, lparam); - } + }); Some(0) } - DS_RUN_IDLE => { - if let Ok(mut s) = self.state.try_borrow_mut() { - let s = s.as_mut().unwrap(); + DS_RUN_IDLE => self + .with_wnd_state(|s| { let queue = self.handle.borrow().take_idle_queue(); for callback in queue { match callback { @@ -1134,15 +1072,8 @@ impl WndProc for MyWndProc { IdleKind::Token(token) => s.handler.idle(token), } } - Some(0) - } else { - None - } - } - DS_HANDLE_DEFERRED => { - self.handle_deferred_queue(); - Some(0) - } + }) + .map(|_| 0), _ => None, } } @@ -1253,7 +1184,7 @@ impl WindowBuilder { wndproc: Box::new(wndproc), idle_queue: Default::default(), timers: Arc::new(Mutex::new(TimerSlots::new(1))), - deferred_queue: RefCell::new(DeferredQueue::new()), + deferred_queue: RefCell::new(Vec::new()), has_titlebar: Cell::new(self.show_titlebar), is_resizable: Cell::new(self.resizable), handle_titlebar: Cell::new(false), @@ -1588,6 +1519,12 @@ impl WindowHandle { self.request_anim_frame(); } + fn defer(&self, op: DeferredOp) { + if let Some(w) = self.state.upgrade() { + w.deferred_queue.borrow_mut().push(op); + } + } + /// Set the title for this menu. pub fn set_title(&self, title: &str) { if let Some(w) = self.state.upgrade() { @@ -1601,19 +1538,13 @@ impl WindowHandle { } pub fn show_titlebar(&self, show_titlebar: bool) { - if let Some(w) = self.state.upgrade() { - w.has_titlebar.set(show_titlebar); - DeferredQueue::add(self.state.clone(), DeferredOp::DecorationChanged()); - } + self.defer(DeferredOp::ShowTitlebar(show_titlebar)); } // Sets the position of the window in virtual screen coordinates pub fn set_position(&self, position: Point) { - DeferredQueue::add( - self.state.clone(), - DeferredOp::SetWindowSizeState(window::WindowState::RESTORED), - ); - DeferredQueue::add(self.state.clone(), DeferredOp::SetPosition(position)); + self.defer(DeferredOp::SetWindowState(window::WindowState::RESTORED)); + self.defer(DeferredOp::SetPosition(position)); } pub fn set_level(&self, _level: WindowLevel) { @@ -1645,7 +1576,7 @@ impl WindowHandle { // Sets the size of the window in DP pub fn set_size(&self, size: Size) { - DeferredQueue::add(self.state.clone(), DeferredOp::SetSize(size)); + self.defer(DeferredOp::SetSize(size)); } // Gets the size of the window in pixels @@ -1674,15 +1605,12 @@ impl WindowHandle { } pub fn resizable(&self, resizable: bool) { - if let Some(w) = self.state.upgrade() { - w.is_resizable.set(resizable); - DeferredQueue::add(self.state.clone(), DeferredOp::DecorationChanged()); - } + self.defer(DeferredOp::SetResizable(resizable)); } // Sets the window state. - pub fn set_window_state(&mut self, state: window::WindowState) { - DeferredQueue::add(self.state.clone(), DeferredOp::SetWindowSizeState(state)); + pub fn set_window_state(&self, state: window::WindowState) { + self.defer(DeferredOp::SetWindowState(state)); } // Gets the window state. @@ -1850,42 +1778,16 @@ impl WindowHandle { } } - //FIXME: these two methods will be reworked to avoid reentrancy problems. - // Currently, calling it may result in important messages being dropped. - /// Prompt the user to chose a file to open. - /// - /// Blocks while the user picks the file. - pub fn open_file_sync(&mut self, options: FileDialogOptions) -> Option { - let hwnd = self.get_hwnd()?; - unsafe { - get_file_dialog_path(hwnd, FileDialogType::Open, options) - .ok() - .map(|s| FileInfo { path: s.into() }) - } - } - - pub fn open_file(&mut self, _options: FileDialogOptions) -> Option { - // TODO: implement this and remove open_file_sync - None - } - - /// Prompt the user to chose a file to open. - /// - /// Blocks while the user picks the file. - pub fn save_as_sync(&mut self, options: FileDialogOptions) -> Option { - let hwnd = self.get_hwnd()?; - unsafe { - get_file_dialog_path(hwnd, FileDialogType::Save, options) - .ok() - .map(|os_str| FileInfo { - path: os_str.into(), - }) - } + pub fn open_file(&mut self, options: FileDialogOptions) -> Option { + let tok = FileDialogToken::next(); + self.defer(DeferredOp::Open(options, tok)); + Some(tok) } - pub fn save_as(&mut self, _options: FileDialogOptions) -> Option { - // TODO: implement this and remove save_as_sync - None + pub fn save_as(&mut self, options: FileDialogOptions) -> Option { + let tok = FileDialogToken::next(); + self.defer(DeferredOp::SaveAs(options, tok)); + Some(tok) } /// Get the raw HWND handle, for uses that are not wrapped in diff --git a/druid-shell/src/platform/x11/window.rs b/druid-shell/src/platform/x11/window.rs index abdfb86004..5ebf1d7159 100644 --- a/druid-shell/src/platform/x11/window.rs +++ b/druid-shell/src/platform/x11/window.rs @@ -38,7 +38,7 @@ use x11rb::wrapper::ConnectionExt as _; use x11rb::xcb_ffi::XCBConnection; use crate::common_util::IdleCallback; -use crate::dialog::{FileDialogOptions, FileInfo}; +use crate::dialog::FileDialogOptions; use crate::error::Error as ShellError; use crate::keyboard::{KeyEvent, KeyState, Modifiers}; use crate::kurbo::{Point, Rect, Size, Vec2}; @@ -1497,22 +1497,12 @@ impl WindowHandle { None } - pub fn open_file_sync(&mut self, _options: FileDialogOptions) -> Option { - log::warn!("WindowHandle::open_file_sync is currently unimplemented for X11 platforms."); - None - } - pub fn open_file(&mut self, _options: FileDialogOptions) -> Option { // TODO(x11/file_dialogs): implement WindowHandle::open_file log::warn!("WindowHandle::open_file is currently unimplemented for X11 platforms."); None } - pub fn save_as_sync(&mut self, _options: FileDialogOptions) -> Option { - log::warn!("WindowHandle::save_as_sync is currently unimplemented for X11 platforms."); - None - } - pub fn save_as(&mut self, _options: FileDialogOptions) -> Option { // TODO(x11/file_dialogs): implement WindowHandle::save_as log::warn!("WindowHandle::save_as is currently unimplemented for X11 platforms."); diff --git a/druid-shell/src/window.rs b/druid-shell/src/window.rs index 3eda399bc1..b78dc9936b 100644 --- a/druid-shell/src/window.rs +++ b/druid-shell/src/window.rs @@ -138,9 +138,15 @@ impl FileDialogToken { /// 4. after some more processing, `WinHandler::mouse_up` returns /// 5. druid-shell displays the "save as" dialog that was requested in step 3. #[allow(dead_code)] +#[derive(Debug)] pub(crate) enum DeferredOp { SaveAs(FileDialogOptions, FileDialogToken), Open(FileDialogOptions, FileDialogToken), + ShowTitlebar(bool), + SetPosition(Point), + SetSize(Size), + SetResizable(bool), + SetWindowState(WindowState), } /// Levels in the window system - Z order for display purposes. @@ -341,13 +347,6 @@ impl WindowHandle { self.0.make_cursor(desc) } - /// Prompt the user to choose a file to open. - /// - /// Blocks while the user picks the file. - pub fn open_file_sync(&mut self, options: FileDialogOptions) -> Option { - self.0.open_file_sync(options) - } - /// Prompt the user to choose a file to open. /// /// This won't block immediately; the file dialog will be shown whenever control returns to @@ -359,13 +358,6 @@ impl WindowHandle { self.0.open_file(options) } - /// Prompt the user to choose a path for saving. - /// - /// Blocks while the user picks a file. - pub fn save_as_sync(&mut self, options: FileDialogOptions) -> Option { - self.0.save_as_sync(options) - } - /// Prompt the user to choose a path for saving. /// /// This won't block immediately; the file dialog will be shown whenever control returns to diff --git a/druid/src/win_handler.rs b/druid/src/win_handler.rs index e7c10cba65..8188abe4ad 100644 --- a/druid/src/win_handler.rs +++ b/druid/src/win_handler.rs @@ -618,24 +618,12 @@ impl AppState { .get_mut(window_id) .map(|w| w.handle.clone()); - let token = handle - .clone() - .and_then(|mut handle| handle.open_file(options.clone())); + let token = handle.and_then(|mut handle| handle.open_file(options)); if let Some(token) = token { self.inner .borrow_mut() .file_dialogs .insert(token, window_id); - } else { - // TODO: remove this (and also some spurious clones above) once all platforms support - // the non-sync version - let file_info = handle.and_then(|mut handle| handle.open_file_sync(options)); - let cmd = if let Some(info) = file_info { - sys_cmd::OPEN_FILE.with(info).to(window_id) - } else { - sys_cmd::OPEN_PANEL_CANCELLED.to(window_id) - }; - self.inner.borrow_mut().dispatch_cmd(cmd); } } @@ -648,24 +636,12 @@ impl AppState { .get_mut(window_id) .map(|w| w.handle.clone()); - let token = handle - .clone() - .and_then(|mut handle| handle.save_as(options.clone())); + let token = handle.and_then(|mut handle| handle.save_as(options)); if let Some(token) = token { self.inner .borrow_mut() .file_dialogs .insert(token, window_id); - } else { - // TODO: remove this (and also some spurious clones above) once all platforms support - // the non-sync version - let file_info = handle.and_then(|mut handle| handle.save_as_sync(options)); - let cmd = if let Some(info) = file_info { - sys_cmd::SAVE_FILE.with(Some(info)).to(window_id) - } else { - sys_cmd::SAVE_PANEL_CANCELLED.to(window_id) - }; - self.inner.borrow_mut().dispatch_cmd(cmd); } }