From 0039e3cd005af9883b20fce7a866c3cda104eb89 Mon Sep 17 00:00:00 2001 From: Kaur Kuut Date: Sat, 18 Apr 2020 01:10:00 +0300 Subject: [PATCH 1/3] Terminate app run loop on Windows when all windows have closed. --- druid-shell/examples/perftest.rs | 7 +- druid-shell/examples/shello.rs | 9 +- druid-shell/src/application.rs | 35 +++- druid-shell/src/lib.rs | 2 +- druid-shell/src/platform/gtk/application.rs | 11 +- druid-shell/src/platform/gtk/window.rs | 4 +- druid-shell/src/platform/mac/application.rs | 11 +- druid-shell/src/platform/mac/window.rs | 3 +- druid-shell/src/platform/web/application.rs | 11 +- druid-shell/src/platform/web/window.rs | 3 +- .../src/platform/windows/application.rs | 119 +++++++++++-- druid-shell/src/platform/windows/error.rs | 3 + druid-shell/src/platform/windows/util.rs | 69 +++++++- druid-shell/src/platform/windows/window.rs | 161 ++++++++++++++++-- druid-shell/src/platform/x11/application.rs | 11 +- druid-shell/src/platform/x11/window.rs | 4 +- druid-shell/src/window.rs | 11 +- druid/src/app.rs | 17 +- druid/src/command.rs | 3 + druid/src/win_handler.rs | 36 +++- 20 files changed, 467 insertions(+), 63 deletions(-) diff --git a/druid-shell/examples/perftest.rs b/druid-shell/examples/perftest.rs index 16fbffba21..968857e46a 100644 --- a/druid-shell/examples/perftest.rs +++ b/druid-shell/examples/perftest.rs @@ -19,7 +19,7 @@ use time::Instant; use piet_common::kurbo::{Line, Rect}; use piet_common::{Color, FontBuilder, Piet, RenderContext, Text, TextLayoutBuilder}; -use druid_shell::{Application, KeyEvent, WinHandler, WindowBuilder, WindowHandle}; +use druid_shell::{AppState, Application, KeyEvent, WinHandler, WindowBuilder, WindowHandle}; const BG_COLOR: Color = Color::rgb8(0x27, 0x28, 0x22); const FG_COLOR: Color = Color::rgb8(0xf0, 0xf0, 0xea); @@ -116,8 +116,9 @@ impl WinHandler for PerfTest { } fn main() { - let mut app = Application::new(None); - let mut builder = WindowBuilder::new(); + let state = AppState::new(); + let mut app = Application::new(state.clone(), None); + let mut builder = WindowBuilder::new(state); let perf_test = PerfTest { size: Default::default(), handle: Default::default(), diff --git a/druid-shell/examples/shello.rs b/druid-shell/examples/shello.rs index 14e41ab102..a86820394d 100644 --- a/druid-shell/examples/shello.rs +++ b/druid-shell/examples/shello.rs @@ -18,8 +18,8 @@ use druid_shell::kurbo::{Line, Rect, Vec2}; use druid_shell::piet::{Color, RenderContext}; use druid_shell::{ - Application, Cursor, FileDialogOptions, FileSpec, HotKey, KeyEvent, KeyModifiers, Menu, - MouseEvent, SysMods, TimerToken, WinHandler, WindowBuilder, WindowHandle, + AppState, Application, Cursor, FileDialogOptions, FileSpec, HotKey, KeyEvent, KeyModifiers, + Menu, MouseEvent, SysMods, TimerToken, WinHandler, WindowBuilder, WindowHandle, }; const BG_COLOR: Color = Color::rgb8(0x27, 0x28, 0x22); @@ -129,8 +129,9 @@ fn main() { menubar.add_dropdown(Menu::new(), "Application", true); menubar.add_dropdown(file_menu, "&File", true); - let mut app = Application::new(None); - let mut builder = WindowBuilder::new(); + let state = AppState::new(); + let mut app = Application::new(state.clone(), None); + let mut builder = WindowBuilder::new(state); builder.set_handler(Box::new(HelloState::default())); builder.set_title("Hello example"); builder.set_menu(menubar); diff --git a/druid-shell/src/application.rs b/druid-shell/src/application.rs index 4306642571..6289de2a09 100644 --- a/druid-shell/src/application.rs +++ b/druid-shell/src/application.rs @@ -14,6 +14,8 @@ //! The top-level application type. +use std::sync::atomic::{AtomicBool, Ordering}; + use crate::clipboard::Clipboard; use crate::platform::application as platform; @@ -36,14 +38,43 @@ pub trait AppHandler { fn command(&mut self, id: u32) {} } +/// The top level application state. +/// +/// This helps the application track all the state that it has created, +/// which it later needs to clean up. +#[derive(Clone)] +pub struct AppState(pub(crate) platform::AppState); + +impl AppState { + /// Create a new `AppState` instance. + pub fn new() -> AppState { + AppState(platform::AppState::new()) + } +} + //TODO: we may want to make the user create an instance of this (Application::global()?) //but for now I'd like to keep changes minimal. /// The top level application object. pub struct Application(platform::Application); +// Used to ensure only one Application instance is ever created. +// This may change in the future. +// For more information see https://github.com/xi-editor/druid/issues/771 +static APPLICATION_CREATED: AtomicBool = AtomicBool::new(false); + impl Application { - pub fn new(handler: Option>) -> Application { - Application(platform::Application::new(handler)) + /// Create a new `Application`. + /// + /// It takes the application `state` and a `handler` which will be used to inform of events. + /// + /// Right now only one application can be created. See [druid#771] for discussion. + /// + /// [druid#771]: https://github.com/xi-editor/druid/issues/771 + pub fn new(state: AppState, handler: Option>) -> Application { + if APPLICATION_CREATED.compare_and_swap(false, true, Ordering::AcqRel) { + panic!("The Application instance has already been created."); + } + Application(platform::Application::new(state.0, handler)) } /// Start the runloop. diff --git a/druid-shell/src/lib.rs b/druid-shell/src/lib.rs index ed9a26edb0..e4c5099690 100644 --- a/druid-shell/src/lib.rs +++ b/druid-shell/src/lib.rs @@ -37,7 +37,7 @@ mod mouse; mod platform; mod window; -pub use application::{AppHandler, Application}; +pub use application::{AppHandler, AppState, Application}; pub use clipboard::{Clipboard, ClipboardFormat, FormatId}; pub use common_util::Counter; pub use dialog::{FileDialogOptions, FileInfo, FileSpec}; diff --git a/druid-shell/src/platform/gtk/application.rs b/druid-shell/src/platform/gtk/application.rs index 39895c4d77..604e2062eb 100644 --- a/druid-shell/src/platform/gtk/application.rs +++ b/druid-shell/src/platform/gtk/application.rs @@ -31,10 +31,19 @@ thread_local!( static GTK_APPLICATION: RefCell> = RefCell::new(None); ); +#[derive(Clone)] +pub struct AppState; + pub struct Application; +impl AppState { + pub(crate) fn new() -> AppState { + AppState + } +} + impl Application { - pub fn new(_handler: Option>) -> Application { + pub fn new(_state: AppState, _handler: Option>) -> Application { // TODO: we should give control over the application ID to the user let application = GtkApplication::new( Some("com.github.xi-editor.druid"), diff --git a/druid-shell/src/platform/gtk/window.rs b/druid-shell/src/platform/gtk/window.rs index 0ca8dc6ed4..fe84219304 100644 --- a/druid-shell/src/platform/gtk/window.rs +++ b/druid-shell/src/platform/gtk/window.rs @@ -33,7 +33,7 @@ use gtk::{AccelGroup, ApplicationWindow}; use crate::kurbo::{Point, Rect, Size, Vec2}; use crate::piet::{Piet, RenderContext}; -use super::application::with_application; +use super::application::{with_application, AppState}; use super::dialog; use super::menu::Menu; use super::util::assert_main_thread; @@ -111,7 +111,7 @@ pub(crate) struct WindowState { } impl WindowBuilder { - pub fn new() -> WindowBuilder { + pub fn new(_app_state: AppState) -> WindowBuilder { WindowBuilder { handler: None, title: String::new(), diff --git a/druid-shell/src/platform/mac/application.rs b/druid-shell/src/platform/mac/application.rs index 4dac5adb62..e8144969c0 100644 --- a/druid-shell/src/platform/mac/application.rs +++ b/druid-shell/src/platform/mac/application.rs @@ -32,12 +32,21 @@ use crate::application::AppHandler; static APP_HANDLER_IVAR: &str = "druidAppHandler"; +#[derive(Clone)] +pub struct AppState; + pub struct Application { ns_app: id, } +impl AppState { + pub(crate) fn new() -> AppState { + AppState + } +} + impl Application { - pub fn new(handler: Option>) -> Application { + pub fn new(_state: AppState, handler: Option>) -> Application { util::assert_main_thread(); unsafe { let _pool = NSAutoreleasePool::new(nil); diff --git a/druid-shell/src/platform/mac/window.rs b/druid-shell/src/platform/mac/window.rs index 9dcd5f30c1..2a4203bb49 100644 --- a/druid-shell/src/platform/mac/window.rs +++ b/druid-shell/src/platform/mac/window.rs @@ -41,6 +41,7 @@ use log::{error, info}; use crate::kurbo::{Point, Rect, Size, Vec2}; use crate::piet::{Piet, RenderContext}; +use super::application::AppState; use super::dialog; use super::menu::Menu; use super::util::{assert_main_thread, make_nsstring}; @@ -104,7 +105,7 @@ struct ViewState { } impl WindowBuilder { - pub fn new() -> WindowBuilder { + pub fn new(_app_state: AppState) -> WindowBuilder { WindowBuilder { handler: None, title: String::new(), diff --git a/druid-shell/src/platform/web/application.rs b/druid-shell/src/platform/web/application.rs index fa3ddd98ba..c2e9f60505 100644 --- a/druid-shell/src/platform/web/application.rs +++ b/druid-shell/src/platform/web/application.rs @@ -17,10 +17,19 @@ use super::clipboard::Clipboard; use crate::application::AppHandler; +#[derive(Clone)] +pub struct AppState; + pub struct Application; +impl AppState { + pub(crate) fn new() -> AppState { + AppState + } +} + impl Application { - pub fn new(_handler: Option>) -> Application { + pub fn new(_state: AppState, _handler: Option>) -> Application { Application } diff --git a/druid-shell/src/platform/web/window.rs b/druid-shell/src/platform/web/window.rs index e94427f34b..230de81ac9 100644 --- a/druid-shell/src/platform/web/window.rs +++ b/druid-shell/src/platform/web/window.rs @@ -29,6 +29,7 @@ use crate::kurbo::{Point, Rect, Size, Vec2}; use crate::piet::RenderContext; +use super::application::AppState; use super::error::Error; use super::keycodes::key_to_text; use super::menu::Menu; @@ -291,7 +292,7 @@ fn setup_web_callbacks(window_state: &Rc) { } impl WindowBuilder { - pub fn new() -> WindowBuilder { + pub fn new(_app_state: AppState) -> WindowBuilder { WindowBuilder { handler: None, title: String::new(), diff --git a/druid-shell/src/platform/windows/application.rs b/druid-shell/src/platform/windows/application.rs index db550505fd..af026b554a 100644 --- a/druid-shell/src/platform/windows/application.rs +++ b/druid-shell/src/platform/windows/application.rs @@ -14,16 +14,21 @@ //! Windows implementation of features at the application scope. +use std::cell::RefCell; +use std::collections::HashSet; use std::mem; use std::ptr; +use std::rc::Rc; -use winapi::shared::minwindef::HINSTANCE; +use winapi::shared::minwindef::{FALSE, HINSTANCE}; use winapi::shared::ntdef::LPCWSTR; -use winapi::shared::windef::HCURSOR; +use winapi::shared::windef::{HCURSOR, HWND}; +use winapi::shared::winerror::HRESULT_FROM_WIN32; +use winapi::um::errhandlingapi::GetLastError; use winapi::um::shellscalingapi::PROCESS_SYSTEM_DPI_AWARE; use winapi::um::wingdi::CreateSolidBrush; use winapi::um::winuser::{ - DispatchMessageW, GetAncestor, GetMessageW, LoadIconW, PostQuitMessage, RegisterClassW, + DispatchMessageW, GetAncestor, GetMessageW, LoadIconW, PostMessageW, RegisterClassW, TranslateAcceleratorW, TranslateMessage, GA_ROOT, IDI_APPLICATION, MSG, WNDCLASSW, }; @@ -31,14 +36,79 @@ use crate::application::AppHandler; use super::accels; use super::clipboard::Clipboard; +use super::error::Error; use super::util::{self, ToWide, CLASS_NAME, OPTIONAL_FUNCTIONS}; -use super::window::win_proc_dispatch; +use super::window::{self, DS_REQUEST_QUIT}; + +thread_local! { + static GLOBAL_STATE: RefCell> = RefCell::new(None); +} + +#[derive(Clone)] +pub struct AppState { + state: Rc>, +} + +struct State { + quitting: bool, + app_hwnd: Option, + windows: HashSet, +} pub struct Application; +impl AppState { + pub(crate) fn new() -> AppState { + let state = Rc::new(RefCell::new(State { + quitting: false, + app_hwnd: None, + windows: HashSet::new(), + })); + AppState { state } + } + + pub(crate) fn quitting(&self) -> bool { + self.state.borrow().quitting + } + + pub(crate) fn set_quitting(&self, quitting: bool) { + self.state.borrow_mut().quitting = quitting; + } + + pub(crate) fn app_hwnd(&self) -> Option { + self.state.borrow().app_hwnd + } + + pub(crate) fn set_app_hwnd(&self, app_hwnd: Option) { + self.state.borrow_mut().app_hwnd = app_hwnd; + } + + /// Returns a set of `HWND` for all the current normal windows. + /// + /// The returned set should be treated with extremely limited lifetime. + /// The window handles it contains can become stale quickly. + #[allow(clippy::mutable_key_type)] + pub(crate) unsafe fn windows(&self) -> HashSet { + self.state.borrow().windows.clone() + } + + pub(crate) fn add_window(&self, hwnd: HWND) -> bool { + self.state.borrow_mut().windows.insert(hwnd) + } + + pub(crate) fn remove_window(&self, hwnd: HWND) -> bool { + self.state.borrow_mut().windows.remove(&hwnd) + } +} + impl Application { - pub fn new(_handler: Option>) -> Application { + pub fn new(state: AppState, _handler: Option>) -> Application { + util::claim_main_thread(); + GLOBAL_STATE.with(|global_state| { + *global_state.borrow_mut() = Some(state.clone()); + }); Application::init(); + window::build_app_window(state).expect("Failed to build main message window"); Application } @@ -49,14 +119,19 @@ impl Application { let mut msg = mem::MaybeUninit::uninit(); let res = GetMessageW(msg.as_mut_ptr(), ptr::null_mut(), 0, 0); if res <= 0 { - return; + if res == -1 { + log::error!( + "GetMessageW failed: {}", + Error::Hr(HRESULT_FROM_WIN32(GetLastError())) + ); + } + break; } let mut msg: MSG = msg.assume_init(); let accels = accels::find_accels(GetAncestor(msg.hwnd, GA_ROOT)); let translated = accels.map_or(false, |it| { TranslateAcceleratorW(msg.hwnd, it.handle(), &mut msg) != 0 }); - if !translated { TranslateMessage(&msg); DispatchMessageW(&msg); @@ -67,6 +142,7 @@ impl Application { /// Initialize the app. At the moment, this is mostly needed for hi-dpi. fn init() { + util::assert_main_thread(); util::attach_console(); if let Some(func) = OPTIONAL_FUNCTIONS.SetProcessDpiAwareness { // This function is only supported on windows 10 @@ -81,7 +157,7 @@ impl Application { let brush = CreateSolidBrush(0xff_ff_ff); let wnd = WNDCLASSW { style: 0, - lpfnWndProc: Some(win_proc_dispatch), + lpfnWndProc: Some(window::win_proc_dispatch), cbClsExtra: 0, cbWndExtra: 0, hInstance: 0 as HINSTANCE, @@ -99,9 +175,21 @@ impl Application { } pub fn quit() { - unsafe { - PostQuitMessage(0); - } + util::assert_main_thread(); + GLOBAL_STATE.with(|global_state| { + if let Some(global_state) = global_state.borrow().as_ref() { + if let Some(app_hwnd) = global_state.app_hwnd() { + unsafe { + if PostMessageW(app_hwnd, DS_REQUEST_QUIT, 0, 0) == FALSE { + log::error!( + "PostMessageW failed: {}", + Error::Hr(HRESULT_FROM_WIN32(GetLastError())) + ); + } + } + } + } + }); } pub fn clipboard() -> Clipboard { @@ -113,3 +201,12 @@ impl Application { "en-US".into() } } + +impl Drop for Application { + fn drop(&mut self) { + GLOBAL_STATE.with(|global_state| { + *global_state.borrow_mut() = None; + }); + util::release_main_thread(); + } +} diff --git a/druid-shell/src/platform/windows/error.rs b/druid-shell/src/platform/windows/error.rs index 7668a00752..34a2a10e12 100644 --- a/druid-shell/src/platform/windows/error.rs +++ b/druid-shell/src/platform/windows/error.rs @@ -38,6 +38,8 @@ pub enum Error { OldWindows, /// The `hwnd` pointer was null. NullHwnd, + /// The main app message window already exists. + AppWindowExists, } fn hresult_description(hr: HRESULT) -> Option { @@ -78,6 +80,7 @@ impl fmt::Display for Error { Error::D2Error => write!(f, "Direct2D error"), Error::OldWindows => write!(f, "Attempted newer API on older Windows"), Error::NullHwnd => write!(f, "Window handle is Null"), + Error::AppWindowExists => write!(f, "The main message window already exists"), } } } diff --git a/druid-shell/src/platform/windows/util.rs b/druid-shell/src/platform/windows/util.rs index 694e58a294..deb930fdf2 100644 --- a/druid-shell/src/platform/windows/util.rs +++ b/druid-shell/src/platform/windows/util.rs @@ -22,11 +22,12 @@ use std::mem; use std::os::windows::ffi::{OsStrExt, OsStringExt}; use std::ptr; use std::slice; +use std::sync::atomic::{AtomicU32, Ordering}; use lazy_static::lazy_static; use winapi::ctypes::c_void; use winapi::shared::guiddef::REFIID; -use winapi::shared::minwindef::{HMODULE, UINT}; +use winapi::shared::minwindef::{DWORD, HMODULE, UINT}; use winapi::shared::ntdef::{HRESULT, LPWSTR}; use winapi::shared::windef::HMONITOR; use winapi::shared::winerror::SUCCEEDED; @@ -34,16 +35,69 @@ use winapi::um::fileapi::{CreateFileA, GetFileType, OPEN_EXISTING}; use winapi::um::handleapi::INVALID_HANDLE_VALUE; use winapi::um::libloaderapi::{GetModuleHandleW, GetProcAddress, LoadLibraryW}; use winapi::um::processenv::{GetStdHandle, SetStdHandle}; +use winapi::um::processthreadsapi::GetCurrentThreadId; use winapi::um::shellscalingapi::{MONITOR_DPI_TYPE, PROCESS_DPI_AWARENESS}; use winapi::um::unknwnbase::IUnknown; use winapi::um::winbase::{FILE_TYPE_UNKNOWN, STD_ERROR_HANDLE, STD_OUTPUT_HANDLE}; use winapi::um::wincon::{AttachConsole, ATTACH_PARENT_PROCESS}; use winapi::um::winnt::{FILE_SHARE_WRITE, GENERIC_READ, GENERIC_WRITE}; -use log::error; - use super::error::Error; +static MAIN_THREAD_ID: AtomicU32 = AtomicU32::new(0); + +#[inline] +fn current_thread_id() -> DWORD { + unsafe { GetCurrentThreadId() } +} + +pub fn assert_main_thread() { + let thread_id = current_thread_id(); + let main_thread_id = MAIN_THREAD_ID.load(Ordering::Acquire); + if thread_id != main_thread_id { + panic!( + "Main thread assertion failed {} != {}", + thread_id, main_thread_id + ); + } +} + +pub fn claim_main_thread() { + let thread_id = current_thread_id(); + let old_thread_id = MAIN_THREAD_ID.compare_and_swap(0, thread_id, Ordering::AcqRel); + if old_thread_id != 0 { + panic!( + "The main thread status has already been claimed by thread {}", + thread_id + ); + } +} + +pub fn release_main_thread() { + let thread_id = current_thread_id(); + let old_thread_id = MAIN_THREAD_ID.compare_and_swap(thread_id, 0, Ordering::AcqRel); + if old_thread_id == 0 { + log::warn!("The main thread status was already vacant."); + } else if old_thread_id != thread_id { + panic!( + "The main thread status is owned by another thread {}", + thread_id + ); + } +} + +#[allow(dead_code)] +pub fn main_thread_id() -> Option { + let thread_id = MAIN_THREAD_ID.load(Ordering::Acquire); + // Although not explicitly documented, zero is an invalid thread id. + // It can be deducted from the behavior of GetThreadId / SetWindowsHookExA. + // https://devblogs.microsoft.com/oldnewthing/20040223-00/?p=40503 + match thread_id { + 0 => None, + id => Some(id), + } +} + pub fn as_result(hr: HRESULT) -> Result<(), Error> { if SUCCEEDED(hr) { Ok(()) @@ -144,9 +198,10 @@ fn load_optional_functions() -> OptionalFunctions { let function_ptr = unsafe { GetProcAddress($lib, cstr.as_ptr()) }; if function_ptr.is_null() { - error!( + log::error!( "Could not load `{}`. Windows {} or later is needed", - name, $min_windows_version + name, + $min_windows_version ); } else { let function = unsafe { mem::transmute::<_, $function>(function_ptr) }; @@ -180,14 +235,14 @@ fn load_optional_functions() -> OptionalFunctions { let mut CreateDXGIFactory2 = None; if shcore.is_null() { - error!("No shcore.dll"); + log::error!("No shcore.dll"); } else { load_function!(shcore, SetProcessDpiAwareness, "8.1"); load_function!(shcore, GetDpiForMonitor, "8.1"); } if user32.is_null() { - error!("No user32.dll"); + log::error!("No user32.dll"); } else { load_function!(user32, GetDpiForSystem, "10"); } diff --git a/druid-shell/src/platform/windows/window.rs b/druid-shell/src/platform/windows/window.rs index d2d9c3a889..1a431002c7 100644 --- a/druid-shell/src/platform/windows/window.rs +++ b/druid-shell/src/platform/windows/window.rs @@ -47,6 +47,7 @@ use crate::kurbo::{Point, Rect, Size, Vec2}; use crate::piet::{Piet, RenderContext}; use super::accels::register_accel; +use super::application::AppState; use super::dcomp::{D3D11Device, DCompositionDevice, DCompositionTarget, DCompositionVisual}; use super::dialog::get_file_dialog_path; use super::error::Error; @@ -68,6 +69,7 @@ extern "system" { /// Builder abstraction for creating new windows. pub struct WindowBuilder { + app_state: AppState, handler: Option>, title: String, menu: Option, @@ -106,6 +108,11 @@ pub enum PresentStrategy { FlipRedirect, } +#[derive(Default, Clone)] +pub struct AppWndHandle { + state: Weak, +} + #[derive(Clone)] pub struct WindowHandle { dwrite_factory: DwriteFactory, @@ -114,7 +121,7 @@ pub struct WindowHandle { /// 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 XI_RUN_IDLE +/// has been destroyed, probably not much will go wrong (the DS_RUN_IDLE /// message may be sent to a stray window). #[derive(Clone)] pub struct IdleHandle { @@ -142,13 +149,24 @@ struct WindowState { trait WndProc { fn connect(&self, handle: &WindowHandle, state: WndState); + fn connect_app(&self, handle: &AppWndHandle); + + fn cleanup(&self, hwnd: HWND); + fn window_proc(&self, hwnd: HWND, msg: UINT, wparam: WPARAM, lparam: LPARAM) -> Option; } +// State for the winapi window procedure entry point of the main message window. +struct AppWndProc { + app_state: AppState, + handle: RefCell, +} + // State and logic for the winapi window procedure entry point. Note that this level // implements policies such as the use of Direct2D for painting. struct MyWndProc { + app_state: AppState, handle: RefCell, d2d_factory: D2DFactory, dwrite_factory: DwriteFactory, @@ -190,9 +208,9 @@ struct DCompState { } /// Message indicating there are idle tasks to run. -const XI_RUN_IDLE: UINT = WM_USER; +const DS_RUN_IDLE: UINT = WM_USER; -/// Message relaying a request to destroy the window +/// Message relaying a request to destroy the window. /// /// Calling `DestroyWindow` from inside the handler is problematic /// because it will recursively cause a `WM_DESTROY` message to be @@ -202,7 +220,12 @@ const XI_RUN_IDLE: UINT = WM_USER; /// As a solution, instead of immediately calling `DestroyWindow`, we /// send this message to request destroying the window, so that at the /// time it is handled, we can successfully borrow the handler. -const XI_REQUEST_DESTROY: UINT = WM_USER + 1; +const DS_REQUEST_DESTROY: UINT = WM_USER + 1; + +/// Message relaying a request to quit the app run loop. +/// +/// Directly calling `PostQuitMessage` won't do proper cleanup. +pub const DS_REQUEST_QUIT: UINT = WM_USER + 2; impl Default for PresentStrategy { fn default() -> PresentStrategy { @@ -303,6 +326,64 @@ impl WndState { } } +impl WndProc for AppWndProc { + fn connect(&self, _handle: &WindowHandle, _state: WndState) {} + + fn connect_app(&self, handle: &AppWndHandle) { + *self.handle.borrow_mut() = handle.clone(); + } + + fn cleanup(&self, _hwnd: HWND) { + self.app_state.set_app_hwnd(None); + } + + fn window_proc( + &self, + hwnd: HWND, + msg: UINT, + _wparam: WPARAM, + _lparam: LPARAM, + ) -> Option { + match msg { + WM_CREATE => { + if let Some(state) = self.handle.borrow().state.upgrade() { + state.hwnd.set(hwnd); + } + Some(0) + } + DS_REQUEST_QUIT => { + if !self.app_state.quitting() { + self.app_state.set_quitting(true); + unsafe { + // We want to queue up the destruction of all our windows. + // Failure to do so will lead to resource leaks + // and an eventual error code exit for the process. + for hwnd in self + .app_state + .windows() + .into_iter() + .chain(self.app_state.app_hwnd()) + { + if PostMessageW(hwnd, DS_REQUEST_DESTROY, 0, 0) == FALSE { + log::warn!( + "PostMessageW failed: {}", + Error::Hr(HRESULT_FROM_WIN32(GetLastError())) + ); + } + } + // PostQuitMessage sets a quit request flag in the OS. + // The actual WM_QUIT message is queued but won't be sent + // until all other important events have been handled. + PostQuitMessage(0); + } + } + Some(0) + } + _ => None, + } + } +} + impl MyWndProc { /// Create debugging output for dropped messages due to wndproc reentrancy. /// @@ -322,6 +403,12 @@ impl WndProc for MyWndProc { *self.state.borrow_mut() = Some(state); } + fn connect_app(&self, _handle: &AppWndHandle) {} + + fn cleanup(&self, hwnd: HWND) { + self.app_state.remove_window(hwnd); + } + #[allow(clippy::cognitive_complexity)] fn window_proc( &self, @@ -761,7 +848,7 @@ impl WndProc for MyWndProc { Some(0) } - XI_REQUEST_DESTROY => { + DS_REQUEST_DESTROY => { unsafe { DestroyWindow(hwnd); } @@ -774,7 +861,7 @@ impl WndProc for MyWndProc { } else { self.log_dropped_msg(hwnd, msg, wparam, lparam); } - None + Some(0) } WM_TIMER => { let id = wparam; @@ -813,7 +900,7 @@ impl WndProc for MyWndProc { } Some(0) } - XI_RUN_IDLE => { + DS_RUN_IDLE => { if let Ok(mut s) = self.state.try_borrow_mut() { let s = s.as_mut().unwrap(); let queue = self.handle.borrow().take_idle_queue(); @@ -834,8 +921,9 @@ impl WndProc for MyWndProc { } impl WindowBuilder { - pub fn new() -> WindowBuilder { + pub fn new(app_state: AppState) -> WindowBuilder { WindowBuilder { + app_state, handler: None, title: String::new(), menu: None, @@ -879,13 +967,11 @@ impl WindowBuilder { pub fn build(self) -> Result { unsafe { - // Maybe separate registration in build api? Probably only need to - // register once even for multiple window creation. - let class_name = super::util::CLASS_NAME.to_wide(); let dwrite_factory = DwriteFactory::new().unwrap(); let dw_clone = dwrite_factory.clone(); let wndproc = MyWndProc { + app_state: self.app_state.clone(), handle: Default::default(), d2d_factory: D2DFactory::new().unwrap(), dwrite_factory: dw_clone, @@ -967,6 +1053,7 @@ impl WindowBuilder { if hwnd.is_null() { return Err(Error::NullHwnd); } + self.app_state.add_window(hwnd); if let Some(accels) = accels { register_accel(hwnd, &accels); @@ -976,6 +1063,51 @@ impl WindowBuilder { } } +/// Create the main message-only app window. +pub(crate) fn build_app_window(app_state: AppState) -> Result { + unsafe { + if app_state.app_hwnd() != None { + return Err(Error::AppWindowExists); + } + let class_name = super::util::CLASS_NAME.to_wide(); + let wndproc = AppWndProc { + app_state: app_state.clone(), + handle: Default::default(), + }; + let window = WindowState { + hwnd: Cell::new(0 as HWND), + dpi: Cell::new(96.0), + wndproc: Box::new(wndproc), + idle_queue: Default::default(), + timers: Arc::new(Mutex::new(TimerSlots::new(1))), + }; + let win = Rc::new(window); + let handle = AppWndHandle { + state: Rc::downgrade(&win), + }; + win.wndproc.connect_app(&handle); + let hwnd = create_window( + 0, + class_name.as_ptr(), + null(), + 0, + 0, + 0, + 0, + 0, + HWND_MESSAGE, + null_mut(), + 0 as HINSTANCE, + win, + ); + if hwnd.is_null() { + return Err(Error::NullHwnd); + } + app_state.set_app_hwnd(Some(hwnd)); + Ok(hwnd) + } +} + /// Choose an adapter. Here the heuristic is to choose the adapter with the /// largest video memory, which will generally be the discrete adapter. It's /// possible that on some systems the integrated adapter might be a better @@ -1108,6 +1240,7 @@ pub(crate) unsafe extern "system" fn win_proc_dispatch( }; if msg == WM_NCDESTROY && !window_ptr.is_null() { + (*window_ptr).wndproc.cleanup(hwnd); SetWindowLongPtrW(hwnd, GWLP_USERDATA, 0); mem::drop(Rc::from_raw(window_ptr)); } @@ -1179,7 +1312,7 @@ impl WindowHandle { if let Some(w) = self.state.upgrade() { let hwnd = w.hwnd.get(); unsafe { - PostMessageW(hwnd, XI_REQUEST_DESTROY, 0, 0); + PostMessageW(hwnd, DS_REQUEST_DESTROY, 0, 0); } } } @@ -1446,7 +1579,7 @@ impl IdleHandle { let mut queue = self.queue.lock().unwrap(); if queue.is_empty() { unsafe { - PostMessageW(self.hwnd, XI_RUN_IDLE, 0, 0); + PostMessageW(self.hwnd, DS_RUN_IDLE, 0, 0); } } queue.push(IdleKind::Callback(Box::new(callback))); @@ -1456,7 +1589,7 @@ impl IdleHandle { let mut queue = self.queue.lock().unwrap(); if queue.is_empty() { unsafe { - PostMessageW(self.hwnd, XI_RUN_IDLE, 0, 0); + PostMessageW(self.hwnd, DS_RUN_IDLE, 0, 0); } } queue.push(IdleKind::Token(token)); diff --git a/druid-shell/src/platform/x11/application.rs b/druid-shell/src/platform/x11/application.rs index a1def73d34..7acb9f0d07 100644 --- a/druid-shell/src/platform/x11/application.rs +++ b/druid-shell/src/platform/x11/application.rs @@ -39,10 +39,19 @@ thread_local! { static WINDOW_MAP: RefCell> = RefCell::new(HashMap::new()); } +#[derive(Clone)] +pub struct AppState; + pub struct Application; +impl AppState { + pub(crate) fn new() -> AppState { + AppState + } +} + impl Application { - pub fn new(_handler: Option>) -> Application { + pub fn new(_state: AppState, _handler: Option>) -> Application { Application } diff --git a/druid-shell/src/platform/x11/window.rs b/druid-shell/src/platform/x11/window.rs index 06d937f8fb..b052996744 100644 --- a/druid-shell/src/platform/x11/window.rs +++ b/druid-shell/src/platform/x11/window.rs @@ -27,7 +27,7 @@ use crate::mouse::{Cursor, MouseEvent}; use crate::piet::{Piet, RenderContext}; use crate::window::{IdleToken, Text, TimerToken, WinHandler}; -use super::application::Application; +use super::application::{AppState, Application}; use super::error::Error; use super::menu::Menu; use super::util; @@ -40,7 +40,7 @@ pub struct WindowBuilder { } impl WindowBuilder { - pub fn new() -> WindowBuilder { + pub fn new(_app_state: AppState) -> WindowBuilder { WindowBuilder { handler: None, title: String::new(), diff --git a/druid-shell/src/window.rs b/druid-shell/src/window.rs index 28bb2498db..5ff01e4a94 100644 --- a/druid-shell/src/window.rs +++ b/druid-shell/src/window.rs @@ -17,6 +17,7 @@ use std::any::Any; use std::time::Duration; +use crate::application::AppState; use crate::common_util::Counter; use crate::dialog::{FileDialogOptions, FileInfo}; use crate::error::Error; @@ -211,9 +212,13 @@ impl WindowHandle { pub struct WindowBuilder(platform::WindowBuilder); impl WindowBuilder { - /// Create a new `WindowBuilder` - pub fn new() -> WindowBuilder { - WindowBuilder(platform::WindowBuilder::new()) + /// Create a new `WindowBuilder`. + /// + /// Takes the [`AppState`] of the application this window is for. + /// + /// [`AppState`]: struct.AppState.html + pub fn new(app_state: AppState) -> WindowBuilder { + WindowBuilder(platform::WindowBuilder::new(app_state.0)) } /// Set the [`WinHandler`]. This is the object that will receive diff --git a/druid/src/app.rs b/druid/src/app.rs index ca729ea43f..9f18dc6a63 100644 --- a/druid/src/app.rs +++ b/druid/src/app.rs @@ -16,7 +16,9 @@ use crate::ext_event::{ExtEventHost, ExtEventSink}; use crate::kurbo::Size; -use crate::shell::{Application, Error as PlatformError, WindowBuilder, WindowHandle}; +use crate::shell::{ + AppState as PlatformState, Application, Error as PlatformError, WindowBuilder, WindowHandle, +}; use crate::widget::LabelText; use crate::win_handler::{AppHandler, AppState}; use crate::window::WindowId; @@ -119,10 +121,17 @@ impl AppLauncher { f(&mut env, &data); } - let mut state = AppState::new(data, env, self.delegate.take(), self.ext_event_host); + let platform_state = PlatformState::new(); + let mut state = AppState::new( + platform_state.clone(), + data, + env, + self.delegate.take(), + self.ext_event_host, + ); let handler = AppHandler::new(state.clone()); - let mut app = Application::new(Some(Box::new(handler))); + let mut app = Application::new(platform_state, Some(Box::new(handler))); for desc in self.windows { let window = desc.build_native(&mut state)?; window.show(); @@ -221,7 +230,7 @@ impl WindowDesc { let handler = DruidHandler::new_shared(state.clone(), self.id); - let mut builder = WindowBuilder::new(); + let mut builder = WindowBuilder::new(state.platform_state()); builder.resizable(self.resizable); builder.show_titlebar(self.show_titlebar); diff --git a/druid/src/command.rs b/druid/src/command.rs index 371ebde8b6..2e2c516a53 100644 --- a/druid/src/command.rs +++ b/druid/src/command.rs @@ -122,6 +122,9 @@ pub mod sys { /// should be the id of the window to close. pub const CLOSE_WINDOW: Selector = Selector::new("druid-builtin.close-window"); + /// Close all windows. + pub const CLOSE_ALL_WINDOWS: Selector = Selector::new("druid-builtin.close-all-windows"); + /// The selector for a command to bring a window to the front, and give it focus. /// /// The command's argument should be the id of the target window. diff --git a/druid/src/win_handler.rs b/druid/src/win_handler.rs index 74372b1c9c..79aa08cd74 100644 --- a/druid/src/win_handler.rs +++ b/druid/src/win_handler.rs @@ -22,7 +22,8 @@ use std::rc::Rc; use crate::kurbo::{Rect, Size, Vec2}; use crate::piet::Piet; use crate::shell::{ - Application, FileDialogOptions, IdleToken, MouseEvent, WinHandler, WindowHandle, + AppState as PlatformState, Application, FileDialogOptions, IdleToken, MouseEvent, WinHandler, + WindowHandle, }; use crate::app_delegate::{AppDelegate, DelegateCtx}; @@ -72,6 +73,7 @@ pub(crate) struct AppState { } struct Inner { + platform_state: PlatformState, delegate: Option>>, command_queue: CommandQueue, ext_event_host: ExtEventHost, @@ -118,6 +120,10 @@ impl Windows { fn get_mut(&mut self, id: WindowId) -> Option<&mut Window> { self.windows.get_mut(&id) } + + fn count(&self) -> usize { + self.windows.len() + self.pending.len() + } } impl AppHandler { @@ -128,12 +134,14 @@ impl AppHandler { impl AppState { pub(crate) fn new( + platform_state: PlatformState, data: T, env: Env, delegate: Option>>, ext_event_host: ExtEventHost, ) -> Self { let inner = Rc::new(RefCell::new(Inner { + platform_state, delegate, command_queue: VecDeque::new(), root_menu: None, @@ -145,6 +153,10 @@ impl AppState { AppState { inner } } + + pub(crate) fn platform_state(&self) -> PlatformState { + self.inner.borrow().platform_state.clone() + } } impl Inner { @@ -220,7 +232,11 @@ impl Inner { if self.windows.windows.is_empty() { // on mac we need to keep the menu around self.root_menu = win.menu.take(); - //FIXME: on windows we need to shutdown the app here? + // If there are even no pending windows, we quit the run loop. + if self.windows.count() == 0 { + #[cfg(target_os = "windows")] + Application::quit(); + } } } @@ -262,6 +278,13 @@ impl Inner { } } + /// Requests the platform to close all windows. + fn request_close_all_windows(&mut self) { + for win in self.windows.iter_mut() { + win.handle.close(); + } + } + fn show_window(&mut self, id: WindowId) { if let Some(win) = self.windows.get_mut(id) { win.handle.bring_to_front_and_focus(); @@ -502,7 +525,7 @@ impl AppState { fn handle_cmd(&mut self, target: Target, cmd: Command) { use Target as T; match (target, &cmd.selector) { - // these are handled the same no matter where they come from + // these are handled the same no matter where they come from (_, &sys_cmd::QUIT_APP) => self.quit(), (_, &sys_cmd::HIDE_APPLICATION) => self.hide_app(), (_, &sys_cmd::HIDE_OTHERS) => self.hide_others(), @@ -511,8 +534,9 @@ impl AppState { log::error!("failed to create window: '{}'", e); } } + (_, &sys_cmd::CLOSE_ALL_WINDOWS) => self.request_close_all_windows(), // these should come from a window - // FIXME: we need to be able to open a file without a window handle + // FIXME: we need to be able to open a file without a window handle (T::Window(id), &sys_cmd::SHOW_OPEN_PANEL) => self.show_open_panel(cmd, id), (T::Window(id), &sys_cmd::SHOW_SAVE_PANEL) => self.show_save_panel(cmd, id), (T::Window(id), &sys_cmd::CLOSE_WINDOW) => self.request_close_window(cmd, id), @@ -574,6 +598,10 @@ impl AppState { self.inner.borrow_mut().request_close_window(*id); } + fn request_close_all_windows(&mut self) { + self.inner.borrow_mut().request_close_all_windows(); + } + fn show_window(&mut self, cmd: Command) { let id: WindowId = *cmd .get_object() From c566164219398d26e35091d2d7534c248c7d545c Mon Sep 17 00:00:00 2001 From: Kaur Kuut Date: Mon, 27 Apr 2020 15:00:05 +0300 Subject: [PATCH 2/3] Move app quit code to the application module. --- druid-shell/examples/invalidate.rs | 6 +- druid-shell/examples/perftest.rs | 11 +- druid-shell/examples/shello.rs | 15 +- druid-shell/src/application.rs | 146 +++++++++++---- druid-shell/src/clipboard.rs | 6 +- druid-shell/src/lib.rs | 3 +- druid-shell/src/platform/gtk/application.rs | 18 +- druid-shell/src/platform/gtk/window.rs | 6 +- druid-shell/src/platform/mac/application.rs | 77 +++++--- druid-shell/src/platform/mac/util.rs | 2 +- druid-shell/src/platform/mac/window.rs | 4 +- druid-shell/src/platform/web/application.rs | 18 +- druid-shell/src/platform/web/window.rs | 6 +- .../src/platform/windows/application.rs | 168 +++++++----------- druid-shell/src/platform/windows/error.rs | 3 - druid-shell/src/platform/windows/util.rs | 58 +----- druid-shell/src/platform/windows/window.rs | 143 ++------------- druid-shell/src/platform/x11/application.rs | 18 +- druid-shell/src/platform/x11/window.rs | 10 +- druid-shell/src/util.rs | 82 +++++++++ druid-shell/src/window.rs | 10 +- druid/src/app.rs | 16 +- druid/src/widget/textbox.rs | 2 +- druid/src/win_handler.rs | 23 ++- 24 files changed, 392 insertions(+), 459 deletions(-) create mode 100644 druid-shell/src/util.rs diff --git a/druid-shell/examples/invalidate.rs b/druid-shell/examples/invalidate.rs index 2e4277083a..44970d2ad1 100644 --- a/druid-shell/examples/invalidate.rs +++ b/druid-shell/examples/invalidate.rs @@ -84,8 +84,8 @@ impl WinHandler for InvalidateTest { } fn main() { - let mut app = Application::new(None); - let mut builder = WindowBuilder::new(); + let app = Application::new(); + let mut builder = WindowBuilder::new(app.clone()); let inv_test = InvalidateTest { size: Default::default(), handle: Default::default(), @@ -98,5 +98,5 @@ fn main() { let window = builder.build().unwrap(); window.show(); - app.run(); + app.run(None); } diff --git a/druid-shell/examples/perftest.rs b/druid-shell/examples/perftest.rs index 968857e46a..e19df78b8c 100644 --- a/druid-shell/examples/perftest.rs +++ b/druid-shell/examples/perftest.rs @@ -19,7 +19,7 @@ use time::Instant; use piet_common::kurbo::{Line, Rect}; use piet_common::{Color, FontBuilder, Piet, RenderContext, Text, TextLayoutBuilder}; -use druid_shell::{AppState, Application, KeyEvent, WinHandler, WindowBuilder, WindowHandle}; +use druid_shell::{Application, KeyEvent, WinHandler, WindowBuilder, WindowHandle}; const BG_COLOR: Color = Color::rgb8(0x27, 0x28, 0x22); const FG_COLOR: Color = Color::rgb8(0xf0, 0xf0, 0xea); @@ -107,7 +107,7 @@ impl WinHandler for PerfTest { } fn destroy(&mut self) { - Application::quit() + Application::global().quit() } fn as_any(&mut self) -> &mut dyn Any { @@ -116,9 +116,8 @@ impl WinHandler for PerfTest { } fn main() { - let state = AppState::new(); - let mut app = Application::new(state.clone(), None); - let mut builder = WindowBuilder::new(state); + let app = Application::new(); + let mut builder = WindowBuilder::new(app.clone()); let perf_test = PerfTest { size: Default::default(), handle: Default::default(), @@ -131,5 +130,5 @@ fn main() { let window = builder.build().unwrap(); window.show(); - app.run(); + app.run(None); } diff --git a/druid-shell/examples/shello.rs b/druid-shell/examples/shello.rs index a86820394d..2124263b75 100644 --- a/druid-shell/examples/shello.rs +++ b/druid-shell/examples/shello.rs @@ -18,8 +18,8 @@ use druid_shell::kurbo::{Line, Rect, Vec2}; use druid_shell::piet::{Color, RenderContext}; use druid_shell::{ - AppState, Application, Cursor, FileDialogOptions, FileSpec, HotKey, KeyEvent, KeyModifiers, - Menu, MouseEvent, SysMods, TimerToken, WinHandler, WindowBuilder, WindowHandle, + Application, Cursor, FileDialogOptions, FileSpec, HotKey, KeyEvent, KeyModifiers, Menu, + MouseEvent, SysMods, TimerToken, WinHandler, WindowBuilder, WindowHandle, }; const BG_COLOR: Color = Color::rgb8(0x27, 0x28, 0x22); @@ -48,7 +48,7 @@ impl WinHandler for HelloState { match id { 0x100 => { self.handle.close(); - Application::quit(); + Application::global().quit() } 0x101 => { let options = FileDialogOptions::new().show_hidden().allowed_types(vec![ @@ -101,7 +101,7 @@ impl WinHandler for HelloState { } fn destroy(&mut self) { - Application::quit() + Application::global().quit() } fn as_any(&mut self) -> &mut dyn Any { @@ -129,9 +129,8 @@ fn main() { menubar.add_dropdown(Menu::new(), "Application", true); menubar.add_dropdown(file_menu, "&File", true); - let state = AppState::new(); - let mut app = Application::new(state.clone(), None); - let mut builder = WindowBuilder::new(state); + let app = Application::new(); + let mut builder = WindowBuilder::new(app.clone()); builder.set_handler(Box::new(HelloState::default())); builder.set_title("Hello example"); builder.set_menu(menubar); @@ -139,5 +138,5 @@ fn main() { let window = builder.build().unwrap(); window.show(); - app.run(); + app.run(None); } diff --git a/druid-shell/src/application.rs b/druid-shell/src/application.rs index 6289de2a09..1e59f642dc 100644 --- a/druid-shell/src/application.rs +++ b/druid-shell/src/application.rs @@ -14,10 +14,13 @@ //! The top-level application type. +use std::cell::RefCell; +use std::rc::Rc; use std::sync::atomic::{AtomicBool, Ordering}; use crate::clipboard::Clipboard; use crate::platform::application as platform; +use crate::util; /// A top-level handler that is not associated with any window. /// @@ -38,73 +41,148 @@ pub trait AppHandler { fn command(&mut self, id: u32) {} } -/// The top level application state. +/// The top level application object. /// -/// This helps the application track all the state that it has created, -/// which it later needs to clean up. +/// This can be thought of as a reference and it can be safely cloned. #[derive(Clone)] -pub struct AppState(pub(crate) platform::AppState); - -impl AppState { - /// Create a new `AppState` instance. - pub fn new() -> AppState { - AppState(platform::AppState::new()) - } +pub struct Application { + state: Rc>, + pub(crate) platform_app: platform::Application, } -//TODO: we may want to make the user create an instance of this (Application::global()?) -//but for now I'd like to keep changes minimal. -/// The top level application object. -pub struct Application(platform::Application); +/// Platform-independent `Application` state. +struct State { + running: bool, +} -// Used to ensure only one Application instance is ever created. -// This may change in the future. -// For more information see https://github.com/xi-editor/druid/issues/771 +/// Used to ensure only one Application instance is ever created. static APPLICATION_CREATED: AtomicBool = AtomicBool::new(false); +thread_local! { + /// A reference object to the current `Application`, if any. + static GLOBAL_APP: RefCell> = RefCell::new(None); +} + impl Application { /// Create a new `Application`. /// - /// It takes the application `state` and a `handler` which will be used to inform of events. + /// # Panics + /// + /// Panics if an `Application` has already been created. /// - /// Right now only one application can be created. See [druid#771] for discussion. + /// This may change in the future. See [druid#771] for discussion. /// /// [druid#771]: https://github.com/xi-editor/druid/issues/771 - pub fn new(state: AppState, handler: Option>) -> Application { + pub fn new() -> Application { if APPLICATION_CREATED.compare_and_swap(false, true, Ordering::AcqRel) { panic!("The Application instance has already been created."); } - Application(platform::Application::new(state.0, handler)) + util::claim_main_thread(); + let state = Rc::new(RefCell::new(State { running: false })); + let app = Application { + state, + platform_app: platform::Application::new(), + }; + GLOBAL_APP.with(|global_app| { + *global_app.borrow_mut() = Some(app.clone()); + }); + app } - /// Start the runloop. + /// Get the current globally active `Application`. /// - /// This will block the current thread until the program has finished executing. - pub fn run(&mut self) { - self.0.run() + /// A globally active `Application` exists + /// after [`new`] is called and until [`run`] returns. + /// + /// # Panics + /// + /// Panics if there is no globally active `Application`. + /// For a non-panicking function use [`try_global`]. + /// + /// This function will also panic if called from a non-main thread. + /// + /// [`new`]: #method.new + /// [`run`]: #method.run + /// [`try_global`]: #method.try_global + #[inline] + pub fn global() -> Application { + // Main thread assertion takes place in try_global() + Application::try_global().expect("There is no globally active Application") } - /// Terminate the application. - pub fn quit() { - platform::Application::quit() + /// Get the current globally active `Application`. + /// + /// A globally active `Application` exists + /// after [`new`] is called and until [`run`] returns. + /// + /// # Panics + /// + /// Panics if called from a non-main thread. + /// + /// [`new`]: #method.new + /// [`run`]: #method.run + pub fn try_global() -> Option { + util::assert_main_thread(); + GLOBAL_APP.with(|global_app| global_app.borrow().clone()) + } + + /// Start the `Application` runloop. + /// + /// The provided `handler` will be used to inform of events. + /// + /// This will consume the `Application` and block the current thread + /// until the `Application` has finished executing. + /// + /// # Panics + /// + /// Panics if the `Application` is already running. + pub fn run(self, handler: Option>) { + // Make sure this application hasn't run() yet. + if let Ok(mut state) = self.state.try_borrow_mut() { + if state.running { + panic!("Application is already running"); + } + state.running = true; + } else { + panic!("Application state already borrowed"); + } + + // Run the platform application + self.platform_app.run(handler); + + // This application is no longer active, so clear the global reference + GLOBAL_APP.with(|global_app| { + *global_app.borrow_mut() = None; + }); + // .. and release the main thread + util::release_main_thread(); + } + + /// Quit the `Application`. + /// + /// This will cause [`run`] to return control back to the calling function. + /// + /// [`run`]: #method.run + pub fn quit(&self) { + self.platform_app.quit() } // TODO: do these two go in some kind of PlatformExt trait? /// Hide the application this window belongs to. (cmd+H) - pub fn hide() { + pub fn hide(&self) { #[cfg(target_os = "macos")] - platform::Application::hide() + self.platform_app.hide() } /// Hide all other applications. (cmd+opt+H) - pub fn hide_others() { + pub fn hide_others(&self) { #[cfg(target_os = "macos")] - platform::Application::hide_others() + self.platform_app.hide_others() } /// Returns a handle to the system clipboard. - pub fn clipboard() -> Clipboard { - platform::Application::clipboard().into() + pub fn clipboard(&self) -> Clipboard { + self.platform_app.clipboard().into() } /// Returns the current locale string. diff --git a/druid-shell/src/clipboard.rs b/druid-shell/src/clipboard.rs index 11cdb492d8..dce0ad3081 100644 --- a/druid-shell/src/clipboard.rs +++ b/druid-shell/src/clipboard.rs @@ -70,7 +70,7 @@ pub use crate::platform::clipboard as platform; /// ```no_run /// use druid_shell::{Application, Clipboard}; /// -/// let mut clipboard = Application::clipboard(); +/// let mut clipboard = Application::global().clipboard(); /// clipboard.put_string("watch it there pal"); /// if let Some(contents) = clipboard.get_string() { /// assert_eq!("what it there pal", contents.as_str()); @@ -83,7 +83,7 @@ pub use crate::platform::clipboard as platform; /// ```no_run /// use druid_shell::{Application, Clipboard, ClipboardFormat}; /// -/// let mut clipboard = Application::clipboard(); +/// let mut clipboard = Application::global().clipboard(); /// /// let custom_type_id = "io.xieditor.path-clipboard-type"; /// @@ -104,7 +104,7 @@ pub use crate::platform::clipboard as platform; /// ```no_run /// use druid_shell::{Application, Clipboard, ClipboardFormat}; /// -/// let clipboard = Application::clipboard(); +/// let clipboard = Application::global().clipboard(); /// /// let custom_type_id = "io.xieditor.path-clipboard-type"; /// let supported_types = &[custom_type_id, ClipboardFormat::SVG, ClipboardFormat::PDF]; diff --git a/druid-shell/src/lib.rs b/druid-shell/src/lib.rs index e4c5099690..82496cf1aa 100644 --- a/druid-shell/src/lib.rs +++ b/druid-shell/src/lib.rs @@ -35,9 +35,10 @@ mod keycodes; mod menu; mod mouse; mod platform; +mod util; mod window; -pub use application::{AppHandler, AppState, Application}; +pub use application::{AppHandler, Application}; pub use clipboard::{Clipboard, ClipboardFormat, FormatId}; pub use common_util::Counter; pub use dialog::{FileDialogOptions, FileInfo, FileSpec}; diff --git a/druid-shell/src/platform/gtk/application.rs b/druid-shell/src/platform/gtk/application.rs index 604e2062eb..1d5a401c61 100644 --- a/druid-shell/src/platform/gtk/application.rs +++ b/druid-shell/src/platform/gtk/application.rs @@ -32,18 +32,10 @@ thread_local!( ); #[derive(Clone)] -pub struct AppState; - -pub struct Application; - -impl AppState { - pub(crate) fn new() -> AppState { - AppState - } -} +pub(crate) struct Application; impl Application { - pub fn new(_state: AppState, _handler: Option>) -> Application { + pub fn new() -> Application { // TODO: we should give control over the application ID to the user let application = GtkApplication::new( Some("com.github.xi-editor.druid"), @@ -68,7 +60,7 @@ impl Application { Application } - pub fn run(&mut self) { + pub fn run(self, _handler: Option>) { util::assert_main_thread(); // TODO: should we pass the command line arguments? @@ -80,7 +72,7 @@ impl Application { }); } - pub fn quit() { + pub fn quit(&self) { util::assert_main_thread(); with_application(|app| { match app.get_active_window() { @@ -95,7 +87,7 @@ impl Application { }); } - pub fn clipboard() -> Clipboard { + pub fn clipboard(&self) -> Clipboard { Clipboard } diff --git a/druid-shell/src/platform/gtk/window.rs b/druid-shell/src/platform/gtk/window.rs index fe84219304..38e529b1c0 100644 --- a/druid-shell/src/platform/gtk/window.rs +++ b/druid-shell/src/platform/gtk/window.rs @@ -33,7 +33,7 @@ use gtk::{AccelGroup, ApplicationWindow}; use crate::kurbo::{Point, Rect, Size, Vec2}; use crate::piet::{Piet, RenderContext}; -use super::application::{with_application, AppState}; +use super::application::{with_application, Application}; use super::dialog; use super::menu::Menu; use super::util::assert_main_thread; @@ -81,7 +81,7 @@ pub struct WindowHandle { } /// Builder abstraction for creating new windows -pub struct WindowBuilder { +pub(crate) struct WindowBuilder { handler: Option>, title: String, menu: Option, @@ -111,7 +111,7 @@ pub(crate) struct WindowState { } impl WindowBuilder { - pub fn new(_app_state: AppState) -> WindowBuilder { + pub fn new(_app: Application) -> WindowBuilder { WindowBuilder { handler: None, title: String::new(), diff --git a/druid-shell/src/platform/mac/application.rs b/druid-shell/src/platform/mac/application.rs index e8144969c0..6049a9089e 100644 --- a/druid-shell/src/platform/mac/application.rs +++ b/druid-shell/src/platform/mac/application.rs @@ -16,11 +16,13 @@ #![allow(non_upper_case_globals)] +use std::cell::RefCell; use std::ffi::c_void; +use std::rc::Rc; use cocoa::appkit::{NSApp, NSApplication, NSApplicationActivationPolicyRegular}; -use cocoa::base::{id, nil, YES}; -use cocoa::foundation::NSAutoreleasePool; +use cocoa::base::{id, nil, NO, YES}; +use cocoa::foundation::{NSArray, NSAutoreleasePool}; use lazy_static::lazy_static; use objc::declare::ClassDecl; use objc::runtime::{Class, Object, Sel}; @@ -33,57 +35,82 @@ use crate::application::AppHandler; static APP_HANDLER_IVAR: &str = "druidAppHandler"; #[derive(Clone)] -pub struct AppState; - -pub struct Application { +pub(crate) struct Application { ns_app: id, + state: Rc>, } -impl AppState { - pub(crate) fn new() -> AppState { - AppState - } +struct State { + quitting: bool, } impl Application { - pub fn new(_state: AppState, handler: Option>) -> Application { + pub fn new() -> Application { + // macOS demands that we run not just on one thread, + // but specifically the first thread of the app. util::assert_main_thread(); unsafe { let _pool = NSAutoreleasePool::new(nil); - let delegate: id = msg_send![APP_DELEGATE.0, alloc]; - let () = msg_send![delegate, init]; - let state = DelegateState { handler }; - let handler_ptr = Box::into_raw(Box::new(state)); - (*delegate).set_ivar(APP_HANDLER_IVAR, handler_ptr as *mut c_void); let ns_app = NSApp(); - let () = msg_send![ns_app, setDelegate: delegate]; ns_app.setActivationPolicy_(NSApplicationActivationPolicyRegular); - Application { ns_app } + + let state = Rc::new(RefCell::new(State { quitting: false })); + + Application { ns_app, state } } } - pub fn run(&mut self) { + pub fn run(self, handler: Option>) { unsafe { + // Initialize the application delegate + let delegate: id = msg_send![APP_DELEGATE.0, alloc]; + let () = msg_send![delegate, init]; + let state = DelegateState { handler }; + let state_ptr = Box::into_raw(Box::new(state)); + (*delegate).set_ivar(APP_HANDLER_IVAR, state_ptr as *mut c_void); + let () = msg_send![self.ns_app, setDelegate: delegate]; + + // Run the main app loop self.ns_app.run(); + + // Clean up the delegate + let () = msg_send![self.ns_app, setDelegate: nil]; + Box::from_raw(state_ptr); // Causes it to drop & dealloc automatically } } - pub fn quit() { - unsafe { - let () = msg_send![NSApp(), terminate: nil]; + pub fn quit(&self) { + if let Ok(mut state) = self.state.try_borrow_mut() { + if !state.quitting { + state.quitting = true; + unsafe { + // We want to queue up the destruction of all our windows. + // Failure to do so will lead to resource leaks. + let windows: id = msg_send![self.ns_app, windows]; + for i in 0..windows.count() { + let window: id = windows.objectAtIndex(i); + let () = msg_send![window, performSelectorOnMainThread: sel!(close) withObject: nil waitUntilDone: NO]; + } + // Stop sets a stop request flag in the OS. + // The run loop is stopped after dealing with events. + let () = msg_send![self.ns_app, stop: nil]; + } + } + } else { + log::warn!("Application state already borrowed"); } } /// Hide the application this window belongs to. (cmd+H) - pub fn hide() { + pub fn hide(&self) { unsafe { - let () = msg_send![NSApp(), hide: nil]; + let () = msg_send![self.ns_app, hide: nil]; } } /// Hide all other applications. (cmd+opt+H) - pub fn hide_others() { + pub fn hide_others(&self) { unsafe { let workspace = class!(NSWorkspace); let shared: id = msg_send![workspace, sharedWorkspace]; @@ -91,7 +118,7 @@ impl Application { } } - pub fn clipboard() -> Clipboard { + pub fn clipboard(&self) -> Clipboard { Clipboard } diff --git a/druid-shell/src/platform/mac/util.rs b/druid-shell/src/platform/mac/util.rs index aa638f2ca3..7d35320b60 100644 --- a/druid-shell/src/platform/mac/util.rs +++ b/druid-shell/src/platform/mac/util.rs @@ -20,7 +20,7 @@ use cocoa::base::{id, nil, BOOL, YES}; use cocoa::foundation::{NSAutoreleasePool, NSString, NSUInteger}; use objc::{class, msg_send, sel, sel_impl}; -/// Panic if not on the main thread.assert_main_thread() +/// Panic if not on the main thread. /// /// Many Cocoa operations are only valid on the main thread, and (I think) /// undefined behavior is possible if invoked from other threads. If so, diff --git a/druid-shell/src/platform/mac/window.rs b/druid-shell/src/platform/mac/window.rs index 2a4203bb49..3110b229d5 100644 --- a/druid-shell/src/platform/mac/window.rs +++ b/druid-shell/src/platform/mac/window.rs @@ -41,7 +41,7 @@ use log::{error, info}; use crate::kurbo::{Point, Rect, Size, Vec2}; use crate::piet::{Piet, RenderContext}; -use super::application::AppState; +use super::application::Application; use super::dialog; use super::menu::Menu; use super::util::{assert_main_thread, make_nsstring}; @@ -105,7 +105,7 @@ struct ViewState { } impl WindowBuilder { - pub fn new(_app_state: AppState) -> WindowBuilder { + pub fn new(_app: Application) -> WindowBuilder { WindowBuilder { handler: None, title: String::new(), diff --git a/druid-shell/src/platform/web/application.rs b/druid-shell/src/platform/web/application.rs index c2e9f60505..41d703c206 100644 --- a/druid-shell/src/platform/web/application.rs +++ b/druid-shell/src/platform/web/application.rs @@ -18,26 +18,18 @@ use super::clipboard::Clipboard; use crate::application::AppHandler; #[derive(Clone)] -pub struct AppState; - -pub struct Application; - -impl AppState { - pub(crate) fn new() -> AppState { - AppState - } -} +pub(crate) struct Application; impl Application { - pub fn new(_state: AppState, _handler: Option>) -> Application { + pub fn new() -> Application { Application } - pub fn run(&mut self) {} + pub fn run(self, _handler: Option>) {} - pub fn quit() {} + pub fn quit(&self) {} - pub fn clipboard() -> Clipboard { + pub fn clipboard(&self) -> Clipboard { Clipboard } diff --git a/druid-shell/src/platform/web/window.rs b/druid-shell/src/platform/web/window.rs index 230de81ac9..30b8eaddd3 100644 --- a/druid-shell/src/platform/web/window.rs +++ b/druid-shell/src/platform/web/window.rs @@ -29,7 +29,7 @@ use crate::kurbo::{Point, Rect, Size, Vec2}; use crate::piet::RenderContext; -use super::application::AppState; +use super::application::Application; use super::error::Error; use super::keycodes::key_to_text; use super::menu::Menu; @@ -60,7 +60,7 @@ type Result = std::result::Result; const NOMINAL_DPI: f32 = 96.0; /// Builder abstraction for creating new windows. -pub struct WindowBuilder { +pub(crate) struct WindowBuilder { handler: Option>, title: String, cursor: Cursor, @@ -292,7 +292,7 @@ fn setup_web_callbacks(window_state: &Rc) { } impl WindowBuilder { - pub fn new(_app_state: AppState) -> WindowBuilder { + pub fn new(_app: Application) -> WindowBuilder { WindowBuilder { handler: None, title: String::new(), diff --git a/druid-shell/src/platform/windows/application.rs b/druid-shell/src/platform/windows/application.rs index af026b554a..630ab06d79 100644 --- a/druid-shell/src/platform/windows/application.rs +++ b/druid-shell/src/platform/windows/application.rs @@ -28,8 +28,9 @@ use winapi::um::errhandlingapi::GetLastError; use winapi::um::shellscalingapi::PROCESS_SYSTEM_DPI_AWARE; use winapi::um::wingdi::CreateSolidBrush; use winapi::um::winuser::{ - DispatchMessageW, GetAncestor, GetMessageW, LoadIconW, PostMessageW, RegisterClassW, - TranslateAcceleratorW, TranslateMessage, GA_ROOT, IDI_APPLICATION, MSG, WNDCLASSW, + DispatchMessageW, GetAncestor, GetMessageW, LoadIconW, PostMessageW, PostQuitMessage, + RegisterClassW, TranslateAcceleratorW, TranslateMessage, GA_ROOT, IDI_APPLICATION, MSG, + WNDCLASSW, }; use crate::application::AppHandler; @@ -38,81 +39,70 @@ 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_REQUEST_QUIT}; - -thread_local! { - static GLOBAL_STATE: RefCell> = RefCell::new(None); -} +use super::window::{self, DS_REQUEST_DESTROY}; #[derive(Clone)] -pub struct AppState { +pub(crate) struct Application { state: Rc>, } struct State { quitting: bool, - app_hwnd: Option, windows: HashSet, } -pub struct Application; - -impl AppState { - pub(crate) fn new() -> AppState { +impl Application { + pub fn new() -> Application { + Application::init(); let state = Rc::new(RefCell::new(State { quitting: false, - app_hwnd: None, windows: HashSet::new(), })); - AppState { state } - } - - pub(crate) fn quitting(&self) -> bool { - self.state.borrow().quitting - } - - pub(crate) fn set_quitting(&self, quitting: bool) { - self.state.borrow_mut().quitting = quitting; - } - - pub(crate) fn app_hwnd(&self) -> Option { - self.state.borrow().app_hwnd + Application { state } } - pub(crate) fn set_app_hwnd(&self, app_hwnd: Option) { - self.state.borrow_mut().app_hwnd = app_hwnd; - } + /// Initialize the app. At the moment, this is mostly needed for hi-dpi. + fn init() { + util::attach_console(); + if let Some(func) = OPTIONAL_FUNCTIONS.SetProcessDpiAwareness { + // This function is only supported on windows 10 + unsafe { + func(PROCESS_SYSTEM_DPI_AWARE); // TODO: per monitor (much harder) + } + } - /// Returns a set of `HWND` for all the current normal windows. - /// - /// The returned set should be treated with extremely limited lifetime. - /// The window handles it contains can become stale quickly. - #[allow(clippy::mutable_key_type)] - pub(crate) unsafe fn windows(&self) -> HashSet { - self.state.borrow().windows.clone() + unsafe { + let class_name = CLASS_NAME.to_wide(); + let icon = LoadIconW(0 as HINSTANCE, IDI_APPLICATION); + let brush = CreateSolidBrush(0xff_ff_ff); + let wnd = WNDCLASSW { + style: 0, + lpfnWndProc: Some(window::win_proc_dispatch), + cbClsExtra: 0, + cbWndExtra: 0, + hInstance: 0 as HINSTANCE, + hIcon: icon, + hCursor: 0 as HCURSOR, + hbrBackground: brush, + lpszMenuName: 0 as LPCWSTR, + lpszClassName: class_name.as_ptr(), + }; + let class_atom = RegisterClassW(&wnd); + if class_atom == 0 { + panic!("Error registering class"); + } + } } - pub(crate) fn add_window(&self, hwnd: HWND) -> bool { + pub fn add_window(&self, hwnd: HWND) -> bool { self.state.borrow_mut().windows.insert(hwnd) } - pub(crate) fn remove_window(&self, hwnd: HWND) -> bool { + pub fn remove_window(&self, hwnd: HWND) -> bool { self.state.borrow_mut().windows.remove(&hwnd) } -} - -impl Application { - pub fn new(state: AppState, _handler: Option>) -> Application { - util::claim_main_thread(); - GLOBAL_STATE.with(|global_state| { - *global_state.borrow_mut() = Some(state.clone()); - }); - Application::init(); - window::build_app_window(state).expect("Failed to build main message window"); - Application - } - pub fn run(&mut self) { + pub fn run(self, _handler: Option>) { unsafe { // Handle windows messages loop { @@ -140,59 +130,34 @@ impl Application { } } - /// Initialize the app. At the moment, this is mostly needed for hi-dpi. - fn init() { - util::assert_main_thread(); - util::attach_console(); - if let Some(func) = OPTIONAL_FUNCTIONS.SetProcessDpiAwareness { - // This function is only supported on windows 10 - unsafe { - func(PROCESS_SYSTEM_DPI_AWARE); // TODO: per monitor (much harder) - } - } - - unsafe { - let class_name = CLASS_NAME.to_wide(); - let icon = LoadIconW(0 as HINSTANCE, IDI_APPLICATION); - let brush = CreateSolidBrush(0xff_ff_ff); - let wnd = WNDCLASSW { - style: 0, - lpfnWndProc: Some(window::win_proc_dispatch), - cbClsExtra: 0, - cbWndExtra: 0, - hInstance: 0 as HINSTANCE, - hIcon: icon, - hCursor: 0 as HCURSOR, - hbrBackground: brush, - lpszMenuName: 0 as LPCWSTR, - lpszClassName: class_name.as_ptr(), - }; - let class_atom = RegisterClassW(&wnd); - if class_atom == 0 { - panic!("Error registering class"); - } - } - } - - pub fn quit() { - util::assert_main_thread(); - GLOBAL_STATE.with(|global_state| { - if let Some(global_state) = global_state.borrow().as_ref() { - if let Some(app_hwnd) = global_state.app_hwnd() { - unsafe { - if PostMessageW(app_hwnd, DS_REQUEST_QUIT, 0, 0) == FALSE { - log::error!( - "PostMessageW failed: {}", + pub fn quit(&self) { + if let Ok(mut state) = self.state.try_borrow_mut() { + if !state.quitting { + state.quitting = true; + unsafe { + // We want to queue up the destruction of all our windows. + // Failure to do so will lead to resource leaks + // and an eventual error code exit for the process. + for hwnd in &state.windows { + if PostMessageW(*hwnd, DS_REQUEST_DESTROY, 0, 0) == FALSE { + log::warn!( + "PostMessageW DS_REQUEST_DESTROY failed: {}", Error::Hr(HRESULT_FROM_WIN32(GetLastError())) ); } } + // PostQuitMessage sets a quit request flag in the OS. + // The actual WM_QUIT message is queued but won't be sent + // until all other important events have been handled. + PostQuitMessage(0); } } - }); + } else { + log::warn!("Application state already borrowed"); + } } - pub fn clipboard() -> Clipboard { + pub fn clipboard(&self) -> Clipboard { Clipboard } @@ -201,12 +166,3 @@ impl Application { "en-US".into() } } - -impl Drop for Application { - fn drop(&mut self) { - GLOBAL_STATE.with(|global_state| { - *global_state.borrow_mut() = None; - }); - util::release_main_thread(); - } -} diff --git a/druid-shell/src/platform/windows/error.rs b/druid-shell/src/platform/windows/error.rs index 34a2a10e12..7668a00752 100644 --- a/druid-shell/src/platform/windows/error.rs +++ b/druid-shell/src/platform/windows/error.rs @@ -38,8 +38,6 @@ pub enum Error { OldWindows, /// The `hwnd` pointer was null. NullHwnd, - /// The main app message window already exists. - AppWindowExists, } fn hresult_description(hr: HRESULT) -> Option { @@ -80,7 +78,6 @@ impl fmt::Display for Error { Error::D2Error => write!(f, "Direct2D error"), Error::OldWindows => write!(f, "Attempted newer API on older Windows"), Error::NullHwnd => write!(f, "Window handle is Null"), - Error::AppWindowExists => write!(f, "The main message window already exists"), } } } diff --git a/druid-shell/src/platform/windows/util.rs b/druid-shell/src/platform/windows/util.rs index deb930fdf2..b231a8d390 100644 --- a/druid-shell/src/platform/windows/util.rs +++ b/druid-shell/src/platform/windows/util.rs @@ -22,12 +22,11 @@ use std::mem; use std::os::windows::ffi::{OsStrExt, OsStringExt}; use std::ptr; use std::slice; -use std::sync::atomic::{AtomicU32, Ordering}; use lazy_static::lazy_static; use winapi::ctypes::c_void; use winapi::shared::guiddef::REFIID; -use winapi::shared::minwindef::{DWORD, HMODULE, UINT}; +use winapi::shared::minwindef::{HMODULE, UINT}; use winapi::shared::ntdef::{HRESULT, LPWSTR}; use winapi::shared::windef::HMONITOR; use winapi::shared::winerror::SUCCEEDED; @@ -35,7 +34,6 @@ use winapi::um::fileapi::{CreateFileA, GetFileType, OPEN_EXISTING}; use winapi::um::handleapi::INVALID_HANDLE_VALUE; use winapi::um::libloaderapi::{GetModuleHandleW, GetProcAddress, LoadLibraryW}; use winapi::um::processenv::{GetStdHandle, SetStdHandle}; -use winapi::um::processthreadsapi::GetCurrentThreadId; use winapi::um::shellscalingapi::{MONITOR_DPI_TYPE, PROCESS_DPI_AWARENESS}; use winapi::um::unknwnbase::IUnknown; use winapi::um::winbase::{FILE_TYPE_UNKNOWN, STD_ERROR_HANDLE, STD_OUTPUT_HANDLE}; @@ -44,60 +42,6 @@ use winapi::um::winnt::{FILE_SHARE_WRITE, GENERIC_READ, GENERIC_WRITE}; use super::error::Error; -static MAIN_THREAD_ID: AtomicU32 = AtomicU32::new(0); - -#[inline] -fn current_thread_id() -> DWORD { - unsafe { GetCurrentThreadId() } -} - -pub fn assert_main_thread() { - let thread_id = current_thread_id(); - let main_thread_id = MAIN_THREAD_ID.load(Ordering::Acquire); - if thread_id != main_thread_id { - panic!( - "Main thread assertion failed {} != {}", - thread_id, main_thread_id - ); - } -} - -pub fn claim_main_thread() { - let thread_id = current_thread_id(); - let old_thread_id = MAIN_THREAD_ID.compare_and_swap(0, thread_id, Ordering::AcqRel); - if old_thread_id != 0 { - panic!( - "The main thread status has already been claimed by thread {}", - thread_id - ); - } -} - -pub fn release_main_thread() { - let thread_id = current_thread_id(); - let old_thread_id = MAIN_THREAD_ID.compare_and_swap(thread_id, 0, Ordering::AcqRel); - if old_thread_id == 0 { - log::warn!("The main thread status was already vacant."); - } else if old_thread_id != thread_id { - panic!( - "The main thread status is owned by another thread {}", - thread_id - ); - } -} - -#[allow(dead_code)] -pub fn main_thread_id() -> Option { - let thread_id = MAIN_THREAD_ID.load(Ordering::Acquire); - // Although not explicitly documented, zero is an invalid thread id. - // It can be deducted from the behavior of GetThreadId / SetWindowsHookExA. - // https://devblogs.microsoft.com/oldnewthing/20040223-00/?p=40503 - match thread_id { - 0 => None, - id => Some(id), - } -} - pub fn as_result(hr: HRESULT) -> Result<(), Error> { if SUCCEEDED(hr) { Ok(()) diff --git a/druid-shell/src/platform/windows/window.rs b/druid-shell/src/platform/windows/window.rs index 1a431002c7..0a47a51d39 100644 --- a/druid-shell/src/platform/windows/window.rs +++ b/druid-shell/src/platform/windows/window.rs @@ -47,7 +47,7 @@ use crate::kurbo::{Point, Rect, Size, Vec2}; use crate::piet::{Piet, RenderContext}; use super::accels::register_accel; -use super::application::AppState; +use super::application::Application; use super::dcomp::{D3D11Device, DCompositionDevice, DCompositionTarget, DCompositionVisual}; use super::dialog::get_file_dialog_path; use super::error::Error; @@ -68,8 +68,8 @@ extern "system" { } /// Builder abstraction for creating new windows. -pub struct WindowBuilder { - app_state: AppState, +pub(crate) struct WindowBuilder { + app: Application, handler: Option>, title: String, menu: Option, @@ -108,11 +108,6 @@ pub enum PresentStrategy { FlipRedirect, } -#[derive(Default, Clone)] -pub struct AppWndHandle { - state: Weak, -} - #[derive(Clone)] pub struct WindowHandle { dwrite_factory: DwriteFactory, @@ -149,24 +144,16 @@ struct WindowState { trait WndProc { fn connect(&self, handle: &WindowHandle, state: WndState); - fn connect_app(&self, handle: &AppWndHandle); - fn cleanup(&self, hwnd: HWND); fn window_proc(&self, hwnd: HWND, msg: UINT, wparam: WPARAM, lparam: LPARAM) -> Option; } -// State for the winapi window procedure entry point of the main message window. -struct AppWndProc { - app_state: AppState, - handle: RefCell, -} - // State and logic for the winapi window procedure entry point. Note that this level // implements policies such as the use of Direct2D for painting. struct MyWndProc { - app_state: AppState, + app: Application, handle: RefCell, d2d_factory: D2DFactory, dwrite_factory: DwriteFactory, @@ -220,12 +207,7 @@ const DS_RUN_IDLE: UINT = WM_USER; /// As a solution, instead of immediately calling `DestroyWindow`, we /// send this message to request destroying the window, so that at the /// time it is handled, we can successfully borrow the handler. -const DS_REQUEST_DESTROY: UINT = WM_USER + 1; - -/// Message relaying a request to quit the app run loop. -/// -/// Directly calling `PostQuitMessage` won't do proper cleanup. -pub const DS_REQUEST_QUIT: UINT = WM_USER + 2; +pub(crate) const DS_REQUEST_DESTROY: UINT = WM_USER + 1; impl Default for PresentStrategy { fn default() -> PresentStrategy { @@ -326,64 +308,6 @@ impl WndState { } } -impl WndProc for AppWndProc { - fn connect(&self, _handle: &WindowHandle, _state: WndState) {} - - fn connect_app(&self, handle: &AppWndHandle) { - *self.handle.borrow_mut() = handle.clone(); - } - - fn cleanup(&self, _hwnd: HWND) { - self.app_state.set_app_hwnd(None); - } - - fn window_proc( - &self, - hwnd: HWND, - msg: UINT, - _wparam: WPARAM, - _lparam: LPARAM, - ) -> Option { - match msg { - WM_CREATE => { - if let Some(state) = self.handle.borrow().state.upgrade() { - state.hwnd.set(hwnd); - } - Some(0) - } - DS_REQUEST_QUIT => { - if !self.app_state.quitting() { - self.app_state.set_quitting(true); - unsafe { - // We want to queue up the destruction of all our windows. - // Failure to do so will lead to resource leaks - // and an eventual error code exit for the process. - for hwnd in self - .app_state - .windows() - .into_iter() - .chain(self.app_state.app_hwnd()) - { - if PostMessageW(hwnd, DS_REQUEST_DESTROY, 0, 0) == FALSE { - log::warn!( - "PostMessageW failed: {}", - Error::Hr(HRESULT_FROM_WIN32(GetLastError())) - ); - } - } - // PostQuitMessage sets a quit request flag in the OS. - // The actual WM_QUIT message is queued but won't be sent - // until all other important events have been handled. - PostQuitMessage(0); - } - } - Some(0) - } - _ => None, - } - } -} - impl MyWndProc { /// Create debugging output for dropped messages due to wndproc reentrancy. /// @@ -403,10 +327,8 @@ impl WndProc for MyWndProc { *self.state.borrow_mut() = Some(state); } - fn connect_app(&self, _handle: &AppWndHandle) {} - fn cleanup(&self, hwnd: HWND) { - self.app_state.remove_window(hwnd); + self.app.remove_window(hwnd); } #[allow(clippy::cognitive_complexity)] @@ -921,9 +843,9 @@ impl WndProc for MyWndProc { } impl WindowBuilder { - pub fn new(app_state: AppState) -> WindowBuilder { + pub fn new(app: Application) -> WindowBuilder { WindowBuilder { - app_state, + app, handler: None, title: String::new(), menu: None, @@ -971,7 +893,7 @@ impl WindowBuilder { let dwrite_factory = DwriteFactory::new().unwrap(); let dw_clone = dwrite_factory.clone(); let wndproc = MyWndProc { - app_state: self.app_state.clone(), + app: self.app.clone(), handle: Default::default(), d2d_factory: D2DFactory::new().unwrap(), dwrite_factory: dw_clone, @@ -1053,7 +975,7 @@ impl WindowBuilder { if hwnd.is_null() { return Err(Error::NullHwnd); } - self.app_state.add_window(hwnd); + self.app.add_window(hwnd); if let Some(accels) = accels { register_accel(hwnd, &accels); @@ -1063,51 +985,6 @@ impl WindowBuilder { } } -/// Create the main message-only app window. -pub(crate) fn build_app_window(app_state: AppState) -> Result { - unsafe { - if app_state.app_hwnd() != None { - return Err(Error::AppWindowExists); - } - let class_name = super::util::CLASS_NAME.to_wide(); - let wndproc = AppWndProc { - app_state: app_state.clone(), - handle: Default::default(), - }; - let window = WindowState { - hwnd: Cell::new(0 as HWND), - dpi: Cell::new(96.0), - wndproc: Box::new(wndproc), - idle_queue: Default::default(), - timers: Arc::new(Mutex::new(TimerSlots::new(1))), - }; - let win = Rc::new(window); - let handle = AppWndHandle { - state: Rc::downgrade(&win), - }; - win.wndproc.connect_app(&handle); - let hwnd = create_window( - 0, - class_name.as_ptr(), - null(), - 0, - 0, - 0, - 0, - 0, - HWND_MESSAGE, - null_mut(), - 0 as HINSTANCE, - win, - ); - if hwnd.is_null() { - return Err(Error::NullHwnd); - } - app_state.set_app_hwnd(Some(hwnd)); - Ok(hwnd) - } -} - /// Choose an adapter. Here the heuristic is to choose the adapter with the /// largest video memory, which will generally be the discrete adapter. It's /// possible that on some systems the integrated adapter might be a better diff --git a/druid-shell/src/platform/x11/application.rs b/druid-shell/src/platform/x11/application.rs index 7acb9f0d07..0ee2c5936d 100644 --- a/druid-shell/src/platform/x11/application.rs +++ b/druid-shell/src/platform/x11/application.rs @@ -40,18 +40,10 @@ thread_local! { } #[derive(Clone)] -pub struct AppState; - -pub struct Application; - -impl AppState { - pub(crate) fn new() -> AppState { - AppState - } -} +pub(crate) struct Application; impl Application { - pub fn new(_state: AppState, _handler: Option>) -> Application { + pub fn new() -> Application { Application } @@ -68,7 +60,7 @@ impl Application { } // TODO(x11/events): handle mouse scroll events - pub fn run(&mut self) { + pub fn run(self, _handler: Option>) { let conn = XCB_CONNECTION.connection_cloned(); loop { if let Some(ev) = conn.wait_for_event() { @@ -191,11 +183,11 @@ impl Application { } } - pub fn quit() { + pub fn quit(&self) { // No-op. } - pub fn clipboard() -> Clipboard { + pub fn clipboard(&self) -> Clipboard { // TODO(x11/clipboard): implement Application::clipboard log::warn!("Application::clipboard is currently unimplemented for X11 platforms."); Clipboard {} diff --git a/druid-shell/src/platform/x11/window.rs b/druid-shell/src/platform/x11/window.rs index b052996744..813883c039 100644 --- a/druid-shell/src/platform/x11/window.rs +++ b/druid-shell/src/platform/x11/window.rs @@ -27,12 +27,12 @@ use crate::mouse::{Cursor, MouseEvent}; use crate::piet::{Piet, RenderContext}; use crate::window::{IdleToken, Text, TimerToken, WinHandler}; -use super::application::{AppState, Application}; +use super::application::Application; use super::error::Error; use super::menu::Menu; use super::util; -pub struct WindowBuilder { +pub(crate) struct WindowBuilder { handler: Option>, title: String, size: Size, @@ -40,7 +40,7 @@ pub struct WindowBuilder { } impl WindowBuilder { - pub fn new(_app_state: AppState) -> WindowBuilder { + pub fn new(_app: Application) -> WindowBuilder { WindowBuilder { handler: None, title: String::new(), @@ -289,7 +289,7 @@ fn get_visual_from_screen(screen: &xcb::Screen<'_>) -> Option(&self, _callback: F) @@ -307,7 +307,7 @@ impl IdleHandle { } #[derive(Clone, Default)] -pub struct WindowHandle { +pub(crate) struct WindowHandle { window_id: u32, } diff --git a/druid-shell/src/util.rs b/druid-shell/src/util.rs new file mode 100644 index 0000000000..b58aefdf1f --- /dev/null +++ b/druid-shell/src/util.rs @@ -0,0 +1,82 @@ +// Copyright 2020 The xi-editor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Utility functions for determining the main thread. + +use std::mem; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::thread; + +static MAIN_THREAD_ID: AtomicU64 = AtomicU64::new(0); + +#[inline] +fn current_thread_id() -> u64 { + // TODO: Use .as_u64() instead of mem::transmute + // when .as_u64() or something similar gets stabilized. + unsafe { mem::transmute(thread::current().id()) } +} + +/// Assert that the current thread is the registered main thread. +/// +/// # Panics +/// +/// Panics when called from a non-main thread. +pub fn assert_main_thread() { + let thread_id = current_thread_id(); + let main_thread_id = MAIN_THREAD_ID.load(Ordering::Acquire); + if thread_id != main_thread_id { + panic!( + "Main thread assertion failed {} != {}", + thread_id, main_thread_id + ); + } +} + +/// Register the current thread as the main thread. +/// +/// # Panics +/// +/// Panics if the main thread has already been claimed by another thread. +pub fn claim_main_thread() { + let thread_id = current_thread_id(); + let old_thread_id = MAIN_THREAD_ID.compare_and_swap(0, thread_id, Ordering::AcqRel); + if old_thread_id != 0 { + if old_thread_id == thread_id { + log::warn!("The main thread status was already claimed by the current thread."); + } else { + panic!( + "The main thread status has already been claimed by thread {}", + thread_id + ); + } + } +} + +/// Removes the main thread status of the current thread. +/// +/// # Panics +/// +/// Panics if the main thread status is owned by another thread. +pub fn release_main_thread() { + let thread_id = current_thread_id(); + let old_thread_id = MAIN_THREAD_ID.compare_and_swap(thread_id, 0, Ordering::AcqRel); + if old_thread_id == 0 { + log::warn!("The main thread status was already vacant."); + } else if old_thread_id != thread_id { + panic!( + "The main thread status is owned by another thread {}", + thread_id + ); + } +} diff --git a/druid-shell/src/window.rs b/druid-shell/src/window.rs index 5ff01e4a94..07be4c52b2 100644 --- a/druid-shell/src/window.rs +++ b/druid-shell/src/window.rs @@ -17,7 +17,7 @@ use std::any::Any; use std::time::Duration; -use crate::application::AppState; +use crate::application::Application; use crate::common_util::Counter; use crate::dialog::{FileDialogOptions, FileInfo}; use crate::error::Error; @@ -214,11 +214,11 @@ pub struct WindowBuilder(platform::WindowBuilder); impl WindowBuilder { /// Create a new `WindowBuilder`. /// - /// Takes the [`AppState`] of the application this window is for. + /// Takes the [`Application`] that this window is for. /// - /// [`AppState`]: struct.AppState.html - pub fn new(app_state: AppState) -> WindowBuilder { - WindowBuilder(platform::WindowBuilder::new(app_state.0)) + /// [`Application`]: struct.Application.html + pub fn new(app: Application) -> WindowBuilder { + WindowBuilder(platform::WindowBuilder::new(app.platform_app)) } /// Set the [`WinHandler`]. This is the object that will receive diff --git a/druid/src/app.rs b/druid/src/app.rs index 9f18dc6a63..7abbbdff3e 100644 --- a/druid/src/app.rs +++ b/druid/src/app.rs @@ -16,9 +16,7 @@ use crate::ext_event::{ExtEventHost, ExtEventSink}; use crate::kurbo::Size; -use crate::shell::{ - AppState as PlatformState, Application, Error as PlatformError, WindowBuilder, WindowHandle, -}; +use crate::shell::{Application, Error as PlatformError, WindowBuilder, WindowHandle}; use crate::widget::LabelText; use crate::win_handler::{AppHandler, AppState}; use crate::window::WindowId; @@ -116,28 +114,28 @@ impl AppLauncher { /// Returns an error if a window cannot be instantiated. This is usually /// a fatal error. pub fn launch(mut self, data: T) -> Result<(), PlatformError> { + let app = Application::new(); + let mut env = theme::init(); if let Some(f) = self.env_setup.take() { f(&mut env, &data); } - let platform_state = PlatformState::new(); let mut state = AppState::new( - platform_state.clone(), + app.clone(), data, env, self.delegate.take(), self.ext_event_host, ); - let handler = AppHandler::new(state.clone()); - let mut app = Application::new(platform_state, Some(Box::new(handler))); for desc in self.windows { let window = desc.build_native(&mut state)?; window.show(); } - app.run(); + let handler = AppHandler::new(state); + app.run(Some(Box::new(handler))); Ok(()) } } @@ -230,7 +228,7 @@ impl WindowDesc { let handler = DruidHandler::new_shared(state.clone(), self.id); - let mut builder = WindowBuilder::new(state.platform_state()); + let mut builder = WindowBuilder::new(state.app()); builder.resizable(self.resizable); builder.show_titlebar(self.show_titlebar); diff --git a/druid/src/widget/textbox.rs b/druid/src/widget/textbox.rs index f53c6d2fcd..46d09479aa 100644 --- a/druid/src/widget/textbox.rs +++ b/druid/src/widget/textbox.rs @@ -283,7 +283,7 @@ impl Widget for TextBox { || cmd.selector == crate::commands::CUT) => { if let Some(text) = data.slice(self.selection.range()) { - Application::clipboard().put_string(text); + Application::global().clipboard().put_string(text); } if !self.selection.is_caret() && cmd.selector == crate::commands::CUT { edit_action = Some(EditAction::Delete); diff --git a/druid/src/win_handler.rs b/druid/src/win_handler.rs index 79aa08cd74..2be1cab7b2 100644 --- a/druid/src/win_handler.rs +++ b/druid/src/win_handler.rs @@ -22,8 +22,7 @@ use std::rc::Rc; use crate::kurbo::{Rect, Size, Vec2}; use crate::piet::Piet; use crate::shell::{ - AppState as PlatformState, Application, FileDialogOptions, IdleToken, MouseEvent, WinHandler, - WindowHandle, + Application, FileDialogOptions, IdleToken, MouseEvent, WinHandler, WindowHandle, }; use crate::app_delegate::{AppDelegate, DelegateCtx}; @@ -73,7 +72,7 @@ pub(crate) struct AppState { } struct Inner { - platform_state: PlatformState, + app: Application, delegate: Option>>, command_queue: CommandQueue, ext_event_host: ExtEventHost, @@ -134,14 +133,14 @@ impl AppHandler { impl AppState { pub(crate) fn new( - platform_state: PlatformState, + app: Application, data: T, env: Env, delegate: Option>>, ext_event_host: ExtEventHost, ) -> Self { let inner = Rc::new(RefCell::new(Inner { - platform_state, + app, delegate, command_queue: VecDeque::new(), root_menu: None, @@ -154,8 +153,8 @@ impl AppState { AppState { inner } } - pub(crate) fn platform_state(&self) -> PlatformState { - self.inner.borrow().platform_state.clone() + pub(crate) fn app(&self) -> Application { + self.inner.borrow().app.clone() } } @@ -235,7 +234,7 @@ impl Inner { // If there are even no pending windows, we quit the run loop. if self.windows.count() == 0 { #[cfg(target_os = "windows")] - Application::quit(); + self.app.quit(); } } } @@ -610,22 +609,22 @@ impl AppState { } fn do_paste(&mut self, window_id: WindowId) { - let event = Event::Paste(Application::clipboard()); + let event = Event::Paste(self.inner.borrow().app.clipboard()); self.inner.borrow_mut().do_window_event(window_id, event); } fn quit(&self) { - Application::quit() + self.inner.borrow().app.quit() } fn hide_app(&self) { #[cfg(target_os = "macos")] - Application::hide() + self.inner.borrow().app.hide() } fn hide_others(&mut self) { #[cfg(target_os = "macos")] - Application::hide_others() + self.inner.borrow().app.hide_others() } } From 27c373a842a2092ffac185871b4a41a168e1d40a Mon Sep 17 00:00:00 2001 From: Kaur Kuut Date: Mon, 27 Apr 2020 20:37:34 +0300 Subject: [PATCH 3/3] Add error reporting instead of panicking to Application::new. --- druid-shell/examples/invalidate.rs | 2 +- druid-shell/examples/perftest.rs | 2 +- druid-shell/examples/shello.rs | 2 +- druid-shell/src/application.rs | 19 ++++++++++++------- druid-shell/src/error.rs | 6 +++++- druid-shell/src/platform/gtk/application.rs | 8 +++++--- druid-shell/src/platform/mac/application.rs | 8 +++++--- druid-shell/src/platform/web/application.rs | 8 +++++--- .../src/platform/windows/application.rs | 11 ++++++----- druid-shell/src/platform/x11/application.rs | 10 ++++++---- druid/src/app.rs | 2 +- 11 files changed, 48 insertions(+), 30 deletions(-) diff --git a/druid-shell/examples/invalidate.rs b/druid-shell/examples/invalidate.rs index 44970d2ad1..82084bce95 100644 --- a/druid-shell/examples/invalidate.rs +++ b/druid-shell/examples/invalidate.rs @@ -84,7 +84,7 @@ impl WinHandler for InvalidateTest { } fn main() { - let app = Application::new(); + let app = Application::new().unwrap(); let mut builder = WindowBuilder::new(app.clone()); let inv_test = InvalidateTest { size: Default::default(), diff --git a/druid-shell/examples/perftest.rs b/druid-shell/examples/perftest.rs index e19df78b8c..6c3914fcdf 100644 --- a/druid-shell/examples/perftest.rs +++ b/druid-shell/examples/perftest.rs @@ -116,7 +116,7 @@ impl WinHandler for PerfTest { } fn main() { - let app = Application::new(); + let app = Application::new().unwrap(); let mut builder = WindowBuilder::new(app.clone()); let perf_test = PerfTest { size: Default::default(), diff --git a/druid-shell/examples/shello.rs b/druid-shell/examples/shello.rs index 2124263b75..93b2f55a91 100644 --- a/druid-shell/examples/shello.rs +++ b/druid-shell/examples/shello.rs @@ -129,7 +129,7 @@ fn main() { menubar.add_dropdown(Menu::new(), "Application", true); menubar.add_dropdown(file_menu, "&File", true); - let app = Application::new(); + let app = Application::new().unwrap(); let mut builder = WindowBuilder::new(app.clone()); builder.set_handler(Box::new(HelloState::default())); builder.set_title("Hello example"); diff --git a/druid-shell/src/application.rs b/druid-shell/src/application.rs index 1e59f642dc..9358d235e8 100644 --- a/druid-shell/src/application.rs +++ b/druid-shell/src/application.rs @@ -19,6 +19,7 @@ use std::rc::Rc; use std::sync::atomic::{AtomicBool, Ordering}; use crate::clipboard::Clipboard; +use crate::error::Error; use crate::platform::application as platform; use crate::util; @@ -46,8 +47,8 @@ pub trait AppHandler { /// This can be thought of as a reference and it can be safely cloned. #[derive(Clone)] pub struct Application { - state: Rc>, pub(crate) platform_app: platform::Application, + state: Rc>, } /// Platform-independent `Application` state. @@ -66,27 +67,31 @@ thread_local! { impl Application { /// Create a new `Application`. /// - /// # Panics + /// # Errors /// - /// Panics if an `Application` has already been created. + /// Errors if an `Application` has already been created. /// /// This may change in the future. See [druid#771] for discussion. /// /// [druid#771]: https://github.com/xi-editor/druid/issues/771 - pub fn new() -> Application { + pub fn new() -> Result { if APPLICATION_CREATED.compare_and_swap(false, true, Ordering::AcqRel) { - panic!("The Application instance has already been created."); + return Err(Error::ApplicationAlreadyExists); } util::claim_main_thread(); + let platform_app = match platform::Application::new() { + Ok(app) => app, + Err(err) => return Err(Error::Platform(err)), + }; let state = Rc::new(RefCell::new(State { running: false })); let app = Application { + platform_app, state, - platform_app: platform::Application::new(), }; GLOBAL_APP.with(|global_app| { *global_app.borrow_mut() = Some(app.clone()); }); - app + Ok(app) } /// Get the current globally active `Application`. diff --git a/druid-shell/src/error.rs b/druid-shell/src/error.rs index 8c7494d93b..04cdb01eed 100644 --- a/druid-shell/src/error.rs +++ b/druid-shell/src/error.rs @@ -20,8 +20,9 @@ use crate::platform::error as platform; /// Error codes. At the moment, this is little more than HRESULT, but that /// might change. -#[derive(Debug)] +#[derive(Debug, Clone)] pub enum Error { + ApplicationAlreadyExists, Other(&'static str), Platform(platform::Error), } @@ -29,6 +30,9 @@ pub enum Error { impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { match self { + Error::ApplicationAlreadyExists => { + write!(f, "An application instance has already been created.") + } Error::Other(s) => write!(f, "{}", s), Error::Platform(p) => fmt::Display::fmt(&p, f), } diff --git a/druid-shell/src/platform/gtk/application.rs b/druid-shell/src/platform/gtk/application.rs index 1d5a401c61..575d74f182 100644 --- a/druid-shell/src/platform/gtk/application.rs +++ b/druid-shell/src/platform/gtk/application.rs @@ -20,9 +20,11 @@ use gio::prelude::ApplicationExtManual; use gio::{ApplicationExt, ApplicationFlags, Cancellable}; use gtk::{Application as GtkApplication, GtkApplicationExt}; +use crate::application::AppHandler; + use super::clipboard::Clipboard; +use super::error::Error; use super::util; -use crate::application::AppHandler; // XXX: The application needs to be global because WindowBuilder::build wants // to construct an ApplicationWindow, which needs the application, but @@ -35,7 +37,7 @@ thread_local!( pub(crate) struct Application; impl Application { - pub fn new() -> Application { + pub fn new() -> Result { // TODO: we should give control over the application ID to the user let application = GtkApplication::new( Some("com.github.xi-editor.druid"), @@ -57,7 +59,7 @@ impl Application { .expect("Could not register GTK application"); GTK_APPLICATION.with(move |x| *x.borrow_mut() = Some(application)); - Application + Ok(Application) } pub fn run(self, _handler: Option>) { diff --git a/druid-shell/src/platform/mac/application.rs b/druid-shell/src/platform/mac/application.rs index 6049a9089e..79fd923343 100644 --- a/druid-shell/src/platform/mac/application.rs +++ b/druid-shell/src/platform/mac/application.rs @@ -28,9 +28,11 @@ use objc::declare::ClassDecl; use objc::runtime::{Class, Object, Sel}; use objc::{class, msg_send, sel, sel_impl}; +use crate::application::AppHandler; + use super::clipboard::Clipboard; +use super::error::Error; use super::util; -use crate::application::AppHandler; static APP_HANDLER_IVAR: &str = "druidAppHandler"; @@ -45,7 +47,7 @@ struct State { } impl Application { - pub fn new() -> Application { + pub fn new() -> Result { // macOS demands that we run not just on one thread, // but specifically the first thread of the app. util::assert_main_thread(); @@ -57,7 +59,7 @@ impl Application { let state = Rc::new(RefCell::new(State { quitting: false })); - Application { ns_app, state } + Ok(Application { ns_app, state }) } } diff --git a/druid-shell/src/platform/web/application.rs b/druid-shell/src/platform/web/application.rs index 41d703c206..1e7da7c465 100644 --- a/druid-shell/src/platform/web/application.rs +++ b/druid-shell/src/platform/web/application.rs @@ -14,15 +14,17 @@ //! Web implementation of features at the application scope. -use super::clipboard::Clipboard; use crate::application::AppHandler; +use super::clipboard::Clipboard; +use super::error::Error; + #[derive(Clone)] pub(crate) struct Application; impl Application { - pub fn new() -> Application { - Application + pub fn new() -> Result { + Ok(Application) } pub fn run(self, _handler: Option>) {} diff --git a/druid-shell/src/platform/windows/application.rs b/druid-shell/src/platform/windows/application.rs index 630ab06d79..cfb5d0fb87 100644 --- a/druid-shell/src/platform/windows/application.rs +++ b/druid-shell/src/platform/windows/application.rs @@ -52,17 +52,18 @@ struct State { } impl Application { - pub fn new() -> Application { - Application::init(); + pub fn new() -> Result { + Application::init()?; let state = Rc::new(RefCell::new(State { quitting: false, windows: HashSet::new(), })); - Application { state } + Ok(Application { state }) } /// Initialize the app. At the moment, this is mostly needed for hi-dpi. - fn init() { + fn init() -> Result<(), Error> { + // TODO: Report back an error instead of panicking util::attach_console(); if let Some(func) = OPTIONAL_FUNCTIONS.SetProcessDpiAwareness { // This function is only supported on windows 10 @@ -70,7 +71,6 @@ impl Application { func(PROCESS_SYSTEM_DPI_AWARE); // TODO: per monitor (much harder) } } - unsafe { let class_name = CLASS_NAME.to_wide(); let icon = LoadIconW(0 as HINSTANCE, IDI_APPLICATION); @@ -92,6 +92,7 @@ impl Application { panic!("Error registering class"); } } + Ok(()) } pub fn add_window(&self, hwnd: HWND) -> bool { diff --git a/druid-shell/src/platform/x11/application.rs b/druid-shell/src/platform/x11/application.rs index 0ee2c5936d..2a884cc7df 100644 --- a/druid-shell/src/platform/x11/application.rs +++ b/druid-shell/src/platform/x11/application.rs @@ -20,12 +20,14 @@ use std::sync::Arc; use lazy_static::lazy_static; -use super::clipboard::Clipboard; -use super::window::XWindow; use crate::application::AppHandler; use crate::kurbo::{Point, Rect}; use crate::{KeyCode, KeyModifiers, MouseButton, MouseEvent}; +use super::clipboard::Clipboard; +use super::error::Error; +use super::window::XWindow; + struct XcbConnection { connection: Arc, screen_num: i32, @@ -43,8 +45,8 @@ thread_local! { pub(crate) struct Application; impl Application { - pub fn new() -> Application { - Application + pub fn new() -> Result { + Ok(Application) } pub(crate) fn add_xwindow(id: u32, xwindow: XWindow) { diff --git a/druid/src/app.rs b/druid/src/app.rs index 7abbbdff3e..db6c5e8cb4 100644 --- a/druid/src/app.rs +++ b/druid/src/app.rs @@ -114,7 +114,7 @@ impl AppLauncher { /// Returns an error if a window cannot be instantiated. This is usually /// a fatal error. pub fn launch(mut self, data: T) -> Result<(), PlatformError> { - let app = Application::new(); + let app = Application::new()?; let mut env = theme::init(); if let Some(f) = self.env_setup.take() {