Safe surface creation

gfx-rs · Oct 28, 2023 · 09fc22c · 09fc22c
1 parent ad3094c
commit 09fc22c
Show file tree

Hide file tree

Showing 9 changed files with 92 additions and 71 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -40,6 +40,10 @@ Bottom level categories:
 
 ## Unreleased
 
+### Safe `Surface` creation
+
+It is now possible to safely create a `wgpu::Surface` with `Surface::create_surface()` and carries a lifetime to the passed `window`. `Surface::create_surface_from_raw()` can be used to produce a `Surface<'static>`, which remains `unsafe`.
+
 For naga changelogs at or before v0.14.0. See [naga's changelog](naga/CHANGELOG.md).
 
 ## v0.18.0 (2023-10-25)

diff --git a/examples/common/src/framework.rs b/examples/common/src/framework.rs
@@ -1,5 +1,6 @@
 #[cfg(target_arch = "wasm32")]
 use std::str::FromStr;
+use std::sync::OnceLock;
 #[cfg(not(target_arch = "wasm32"))]
 use std::time::Instant;
 #[cfg(target_arch = "wasm32")]
@@ -70,11 +71,10 @@ pub trait Example: 'static + Sized {
 }
 
 struct Setup {
-    _window: Window,
     event_loop: EventLoop<()>,
     instance: wgpu::Instance,
     size: winit::dpi::PhysicalSize<u32>,
-    surface: wgpu::Surface,
+    surface: wgpu::Surface<'static>,
     adapter: wgpu::Adapter,
     device: wgpu::Device,
     queue: wgpu::Queue,
@@ -102,7 +102,8 @@ async fn setup<E: Example>(title: &str) -> Setup {
         use winit::platform::windows::WindowBuilderExtWindows;
         builder = builder.with_no_redirection_bitmap(true);
     }
-    let window = builder.build(&event_loop).unwrap();
+    static WINDOW: OnceLock<Window> = OnceLock::new();
+    let window = WINDOW.get_or_init(|| builder.build(&event_loop).unwrap());
 
     #[cfg(target_arch = "wasm32")]
     {
@@ -171,26 +172,22 @@ async fn setup<E: Example>(title: &str) -> Setup {
         dx12_shader_compiler,
         gles_minor_version,
     });
-    let (size, surface) = unsafe {
-        let size = window.inner_size();
-
-        #[cfg(any(not(target_arch = "wasm32"), target_os = "emscripten"))]
-        let surface = instance.create_surface(&window).unwrap();
-        #[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
-        let surface = {
-            if let Some(offscreen_canvas_setup) = &offscreen_canvas_setup {
-                log::info!("Creating surface from OffscreenCanvas");
-                instance.create_surface_from_offscreen_canvas(
-                    offscreen_canvas_setup.offscreen_canvas.clone(),
-                )
-            } else {
-                instance.create_surface(&window)
-            }
+    let size = window.inner_size();
+
+    #[cfg(any(not(target_arch = "wasm32"), target_os = "emscripten"))]
+    let surface = instance.create_surface(window).unwrap();
+    #[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
+    let surface = {
+        if let Some(offscreen_canvas_setup) = &offscreen_canvas_setup {
+            log::info!("Creating surface from OffscreenCanvas");
+            instance.create_surface_from_offscreen_canvas(
+                offscreen_canvas_setup.offscreen_canvas.clone(),
+            )
+        } else {
+            instance.create_surface(window)
         }
-        .unwrap();
-
-        (size, surface)
-    };
+    }
+    .unwrap();
     let adapter = wgpu::util::initialize_adapter_from_env_or_default(&instance, Some(&surface))
         .await
         .expect("No suitable GPU adapters found on the system!");
@@ -242,7 +239,6 @@ async fn setup<E: Example>(title: &str) -> Setup {
         .expect("Unable to find a suitable GPU adapter!");
 
     Setup {
-        _window: window,
         event_loop,
         instance,
         size,
@@ -264,7 +260,6 @@ fn start<E: Example>(
         adapter,
         device,
         queue,
-        ..
     }: Setup,
     #[cfg(target_arch = "wasm32")] Setup {
         event_loop,
@@ -275,7 +270,6 @@ fn start<E: Example>(
         device,
         queue,
         offscreen_canvas_setup,
-        ..
     }: Setup,
 ) {
     let mut config = surface

diff --git a/examples/hello-triangle/src/main.rs b/examples/hello-triangle/src/main.rs
@@ -10,7 +10,7 @@ async fn run(event_loop: EventLoop<()>, window: Window) {
 
     let instance = wgpu::Instance::default();
 
-    let surface = unsafe { instance.create_surface(&window) }.unwrap();
+    let surface = instance.create_surface(&window).unwrap();
     let adapter = instance
         .request_adapter(&wgpu::RequestAdapterOptions {
             power_preference: wgpu::PowerPreference::default(),
@@ -82,6 +82,7 @@ async fn run(event_loop: EventLoop<()>, window: Window) {
 
     surface.configure(&device, &config);
 
+    let window = &window;
     event_loop
         .run(move |event, target| {
             // Have the closure take ownership of the resources.

diff --git a/examples/hello-windows/src/main.rs b/examples/hello-windows/src/main.rs
@@ -7,28 +7,28 @@ use winit::{
     window::{Window, WindowId},
 };
 
-struct ViewportDesc {
-    window: Window,
+struct ViewportDesc<'window> {
+    window: &'window Window,
     background: wgpu::Color,
-    surface: wgpu::Surface,
+    surface: wgpu::Surface<'window>,
 }
 
-struct Viewport {
-    desc: ViewportDesc,
+struct Viewport<'window> {
+    desc: ViewportDesc<'window>,
     config: wgpu::SurfaceConfiguration,
 }
 
-impl ViewportDesc {
-    fn new(window: Window, background: wgpu::Color, instance: &wgpu::Instance) -> Self {
-        let surface = unsafe { instance.create_surface(&window) }.unwrap();
+impl<'window> ViewportDesc<'window> {
+    fn new(window: &'window Window, background: wgpu::Color, instance: &wgpu::Instance) -> Self {
+        let surface = instance.create_surface(window).unwrap();
         Self {
             window,
             background,
             surface,
         }
     }
 
-    fn build(self, adapter: &wgpu::Adapter, device: &wgpu::Device) -> Viewport {
+    fn build(self, adapter: &wgpu::Adapter, device: &wgpu::Device) -> Viewport<'window> {
         let size = self.window.inner_size();
 
         let caps = self.surface.get_capabilities(adapter);
@@ -48,7 +48,7 @@ impl ViewportDesc {
     }
 }
 
-impl Viewport {
+impl Viewport<'_> {
     fn resize(&mut self, device: &wgpu::Device, size: winit::dpi::PhysicalSize<u32>) {
         self.config.width = size.width;
         self.config.height = size.height;
@@ -62,11 +62,11 @@ impl Viewport {
     }
 }
 
-async fn run(event_loop: EventLoop<()>, viewports: Vec<(Window, wgpu::Color)>) {
+async fn run(event_loop: EventLoop<()>, viewports: &[(Window, wgpu::Color)]) {
     let instance = wgpu::Instance::default();
     let viewports: Vec<_> = viewports
-        .into_iter()
-        .map(|(window, color)| ViewportDesc::new(window, color, &instance))
+        .iter()
+        .map(|(window, color)| ViewportDesc::new(window, *color, &instance))
         .collect();
     let adapter = instance
         .request_adapter(&wgpu::RequestAdapterOptions {
@@ -200,7 +200,7 @@ fn main() {
         }
 
         env_logger::init();
-        pollster::block_on(run(event_loop, viewports));
+        pollster::block_on(run(event_loop, &viewports));
     }
     #[cfg(target_arch = "wasm32")]
     {

diff --git a/examples/uniform-values/src/main.rs b/examples/uniform-values/src/main.rs
@@ -83,9 +83,9 @@ impl Default for AppState {
     }
 }
 
-struct WgpuContext {
-    pub window: Window,
-    pub surface: wgpu::Surface,
+struct WgpuContext<'window> {
+    pub window: &'window Window,
+    pub surface: wgpu::Surface<'window>,
     pub surface_config: wgpu::SurfaceConfiguration,
     pub device: wgpu::Device,
     pub queue: wgpu::Queue,
@@ -94,12 +94,12 @@ struct WgpuContext {
     pub uniform_buffer: wgpu::Buffer,
 }
 
-impl WgpuContext {
-    async fn new(window: Window) -> WgpuContext {
+impl WgpuContext<'_> {
+    async fn new(window: &Window) -> WgpuContext {
         let size = window.inner_size();
 
         let instance = wgpu::Instance::default();
-        let surface = unsafe { instance.create_surface(&window) }.unwrap();
+        let surface = instance.create_surface(window).unwrap();
         let adapter = instance
             .request_adapter(&wgpu::RequestAdapterOptions {
                 power_preference: wgpu::PowerPreference::HighPerformance,
@@ -224,7 +224,7 @@ impl WgpuContext {
 }
 
 async fn run(event_loop: EventLoop<()>, window: Window) {
-    let mut wgpu_context = Some(WgpuContext::new(window).await);
+    let mut wgpu_context = Some(WgpuContext::new(&window).await);
     // (6)
     let mut state = Some(AppState::default());
     let main_window_id = wgpu_context.as_ref().unwrap().window.id();

diff --git a/wgpu/src/backend/web.rs b/wgpu/src/backend/web.rs
@@ -1109,7 +1109,7 @@ impl crate::context::Context for Context {
 
     fn instance_request_adapter(
         &self,
-        options: &crate::RequestAdapterOptions<'_>,
+        options: &crate::RequestAdapterOptions<'_, '_>,
     ) -> Self::RequestAdapterFuture {
         //TODO: support this check, return `None` if the flag is not set.
         // It's not trivial, since we need the Future logic to have this check,

diff --git a/wgpu/src/context.rs b/wgpu/src/context.rs
@@ -103,7 +103,7 @@ pub trait Context: Debug + WasmNotSend + WasmNotSync + Sized {
     ) -> Result<(Self::SurfaceId, Self::SurfaceData), crate::CreateSurfaceError>;
     fn instance_request_adapter(
         &self,
-        options: &RequestAdapterOptions<'_>,
+        options: &RequestAdapterOptions<'_, '_>,
     ) -> Self::RequestAdapterFuture;
     fn adapter_request_device(
         &self,
@@ -1212,7 +1212,7 @@ pub(crate) trait DynContext: Debug + WasmNotSend + WasmNotSync {
     #[allow(clippy::type_complexity)]
     fn instance_request_adapter(
         &self,
-        options: &RequestAdapterOptions<'_>,
+        options: &RequestAdapterOptions<'_, '_>,
     ) -> Pin<InstanceRequestAdapterFuture>;
     fn adapter_request_device(
         &self,
@@ -2070,7 +2070,7 @@ where
 
     fn instance_request_adapter(
         &self,
-        options: &RequestAdapterOptions<'_>,
+        options: &RequestAdapterOptions<'_, '_>,
     ) -> Pin<InstanceRequestAdapterFuture> {
         let future: T::RequestAdapterFuture = Context::instance_request_adapter(self, options);
         Box::pin(async move {