Merge #668

668: Return failures to the user in swap_chain_get_next_texture, rather than transparently reconfiguring. r=kvark a=AlphaModder

TODO:
- [x] Change `Global::swap_chain_get_next_texture` in `wgpu-core`.
- [x] Update `wgpu_swap_chain_get_next_texture` in `wgpu-native`. (https://github.com/gfx-rs/wgpu-native/pull/32)
- [x] Wrap `SwapChainOutput`/`SwapChainStatus` in a nice enum in `wgpu-rs`. (https://github.com/gfx-rs/wgpu-rs/pull/323)
- [ ] Update `wgpu_bindings` (?)

Co-authored-by: AlphaModder <quasiflux@gmail.com>

player/src/main.rs

@@ -247,8 +247,11 @@ impl GlobalExt for wgc::hub::Global<IdentityPassThroughFactory> {
                 self.sampler_destroy::<B>(id);
             }
             A::GetSwapChainTexture { id, parent_id } => {
-                self.swap_chain_get_next_texture::<B>(parent_id, id)
-                    .unwrap();
+                if let Some(id) = id {
+                    self.swap_chain_get_next_texture::<B>(parent_id, id)
+                        .view_id
+                        .unwrap();
+                }
             }
             A::CreateBindGroupLayout { id, label, entries } => {
                 let label = Label::new(&label);

wgpu-core/src/device/trace.rs

@@ -155,7 +155,7 @@ pub enum Action {
         desc: wgt::SwapChainDescriptor,
     },
     GetSwapChainTexture {
-        id: id::TextureViewId,
+        id: Option<id::TextureViewId>,
         parent_id: id::SwapChainId,
     },
     PresentSwapChain(id::SwapChainId),

wgpu-core/src/swap_chain.rs

@@ -42,7 +42,7 @@ use crate::{
 };
 
 use hal::{self, device::Device as _, queue::CommandQueue as _, window::PresentationSurface as _};
-use wgt::SwapChainDescriptor;
+use wgt::{SwapChainDescriptor, SwapChainStatus};
 
 const FRAME_TIMEOUT_MS: u64 = 1000;
 pub const DESIRED_NUM_FRAMES: u32 = 3;
@@ -83,20 +83,16 @@ pub(crate) fn swap_chain_descriptor_to_hal(
 #[repr(C)]
 #[derive(Debug)]
 pub struct SwapChainOutput {
+    pub status: SwapChainStatus,
     pub view_id: Option<TextureViewId>,
 }
 
-#[derive(Debug)]
-pub enum SwapChainGetNextTextureError {
-    GpuProcessingTimeout,
-}
-
 impl<G: GlobalIdentityHandlerFactory> Global<G> {
     pub fn swap_chain_get_next_texture<B: GfxBackend>(
         &self,
         swap_chain_id: SwapChainId,
         view_id_in: Input<G, TextureViewId>,
-    ) -> Result<SwapChainOutput, SwapChainGetNextTextureError> {
+    ) -> SwapChainOutput {
         let hub = B::hub(self);
         let mut token = Token::root();
 
@@ -107,74 +103,75 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
         let sc = &mut swap_chain_guard[swap_chain_id];
         let device = &device_guard[sc.device_id.value];
 
-        let (image, _) = {
-            let suf = B::get_surface_mut(surface);
-            match unsafe { suf.acquire_image(FRAME_TIMEOUT_MS * 1_000_000) } {
-                Ok(surface_image) => surface_image,
-                Err(hal::window::AcquireError::Timeout) => {
-                    return Err(SwapChainGetNextTextureError::GpuProcessingTimeout);
-                }
-                Err(e) => {
-                    log::warn!("acquire_image() failed ({:?}), reconfiguring swapchain", e);
-                    let desc = swap_chain_descriptor_to_hal(
-                        &sc.desc,
-                        sc.num_frames,
-                        device.private_features,
-                    );
-                    unsafe {
-                        suf.configure_swapchain(&device.raw, desc).unwrap();
-                        suf.acquire_image(FRAME_TIMEOUT_MS * 1_000_000).unwrap()
-                    }
-                }
-            }
+        let suf = B::get_surface_mut(surface);
+        let (image, status) = match unsafe { suf.acquire_image(FRAME_TIMEOUT_MS * 1_000_000) } {
+            Ok((surface_image, None)) => (Some(surface_image), SwapChainStatus::Good),
+            Ok((surface_image, Some(_))) => (Some(surface_image), SwapChainStatus::Suboptimal),
+            Err(err) => (
+                None,
+                match err {
+                    hal::window::AcquireError::OutOfMemory(_) => SwapChainStatus::OutOfMemory,
+                    hal::window::AcquireError::NotReady => unreachable!(), // we always set a timeout
+                    hal::window::AcquireError::Timeout => SwapChainStatus::Timeout,
+                    hal::window::AcquireError::OutOfDate => SwapChainStatus::Outdated,
+                    hal::window::AcquireError::SurfaceLost(_) => SwapChainStatus::Lost,
+                    hal::window::AcquireError::DeviceLost(_) => SwapChainStatus::Lost,
+                },
+            ),
         };
 
-        let view = resource::TextureView {
-            inner: resource::TextureViewInner::SwapChain {
-                image,
-                source_id: Stored {
-                    value: swap_chain_id,
-                    ref_count: sc.life_guard.add_ref(),
+        let view_id = image.map(|image| {
+            let view = resource::TextureView {
+                inner: resource::TextureViewInner::SwapChain {
+                    image,
+                    source_id: Stored {
+                        value: swap_chain_id,
+                        ref_count: sc.life_guard.add_ref(),
+                    },
                 },
-            },
-            format: sc.desc.format,
-            extent: hal::image::Extent {
-                width: sc.desc.width,
-                height: sc.desc.height,
-                depth: 1,
-            },
-            samples: 1,
-            range: hal::image::SubresourceRange {
-                aspects: hal::format::Aspects::COLOR,
-                layers: 0..1,
-                levels: 0..1,
-            },
-            life_guard: LifeGuard::new(),
-        };
-        let ref_count = view.life_guard.add_ref();
-        let id = hub
-            .texture_views
-            .register_identity(view_id_in, view, &mut token);
+                format: sc.desc.format,
+                extent: hal::image::Extent {
+                    width: sc.desc.width,
+                    height: sc.desc.height,
+                    depth: 1,
+                },
+                samples: 1,
+                range: hal::image::SubresourceRange {
+                    aspects: hal::format::Aspects::COLOR,
+                    layers: 0..1,
+                    levels: 0..1,
+                },
+                life_guard: LifeGuard::new(),
+            };
+
+            let ref_count = view.life_guard.add_ref();
+            let id = hub
+                .texture_views
+                .register_identity(view_id_in, view, &mut token);
+
+            assert!(
+                sc.acquired_view_id.is_none(),
+                "Swap chain image is already acquired"
+            );
+
+            sc.acquired_view_id = Some(Stored {
+                value: id,
+                ref_count,
+            });
+
+            id
+        });
 
         #[cfg(feature = "trace")]
         match device.trace {
             Some(ref trace) => trace.lock().add(Action::GetSwapChainTexture {
-                id,
+                id: view_id,
                 parent_id: swap_chain_id,
             }),
             None => (),
         };
 
-        assert!(
-            sc.acquired_view_id.is_none(),
-            "Swap chain image is already acquired"
-        );
-        sc.acquired_view_id = Some(Stored {
-            value: id,
-            ref_count,
-        });
-
-        Ok(SwapChainOutput { view_id: Some(id) })
+        SwapChainOutput { status, view_id }
     }
 
     pub fn swap_chain_present<B: GfxBackend>(&self, swap_chain_id: SwapChainId) {

wgpu-types/src/lib.rs

@@ -641,6 +641,17 @@ pub struct SwapChainDescriptor {
     pub present_mode: PresentMode,
 }
 
+#[repr(C)]
+#[derive(Debug)]
+pub enum SwapChainStatus {
+    Good,
+    Suboptimal,
+    Timeout,
+    Outdated,
+    Lost,
+    OutOfMemory,
+}
+
 #[repr(C)]
 #[derive(Copy, Clone, Debug, Hash, Eq, PartialEq)]
 #[cfg_attr(feature = "trace", derive(Serialize))]