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)
This commit is contained in:
Robert Bragg 2022-06-04 17:05:40 +01:00 committed by GitHub
parent 46420212d7
commit 444836f747
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 90 additions and 14 deletions

7
Cargo.lock generated
View File

@ -34,6 +34,12 @@ dependencies = [
"memchr",
]
[[package]]
name = "android-properties"
version = "0.2.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "fc7eb209b1518d6bb87b283c20095f5228ecda460da70b44f0802523dea6da04"
[[package]]
name = "android_glue"
version = "0.2.3"
@ -2101,6 +2107,7 @@ dependencies = [
name = "wgpu-hal"
version = "0.12.0"
dependencies = [
"android-properties",
"arrayvec",
"ash",
"bit-set",

View File

@ -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"),

View File

@ -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"

View File

@ -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(),

View File

@ -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!()
}

View File

@ -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();

View File

@ -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)
}

View File

@ -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 {

View File

@ -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 {

View File

@ -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);
}

View File

@ -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(|| {

View File

@ -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) =

View File

@ -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,
}