Skip to content

Commit

Permalink
surface.acquire_texture: pass Option<Duration> for timeout
Browse files Browse the repository at this point in the history
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
  • Loading branch information
rib committed Jun 4, 2022
1 parent 9e3cd08 commit 3657915
Show file tree
Hide file tree
Showing 10 changed files with 39 additions and 14 deletions.
7 changes: 6 additions & 1 deletion wgpu-core/src/present.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
2 changes: 1 addition & 1 deletion wgpu-hal/examples/halmark/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
2 changes: 1 addition & 1 deletion wgpu-hal/src/dx11/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!()
}
Expand Down
15 changes: 11 additions & 4 deletions wgpu-hal/src/dx12/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion wgpu-hal/src/empty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion wgpu-hal/src/gles/egl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion wgpu-hal/src/gles/web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
11 changes: 10 additions & 1 deletion wgpu-hal/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion wgpu-hal/src/metal/surface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(|| {
Expand Down
8 changes: 6 additions & 2 deletions wgpu-hal/src/vulkan/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -707,10 +707,14 @@ 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 timeout_ns = match timeout {
Some(duration) => duration.as_nanos() as u64,
None => u64::MAX,
};

// will block if no image is available
let (index, suboptimal) =
Expand Down

0 comments on commit 3657915

Please sign in to comment.