Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vulkan: Replace fence with semaphore when acquiring surfaces #4967

Merged
merged 2 commits into from
Jan 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 36 additions & 3 deletions wgpu-core/src/device/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use crate::{

use hal::{CommandEncoder as _, Device as _, Queue as _};
use parking_lot::Mutex;
use smallvec::SmallVec;

use std::{
iter, mem, ptr,
Expand Down Expand Up @@ -1115,10 +1116,13 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
.fetch_add(1, Ordering::Relaxed)
+ 1;
let mut active_executions = Vec::new();

let mut used_surface_textures = track::TextureUsageScope::new();

let snatch_guard = device.snatchable_lock.read();

let mut submit_surface_textures_owned = SmallVec::<[_; 2]>::new();

{
let mut command_buffer_guard = hub.command_buffers.write();

Expand Down Expand Up @@ -1217,8 +1221,17 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
return Err(QueueSubmitError::DestroyedTexture(id));
}
Some(TextureInner::Native { .. }) => false,
Some(TextureInner::Surface { ref has_work, .. }) => {
Some(TextureInner::Surface {
ref has_work,
ref raw,
..
}) => {
has_work.store(true, Ordering::Relaxed);

if raw.is_some() {
submit_surface_textures_owned.push(texture.clone());
}

true
}
};
Expand Down Expand Up @@ -1409,8 +1422,17 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
return Err(QueueSubmitError::DestroyedTexture(id));
}
Some(TextureInner::Native { .. }) => {}
Some(TextureInner::Surface { ref has_work, .. }) => {
Some(TextureInner::Surface {
ref has_work,
ref raw,
..
}) => {
has_work.store(true, Ordering::Relaxed);

if raw.is_some() {
submit_surface_textures_owned.push(texture.clone());
}

unsafe {
used_surface_textures
.merge_single(texture, None, hal::TextureUses::PRESENT)
Expand Down Expand Up @@ -1449,12 +1471,23 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
.flat_map(|pool_execution| pool_execution.cmd_buffers.iter()),
)
.collect::<Vec<_>>();

let mut submit_surface_textures =
SmallVec::<[_; 2]>::with_capacity(submit_surface_textures_owned.len());

for texture in &submit_surface_textures_owned {
submit_surface_textures.extend(match texture.inner.get(&snatch_guard) {
Some(TextureInner::Surface { raw, .. }) => raw.as_ref(),
_ => None,
});
}

unsafe {
queue
.raw
.as_ref()
.unwrap()
.submit(&refs, Some((fence, submit_index)))
.submit(&refs, &submit_surface_textures, Some((fence, submit_index)))
.map_err(DeviceError::from)?;
}

Expand Down
8 changes: 5 additions & 3 deletions wgpu-hal/examples/halmark/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ impl<A: hal::Api> Example<A> {
let mut fence = device.create_fence().unwrap();
let init_cmd = cmd_encoder.end_encoding().unwrap();
queue
.submit(&[&init_cmd], Some((&mut fence, init_fence_value)))
.submit(&[&init_cmd], &[], Some((&mut fence, init_fence_value)))
.unwrap();
device.wait(&fence, init_fence_value, !0).unwrap();
device.destroy_buffer(staging_buffer);
Expand Down Expand Up @@ -542,7 +542,7 @@ impl<A: hal::Api> Example<A> {
{
let ctx = &mut self.contexts[self.context_index];
self.queue
.submit(&[], Some((&mut ctx.fence, ctx.fence_value)))
.submit(&[], &[], Some((&mut ctx.fence, ctx.fence_value)))
.unwrap();
}

Expand Down Expand Up @@ -729,7 +729,9 @@ impl<A: hal::Api> Example<A> {
} else {
None
};
self.queue.submit(&[&cmd_buf], fence_param).unwrap();
self.queue
.submit(&[&cmd_buf], &[&surface_tex], fence_param)
.unwrap();
self.queue.present(&self.surface, surface_tex).unwrap();
ctx.used_cmd_bufs.push(cmd_buf);
ctx.used_views.push(surface_tex_view);
Expand Down
2 changes: 1 addition & 1 deletion wgpu-hal/examples/raw-gles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,6 @@ fn fill_screen(exposed: &hal::ExposedAdapter<hal::api::Gles>, width: u32, height
encoder.begin_render_pass(&rp_desc);
encoder.end_render_pass();
let cmd_buf = encoder.end_encoding().unwrap();
od.queue.submit(&[&cmd_buf], None).unwrap();
od.queue.submit(&[&cmd_buf], &[], None).unwrap();
}
}
8 changes: 5 additions & 3 deletions wgpu-hal/examples/ray-traced-triangle/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,7 @@ impl<A: hal::Api> Example<A> {
let mut fence = device.create_fence().unwrap();
let init_cmd = cmd_encoder.end_encoding().unwrap();
queue
.submit(&[&init_cmd], Some((&mut fence, init_fence_value)))
.submit(&[&init_cmd], &[], Some((&mut fence, init_fence_value)))
.unwrap();
device.wait(&fence, init_fence_value, !0).unwrap();
cmd_encoder.reset_all(iter::once(init_cmd));
Expand Down Expand Up @@ -960,7 +960,9 @@ impl<A: hal::Api> Example<A> {
} else {
None
};
self.queue.submit(&[&cmd_buf], fence_param).unwrap();
self.queue
.submit(&[&cmd_buf], &[&surface_tex], fence_param)
.unwrap();
self.queue.present(&self.surface, surface_tex).unwrap();
ctx.used_cmd_bufs.push(cmd_buf);
ctx.used_views.push(surface_tex_view);
Expand Down Expand Up @@ -999,7 +1001,7 @@ impl<A: hal::Api> Example<A> {
{
let ctx = &mut self.contexts[self.context_index];
self.queue
.submit(&[], Some((&mut ctx.fence, ctx.fence_value)))
.submit(&[], &[], Some((&mut ctx.fence, ctx.fence_value)))
.unwrap();
}

Expand Down
1 change: 1 addition & 0 deletions wgpu-hal/src/dx12/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -882,6 +882,7 @@ impl crate::Queue<Api> for Queue {
unsafe fn submit(
&self,
command_buffers: &[&CommandBuffer],
_surface_textures: &[&Texture],
signal_fence: Option<(&mut Fence, crate::FenceValue)>,
) -> Result<(), crate::DeviceError> {
let mut temp_lists = self.temp_lists.lock();
Expand Down
1 change: 1 addition & 0 deletions wgpu-hal/src/empty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ impl crate::Queue<Api> for Context {
unsafe fn submit(
&self,
command_buffers: &[&Resource],
surface_textures: &[&Resource],
signal_fence: Option<(&mut Resource, crate::FenceValue)>,
) -> DeviceResult<()> {
Ok(())
Expand Down
1 change: 1 addition & 0 deletions wgpu-hal/src/gles/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1748,6 +1748,7 @@ impl crate::Queue<super::Api> for super::Queue {
unsafe fn submit(
&self,
command_buffers: &[&super::CommandBuffer],
_surface_textures: &[&super::Texture],
signal_fence: Option<(&mut super::Fence, crate::FenceValue)>,
) -> Result<(), crate::DeviceError> {
let shared = Arc::clone(&self.shared);
Expand Down
3 changes: 3 additions & 0 deletions wgpu-hal/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,9 +413,12 @@ pub trait Queue<A: Api>: WasmNotSendSync {
/// - all of the command buffers were created from command pools
/// that are associated with this queue.
/// - all of the command buffers had `CommadBuffer::finish()` called.
/// - all surface textures that the command buffers write to must be
/// passed to the surface_textures argument.
unsafe fn submit(
&self,
command_buffers: &[&A::CommandBuffer],
surface_textures: &[&A::SurfaceTexture],
signal_fence: Option<(&mut A::Fence, FenceValue)>,
) -> Result<(), DeviceError>;
unsafe fn present(
Expand Down
1 change: 1 addition & 0 deletions wgpu-hal/src/metal/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ impl crate::Queue<Api> for Queue {
unsafe fn submit(
&self,
command_buffers: &[&CommandBuffer],
_surface_textures: &[&SurfaceTexture],
signal_fence: Option<(&mut Fence, crate::FenceValue)>,
) -> Result<(), crate::DeviceError> {
objc::rc::autoreleasepool(|| {
Expand Down
15 changes: 12 additions & 3 deletions wgpu-hal/src/vulkan/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -627,19 +627,28 @@ impl super::Device {
let images =
unsafe { functor.get_swapchain_images(raw) }.map_err(crate::DeviceError::from)?;

let vk_info = vk::FenceCreateInfo::builder().build();
let fence = unsafe { self.shared.raw.create_fence(&vk_info, None) }
// NOTE: It's important that we define at least images.len() + 1 wait
// semaphores, since we prospectively need to provide the call to
// acquire the next image with an unsignaled semaphore.
let surface_semaphores = (0..images.len() + 1)
.map(|_| unsafe {
self.shared
.raw
.create_semaphore(&vk::SemaphoreCreateInfo::builder(), None)
})
.collect::<Result<Vec<_>, _>>()
.map_err(crate::DeviceError::from)?;

Ok(super::Swapchain {
raw,
raw_flags,
functor,
device: Arc::clone(&self.shared),
fence,
images,
config: config.clone(),
view_formats: wgt_view_formats,
surface_semaphores,
next_surface_index: 0,
})
}

Expand Down
24 changes: 15 additions & 9 deletions wgpu-hal/src/vulkan/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,15 +169,21 @@ impl super::Swapchain {
/// # Safety
///
/// - The device must have been made idle before calling this function.
unsafe fn release_resources(self, device: &ash::Device) -> Self {
unsafe fn release_resources(mut self, device: &ash::Device) -> Self {
profiling::scope!("Swapchain::release_resources");
{
profiling::scope!("vkDeviceWaitIdle");
// We need to also wait until all presentation work is done. Because there is no way to portably wait until
// the presentation work is done, we are forced to wait until the device is idle.
let _ = unsafe { device.device_wait_idle() };
};
unsafe { device.destroy_fence(self.fence, None) };

for semaphore in self.surface_semaphores.drain(..) {
unsafe {
device.destroy_semaphore(semaphore, None);
}
}

self
}
}
Expand Down Expand Up @@ -934,10 +940,12 @@ impl crate::Surface<super::Api> for super::Surface {
timeout_ns = u64::MAX;
}

let wait_semaphore = sc.surface_semaphores[sc.next_surface_index];

// will block if no image is available
let (index, suboptimal) = match unsafe {
sc.functor
.acquire_next_image(sc.raw, timeout_ns, vk::Semaphore::null(), sc.fence)
.acquire_next_image(sc.raw, timeout_ns, wait_semaphore, vk::Fence::null())
} {
// We treat `VK_SUBOPTIMAL_KHR` as `VK_SUCCESS` on Android.
// See the comment in `Queue::present`.
Expand All @@ -957,17 +965,14 @@ impl crate::Surface<super::Api> for super::Surface {
}
};

sc.next_surface_index += 1;
sc.next_surface_index %= sc.surface_semaphores.len();

// special case for Intel Vulkan returning bizzare values (ugh)
if sc.device.vendor_id == crate::auxil::db::intel::VENDOR && index > 0x100 {
return Err(crate::SurfaceError::Outdated);
}

let fences = &[sc.fence];

unsafe { sc.device.raw.wait_for_fences(fences, true, !0) }
.map_err(crate::DeviceError::from)?;
unsafe { sc.device.raw.reset_fences(fences) }.map_err(crate::DeviceError::from)?;

// https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkRenderPassBeginInfo.html#VUID-VkRenderPassBeginInfo-framebuffer-03209
let raw_flags = if sc
.raw_flags
Expand All @@ -994,6 +999,7 @@ impl crate::Surface<super::Api> for super::Surface {
},
view_formats: sc.view_formats.clone(),
},
wait_semaphore,
};
Ok(Some(crate::AcquiredSurfaceTexture {
texture,
Expand Down
Loading