Skip to content

Commit

Permalink
vulkan: Replace fence with semaphore when acquiring surfaces
Browse files Browse the repository at this point in the history
  • Loading branch information
udoprog committed Jan 3, 2024
1 parent a0c2b25 commit 9bea671
Show file tree
Hide file tree
Showing 10 changed files with 137 additions and 30 deletions.
37 changes: 33 additions & 4 deletions wgpu-core/src/device/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use crate::{
resource_log, track, FastHashMap, SubmissionIndex,
};

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

use std::{
Expand Down Expand Up @@ -1139,6 +1139,12 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
.fetch_add(1, Ordering::Relaxed)
+ 1;
let mut active_executions = Vec::new();

// SAFETY: We're constructing this during the submission phase,
// where all resources it uses are guaranteed to outlive this
// short-lived set.
let mut submit_surface_textures = A::SubmitSurfaceTextureSet::new();

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

let snatch_guard = device.snatchable_lock.read();
Expand Down Expand Up @@ -1243,8 +1249,19 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
return Err(QueueSubmitError::DestroyedTexture(id));
}
TextureInner::Native { raw: Some(_) } => false,
TextureInner::Surface { ref has_work, .. } => {
TextureInner::Surface {
ref has_work,
ref raw,
..
} => {
has_work.store(true, Ordering::Relaxed);

if let Some(raw) = raw {
unsafe {
submit_surface_textures.insert(raw);
}
}

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

if let Some(raw) = raw {
unsafe {
submit_surface_textures.insert(raw);
}
}

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

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
2 changes: 2 additions & 0 deletions wgpu-hal/src/dx12/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ impl crate::Api for Api {
type ComputePipeline = ComputePipeline;

type AccelerationStructure = AccelerationStructure;
type SubmitSurfaceTextureSet = ();
}

// Limited by D3D12's root signature size of 64. Each element takes 1 or 2 entries.
Expand Down Expand Up @@ -877,6 +878,7 @@ impl crate::Queue<Api> for Queue {
unsafe fn submit(
&self,
command_buffers: &[&CommandBuffer],
surface_textures: &(),
signal_fence: Option<(&mut Fence, crate::FenceValue)>,
) -> Result<(), crate::DeviceError> {
let mut temp_lists = self.temp_lists.lock();
Expand Down
2 changes: 2 additions & 0 deletions wgpu-hal/src/empty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ impl crate::Api for Api {
type ShaderModule = Resource;
type RenderPipeline = Resource;
type ComputePipeline = Resource;
type SubmitSurfaceTextureSet = ();
}

impl crate::Instance<Api> for Context {
Expand Down Expand Up @@ -104,6 +105,7 @@ impl crate::Queue<Api> for Context {
unsafe fn submit(
&self,
command_buffers: &[&Resource],
surface_textures: &(),
signal_fence: Option<(&mut Resource, crate::FenceValue)>,
) -> DeviceResult<()> {
Ok(())
Expand Down
1 change: 1 addition & 0 deletions wgpu-hal/src/gles/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ impl crate::Api for Api {
type ShaderModule = ShaderModule;
type RenderPipeline = RenderPipeline;
type ComputePipeline = ComputePipeline;
type SubmitSurfaceTextureSet = ();
}

bitflags::bitflags! {
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: &(),
signal_fence: Option<(&mut super::Fence, crate::FenceValue)>,
) -> Result<(), crate::DeviceError> {
let shared = Arc::clone(&self.shared);
Expand Down
23 changes: 23 additions & 0 deletions wgpu-hal/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ pub trait Api: Clone + fmt::Debug + Sized {
type ComputePipeline: fmt::Debug + WasmNotSendSync;

type AccelerationStructure: fmt::Debug + WasmNotSendSync + 'static;
type SubmitSurfaceTextureSet: RawSet<Self::SurfaceTexture>;
}

pub trait Instance<A: Api>: Sized + WasmNotSendSync {
Expand Down Expand Up @@ -413,9 +414,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::SubmitSurfaceTextureSet,
signal_fence: Option<(&mut A::Fence, FenceValue)>,
) -> Result<(), DeviceError>;
unsafe fn present(
Expand Down Expand Up @@ -718,6 +722,25 @@ bitflags!(
}
);

pub trait RawSet<T> {
/// Construct a new set unsafely.
fn new() -> Self;

/// Insert a value into the raw set.
///
/// The caller is responsible for ensuring that the set doesn't outlive the
/// values it contains. The exact requirements depends on which set is being
/// constructed.
unsafe fn insert(&mut self, value: &T);
}

/// Provide a default implementation for () for backends which do not need to
/// track any raw resources so they can easily be stubbed out.
impl<T> RawSet<T> for () {
fn new() -> Self {}
unsafe fn insert(&mut self, _: &T) {}
}

bitflags!(
/// Texture format capability flags.
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
Expand Down
2 changes: 2 additions & 0 deletions wgpu-hal/src/metal/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ impl crate::Api for Api {
type ComputePipeline = ComputePipeline;

type AccelerationStructure = AccelerationStructure;
type SubmitSurfaceTextureSet = ();
}

pub struct Instance {
Expand Down Expand Up @@ -368,6 +369,7 @@ impl crate::Queue<Api> for Queue {
unsafe fn submit(
&self,
command_buffers: &[&CommandBuffer],
_surface_textures: &(),
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 images.len() + 1 wait semaphores,
// since we need to provide the call to acquireNextImage with an
// unsignaled semaphore, and calls to acquire next image
let surface_semaphores = (0..(images.len() * 2))
.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 @@ -168,15 +168,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 @@ -928,10 +934,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 @@ -951,17 +959,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 @@ -988,6 +993,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

0 comments on commit 9bea671

Please sign in to comment.