From 48f62855a662d097f40d52870dbe487a8529395a Mon Sep 17 00:00:00 2001 From: John Nunley Date: Tue, 8 Aug 2023 15:13:02 -0700 Subject: [PATCH] Excise the HasRaw*Handle traits After reading #135, I've realized that the HasRawDisplayHandle and HasRawWindowHandle traits have little value in a post-borrowed-handle world. Borrowed handles do everything better, and the raw handle can be extracted from the borrowed handle. Therefore it makes sense to remove these traits. Closes #135. Signed-off-by: John Nunley --- .github/workflows/ci.yml | 8 ---- src/borrowed.rs | 91 ++++++++++++++++++++++++++++++++-------- src/lib.rs | 8 ++-- 3 files changed, 79 insertions(+), 28 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c997113..d10d88b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,8 +40,6 @@ jobs: with: rust-version: ${{ matrix.rust_version }} - - run: rustup target add x86_64-linux-android - - name: Check documentation run: cargo doc --no-deps --document-private-items @@ -50,9 +48,3 @@ jobs: - name: Run tests with std run: cargo test --verbose --features std - - - name: Check on Android - run: cargo check --verbose --target x86_64-linux-android - - - name: Check on Android with std - run: cargo check --verbose --target x86_64-linux-android --features std diff --git a/src/borrowed.rs b/src/borrowed.rs index 9e3d3a0..bfa0073 100644 --- a/src/borrowed.rs +++ b/src/borrowed.rs @@ -2,12 +2,11 @@ //! //! These should be 100% safe to pass around and use, no possibility of dangling or invalidity. +use core::borrow::Borrow; use core::fmt; use core::marker::PhantomData; -use crate::{ - HandleError, HasRawDisplayHandle, HasRawWindowHandle, RawDisplayHandle, RawWindowHandle, -}; +use crate::{HandleError, RawDisplayHandle, RawWindowHandle}; /// A display that acts as a wrapper around a display handle. /// @@ -16,7 +15,7 @@ use crate::{ /// 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 trait on types that already implement [`HasRawDisplayHandle`]. It +/// systems should implement this trait on types that represent the top-level display server. It /// should be implemented by tying the lifetime of the [`DisplayHandle`] to the lifetime of the /// display object. /// @@ -26,15 +25,22 @@ use crate::{ /// /// # Safety /// -/// The safety requirements of [`HasRawDisplayHandle`] apply here as well. To reiterate, the -/// [`DisplayHandle`] must contain a valid window handle for its lifetime. +/// Users can safely assume that non-`null`/`0` fields are valid handles, and it is up to the +/// implementer of this trait to ensure that condition is upheld. +/// +/// Despite that qualification, implementers should still make a best-effort attempt to fill in all +/// available fields. If an implementation doesn't, and a downstream user needs the field, it should +/// try to derive the field from other fields the implementer *does* provide via whatever methods the +/// platform provides. +/// +/// The exact handles returned by `raw_display_handle` must remain consistent between multiple calls +/// to `raw_display_handle` as long as not indicated otherwise by platform specific events. /// /// 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 [`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 @@ -57,6 +63,7 @@ impl HasDisplayHandle for &mut H { } #[cfg(feature = "alloc")] +#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] impl HasDisplayHandle for alloc::boxed::Box { fn display_handle(&self) -> Result, HandleError> { (**self).display_handle() @@ -64,6 +71,7 @@ impl HasDisplayHandle for alloc::boxed::Box { } #[cfg(feature = "alloc")] +#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] impl HasDisplayHandle for alloc::rc::Rc { fn display_handle(&self) -> Result, HandleError> { (**self).display_handle() @@ -71,6 +79,7 @@ impl HasDisplayHandle for alloc::rc::Rc { } #[cfg(feature = "alloc")] +#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] impl HasDisplayHandle for alloc::sync::Arc { fn display_handle(&self) -> Result, HandleError> { (**self).display_handle() @@ -81,8 +90,6 @@ impl HasDisplayHandle for alloc::sync::Arc { /// /// 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, Copy, Clone)] pub struct DisplayHandle<'a> { @@ -108,11 +115,28 @@ impl<'a> DisplayHandle<'a> { _marker: PhantomData, } } + + /// Get the underlying raw display handle. + pub fn as_raw(&self) -> RawDisplayHandle { + self.raw + } +} + +impl AsRef for DisplayHandle<'_> { + fn as_ref(&self) -> &RawDisplayHandle { + &self.raw + } +} + +impl Borrow for DisplayHandle<'_> { + fn borrow(&self) -> &RawDisplayHandle { + &self.raw + } } -unsafe impl HasRawDisplayHandle for DisplayHandle<'_> { - fn raw_display_handle(&self) -> Result { - Ok(self.raw) +impl From> for RawDisplayHandle { + fn from(handle: DisplayHandle<'_>) -> Self { + handle.raw } } @@ -129,7 +153,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 trait on types that already implement [`HasRawWindowHandle`]. +/// systems should implement this trait on types that represent windows. /// /// 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 @@ -139,8 +163,16 @@ impl<'a> HasDisplayHandle for DisplayHandle<'a> { /// /// # Safety /// -/// All pointers within the resulting [`WindowHandle`] must be valid and not dangling for the lifetime of -/// the handle. +/// Users can safely assume that non-`null`/`0` fields are valid handles, and it is up to the +/// implementer of this trait to ensure that condition is upheld. +/// +/// Despite that qualification, implementers should still make a best-effort attempt to fill in all +/// available fields. If an implementation doesn't, and a downstream user needs the field, it should +/// try to derive the field from other fields the implementer *does* provide via whatever methods the +/// platform provides. +/// +/// The exact handles returned by `raw_window_handle` must remain consistent between multiple calls +/// to `raw_window_handle` as long as not indicated otherwise by platform specific events. /// /// Note that this guarantee only applies to *pointers*, and not any window ID types in the handle. /// This includes Window IDs (XIDs) from X11 and the window ID for web platforms. There is no way for @@ -180,6 +212,7 @@ impl HasWindowHandle for &mut H { } #[cfg(feature = "alloc")] +#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] impl HasWindowHandle for alloc::boxed::Box { fn window_handle(&self) -> Result, HandleError> { (**self).window_handle() @@ -187,6 +220,7 @@ impl HasWindowHandle for alloc::boxed::Box { } #[cfg(feature = "alloc")] +#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] impl HasWindowHandle for alloc::rc::Rc { fn window_handle(&self) -> Result, HandleError> { (**self).window_handle() @@ -194,6 +228,7 @@ impl HasWindowHandle for alloc::rc::Rc { } #[cfg(feature = "alloc")] +#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] impl HasWindowHandle for alloc::sync::Arc { fn window_handle(&self) -> Result, HandleError> { (**self).window_handle() @@ -207,8 +242,7 @@ impl HasWindowHandle for alloc::sync::Arc { /// 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. +/// This handle is guaranteed to be safe and valid. #[derive(PartialEq, Eq, Hash, Clone)] pub struct WindowHandle<'a> { raw: RawWindowHandle, @@ -233,6 +267,11 @@ impl<'a> WindowHandle<'a> { _marker: PhantomData, } } + + /// Get the underlying raw window handle. + pub fn as_raw(&self) -> RawWindowHandle { + self.raw + } } unsafe impl HasRawWindowHandle for WindowHandle<'_> { @@ -241,6 +280,24 @@ unsafe impl HasRawWindowHandle for WindowHandle<'_> { } } +impl AsRef for WindowHandle<'_> { + fn as_ref(&self) -> &RawWindowHandle { + &self.raw + } +} + +impl Borrow for WindowHandle<'_> { + fn borrow(&self) -> &RawWindowHandle { + &self.raw + } +} + +impl From> for RawWindowHandle { + fn from(handle: WindowHandle<'_>) -> Self { + handle.raw + } +} + impl HasWindowHandle for WindowHandle<'_> { fn window_handle(&self) -> Result { Ok(self.clone()) diff --git a/src/lib.rs b/src/lib.rs index 3b81564..eb042bc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -10,7 +10,7 @@ //! //! ## Safety guarantees //! -//! Please see the docs of [`HasRawWindowHandle`] and [`HasRawDisplayHandle`]. +//! Please see the docs of [`HasWindowHandle`] and [`HasDisplayHandle`]. //! //! ## Platform handle initialization //! @@ -70,6 +70,7 @@ use core::fmt; /// /// The exact handles returned by `raw_window_handle` must remain consistent between multiple calls /// to `raw_window_handle` as long as not indicated otherwise by platform specific events. +#[deprecated = "Use `HasWindowHandle` instead"] pub unsafe trait HasRawWindowHandle { fn raw_window_handle(&self) -> Result; } @@ -115,7 +116,7 @@ unsafe impl HasRawWindowHandle for alloc::sync:: /// some hints on where this variant might be expected. /// /// Note that these "Availability Hints" are not normative. That is to say, a -/// [`HasRawWindowHandle`] implementor is completely allowed to return something +/// [`HasWindowHandle`] implementor is completely allowed to return something /// unexpected. (For example, it's legal for someone to return a /// [`RawWindowHandle::Xlib`] on macOS, it would just be weird, and probably /// requires something like XQuartz be used). @@ -215,6 +216,7 @@ pub enum RawWindowHandle { /// /// The exact handles returned by `raw_display_handle` must remain consistent between multiple calls /// to `raw_display_handle` as long as not indicated otherwise by platform specific events. +#[deprecated = "Use `HasDisplayHandle` instead"] pub unsafe trait HasRawDisplayHandle { fn raw_display_handle(&self) -> Result; } @@ -269,7 +271,7 @@ unsafe impl HasRawDisplayHandle for alloc::sync /// some hints on where this variant might be expected. /// /// Note that these "Availability Hints" are not normative. That is to say, a -/// [`HasRawDisplayHandle`] implementor is completely allowed to return something +/// [`HasDisplayHandle`] implementor is completely allowed to return something /// unexpected. (For example, it's legal for someone to return a /// [`RawDisplayHandle::Xlib`] on macOS, it would just be weird, and probably /// requires something like XQuartz be used).