Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Excise the HasRaw*Handle traits #139

Merged
merged 5 commits into from
Sep 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
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
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
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
Loading