From 98d4b4058e10e005bfc863dd95a62ef805a43946 Mon Sep 17 00:00:00 2001 From: jtnunley Date: Sun, 19 Mar 2023 23:02:01 -0700 Subject: [PATCH 01/13] Use a new strategy for ensuring window handles are active --- Cargo.toml | 1 + src/borrowed.rs | 355 +++++++++++++++++++++++++++++++----------------- src/lib.rs | 7 +- 3 files changed, 235 insertions(+), 128 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1ee078b..1aa39c9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,6 +13,7 @@ rust-version = "1.64" [features] alloc = [] +std = ["alloc"] [package.metadata.docs.rs] all-features = true diff --git a/src/borrowed.rs b/src/borrowed.rs index 823c120..ecef5a8 100644 --- a/src/borrowed.rs +++ b/src/borrowed.rs @@ -1,90 +1,72 @@ //! Borrowable window handles based on the ones in this crate. //! -//! These should be 100% safe to pass around and use, no possibility of dangling or -//! invalidity. +//! These should be 100% safe to pass around and use, no possibility of dangling or invalidity. use core::fmt; +use core::hash::{Hash, Hasher}; use core::marker::PhantomData; use crate::{HasRawDisplayHandle, HasRawWindowHandle, RawDisplayHandle, RawWindowHandle}; -/// The application is currently active. -/// -/// This structure is a token that indicates that the application is -/// not presently suspended. It is used to ensure that the window handle -/// is only used when the application is active. -/// -/// It is safe to create this token on platforms where the application -/// is guaranteed to be active, such as on desktop platforms. On Android -/// platforms, the application may be suspended, so this token must be -/// either created with `unsafe` or derived from a `HasDisplayHandle` -/// type. -pub struct Active<'a> { - _marker: PhantomData<&'a *const ()>, -} +/// Keeps track of whether the application is currently active. +pub struct Active(imp::Active); -impl fmt::Debug for Active<'_> { +impl fmt::Debug for Active { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.write_str("Active { .. }") } } -impl<'a> Active<'a> { - /// Create a new active token. - /// - /// # Safety - /// - /// The application must not be `Suspend`ed. +/// Represents a live window handle. +#[derive(Clone)] +pub struct ActiveHandle<'a>(imp::ActiveHandle<'a>); + +impl<'a> fmt::Debug for ActiveHandle<'a> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("ActiveHandle { .. }") + } +} + +impl Active { + /// Create a new `Active` tracker. + pub const fn new() -> Self { + Self(imp::Active::new()) + } + + /// Get a live window handle. + pub fn handle(&self) -> Option> { + self.0.handle().map(ActiveHandle) + } + + /// Set the application to be inactive. /// - /// # Examples + /// This function may block until there are no more active handles. /// - /// ``` - /// use raw_window_handle::Active; + /// # Safety /// - /// // SAFETY: The application is active. - /// let active = unsafe { Active::new_unchecked() }; - /// ``` - pub unsafe fn new_unchecked() -> Self { - Self { - _marker: PhantomData, - } + /// The application must actually be inactive. + pub unsafe fn set_inactive(&self) { + self.0.set_inactive() } - /// Create a new active token on a system where the application is - /// guaranteed to be active. + /// Set the application to be active. /// - /// On most platforms, there is no event where the application is - /// suspended, so there are no cases where this function is unsafe. - /// - /// ``` - /// use raw_window_handle::Active; + /// # Safety /// - /// let with_active = |active: Active<'_>| { - /// /* ... */ - /// }; + /// The application must actually be active. + pub unsafe fn set_active(&self) { + self.0.set_active() + } +} + +impl ActiveHandle<'_> { + /// Create a new freestanding active handle. /// - /// // Only use this code-path on non-android platforms. - /// #[cfg(not(target_os = "android"))] - /// { - /// let active = Active::new(); - /// with_active(active); - /// } + /// # Safety /// - /// // Only use this code-path on android platforms. - /// #[cfg(target_os = "android")] - /// { - /// if application_is_active() { - /// let active = unsafe { Active::new_unchecked() }; - /// with_active(active); - /// } - /// } - /// # fn application_is_active() -> bool { false } - /// ``` - #[cfg(not(target_os = "android"))] - #[cfg_attr(docsrs, doc(cfg(not(target_os = "android"))))] - pub fn new() -> Self { - // SAFETY: The application is guaranteed to be active. - unsafe { Self::new_unchecked() } + /// The application must actually be active. + pub unsafe fn new_unchecked() -> Self { + Self(imp::ActiveHandle::new_unchecked()) } } @@ -92,31 +74,18 @@ impl<'a> Active<'a> { /// /// # Safety /// -/// The safety requirements of [`HasRawDisplayHandle`] apply here as -/// well. To clarify, the [`DisplayHandle`] must contain a valid window -/// handle for its lifetime. In addition, the handle must be consistent -/// between multiple calls barring platform-specific events. +/// The safety requirements of [`HasRawDisplayHandle`] apply here as well. To reiterate, the +/// [`DisplayHandle`] must contain a valid window handle for its lifetime. /// -/// In addition, the `active` function must only return an `Active` -/// token if the application is active. +/// It is not possible to invalidate a [`DisplayHandle`] on any platform without additional unsafe code. /// -/// Note that these requirements are not enforced on `HasDisplayHandle`, -/// rather, they are enforced on the constructors of [`Active`] and -/// [`DisplayHandle`]. This is because the `HasDisplayHandle` trait is -/// safe to implement. +/// Note that these requirements are not enforced on `HasDisplayHandle`, rather, they are enforced on the +/// constructors of [`DisplayHandle`]. This is because the `HasDisplayHandle` trait is safe to implement. /// /// [`HasRawDisplayHandle`]: crate::HasRawDisplayHandle pub trait HasDisplayHandle { - /// Get a token indicating whether the application is active. - fn active(&self) -> Option>; - /// Get a handle to the display controller of the windowing system. - fn display_handle<'this, 'active>( - &'this self, - active: &'active Active<'_>, - ) -> DisplayHandle<'this> - where - 'active: 'this; + fn display_handle(&self) -> DisplayHandle<'_>; } impl HasDisplayHandle for &T { @@ -191,7 +160,10 @@ impl HasDisplayHandle for alloc::sync::Arc { /// The handle to the display controller of the windowing system. /// -/// Get the underlying raw display handle with the `HasRawDisplayHandle` trait. +/// This is the primary return type of the [`HasDisplayHandle`] trait. It is guaranteed to contain +/// a valid platform-specific display handle for its lifetime. +/// +/// Get the underlying raw display handle with the [`HasRawDisplayHandle`] trait. #[repr(transparent)] #[derive(PartialEq, Eq, Hash)] pub struct DisplayHandle<'a> { @@ -213,15 +185,12 @@ impl<'a> Clone for DisplayHandle<'a> { } impl<'a> DisplayHandle<'a> { - /// Borrow a `DisplayHandle` from a `RawDisplayHandle`. + /// Borrow a `DisplayHandle` from a [`RawDisplayHandle`]. /// /// # Safety /// - /// The `RawDisplayHandle` must be valid for the lifetime and the - /// application must be `Active`. See the requirements on the - /// [`HasDisplayHandle`] trait for more information. - pub unsafe fn borrow_raw(raw: RawDisplayHandle, active: &Active<'a>) -> Self { - let _ = active; + /// The `RawDisplayHandle` must be valid for the lifetime. + pub unsafe fn borrow_raw(raw: RawDisplayHandle) -> Self { Self { raw, _marker: PhantomData, @@ -236,19 +205,7 @@ unsafe impl HasRawDisplayHandle for DisplayHandle<'_> { } impl<'a> HasDisplayHandle for DisplayHandle<'a> { - fn active(&self) -> Option> { - // SAFETY: The fact that this handle was created means that the - // application is active. - Some(unsafe { Active::new_unchecked() }) - } - - fn display_handle<'this, 'active>( - &'this self, - _active: &'active Active<'_>, - ) -> DisplayHandle<'this> - where - 'active: 'this, - { + fn display_handle(&self) -> DisplayHandle<'_> { *self } } @@ -257,20 +214,31 @@ impl<'a> HasDisplayHandle for DisplayHandle<'a> { /// /// # Safety /// -/// The safety requirements of [`HasRawWindowHandle`] apply here as -/// well. To clarify, the [`WindowHandle`] must contain a valid window -/// handle for its lifetime. In addition, the handle must be consistent -/// between multiple calls barring platform-specific events. +/// All pointers within the resulting [`WindowHandle`] must be valid and not dangling for the lifetime of +/// the handle. +/// +/// Note that this guarantee only applies to *pointers*, and not any window ID types in the handle. +/// This includes Window IDs (XIDs) from X11, `HWND`s from Win32, and the window ID for web platforms. +/// There is no way for Rust to enforce any kind of invariant on these types, since: +/// +/// - For all three listed platforms, it is possible for safe code in the same process to delete +/// the window. +/// - For X11 and Win32, it is possible for code in a different process to delete the window. +/// - For X11, it is possible for code on a different *machine* to delete the window. /// -/// Note that these requirements are not enforced on `HasWindowHandle`, -/// rather, they are enforced on the constructors of -/// [`WindowHandle`]. This is because the `HasWindowHandle` trait is -/// safe to implement. +/// It is *also* possible for the window to be replaced with another, valid-but-different window. User +/// code should be aware of this possibility, and should be ready to soundly handle the possible error +/// conditions that can arise from this. +/// +/// In addition, the window handle must not be invalidated for the duration of the [`ActiveHandle`] token. +/// +/// Note that these requirements are not enforced on `HasWindowHandle`, rather, they are enforced on the +/// constructors of [`WindowHandle`]. This is because the `HasWindowHandle` trait is safe to implement. pub trait HasWindowHandle { /// Get a handle to the window. fn window_handle<'this, 'active>( &'this self, - active: &'active Active<'_>, + active: ActiveHandle<'active>, ) -> WindowHandle<'this> where 'active: 'this; @@ -332,12 +300,17 @@ impl HasWindowHandle for alloc::sync::Arc { /// The handle to a window. /// -/// This handle is guaranteed to be safe and valid. Get the underlying -/// raw window handle with the `HasRawWindowHandle` trait. -#[repr(transparent)] -#[derive(PartialEq, Eq, Hash)] +/// This is the primary return type of the [`HasWindowHandle`] trait. All *pointers* within this type +/// are guaranteed to be valid and not dangling for the lifetime of the handle. This excludes window IDs +/// like XIDs, `HWND`s, and the window ID for web platforms. See the documentation on the +/// [`HasWindowHandle`] trait for more information about these safety requirements. +/// +/// This handle is guaranteed to be safe and valid. Get the underlying raw window handle with the +/// [`HasRawWindowHandle`] trait. +#[derive(Clone)] pub struct WindowHandle<'a> { raw: RawWindowHandle, + _active: ActiveHandle<'a>, _marker: PhantomData<&'a *const ()>, } @@ -347,24 +320,30 @@ impl fmt::Debug for WindowHandle<'_> { } } -impl<'a> Copy for WindowHandle<'a> {} -impl<'a> Clone for WindowHandle<'a> { - fn clone(&self) -> Self { - *self +impl PartialEq for WindowHandle<'_> { + fn eq(&self, other: &Self) -> bool { + self.raw == other.raw + } +} + +impl Eq for WindowHandle<'_> {} + +impl Hash for WindowHandle<'_> { + fn hash(&self, state: &mut H) { + self.raw.hash(state); } } impl<'a> WindowHandle<'a> { - /// Borrow a `WindowHandle` from a `RawWindowHandle`. + /// Borrow a `WindowHandle` from a [`RawWindowHandle`]. /// /// # Safety /// - /// The `RawWindowHandle` must be valid for the lifetime and the - /// application must be `Active`. - pub unsafe fn borrow_raw(raw: RawWindowHandle, active: &Active<'a>) -> Self { - let _ = active; + /// The [`RawWindowHandle`] must be valid for the lifetime and the application must be `Active`. + pub unsafe fn borrow_raw(raw: RawWindowHandle, active: ActiveHandle<'a>) -> Self { Self { raw, + _active: active, _marker: PhantomData, } } @@ -379,12 +358,16 @@ unsafe impl HasRawWindowHandle for WindowHandle<'_> { impl<'a> HasWindowHandle for WindowHandle<'a> { fn window_handle<'this, 'active>( &'this self, - _active: &'active Active<'_>, + active: ActiveHandle<'active>, ) -> WindowHandle<'this> where 'active: 'this, { - *self + WindowHandle { + raw: self.raw, + _active: active, + _marker: PhantomData, + } } } @@ -396,3 +379,121 @@ impl<'a> HasWindowHandle for WindowHandle<'a> { /// _assert::>(); /// ``` fn _not_send_or_sync() {} + +#[cfg(not(any(target_os = "android", raw_window_handle_force_refcount)))] +#[cfg_attr(docsrs, doc(cfg(not(target_os = "android"))))] +mod imp { + //! We don't need to refcount the handles, so we can just use no-ops. + + use core::marker::PhantomData; + + pub(super) struct Active; + + #[derive(Clone)] + pub(super) struct ActiveHandle<'a> { + _marker: PhantomData<&'a ()>, + } + + impl Active { + pub(super) const fn new() -> Self { + Self + } + + pub(super) fn handle(&self) -> Option> { + // SAFETY: The handle is always active. + Some(unsafe { ActiveHandle::new_unchecked() }) + } + + pub(super) unsafe fn set_active(&self) {} + + pub(super) unsafe fn set_inactive(&self) {} + } + + impl ActiveHandle<'_> { + pub(super) unsafe fn new_unchecked() -> Self { + Self { + _marker: PhantomData, + } + } + } + + impl super::ActiveHandle<'_> { + /// Create a new `ActiveHandle`. + /// + /// This is safe because the handle is always active. + #[allow(clippy::new_without_default)] + pub fn new() -> Self { + // SAFETY: The handle is always active. + unsafe { super::ActiveHandle::new_unchecked() } + } + } +} + +#[cfg(any(target_os = "android", raw_window_handle_force_refcount))] +#[cfg_attr(docsrs, doc(cfg(any(target_os = "android"))))] +mod imp { + //! We need to refcount the handles, so we use an `RwLock` to do so. + + extern crate std; + use std::sync::{RwLock, RwLockReadGuard}; + + pub(super) struct Active { + active: RwLock, + } + + pub(super) struct ActiveHandle<'a> { + inner: Option>, + } + + struct Inner<'a> { + _read_guard: RwLockReadGuard<'a, bool>, + active: &'a Active, + } + + impl Clone for ActiveHandle<'_> { + fn clone(&self) -> Self { + Self { + inner: self.inner.as_ref().map(|inner| Inner { + _read_guard: inner.active.active.read().unwrap(), + active: inner.active, + }), + } + } + } + + impl Active { + pub(super) const fn new() -> Self { + Self { + active: RwLock::new(false), + } + } + + pub(super) fn handle(&self) -> Option> { + let active = self.active.read().ok()?; + if !*active { + return None; + } + + Some(ActiveHandle { + inner: Some(Inner { + _read_guard: active, + active: self, + }), + }) + } + + pub(super) unsafe fn set_active(&self) { + *self.active.write().unwrap() = true; + } + + pub(super) unsafe fn set_inactive(&self) { + *self.active.write().unwrap() = false; + } + } + + impl ActiveHandle<'_> { + pub(super) unsafe fn new_unchecked() -> Self { + Self { inner: None } + } + } +} diff --git a/src/lib.rs b/src/lib.rs index f299a09..a3e1e69 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -28,6 +28,9 @@ #[cfg(feature = "alloc")] extern crate alloc; +#[cfg(feature = "std")] +extern crate std; + mod android; mod appkit; mod borrowed; @@ -40,7 +43,9 @@ mod windows; pub use android::{AndroidDisplayHandle, AndroidNdkWindowHandle}; pub use appkit::{AppKitDisplayHandle, AppKitWindowHandle}; -pub use borrowed::{Active, DisplayHandle, HasDisplayHandle, HasWindowHandle, WindowHandle}; +pub use borrowed::{ + Active, ActiveHandle, DisplayHandle, HasDisplayHandle, HasWindowHandle, WindowHandle, +}; pub use haiku::{HaikuDisplayHandle, HaikuWindowHandle}; pub use redox::{OrbitalDisplayHandle, OrbitalWindowHandle}; pub use uikit::{UiKitDisplayHandle, UiKitWindowHandle}; From b17e4fec1b7a4ceddc46f5ee5f2d27c38c0731b1 Mon Sep 17 00:00:00 2001 From: jtnunley Date: Fri, 24 Mar 2023 04:34:57 -0700 Subject: [PATCH 02/13] Make sure android requires std --- src/borrowed.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/borrowed.rs b/src/borrowed.rs index ecef5a8..8563df0 100644 --- a/src/borrowed.rs +++ b/src/borrowed.rs @@ -2,6 +2,9 @@ //! //! These should be 100% safe to pass around and use, no possibility of dangling or invalidity. +#[cfg(all(not(feature = "std"), target_os = "android"))] +compile_error!("Using borrowed handles on Android requires the `std` feature to be enabled."); + use core::fmt; use core::hash::{Hash, Hasher}; use core::marker::PhantomData; @@ -434,7 +437,6 @@ mod imp { mod imp { //! We need to refcount the handles, so we use an `RwLock` to do so. - extern crate std; use std::sync::{RwLock, RwLockReadGuard}; pub(super) struct Active { From 2dbf7d07007674d9733aad349472a4a9fa5e45be Mon Sep 17 00:00:00 2001 From: jtnunley Date: Fri, 24 Mar 2023 04:49:26 -0700 Subject: [PATCH 03/13] Second look at HasWindowHandle trait --- src/borrowed.rs | 160 ++++++++++++++---------------------------------- 1 file changed, 47 insertions(+), 113 deletions(-) diff --git a/src/borrowed.rs b/src/borrowed.rs index 8563df0..98c1469 100644 --- a/src/borrowed.rs +++ b/src/borrowed.rs @@ -91,73 +91,30 @@ pub trait HasDisplayHandle { fn display_handle(&self) -> DisplayHandle<'_>; } -impl HasDisplayHandle for &T { - fn active(&self) -> Option> { - (**self).active() - } - - fn display_handle<'this, 'active>( - &'this self, - active: &'active Active<'_>, - ) -> DisplayHandle<'this> - where - 'active: 'this, - { - (**self).display_handle(active) +impl HasDisplayHandle for &H { + fn display_handle(&self) -> DisplayHandle<'_> { + (**self).display_handle() } } #[cfg(feature = "alloc")] -#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] -impl HasDisplayHandle for alloc::boxed::Box { - fn active(&self) -> Option> { - (**self).active() - } - - fn display_handle<'this, 'active>( - &'this self, - active: &'active Active<'_>, - ) -> DisplayHandle<'this> - where - 'active: 'this, - { - (**self).display_handle(active) +impl HasDisplayHandle for alloc::boxed::Box { + fn display_handle(&self) -> DisplayHandle<'_> { + (**self).display_handle() } } #[cfg(feature = "alloc")] -#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] -impl HasDisplayHandle for alloc::rc::Rc { - fn active(&self) -> Option> { - (**self).active() - } - - fn display_handle<'this, 'active>( - &'this self, - active: &'active Active<'_>, - ) -> DisplayHandle<'this> - where - 'active: 'this, - { - (**self).display_handle(active) +impl HasDisplayHandle for alloc::rc::Rc { + fn display_handle(&self) -> DisplayHandle<'_> { + (**self).display_handle() } } #[cfg(feature = "alloc")] -#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] -impl HasDisplayHandle for alloc::sync::Arc { - fn active(&self) -> Option> { - (**self).active() - } - - fn display_handle<'this, 'active>( - &'this self, - active: &'active Active<'_>, - ) -> DisplayHandle<'this> - where - 'active: 'this, - { - (**self).display_handle(active) +impl HasDisplayHandle for alloc::sync::Arc { + fn display_handle(&self) -> DisplayHandle<'_> { + (**self).display_handle() } } @@ -239,68 +196,55 @@ impl<'a> HasDisplayHandle for DisplayHandle<'a> { /// constructors of [`WindowHandle`]. This is because the `HasWindowHandle` trait is safe to implement. pub trait HasWindowHandle { /// Get a handle to the window. - fn window_handle<'this, 'active>( - &'this self, - active: ActiveHandle<'active>, - ) -> WindowHandle<'this> - where - 'active: 'this; + fn window_handle(&self) -> Result, WindowHandleError>; } -impl HasWindowHandle for &T { - fn window_handle<'this, 'active>( - &'this self, - active: &'active Active<'_>, - ) -> WindowHandle<'this> - where - 'active: 'this, - { - (**self).window_handle(active) +impl HasWindowHandle for &H { + fn window_handle(&self) -> Result, WindowHandleError> { + (**self).window_handle() } } #[cfg(feature = "alloc")] -#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] -impl HasWindowHandle for alloc::boxed::Box { - fn window_handle<'this, 'active>( - &'this self, - active: &'active Active<'_>, - ) -> WindowHandle<'this> - where - 'active: 'this, - { - (**self).window_handle(active) +impl HasWindowHandle for alloc::boxed::Box { + fn window_handle(&self) -> Result, WindowHandleError> { + (**self).window_handle() } } #[cfg(feature = "alloc")] -#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] -impl HasWindowHandle for alloc::rc::Rc { - fn window_handle<'this, 'active>( - &'this self, - active: &'active Active<'_>, - ) -> WindowHandle<'this> - where - 'active: 'this, - { - (**self).window_handle(active) +impl HasWindowHandle for alloc::rc::Rc { + fn window_handle(&self) -> Result, WindowHandleError> { + (**self).window_handle() } } #[cfg(feature = "alloc")] -#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] -impl HasWindowHandle for alloc::sync::Arc { - fn window_handle<'this, 'active>( - &'this self, - active: &'active Active<'_>, - ) -> WindowHandle<'this> - where - 'active: 'this, - { - (**self).window_handle(active) +impl HasWindowHandle for alloc::sync::Arc { + fn window_handle(&self) -> Result, WindowHandleError> { + (**self).window_handle() } } +/// The error type returned when a window handle cannot be obtained. +#[derive(Debug)] +#[non_exhaustive] +pub enum WindowHandleError { + /// The window is not currently active. + Inactive, +} + +impl fmt::Display for WindowHandleError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Inactive => write!(f, "the window is not currently active"), + } + } +} + +#[cfg(feature = "std")] +impl std::error::Error for WindowHandleError {} + /// The handle to a window. /// /// This is the primary return type of the [`HasWindowHandle`] trait. All *pointers* within this type @@ -358,19 +302,9 @@ unsafe impl HasRawWindowHandle for WindowHandle<'_> { } } -impl<'a> HasWindowHandle for WindowHandle<'a> { - fn window_handle<'this, 'active>( - &'this self, - active: ActiveHandle<'active>, - ) -> WindowHandle<'this> - where - 'active: 'this, - { - WindowHandle { - raw: self.raw, - _active: active, - _marker: PhantomData, - } +impl HasWindowHandle for WindowHandle<'_> { + fn window_handle(&self) -> Result { + Ok(self.clone()) } } From 2397bc03a363dcf77588779b35d198ff7cafd797 Mon Sep 17 00:00:00 2001 From: jtnunley Date: Fri, 24 Mar 2023 04:54:21 -0700 Subject: [PATCH 04/13] Export the error type --- src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib.rs b/src/lib.rs index a3e1e69..125ff89 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -45,6 +45,7 @@ pub use android::{AndroidDisplayHandle, AndroidNdkWindowHandle}; pub use appkit::{AppKitDisplayHandle, AppKitWindowHandle}; pub use borrowed::{ Active, ActiveHandle, DisplayHandle, HasDisplayHandle, HasWindowHandle, WindowHandle, + WindowHandleError, }; pub use haiku::{HaikuDisplayHandle, HaikuWindowHandle}; pub use redox::{OrbitalDisplayHandle, OrbitalWindowHandle}; From c9e29fded255a87fb1b209897ad80765f79c5d45 Mon Sep 17 00:00:00 2001 From: jtnunley Date: Fri, 24 Mar 2023 10:25:40 -0700 Subject: [PATCH 05/13] Make DisplayHandle more consistent with WindowHandle --- src/borrowed.rs | 44 +++++++++++++++++++++++--------------------- src/lib.rs | 4 ++-- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/src/borrowed.rs b/src/borrowed.rs index 98c1469..81e957c 100644 --- a/src/borrowed.rs +++ b/src/borrowed.rs @@ -88,32 +88,32 @@ impl ActiveHandle<'_> { /// [`HasRawDisplayHandle`]: crate::HasRawDisplayHandle pub trait HasDisplayHandle { /// Get a handle to the display controller of the windowing system. - fn display_handle(&self) -> DisplayHandle<'_>; + fn display_handle(&self) -> Result, HandleError>; } impl HasDisplayHandle for &H { - fn display_handle(&self) -> DisplayHandle<'_> { + fn display_handle(&self) -> Result, HandleError> { (**self).display_handle() } } #[cfg(feature = "alloc")] impl HasDisplayHandle for alloc::boxed::Box { - fn display_handle(&self) -> DisplayHandle<'_> { + fn display_handle(&self) -> Result, HandleError> { (**self).display_handle() } } #[cfg(feature = "alloc")] impl HasDisplayHandle for alloc::rc::Rc { - fn display_handle(&self) -> DisplayHandle<'_> { + fn display_handle(&self) -> Result, HandleError> { (**self).display_handle() } } #[cfg(feature = "alloc")] impl HasDisplayHandle for alloc::sync::Arc { - fn display_handle(&self) -> DisplayHandle<'_> { + fn display_handle(&self) -> Result, HandleError> { (**self).display_handle() } } @@ -137,10 +137,12 @@ impl fmt::Debug for DisplayHandle<'_> { } } -impl<'a> Copy for DisplayHandle<'a> {} impl<'a> Clone for DisplayHandle<'a> { fn clone(&self) -> Self { - *self + Self { + raw: self.raw, + _marker: PhantomData, + } } } @@ -165,8 +167,8 @@ unsafe impl HasRawDisplayHandle for DisplayHandle<'_> { } impl<'a> HasDisplayHandle for DisplayHandle<'a> { - fn display_handle(&self) -> DisplayHandle<'_> { - *self + fn display_handle(&self) -> Result, HandleError> { + Ok(self.clone()) } } @@ -196,54 +198,54 @@ impl<'a> HasDisplayHandle for DisplayHandle<'a> { /// constructors of [`WindowHandle`]. This is because the `HasWindowHandle` trait is safe to implement. pub trait HasWindowHandle { /// Get a handle to the window. - fn window_handle(&self) -> Result, WindowHandleError>; + fn window_handle(&self) -> Result, HandleError>; } impl HasWindowHandle for &H { - fn window_handle(&self) -> Result, WindowHandleError> { + fn window_handle(&self) -> Result, HandleError> { (**self).window_handle() } } #[cfg(feature = "alloc")] impl HasWindowHandle for alloc::boxed::Box { - fn window_handle(&self) -> Result, WindowHandleError> { + fn window_handle(&self) -> Result, HandleError> { (**self).window_handle() } } #[cfg(feature = "alloc")] impl HasWindowHandle for alloc::rc::Rc { - fn window_handle(&self) -> Result, WindowHandleError> { + fn window_handle(&self) -> Result, HandleError> { (**self).window_handle() } } #[cfg(feature = "alloc")] impl HasWindowHandle for alloc::sync::Arc { - fn window_handle(&self) -> Result, WindowHandleError> { + fn window_handle(&self) -> Result, HandleError> { (**self).window_handle() } } -/// The error type returned when a window handle cannot be obtained. +/// The error type returned when a handle cannot be obtained. #[derive(Debug)] #[non_exhaustive] -pub enum WindowHandleError { - /// The window is not currently active. +pub enum HandleError { + /// The handle is not currently active. Inactive, } -impl fmt::Display for WindowHandleError { +impl fmt::Display for HandleError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - Self::Inactive => write!(f, "the window is not currently active"), + Self::Inactive => write!(f, "the handle is not currently active"), } } } #[cfg(feature = "std")] -impl std::error::Error for WindowHandleError {} +impl std::error::Error for HandleError {} /// The handle to a window. /// @@ -303,7 +305,7 @@ unsafe impl HasRawWindowHandle for WindowHandle<'_> { } impl HasWindowHandle for WindowHandle<'_> { - fn window_handle(&self) -> Result { + fn window_handle(&self) -> Result { Ok(self.clone()) } } diff --git a/src/lib.rs b/src/lib.rs index 125ff89..522e49e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -44,8 +44,8 @@ mod windows; pub use android::{AndroidDisplayHandle, AndroidNdkWindowHandle}; pub use appkit::{AppKitDisplayHandle, AppKitWindowHandle}; pub use borrowed::{ - Active, ActiveHandle, DisplayHandle, HasDisplayHandle, HasWindowHandle, WindowHandle, - WindowHandleError, + Active, ActiveHandle, DisplayHandle, HandleError, HasDisplayHandle, HasWindowHandle, + WindowHandle, }; pub use haiku::{HaikuDisplayHandle, HaikuWindowHandle}; pub use redox::{OrbitalDisplayHandle, OrbitalWindowHandle}; From 143079b9d3bbb957f074eea5d82641b43a468998 Mon Sep 17 00:00:00 2001 From: jtnunley Date: Mon, 27 Mar 2023 11:53:22 -0700 Subject: [PATCH 06/13] Fix active handle resemblance to refcounted version --- src/borrowed.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/borrowed.rs b/src/borrowed.rs index 81e957c..ae4cef4 100644 --- a/src/borrowed.rs +++ b/src/borrowed.rs @@ -323,14 +323,15 @@ fn _not_send_or_sync() {} #[cfg_attr(docsrs, doc(cfg(not(target_os = "android"))))] mod imp { //! We don't need to refcount the handles, so we can just use no-ops. - + + use core::cell::UnsafeCell; use core::marker::PhantomData; pub(super) struct Active; #[derive(Clone)] pub(super) struct ActiveHandle<'a> { - _marker: PhantomData<&'a ()>, + _marker: PhantomData<&'a UnsafeCell<()>>, } impl Active { @@ -356,6 +357,12 @@ mod imp { } } + impl Drop for ActiveHandle<'_> { + fn drop(&mut self) { + // Done for consistency with the refcounted version. + } + } + impl super::ActiveHandle<'_> { /// Create a new `ActiveHandle`. /// From b0f06d82d74bcf456d5a9ffb24061d6036c0398a Mon Sep 17 00:00:00 2001 From: jtnunley Date: Mon, 27 Mar 2023 12:59:16 -0700 Subject: [PATCH 07/13] fmt --- src/borrowed.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/borrowed.rs b/src/borrowed.rs index ae4cef4..e2f6709 100644 --- a/src/borrowed.rs +++ b/src/borrowed.rs @@ -323,7 +323,7 @@ fn _not_send_or_sync() {} #[cfg_attr(docsrs, doc(cfg(not(target_os = "android"))))] mod imp { //! We don't need to refcount the handles, so we can just use no-ops. - + use core::cell::UnsafeCell; use core::marker::PhantomData; From 263d32e5eaddbd8721bf5bf833b94dbe1c005ef7 Mon Sep 17 00:00:00 2001 From: jtnunley Date: Tue, 28 Mar 2023 09:50:06 -0700 Subject: [PATCH 08/13] Mark `set_inactive` as unsafe --- src/borrowed.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/borrowed.rs b/src/borrowed.rs index e2f6709..6c4d717 100644 --- a/src/borrowed.rs +++ b/src/borrowed.rs @@ -44,11 +44,7 @@ impl Active { /// Set the application to be inactive. /// /// This function may block until there are no more active handles. - /// - /// # Safety - /// - /// The application must actually be inactive. - pub unsafe fn set_inactive(&self) { + pub fn set_inactive(&self) { self.0.set_inactive() } @@ -346,7 +342,7 @@ mod imp { pub(super) unsafe fn set_active(&self) {} - pub(super) unsafe fn set_inactive(&self) {} + pub(super) fn set_inactive(&self) {} } impl ActiveHandle<'_> { @@ -431,7 +427,7 @@ mod imp { *self.active.write().unwrap() = true; } - pub(super) unsafe fn set_inactive(&self) { + pub(super) fn set_inactive(&self) { *self.active.write().unwrap() = false; } } From 8ae48505e5ba232ad7c89c728c9d9386904cebcc Mon Sep 17 00:00:00 2001 From: jtnunley Date: Wed, 29 Mar 2023 10:53:15 -0700 Subject: [PATCH 09/13] Add more docs --- src/borrowed.rs | 174 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 170 insertions(+), 4 deletions(-) diff --git a/src/borrowed.rs b/src/borrowed.rs index 6c4d717..d79b768 100644 --- a/src/borrowed.rs +++ b/src/borrowed.rs @@ -12,6 +12,54 @@ use core::marker::PhantomData; use crate::{HasRawDisplayHandle, HasRawWindowHandle, RawDisplayHandle, RawWindowHandle}; /// Keeps track of whether the application is currently active. +/// +/// On certain platforms (e.g. Android), it is possible for the application to enter a "suspended" +/// state. While in this state, all previously valid window handles become invalid. Therefore, in +/// order for window handles to be valid, the application must be active. +/// +/// On platforms where the graphical user interface is always active, this type is a ZST and all +/// of its methods are noops. On Android, this type acts as a reference counter that keeps track +/// of all currently active window handles. Before the application enters the suspended state, it +/// blocks until all of the currently active window handles are dropped. +/// +/// ## Explanation +/// +/// On Android, there is an [Activity]-global [`ANativeWindow`] object that is used for drawing. This +/// handle is used [within the `RawWindowHandle` type] for Android NDK, since it is necessary for GFX +/// APIs to draw to the screen. +/// +/// However, the [`ANativeWindow`] type can be arbitrarily invalidated by the underlying Android runtime. +/// The reasoning for this is complicated, but this idea is exposed to native code through the +/// [`onNativeWindowCreated`] and [`onNativeWindowDestroyed`] callbacks. To save you a click, the +/// conditions associated with these callbacks are: +/// +/// - [`onNativeWindowCreated`] provides a valid [`ANativeWindow`] pointer that can be used for drawing. +/// - [`onNativeWindowDestroyed`] indicates that the previous [`ANativeWindow`] pointer is no longer +/// valid. The documentation clarifies that, *once the function returns*, the [`ANativeWindow`] pointer +/// can no longer be used for drawing without resulting in undefined behavior. +/// +/// In [`winit`], these are exposed via the [`Resumed`] and [`Suspended`] events, respectively. Therefore, +/// between the last [`Suspended`] event and the next [`Resumed`] event, it is undefined behavior to use +/// the raw window handle. This condition makes it tricky to define an API that safely wraps the raw +/// window handles, since an existing window handle can be made invalid at any time. +/// +/// The Android docs specifies that the [`ANativeWindow`] pointer is still valid while the application +/// is still in the [`onNativeWindowDestroyed`] block, and suggests that synchronization needs to take +/// place to ensure that the pointer has been invalidated before the function returns. `Active` aims +/// to be the solution to this problem. It keeps track of all currently active window handles, and +/// blocks until all of them are dropped before allowing the application to enter the suspended state. +/// +/// [Activity]: https://developer.android.com/reference/android/app/Activity +/// [`ANativeWindow`]: https://developer.android.com/ndk/reference/group/a-native-window +/// [within the `RawWindowHandle` type]: struct.AndroidNdkWindowHandle.html#structfield.a_native_window +/// [`onNativeWindowCreated`]: https://developer.android.com/ndk/reference/struct/a-native-activity-callbacks#onnativewindowcreated +/// [`onNativeWindowDestroyed`]: https://developer.android.com/ndk/reference/struct/a-native-activity-callbacks#onnativewindowdestroyed +/// [`Resumed`]: https://docs.rs/winit/latest/winit/event/enum.Event.html#variant.Resumed +/// [`Suspended`]: https://docs.rs/winit/latest/winit/event/enum.Event.html#variant.Suspended +/// [`sdl2`]: https://crates.io/crates/sdl2 +/// [`RawWindowHandle`]: https://docs.rs/raw-window-handle/latest/raw_window_handle/enum.RawWindowHandle.html +/// [`HasRawWindowHandle`]: https://docs.rs/raw-window-handle/latest/raw_window_handle/trait.HasRawWindowHandle.html +/// [`winit`]: https://crates.io/crates/winit pub struct Active(imp::Active); impl fmt::Debug for Active { @@ -21,6 +69,13 @@ impl fmt::Debug for Active { } /// Represents a live window handle. +/// +/// This is carried around by the [`Active`] type, and is used to ensure that the application doesn't +/// enter the suspended state while there are still live window handles. See documentation on the +/// [`Active`] type for more information. +/// +/// On non-Android platforms, this is a ZST. On Android, this is a reference counted handle that +/// keeps the application active while it is alive. #[derive(Clone)] pub struct ActiveHandle<'a>(imp::ActiveHandle<'a>); @@ -32,11 +87,39 @@ impl<'a> fmt::Debug for ActiveHandle<'a> { impl Active { /// Create a new `Active` tracker. + /// + /// Only one of these should exist per display connection. + /// + /// # Example + /// + /// ``` + /// use raw_window_handle::Active; + /// let active = Active::new(); + /// ``` pub const fn new() -> Self { Self(imp::Active::new()) } /// Get a live window handle. + /// + /// This function returns an active handle if the application is active, and `None` otherwise. + /// + /// # Example + /// + /// ``` + /// use raw_window_handle::Active; + /// + /// // Set the application to be active. + /// let active = Active::new(); + /// unsafe { active.set_active() }; + /// + /// // Get a live window handle. + /// let handle = active.handle(); + /// + /// // Drop it and set the application to be inactive. + /// drop(handle); + /// active.set_inactive(); + /// ``` pub fn handle(&self) -> Option> { self.0.handle().map(ActiveHandle) } @@ -44,6 +127,19 @@ impl Active { /// Set the application to be inactive. /// /// This function may block until there are no more active handles. + /// + /// # Example + /// + /// ``` + /// use raw_window_handle::Active; + /// + /// // Set the application to be active. + /// let active = Active::new(); + /// unsafe { active.set_active() }; + /// + /// // Set the application to be inactive. + /// active.set_inactive(); + /// ``` pub fn set_inactive(&self) { self.0.set_inactive() } @@ -52,7 +148,21 @@ impl Active { /// /// # Safety /// - /// The application must actually be active. + /// The application must actually be active. Setting to active when the application is not active + /// will result in undefined behavior. + /// + /// # Example + /// + /// ``` + /// use raw_window_handle::Active; + /// + /// // Set the application to be active. + /// let active = Active::new(); + /// unsafe { active.set_active() }; + /// + /// // Set the application to be inactive. + /// active.set_inactive(); + /// ``` pub unsafe fn set_active(&self) { self.0.set_active() } @@ -61,9 +171,24 @@ impl Active { impl ActiveHandle<'_> { /// Create a new freestanding active handle. /// + /// This function acts as an "escape hatch" to allow the user to create a live window handle + /// without having to go through the [`Active`] type. This is useful if the user *knows* that the + /// application is active, and wants to create a live window handle without having to go through + /// the [`Active`] type. + /// /// # Safety /// /// The application must actually be active. + /// + /// # Example + /// + /// ``` + /// use raw_window_handle::ActiveHandle; + /// + /// // Create a freestanding active handle. + /// // SAFETY: The application must actually be active. + /// let handle = unsafe { ActiveHandle::new_unchecked() }; + /// ``` pub unsafe fn new_unchecked() -> Self { Self(imp::ActiveHandle::new_unchecked()) } @@ -71,9 +196,22 @@ impl ActiveHandle<'_> { /// A display that acts as a wrapper around a display handle. /// +/// Objects that implement this trait should be able to return a [`DisplayHandle`] for the display +/// that they are associated with. This handle should last for the lifetime of the object, and should +/// return an error if the application is inactive. +/// +/// Implementors of this trait will be windowing systems, like [`winit`] and [`sdl2`]. These windowing +/// systems should implement this tait on types that already implement [`HasRawDisplayHandle`]. It +/// should be implemented by tying the lifetime of the [`DisplayHandle`] to the lifetime of the +/// display object. +/// +/// Users of this trait will include graphics libraries, like [`wgpu`] and [`glutin`]. These APIs +/// should be generic over a type that implements `HasDisplayHandle`, and should use the +/// [`DisplayHandle`] type to access the display handle. +/// /// # Safety /// -/// The safety requirements of [`HasRawDisplayHandle`] apply here as well. To reiterate, the +/// The safety requirements of [`HasRawDisplayHandle`] apply here as well. To reiterate, the /// [`DisplayHandle`] must contain a valid window handle for its lifetime. /// /// It is not possible to invalidate a [`DisplayHandle`] on any platform without additional unsafe code. @@ -143,7 +281,7 @@ impl<'a> Clone for DisplayHandle<'a> { } impl<'a> DisplayHandle<'a> { - /// Borrow a `DisplayHandle` from a [`RawDisplayHandle`]. + /// Create a `DisplayHandle` from a [`RawDisplayHandle`]. /// /// # Safety /// @@ -170,6 +308,23 @@ impl<'a> HasDisplayHandle for DisplayHandle<'a> { /// A handle to a window. /// +/// Objects that implement this trait should be able to return a [`WindowHandle`] for the window +/// that they are associated with. This handle should last for the lifetime of the object, and should +/// return an error if the application is inactive. +/// +/// Implementors of this trait will be windowing systems, like [`winit`] and [`sdl2`]. These windowing +/// systems should implement this tait on types that already implement [`HasRawWindowHandle`]. First, +/// it should be made sure that the display type contains a unique [`Active`] ref-counted handle. +/// To create a [`WindowHandle`], the [`Active`] should be used to create an [`ActiveHandle`] that is +/// then used to create a [`WindowHandle`]. Finally, the raw window handle should be retrieved from +/// the type and used to create a [`WindowHandle`]. +/// +/// Users of this trait will include graphics libraries, like [`wgpu`] and [`glutin`]. These APIs +/// should be generic over a type that implements `HasWindowHandle`, and should use the +/// [`WindowHandle`] type to access the window handle. The window handle should be acquired and held +/// while the window is being used, in order to ensure that the window is not deleted while it is in +/// use. +/// /// # Safety /// /// All pointers within the resulting [`WindowHandle`] must be valid and not dangling for the lifetime of @@ -229,6 +384,8 @@ impl HasWindowHandle for alloc::sync::Arc { #[non_exhaustive] pub enum HandleError { /// The handle is not currently active. + /// + /// See documentation on [`Active`] for more information. Inactive, } @@ -284,7 +441,9 @@ impl<'a> WindowHandle<'a> { /// /// # Safety /// - /// The [`RawWindowHandle`] must be valid for the lifetime and the application must be `Active`. + /// The [`RawWindowHandle`] must be valid for the lifetime and the application must not be + /// suspended. The [`Active`] object that the [`ActiveHandle`] was created from must be + /// associated directly with the display that the window handle is associated with. pub unsafe fn borrow_raw(raw: RawWindowHandle, active: ActiveHandle<'a>) -> Self { Self { raw, @@ -363,6 +522,13 @@ mod imp { /// Create a new `ActiveHandle`. /// /// This is safe because the handle is always active. + /// + /// # Example + /// + /// ``` + /// use raw_window_handle::ActiveHandle; + /// let handle = ActiveHandle::new(); + /// ``` #[allow(clippy::new_without_default)] pub fn new() -> Self { // SAFETY: The handle is always active. From 6362dc43ef3bb851f5d986fae7b78392fd799a34 Mon Sep 17 00:00:00 2001 From: jtnunley Date: Wed, 29 Mar 2023 13:03:46 -0700 Subject: [PATCH 10/13] Fix doc links --- src/borrowed.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/borrowed.rs b/src/borrowed.rs index d79b768..8e983f3 100644 --- a/src/borrowed.rs +++ b/src/borrowed.rs @@ -220,6 +220,10 @@ impl ActiveHandle<'_> { /// constructors of [`DisplayHandle`]. This is because the `HasDisplayHandle` trait is safe to implement. /// /// [`HasRawDisplayHandle`]: crate::HasRawDisplayHandle +/// [`winit`]: https://crates.io/crates/winit +/// [`sdl2`]: https://crates.io/crates/sdl2 +/// [`wgpu`]: https://crates.io/crates/wgpu +/// [`glutin`]: https://crates.io/crates/glutin pub trait HasDisplayHandle { /// Get a handle to the display controller of the windowing system. fn display_handle(&self) -> Result, HandleError>; @@ -347,6 +351,11 @@ impl<'a> HasDisplayHandle for DisplayHandle<'a> { /// /// Note that these requirements are not enforced on `HasWindowHandle`, rather, they are enforced on the /// constructors of [`WindowHandle`]. This is because the `HasWindowHandle` trait is safe to implement. +/// +/// [`winit`]: https://crates.io/crates/winit +/// [`sdl2`]: https://crates.io/crates/sdl2 +/// [`wgpu`]: https://crates.io/crates/wgpu +/// [`glutin`]: https://crates.io/crates/glutin pub trait HasWindowHandle { /// Get a handle to the window. fn window_handle(&self) -> Result, HandleError>; From c0e30026e02f36822a2aa7eea5b0de9de43d8f58 Mon Sep 17 00:00:00 2001 From: jtnunley Date: Wed, 29 Mar 2023 13:24:30 -0700 Subject: [PATCH 11/13] tait -> trait --- src/borrowed.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/borrowed.rs b/src/borrowed.rs index 8e983f3..42f6a34 100644 --- a/src/borrowed.rs +++ b/src/borrowed.rs @@ -201,7 +201,7 @@ impl ActiveHandle<'_> { /// return an error if the application is inactive. /// /// Implementors of this trait will be windowing systems, like [`winit`] and [`sdl2`]. These windowing -/// systems should implement this tait on types that already implement [`HasRawDisplayHandle`]. It +/// systems should implement this trait on types that already implement [`HasRawDisplayHandle`]. It /// should be implemented by tying the lifetime of the [`DisplayHandle`] to the lifetime of the /// display object. /// @@ -317,7 +317,7 @@ impl<'a> HasDisplayHandle for DisplayHandle<'a> { /// return an error if the application is inactive. /// /// Implementors of this trait will be windowing systems, like [`winit`] and [`sdl2`]. These windowing -/// systems should implement this tait on types that already implement [`HasRawWindowHandle`]. First, +/// systems should implement this trait on types that already implement [`HasRawWindowHandle`]. First, /// it should be made sure that the display type contains a unique [`Active`] ref-counted handle. /// To create a [`WindowHandle`], the [`Active`] should be used to create an [`ActiveHandle`] that is /// then used to create a [`WindowHandle`]. Finally, the raw window handle should be retrieved from From ed90ef0727c3781eb60a92158056001b42b612c2 Mon Sep 17 00:00:00 2001 From: jtnunley Date: Wed, 29 Mar 2023 13:45:45 -0700 Subject: [PATCH 12/13] Win32 handles can't be arbitrarily deleted --- src/borrowed.rs | 54 ++++++++++++++++++++++++------------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/src/borrowed.rs b/src/borrowed.rs index 42f6a34..8cc3f9e 100644 --- a/src/borrowed.rs +++ b/src/borrowed.rs @@ -335,13 +335,13 @@ impl<'a> HasDisplayHandle for DisplayHandle<'a> { /// the handle. /// /// Note that this guarantee only applies to *pointers*, and not any window ID types in the handle. -/// This includes Window IDs (XIDs) from X11, `HWND`s from Win32, and the window ID for web platforms. -/// There is no way for Rust to enforce any kind of invariant on these types, since: +/// This includes Window IDs (XIDs) from X11 and the window ID for web platforms. There is no way for +/// Rust to enforce any kind of invariant on these types, since: /// /// - For all three listed platforms, it is possible for safe code in the same process to delete /// the window. -/// - For X11 and Win32, it is possible for code in a different process to delete the window. -/// - For X11, it is possible for code on a different *machine* to delete the window. +/// - For X11, it is possible for code in a different process to delete the window. In fact, it is +/// possible for code on a different *machine* to delete the window. /// /// It is *also* possible for the window to be replaced with another, valid-but-different window. User /// code should be aware of this possibility, and should be ready to soundly handle the possible error @@ -388,33 +388,12 @@ impl HasWindowHandle for alloc::sync::Arc { } } -/// The error type returned when a handle cannot be obtained. -#[derive(Debug)] -#[non_exhaustive] -pub enum HandleError { - /// The handle is not currently active. - /// - /// See documentation on [`Active`] for more information. - Inactive, -} - -impl fmt::Display for HandleError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::Inactive => write!(f, "the handle is not currently active"), - } - } -} - -#[cfg(feature = "std")] -impl std::error::Error for HandleError {} - /// The handle to a window. /// /// This is the primary return type of the [`HasWindowHandle`] trait. All *pointers* within this type /// are guaranteed to be valid and not dangling for the lifetime of the handle. This excludes window IDs -/// like XIDs, `HWND`s, and the window ID for web platforms. See the documentation on the -/// [`HasWindowHandle`] trait for more information about these safety requirements. +/// like XIDs and the window ID for web platforms. See the documentation on the [`HasWindowHandle`] +/// trait for more information about these safety requirements. /// /// This handle is guaranteed to be safe and valid. Get the underlying raw window handle with the /// [`HasRawWindowHandle`] trait. @@ -474,6 +453,27 @@ impl HasWindowHandle for WindowHandle<'_> { } } +/// The error type returned when a handle cannot be obtained. +#[derive(Debug)] +#[non_exhaustive] +pub enum HandleError { + /// The handle is not currently active. + /// + /// See documentation on [`Active`] for more information. + Inactive, +} + +impl fmt::Display for HandleError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Inactive => write!(f, "the handle is not currently active"), + } + } +} + +#[cfg(feature = "std")] +impl std::error::Error for HandleError {} + /// ```compile_fail /// use raw_window_handle::{Active, DisplayHandle, WindowHandle}; /// fn _assert() {} From ee93e21379befc4e9c7335d1fc7cea3cd8d0a1ac Mon Sep 17 00:00:00 2001 From: jtnunley Date: Wed, 29 Mar 2023 13:46:53 -0700 Subject: [PATCH 13/13] fmt --- src/borrowed.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/borrowed.rs b/src/borrowed.rs index 8cc3f9e..b38d3d8 100644 --- a/src/borrowed.rs +++ b/src/borrowed.rs @@ -340,7 +340,7 @@ impl<'a> HasDisplayHandle for DisplayHandle<'a> { /// /// - For all three listed platforms, it is possible for safe code in the same process to delete /// the window. -/// - For X11, it is possible for code in a different process to delete the window. In fact, it is +/// - For X11, it is possible for code in a different process to delete the window. In fact, it is /// possible for code on a different *machine* to delete the window. /// /// It is *also* possible for the window to be replaced with another, valid-but-different window. User @@ -392,7 +392,7 @@ impl HasWindowHandle for alloc::sync::Arc { /// /// This is the primary return type of the [`HasWindowHandle`] trait. All *pointers* within this type /// are guaranteed to be valid and not dangling for the lifetime of the handle. This excludes window IDs -/// like XIDs and the window ID for web platforms. See the documentation on the [`HasWindowHandle`] +/// like XIDs and the window ID for web platforms. See the documentation on the [`HasWindowHandle`] /// trait for more information about these safety requirements. /// /// This handle is guaranteed to be safe and valid. Get the underlying raw window handle with the