Skip to content

Commit

Permalink
Excise the HasRaw*Handle traits
Browse files Browse the repository at this point in the history
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 <dev@notgull.net>
  • Loading branch information
notgull committed Aug 8, 2023
1 parent 880b305 commit fcdbcc8
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 120 deletions.
8 changes: 0 additions & 8 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
91 changes: 71 additions & 20 deletions src/borrowed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand All @@ -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.
///
Expand All @@ -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
Expand All @@ -57,20 +63,23 @@ impl<H: HasDisplayHandle + ?Sized> HasDisplayHandle for &mut H {
}

#[cfg(feature = "alloc")]
#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
impl<H: HasDisplayHandle + ?Sized> HasDisplayHandle for alloc::boxed::Box<H> {
fn display_handle(&self) -> Result<DisplayHandle<'_>, HandleError> {
(**self).display_handle()
}
}

#[cfg(feature = "alloc")]
#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
impl<H: HasDisplayHandle + ?Sized> HasDisplayHandle for alloc::rc::Rc<H> {
fn display_handle(&self) -> Result<DisplayHandle<'_>, HandleError> {
(**self).display_handle()
}
}

#[cfg(feature = "alloc")]
#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
impl<H: HasDisplayHandle + ?Sized> HasDisplayHandle for alloc::sync::Arc<H> {
fn display_handle(&self) -> Result<DisplayHandle<'_>, HandleError> {
(**self).display_handle()
Expand All @@ -81,8 +90,6 @@ impl<H: HasDisplayHandle + ?Sized> HasDisplayHandle for alloc::sync::Arc<H> {
///
/// 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> {
Expand All @@ -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<RawDisplayHandle> for DisplayHandle<'_> {
fn as_ref(&self) -> &RawDisplayHandle {
&self.raw
}
}

impl Borrow<RawDisplayHandle> for DisplayHandle<'_> {
fn borrow(&self) -> &RawDisplayHandle {
&self.raw
}
}

unsafe impl HasRawDisplayHandle for DisplayHandle<'_> {
fn raw_display_handle(&self) -> Result<RawDisplayHandle, HandleError> {
Ok(self.raw)
impl From<DisplayHandle<'_>> for RawDisplayHandle {
fn from(handle: DisplayHandle<'_>) -> Self {
handle.raw
}
}

Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -180,20 +212,23 @@ impl<H: HasWindowHandle + ?Sized> HasWindowHandle for &mut H {
}

#[cfg(feature = "alloc")]
#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
impl<H: HasWindowHandle + ?Sized> HasWindowHandle for alloc::boxed::Box<H> {
fn window_handle(&self) -> Result<WindowHandle<'_>, HandleError> {
(**self).window_handle()
}
}

#[cfg(feature = "alloc")]
#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
impl<H: HasWindowHandle + ?Sized> HasWindowHandle for alloc::rc::Rc<H> {
fn window_handle(&self) -> Result<WindowHandle<'_>, HandleError> {
(**self).window_handle()
}
}

#[cfg(feature = "alloc")]
#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
impl<H: HasWindowHandle + ?Sized> HasWindowHandle for alloc::sync::Arc<H> {
fn window_handle(&self) -> Result<WindowHandle<'_>, HandleError> {
(**self).window_handle()
Expand All @@ -207,8 +242,7 @@ impl<H: HasWindowHandle + ?Sized> HasWindowHandle for alloc::sync::Arc<H> {
/// 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, Copy)]
pub struct WindowHandle<'a> {
raw: RawWindowHandle,
Expand All @@ -233,11 +267,28 @@ impl<'a> WindowHandle<'a> {
_marker: PhantomData,
}
}

/// Get the underlying raw window handle.
pub fn as_raw(&self) -> RawWindowHandle {
self.raw
}
}

impl AsRef<RawWindowHandle> for WindowHandle<'_> {
fn as_ref(&self) -> &RawWindowHandle {
&self.raw
}
}

impl Borrow<RawWindowHandle> for WindowHandle<'_> {
fn borrow(&self) -> &RawWindowHandle {
&self.raw
}
}

unsafe impl HasRawWindowHandle for WindowHandle<'_> {
fn raw_window_handle(&self) -> Result<RawWindowHandle, HandleError> {
Ok(self.raw)
impl From<WindowHandle<'_>> for RawWindowHandle {
fn from(handle: WindowHandle<'_>) -> Self {
handle.raw
}
}

Expand Down
95 changes: 3 additions & 92 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
//!
Expand Down Expand Up @@ -56,49 +56,6 @@ pub use windows::{Win32WindowHandle, WinRtWindowHandle, WindowsDisplayHandle};

use core::fmt;

/// Window that wraps around a raw window handle.
///
/// # Safety
///
/// 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.
pub unsafe trait HasRawWindowHandle {
fn raw_window_handle(&self) -> Result<RawWindowHandle, HandleError>;
}

unsafe impl<'a, T: HasRawWindowHandle + ?Sized> HasRawWindowHandle for &'a T {
fn raw_window_handle(&self) -> Result<RawWindowHandle, HandleError> {
(*self).raw_window_handle()
}
}
unsafe impl<'a, T: HasRawWindowHandle + ?Sized> HasRawWindowHandle for &'a mut T {
fn raw_window_handle(&self) -> Result<RawWindowHandle, HandleError> {
(**self).raw_window_handle()
}
}
#[cfg(feature = "alloc")]
#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
unsafe impl<T: HasRawWindowHandle + ?Sized> HasRawWindowHandle for alloc::rc::Rc<T> {
fn raw_window_handle(&self) -> Result<RawWindowHandle, HandleError> {
(**self).raw_window_handle()
}
}
#[cfg(feature = "alloc")]
#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
unsafe impl<T: HasRawWindowHandle + ?Sized> HasRawWindowHandle for alloc::sync::Arc<T> {
fn raw_window_handle(&self) -> Result<RawWindowHandle, HandleError> {
(**self).raw_window_handle()
}
}

/// A window handle for a particular windowing system.
///
/// Each variant contains a struct with fields specific to that windowing system
Expand All @@ -115,7 +72,7 @@ unsafe impl<T: HasRawWindowHandle + ?Sized> 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).
Expand Down Expand Up @@ -201,52 +158,6 @@ pub enum RawWindowHandle {
Haiku(HaikuWindowHandle),
}

/// Display that wraps around a raw display handle.
///
/// # Safety
///
/// 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.
pub unsafe trait HasRawDisplayHandle {
fn raw_display_handle(&self) -> Result<RawDisplayHandle, HandleError>;
}

unsafe impl<'a, T: HasRawDisplayHandle + ?Sized> HasRawDisplayHandle for &'a T {
fn raw_display_handle(&self) -> Result<RawDisplayHandle, HandleError> {
(*self).raw_display_handle()
}
}

unsafe impl<'a, T: HasRawDisplayHandle + ?Sized> HasRawDisplayHandle for &'a mut T {
fn raw_display_handle(&self) -> Result<RawDisplayHandle, HandleError> {
(**self).raw_display_handle()
}
}

#[cfg(feature = "alloc")]
#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
unsafe impl<T: HasRawDisplayHandle + ?Sized> HasRawDisplayHandle for alloc::rc::Rc<T> {
fn raw_display_handle(&self) -> Result<RawDisplayHandle, HandleError> {
(**self).raw_display_handle()
}
}

#[cfg(feature = "alloc")]
#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
unsafe impl<T: HasRawDisplayHandle + ?Sized> HasRawDisplayHandle for alloc::sync::Arc<T> {
fn raw_display_handle(&self) -> Result<RawDisplayHandle, HandleError> {
(**self).raw_display_handle()
}
}

/// A display server handle for a particular windowing system.
///
/// The display usually represents a connection to some display server, but it is not necessarily
Expand All @@ -269,7 +180,7 @@ unsafe impl<T: HasRawDisplayHandle + ?Sized> 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).
Expand Down

0 comments on commit fcdbcc8

Please sign in to comment.