Acquire texture: Option<std::time::Duration> timeouts (#2724)

* surface.acquire_texture: pass Option<Duration> for timeout

A std::time::Duration allows for timeouts to be specified more clearly in
Rust using whatever units are convenient for the caller, and an Option also
makes it clearer in case no timeout is wanted, as opposed to passing a
bitwise !0 as special timeout value.

Notably there was an impedance mismatch with the Vulkan backend that
takes a 64bit timeout in nanoseconds and uses u64::MAX to indicate no
timeout and the backend was not mapping a given u32::MAX into a u64::MAX

* surface.acquire_texture: ignore timeout for Android < 11

Prior to Android 11 then Android's vkAcquireNextImageKHR implementation was
non-conformant and didn't support timeouts and additionally would log a
verbose warning if a timeout was requested.

For reference this version of AcquireNextImageKHR doesn't support timeouts:
https://android.googlesource.com/platform/frameworks/native/+/refs/tags/android-mainline-10.0.0_r13/vulkan/libvulkan/swapchain.cpp#1426
and this version does:
https://android.googlesource.com/platform/frameworks/native/+/refs/tags/android-mainline-11.0.0_r45/vulkan/libvulkan/swapchain.cpp#1438
(i.e. timeout support was added in Android 11)

This patch adds a dependency on the `android-properties` crate that provides
a simple wrapper for the `__system_property_set` syscall so that the
platform version can be read via the `ro.build.version.sdk` property
and then for versions < 30 (corresponds to Android 11) any timeout
given to `acquire_texture` will be ignored (and `u64::MAX` will be
passed to Vulkan)

wgpu-core/src/present.rs

@@ -123,7 +123,12 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
  let _ = device;
 
  let suf = A::get_surface_mut(surface);
- let (texture_id, status) = match unsafe { suf.raw.acquire_texture(FRAME_TIMEOUT_MS) } {
+ let (texture_id, status) = match unsafe {
+ suf.raw
+ .acquire_texture(Some(std::time::Duration::from_millis(
+ FRAME_TIMEOUT_MS as u64,
+ )))
+ } {
  Ok(Some(ast)) => {
  let clear_view_desc = hal::TextureViewDescriptor {
  label: Some("(wgpu internal) clear surface texture view"),

wgpu-hal/Cargo.toml

@@ -90,6 +90,9 @@ wasm-bindgen = { version = "0.2" }
 web-sys = { version = "0.3", features = ["Window", "HtmlCanvasElement", "WebGl2RenderingContext"] }
 js-sys = { version = "0.3" }
 
+[target.'cfg(target_os = "android")'.dependencies]
+android-properties = "0.2"
+
 [dependencies.naga]
 git = "https://github.com/gfx-rs/naga"
 rev = "571302e"

wgpu-hal/examples/halmark/main.rs

@@ -614,7 +614,7 @@ impl<A: hal::Api> Example<A> {
 
  let ctx = &mut self.contexts[self.context_index];
 
- let surface_tex = unsafe { self.surface.acquire_texture(!0).unwrap().unwrap().texture };
+ let surface_tex = unsafe { self.surface.acquire_texture(None).unwrap().unwrap().texture };
 
  let target_barrier0 = hal::TextureBarrier {
  texture: surface_tex.borrow(),

wgpu-hal/src/dx11/mod.rs

@@ -124,7 +124,7 @@ impl crate::Surface<Api> for Surface {
 
  unsafe fn acquire_texture(
  &mut self,
- timeout_ms: u32,
+ _timeout: Option<std::time::Duration>,
  ) -> Result<Option<crate::AcquiredSurfaceTexture<Api>>, crate::SurfaceError> {
  todo!()
  }

wgpu-hal/src/dx12/mod.rs

@@ -527,7 +527,14 @@ impl SwapChain {
  self.raw
  }
 
- unsafe fn wait(&mut self, timeout_ms: u32) -> Result<bool, crate::SurfaceError> {
+ unsafe fn wait(
+ &mut self,
+ timeout: Option<std::time::Duration>,
+ ) -> Result<bool, crate::SurfaceError> {
+ let timeout_ms = match timeout {
+ Some(duration) => duration.as_millis() as u32,
+ None => winbase::INFINITE,
+ };
  match synchapi::WaitForSingleObject(self.waitable, timeout_ms) {
  winbase::WAIT_ABANDONED | winbase::WAIT_FAILED => Err(crate::SurfaceError::Lost),
  winbase::WAIT_OBJECT_0 => Ok(true),
@@ -690,7 +697,7 @@ impl crate::Surface<Api> for Surface {
 
  unsafe fn unconfigure(&mut self, device: &Device) {
  if let Some(mut sc) = self.swap_chain.take() {
- let _ = sc.wait(winbase::INFINITE);
+ let _ = sc.wait(None);
  //TODO: this shouldn't be needed,
  // but it complains that the queue is still used otherwise
  let _ = device.wait_idle();
@@ -701,11 +708,11 @@ impl crate::Surface<Api> for Surface {
 
  unsafe fn acquire_texture(
  &mut self,
- timeout_ms: u32,
+ timeout: Option<std::time::Duration>,
  ) -> Result<Option<crate::AcquiredSurfaceTexture<Api>>, crate::SurfaceError> {
  let sc = self.swap_chain.as_mut().unwrap();
 
- sc.wait(timeout_ms)?;
+ sc.wait(timeout)?;
 
  let base_index = sc.raw.GetCurrentBackBufferIndex() as usize;
  let index = (base_index + sc.acquired_count) % sc.resources.len();

wgpu-hal/src/empty.rs

@@ -66,7 +66,7 @@ impl crate::Surface<Api> for Context {
 
  unsafe fn acquire_texture(
  &mut self,
- timeout_ms: u32,
+ timeout: Option<std::time::Duration>,
  ) -> Result<Option<crate::AcquiredSurfaceTexture<Api>>, crate::SurfaceError> {
  Ok(None)
  }

wgpu-hal/src/gles/egl.rs

@@ -1194,7 +1194,7 @@ impl crate::Surface<super::Api> for Surface {
 
  unsafe fn acquire_texture(
  &mut self,
- _timeout_ms: u32, //TODO
+ _timeout_ms: Option<Duration>, //TODO
  ) -> Result<Option<crate::AcquiredSurfaceTexture<super::Api>>, crate::SurfaceError> {
  let sc = self.swapchain.as_ref().unwrap();
  let texture = super::Texture {

wgpu-hal/src/gles/web.rs

@@ -253,7 +253,7 @@ impl crate::Surface<super::Api> for Surface {
 
  unsafe fn acquire_texture(
  &mut self,
- _timeout_ms: u32,
+ _timeout_ms: Option<std::time::Duration>, //TODO
  ) -> Result<Option<crate::AcquiredSurfaceTexture<super::Api>>, crate::SurfaceError> {
  let sc = self.swapchain.as_ref().unwrap();
  let texture = super::Texture {

wgpu-hal/src/lib.rs

@@ -193,10 +193,19 @@ pub trait Surface<A: Api>: Send + Sync {
 
  unsafe fn unconfigure(&mut self, device: &A::Device);
 
+ /// Returns the next texture to be presented by the swapchain for drawing
+ ///
+ /// A `timeout` of `None` means to wait indefinitely, with no timeout.
+ ///
+ /// # Portability
+ ///
+ /// Some backends can't support a timeout when acquiring a texture and
+ /// the timeout will be ignored.
+ ///
  /// Returns `None` on timing out.
  unsafe fn acquire_texture(
  &mut self,
- timeout_ms: u32,
+ timeout: Option<std::time::Duration>,
  ) -> Result<Option<AcquiredSurfaceTexture<A>>, SurfaceError>;
  unsafe fn discard_texture(&mut self, texture: A::SurfaceTexture);
 }

wgpu-hal/src/metal/surface.rs

@@ -216,7 +216,7 @@ impl crate::Surface<super::Api> for super::Surface {
 
  unsafe fn acquire_texture(
  &mut self,
- _timeout_ms: u32, //TODO
+ _timeout_ms: Option<std::time::Duration>, //TODO
  ) -> Result<Option<crate::AcquiredSurfaceTexture<super::Api>>, crate::SurfaceError> {
  let render_layer = self.render_layer.lock();
  let (drawable, texture) = match autoreleasepool(|| {

wgpu-hal/src/vulkan/instance.rs

@@ -214,10 +214,13 @@ impl super::Instance {
  /// - `raw_instance` must be created respecting `driver_api_version`, `extensions` and `flags`
  /// - `extensions` must be a superset of `required_extensions()` and must be created from the
  /// same entry, driver_api_version and flags.
+ /// - `android_sdk_version` is ignored and can be `0` for all platforms besides Android
+ #[allow(clippy::too_many_arguments)]
  pub unsafe fn from_raw(
  entry: ash::Entry,
  raw_instance: ash::Instance,
  driver_api_version: u32,
+ android_sdk_version: u32,
  extensions: Vec<&'static CStr>,
  flags: crate::InstanceFlags,
  has_nv_optimus: bool,
@@ -283,6 +286,7 @@ impl super::Instance {
  entry,
  has_nv_optimus,
  driver_api_version,
+ android_sdk_version,
  }),
  extensions,
  })
@@ -557,6 +561,28 @@ impl crate::Instance<super::Api> for super::Instance {
  layers
  };
 
+ #[cfg(target_os = "android")]
+ let android_sdk_version = {
+ // See: https://developer.android.com/reference/android/os/Build.VERSION_CODES
+ let mut prop = android_properties::getprop("ro.build.version.sdk");
+ if let Some(val) = prop.value() {
+ match val.parse::<u32>() {
+ Ok(sdk_ver) => sdk_ver,
+ Err(err) => {
+ log::error!(
+ "Couldn't parse Android's ro.build.version.sdk system property ({val}): {err}"
+ );
+ 0
+ }
+ }
+ } else {
+ log::error!("Couldn't read Android's ro.build.version.sdk system property");
+ 0
+ }
+ };
+ #[cfg(not(target_os = "android"))]
+ let android_sdk_version = 0;
+
  let vk_instance = {
  let str_pointers = layers
  .iter()
@@ -583,6 +609,7 @@ impl crate::Instance<super::Api> for super::Instance {
  entry,
  vk_instance,
  driver_api_version,
+ android_sdk_version,
  extensions,
  desc.flags,
  has_nv_optimus,
@@ -707,10 +734,27 @@ impl crate::Surface<super::Api> for super::Surface {
 
  unsafe fn acquire_texture(
  &mut self,
- timeout_ms: u32,
+ timeout: Option<std::time::Duration>,
  ) -> Result<Option<crate::AcquiredSurfaceTexture<super::Api>>, crate::SurfaceError> {
  let sc = self.swapchain.as_mut().unwrap();
- let timeout_ns = timeout_ms as u64 * super::MILLIS_TO_NANOS;
+
+ let mut timeout_ns = match timeout {
+ Some(duration) => duration.as_nanos() as u64,
+ None => u64::MAX,
+ };
+
+ // AcquireNextImageKHR on Android (prior to Android 11) doesn't support timeouts
+ // and will also log verbose warnings if tying to use a timeout.
+ //
+ // Android 10 implementation for reference:
+ // https://android.googlesource.com/platform/frameworks/native/+/refs/tags/android-mainline-10.0.0_r13/vulkan/libvulkan/swapchain.cpp#1426
+ // Android 11 implementation for reference:
+ // https://android.googlesource.com/platform/frameworks/native/+/refs/tags/android-mainline-11.0.0_r45/vulkan/libvulkan/swapchain.cpp#1438
+ //
+ // Android 11 corresponds to an SDK_INT/ro.build.version.sdk of 30
+ if cfg!(target_os = "android") && self.instance.android_sdk_version < 30 {
+ timeout_ns = u64::MAX;
+ }
 
  // will block if no image is available
  let (index, suboptimal) =

wgpu-hal/src/vulkan/mod.rs

@@ -87,6 +87,7 @@ struct InstanceShared {
  get_physical_device_properties: Option<khr::GetPhysicalDeviceProperties2>,
  entry: ash::Entry,
  has_nv_optimus: bool,
+ android_sdk_version: u32,
  driver_api_version: u32,
 }