From 14caa8a50d777ffa8b9e1be02e47fa7cf887d81b Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Sun, 14 Dec 2025 23:17:40 +0100 Subject: [PATCH] Make `HasDisplayHandle` optional in `WindowHandle` While true for some window objects like `winit`'s `Window` object, not everyone is or should be required to return their display handle in the same place as the window handle, by means of implementing both `RawDisplayHandle` and `RawWindowHandle` on the same object. Additionally some APIs, specifically most/all GL(ES) context creation APIs require the `RawDisplayHandle` up-front to create a context that is compatible with that "compositor connection". For this reason callers should no longer be required to pass a single object that implements both `HasDisplayHandle` and `HasWindowHandle`, allowing the former to become `Option`al in favour of taking the one stored (and kept alive!) in the `Instance`. `HasDisplayHandle` can still be passed in this position however, because APIs like Vulkan have completely pushed out compositor integration via surfaces to the edge of the API and can technically let one instance or device render into multiple different `RawDisplayHandle` "compositor connections" concurrently. --- CHANGELOG.md | 4 ++ deno_webgpu/byow.rs | 3 +- .../standalone/02_hello_window/src/main.rs | 3 ++ player/src/bin/play.rs | 9 +--- wgpu-core/src/instance.rs | 32 ++++++++----- wgpu-hal/src/dx12/instance.rs | 4 ++ wgpu-hal/src/gles/egl.rs | 1 - wgpu-hal/src/gles/wgl.rs | 2 +- wgpu-hal/src/metal/mod.rs | 22 +++++---- wgpu-types/src/instance.rs | 3 -- wgpu/src/api/instance.rs | 13 +++++- wgpu/src/api/surface.rs | 46 +++++++++++++++---- 12 files changed, 97 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 880c8cf73a..ff70441062 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,10 @@ Bottom level categories: - Expanded documentation of `QuerySet`, `QueryType`, and `resolve_query_set()` describing how to use queries. By @kpreid in [#8776](https://github.com/gfx-rs/wgpu/pull/8776). +### Hacks (for lack of a better description) + +- `DisplayHandle` is now optional in surface creation if it was passed to `InstanceDescriptor::display`. By @MarijnS95 in [#8782](https://github.com/gfx-rs/wgpu/pull/8782) + ## v28.0.0 (2025-12-17) ### Major Changes diff --git a/deno_webgpu/byow.rs b/deno_webgpu/byow.rs index c109127013..3847e0dafc 100644 --- a/deno_webgpu/byow.rs +++ b/deno_webgpu/byow.rs @@ -137,7 +137,8 @@ impl UnsafeWindowSurface { // SAFETY: see above comment let id = unsafe { instance - .instance_create_surface(display_handle, win_handle, None) + // TODO: May need to be None in some cases + .instance_create_surface(Some(display_handle), win_handle, None) .map_err(ByowError::CreateSurface)? }; diff --git a/examples/standalone/02_hello_window/src/main.rs b/examples/standalone/02_hello_window/src/main.rs index f902b93720..4de3894cd7 100644 --- a/examples/standalone/02_hello_window/src/main.rs +++ b/examples/standalone/02_hello_window/src/main.rs @@ -32,6 +32,9 @@ impl State { let size = window.inner_size(); + // BREAKING CHANGE: The Into here will now only + // pass the window handle, and fails if the "optional" display + // handle wasn't passed above to with_display_handle() too let surface = instance.create_surface(window.clone()).unwrap(); let cap = surface.get_capabilities(&adapter); let surface_format = cap.formats[0]; diff --git a/player/src/bin/play.rs b/player/src/bin/play.rs index d6213077fb..9d1acd0884 100644 --- a/player/src/bin/play.rs +++ b/player/src/bin/play.rs @@ -79,13 +79,8 @@ fn main() { let instance = wgc::instance::Instance::new("player", instance_desc, None); #[cfg(feature = "winit")] - let surface = unsafe { - instance.create_surface( - window.display_handle().unwrap().into(), - window.window_handle().unwrap().into(), - ) - } - .unwrap(); + let surface = + unsafe { instance.create_surface(None, window.window_handle().unwrap().into()) }.unwrap(); let (backends, device_desc) = match actions.pop_if(|action| matches!(action, trace::Action::Init { .. })) { diff --git a/wgpu-core/src/instance.rs b/wgpu-core/src/instance.rs index 508543b18c..bf9c35bf8a 100644 --- a/wgpu-core/src/instance.rs +++ b/wgpu-core/src/instance.rs @@ -227,26 +227,33 @@ impl Instance { /// /// # Safety /// - /// - `display_handle` must be a valid object to create a surface upon. + /// - `display_handle` must be a valid object to create a surface upon, + /// falls back to the instance display handle otherwise. /// - `window_handle` must remain valid as long as the returned /// [`SurfaceId`] is being used. pub unsafe fn create_surface( &self, - display_handle: raw_window_handle::RawDisplayHandle, + display_handle: Option, window_handle: raw_window_handle::RawWindowHandle, ) -> Result { profiling::scope!("Instance::create_surface"); - if let Some(instance_display_handle) = &self.display { - if instance_display_handle - .display_handle() + let instance_display_handle = self.display.as_ref().map(|d| { + d.display_handle() .expect("Implementation did not provide a DisplayHandle") .as_raw() - != display_handle - { - return Err(CreateSurfaceError::MismatchingDisplayHandle); + }); + let display_handle = match (instance_display_handle, display_handle) { + (Some(a), Some(b)) => { + if a != b { + return Err(CreateSurfaceError::MismatchingDisplayHandle); + } + a } - } + (Some(hnd), None) => hnd, + (None, Some(hnd)) => hnd, + (None, None) => return Err(CreateSurfaceError::MissingDisplayHandle), + }; let mut errors = HashMap::default(); let mut surface_per_backend = HashMap::default(); @@ -932,6 +939,8 @@ pub enum CreateSurfaceError { FailedToCreateSurfaceForAnyBackend(HashMap), #[error("The display handle used to create this Instance does not match the one used to create a surface on it")] MismatchingDisplayHandle, + #[error("Neither the `Instance` nor `create_surface()` parameters received a `DisplayHandle`")] + MissingDisplayHandle, } impl Global { @@ -949,12 +958,13 @@ impl Global { /// /// # Safety /// - /// - `display_handle` must be a valid object to create a surface upon. + /// - `display_handle` must be a valid object to create a surface upon, + /// falls back to the instance display handle otherwise. /// - `window_handle` must remain valid as long as the returned /// [`SurfaceId`] is being used. pub unsafe fn instance_create_surface( &self, - display_handle: raw_window_handle::RawDisplayHandle, + display_handle: Option, window_handle: raw_window_handle::RawWindowHandle, id_in: Option, ) -> Result { diff --git a/wgpu-hal/src/dx12/instance.rs b/wgpu-hal/src/dx12/instance.rs index 215e48c4f6..de10ab8dc5 100644 --- a/wgpu-hal/src/dx12/instance.rs +++ b/wgpu-hal/src/dx12/instance.rs @@ -121,6 +121,10 @@ impl crate::Instance for super::Instance { _display_handle: raw_window_handle::RawDisplayHandle, window_handle: raw_window_handle::RawWindowHandle, ) -> Result { + assert!(matches!( + _display_handle, + raw_window_handle::RawDisplayHandle::Windows(_) + )); match window_handle { raw_window_handle::RawWindowHandle::Win32(handle) => { // https://github.com/rust-windowing/raw-window-handle/issues/171 diff --git a/wgpu-hal/src/gles/egl.rs b/wgpu-hal/src/gles/egl.rs index 606d1b2d4a..2fb5522ab0 100644 --- a/wgpu-hal/src/gles/egl.rs +++ b/wgpu-hal/src/gles/egl.rs @@ -888,7 +888,6 @@ impl crate::Instance for Instance { }) } - #[cfg_attr(target_os = "macos", allow(unused, unused_mut, unreachable_code))] unsafe fn create_surface( &self, display_handle: raw_window_handle::RawDisplayHandle, diff --git a/wgpu-hal/src/gles/wgl.rs b/wgpu-hal/src/gles/wgl.rs index f9a1af09be..e09db679e3 100644 --- a/wgpu-hal/src/gles/wgl.rs +++ b/wgpu-hal/src/gles/wgl.rs @@ -563,12 +563,12 @@ impl crate::Instance for Instance { }) } - #[cfg_attr(target_os = "macos", allow(unused, unused_mut, unreachable_code))] unsafe fn create_surface( &self, _display_handle: RawDisplayHandle, window_handle: RawWindowHandle, ) -> Result { + assert!(matches!(_display_handle, RawDisplayHandle::Windows(_))); let window = if let RawWindowHandle::Win32(handle) = window_handle { handle } else { diff --git a/wgpu-hal/src/metal/mod.rs b/wgpu-hal/src/metal/mod.rs index d613316148..fabe21fd76 100644 --- a/wgpu-hal/src/metal/mod.rs +++ b/wgpu-hal/src/metal/mod.rs @@ -122,20 +122,22 @@ impl crate::Instance for Instance { unsafe fn create_surface( &self, - _display_handle: raw_window_handle::RawDisplayHandle, + display_handle: raw_window_handle::RawDisplayHandle, window_handle: raw_window_handle::RawWindowHandle, ) -> Result { - match window_handle { + match (display_handle, window_handle) { #[cfg(any(target_os = "ios", target_os = "visionos"))] - raw_window_handle::RawWindowHandle::UiKit(handle) => { - Ok(unsafe { Surface::from_view(handle.ui_view.cast()) }) - } + ( + raw_window_handle::RawDisplayHandle::UiKit(_), + raw_window_handle::RawWindowHandle::UiKit(handle), + ) => Ok(unsafe { Surface::from_view(handle.ui_view.cast()) }), #[cfg(target_os = "macos")] - raw_window_handle::RawWindowHandle::AppKit(handle) => { - Ok(unsafe { Surface::from_view(handle.ns_view.cast()) }) - } - _ => Err(crate::InstanceError::new(format!( - "window handle {window_handle:?} is not a Metal-compatible handle" + ( + raw_window_handle::RawDisplayHandle::AppKit(_), + raw_window_handle::RawWindowHandle::AppKit(handle), + ) => Ok(unsafe { Surface::from_view(handle.ns_view.cast()) }), + x => Err(crate::InstanceError::new(format!( + "window handle {x:?} is not a Metal-compatible handle" ))), } } diff --git a/wgpu-types/src/instance.rs b/wgpu-types/src/instance.rs index ded70816ce..be11ba4455 100644 --- a/wgpu-types/src/instance.rs +++ b/wgpu-types/src/instance.rs @@ -58,9 +58,6 @@ pub struct InstanceDescriptor { /// the `EventLoop`) here. /// /// [`OwnedDisplayHandle`]: https://docs.rs/winit/latest/winit/event_loop/struct.OwnedDisplayHandle.html - // FUTURE: The RawDisplayHandle trait can/should be removed entirely from create_display()? At - // least `trait WindowHandle: HasWindowHandle + HasDisplayHandle` should really be removed as - // it's impractical and not implementable everywhere. pub display: Option>, } diff --git a/wgpu/src/api/instance.rs b/wgpu/src/api/instance.rs index 12d144c7c3..86a69fd224 100644 --- a/wgpu/src/api/instance.rs +++ b/wgpu/src/api/instance.rs @@ -176,10 +176,19 @@ impl Instance { /// Internally, this creates surfaces for all backends that are enabled for this instance. /// /// See [`SurfaceTarget`] for what targets are supported. - /// See [`Instance::create_surface_unsafe`] for surface creation with unsafe target variants. + /// See [`Instance::create_surface_unsafe()`] for surface creation with unsafe target variants. /// /// Most commonly used are window handles (or provider of windows handles) /// which can be passed directly as they're automatically converted to [`SurfaceTarget`]. + /// + /// # Warning + /// + /// This function does _not_ consume a [`raw_window_handle::HasDisplayHandle`] from your + /// `target` (no longer a trait requirement for [`WindowHandle`]) and instead expects + /// a display handle in [`InstanceDescriptor::display`]. + /// + /// If that cannot be provided, call [`Instance::create_surface_unsafe()`] with a + /// [`SurfaceTargetUnsafe::from_display_and_window()`]. pub fn create_surface<'window>( &self, target: impl Into>, @@ -191,6 +200,8 @@ impl Instance { let mut surface = match target { SurfaceTarget::Window(window) => unsafe { let surface = self.create_surface_unsafe( + // TODO: Could also call from_display_and_window depending on the argument; + // currently this is a hidden breaking change. SurfaceTargetUnsafe::from_window(&window).map_err(|e| CreateSurfaceError { inner: CreateSurfaceErrorKind::RawHandle(e), })?, diff --git a/wgpu/src/api/surface.rs b/wgpu/src/api/surface.rs index f063f2ba5f..0aa431ac4f 100644 --- a/wgpu/src/api/surface.rs +++ b/wgpu/src/api/surface.rs @@ -230,10 +230,10 @@ static_assertions::assert_impl_all!(Surface<'_>: Send, Sync); crate::cmp::impl_eq_ord_hash_proxy!(Surface<'_> => .inner); -/// Super trait for window handles as used in [`SurfaceTarget`]. -pub trait WindowHandle: HasWindowHandle + HasDisplayHandle + WasmNotSendSync {} +/// [`Send`]/[`Sync`] blanket trait for [`HasWindowHandle`] used in [`SurfaceTarget`]. +pub trait WindowHandle: HasWindowHandle + WasmNotSendSync {} -impl WindowHandle for T where T: HasWindowHandle + HasDisplayHandle + WasmNotSendSync {} +impl WindowHandle for T {} /// The window/canvas/surface/swap-chain/etc. a surface is attached to, for use with safe surface creation. /// @@ -259,6 +259,9 @@ pub enum SurfaceTarget<'window> { /// - On macOS/Metal: will panic if not called on the main thread. /// - On web: will panic if the `raw_window_handle` does not properly refer to a /// canvas element. + // TODO: Certain backends (Vulkan) have the window/display extensions and handles completely + // separate from the rendering instance, and should hence support creating surfaces + // for different display handles/conncetions. Window(Box), /// Surface from a `web_sys::HtmlCanvasElement`. @@ -309,6 +312,9 @@ pub enum SurfaceTargetUnsafe { /// If the specified display and window handle are not supported by any of the backends, then the surface /// will not be supported by any adapters. /// + /// If the `raw_display_handle` is not [`None`] here and was not [`None`] in + /// [`wgt::InstanceDescriptor::display`], their values _must_ be identical. + /// /// # Safety /// /// - `raw_window_handle` & `raw_display_handle` must be valid objects to create a surface upon. @@ -316,9 +322,9 @@ pub enum SurfaceTargetUnsafe { /// [`Surface`] is dropped. RawHandle { /// Raw display handle, underlying display must outlive the surface created from this. - raw_display_handle: raw_window_handle::RawDisplayHandle, + raw_display_handle: Option, - /// Raw display handle, underlying window must outlive the surface created from this. + /// Raw window handle, underlying window must outlive the surface created from this. raw_window_handle: raw_window_handle::RawWindowHandle, }, @@ -384,18 +390,38 @@ pub enum SurfaceTargetUnsafe { } impl SurfaceTargetUnsafe { + /// Creates a [`SurfaceTargetUnsafe::RawHandle`] from a display and window. + /// + /// The `display` is optional and may be omitted if it was also passed to + /// [`wgt::InstanceDescriptor::display`]. If passed to both it must (currently) be identical. + /// + /// # Safety + /// + /// - `display` must outlive the resulting surface target + /// (and subsequently the surface created for this target). + /// - `window` must outlive the resulting surface target + /// (and subsequently the surface created for this target). + pub unsafe fn from_display_and_window( + display: &impl HasDisplayHandle, + window: &impl HasWindowHandle, + ) -> Result { + Ok(Self::RawHandle { + raw_display_handle: Some(display.display_handle()?.as_raw()), + raw_window_handle: window.window_handle()?.as_raw(), + }) + } + /// Creates a [`SurfaceTargetUnsafe::RawHandle`] from a window. /// /// # Safety /// /// - `window` must outlive the resulting surface target /// (and subsequently the surface created for this target). - pub unsafe fn from_window(window: &T) -> Result - where - T: HasDisplayHandle + HasWindowHandle, - { + pub unsafe fn from_window( + window: &impl HasWindowHandle, + ) -> Result { Ok(Self::RawHandle { - raw_display_handle: window.display_handle()?.as_raw(), + raw_display_handle: None, raw_window_handle: window.window_handle()?.as_raw(), }) }