Skip to content

Commit

Permalink
breaking: Deprecate 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.

Thus far I've deprecated them instead of removing them, in order to be more kind to downstream crates.

Closes #135.

Signed-off-by: John Nunley <[email protected]>
  • Loading branch information
@notgull
notgull authored Sep 10, 2023
1 parent 809b130 commit e3f68a7
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 95 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ jobs:
- uses: hecrj/setup-rust-action@v1
with:
rust-version: ${{ matrix.rust_version }}

- run: rustup target add wasm32-unknown-unknown

- name: Check documentation
Expand All @@ -52,3 +51,4 @@ jobs:

- name: Run tests for wasm32-unknown-unknown
run: cargo hack check --target wasm32-unknown-unknown --feature-powerset

126 changes: 80 additions & 46 deletions src/borrowed.rs
View file
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,25 +15,17 @@ 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.
///
/// 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
/// [`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.
///
/// 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 +48,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 +75,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 @@ -101,18 +93,43 @@ impl<'a> DisplayHandle<'a> {
///
/// # Safety
///
/// The `RawDisplayHandle` must be valid for the 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, implementors 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.
///
/// It is not possible to invalidate a [`DisplayHandle`] on any platform without additional unsafe code.
pub unsafe fn borrow_raw(raw: RawDisplayHandle) -> Self {
Self {
raw,
_marker: PhantomData,
}
}

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

unsafe impl HasRawDisplayHandle for DisplayHandle<'_> {
fn raw_display_handle(&self) -> Result<RawDisplayHandle, HandleError> {
Ok(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
}
}

impl From<DisplayHandle<'_>> for RawDisplayHandle {
fn from(handle: DisplayHandle<'_>) -> Self {
handle.raw
}
}

Expand All @@ -129,35 +146,14 @@ 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
/// [`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
/// 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 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, 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
/// conditions that can arise from this.
///
/// 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
Expand All @@ -180,20 +176,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 +206,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, Copy, Clone)]
pub struct WindowHandle<'a> {
raw: RawWindowHandle,
Expand All @@ -226,18 +224,54 @@ impl<'a> WindowHandle<'a> {
///
/// # Safety
///
/// The [`RawWindowHandle`] must be valid for the lifetime provided.
/// 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.
///
/// 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
/// 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, 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
/// conditions that can arise from this.
pub unsafe fn borrow_raw(raw: RawWindowHandle) -> Self {
Self {
raw,
_marker: PhantomData,
}
}

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

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
59 changes: 11 additions & 48 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,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 @@ -74,32 +74,15 @@ 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<RawWindowHandle, HandleError>;
}

unsafe impl<'a, T: HasRawWindowHandle + ?Sized> HasRawWindowHandle for &'a T {
#[allow(deprecated)]
unsafe impl<T: HasWindowHandle + ?Sized> HasRawWindowHandle for 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()
self.window_handle().map(Into::into)
}
}

Expand All @@ -119,7 +102,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 @@ -233,35 +216,15 @@ 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<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> {
#[allow(deprecated)]
unsafe impl<T: HasDisplayHandle + ?Sized> HasRawDisplayHandle for T {
fn raw_display_handle(&self) -> Result<RawDisplayHandle, HandleError> {
(**self).raw_display_handle()
self.display_handle().map(Into::into)
}
}

Expand All @@ -287,7 +250,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 e3f68a7

Please sign in to comment.