fix swapchain recycling, copies, device destruction, RODS

This commit is contained in:
Dzmitry Malyshau 2021-06-16 23:18:50 -04:00
parent 54d7391df0
commit 7410b700d6
17 changed files with 113 additions and 66 deletions

View File

@ -269,22 +269,14 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
let hub = A::hub(self);
let mut token = Token::root();
let (swap_chain_guard, mut token) = hub.swap_chains.read(&mut token);
//TODO: actually close the last recorded command buffer
let (mut cmd_buf_guard, _) = hub.command_buffers.write(&mut token);
let error = match CommandBuffer::get_encoder_mut(&mut *cmd_buf_guard, encoder_id) {
Ok(cmd_buf) => {
cmd_buf.encoder.close();
cmd_buf.status = CommandEncoderStatus::Finished;
// stop tracking the swapchain image, if used
for sc_id in cmd_buf.used_swap_chains.iter() {
let &(ref view_id, _) = swap_chain_guard[sc_id.value]
.acquired_texture
.as_ref()
.expect("Used swap chain frame has already presented");
cmd_buf.trackers.views.remove(view_id.value);
}
//Note: if we want to stop tracking the swapchain texture view,
// this is the place to do it.
log::trace!("Command buffer {:?} {:#?}", encoder_id, cmd_buf.trackers);
None
}

View File

@ -235,7 +235,7 @@ pub(crate) fn validate_linear_texture_data(
}
/// Function copied with minor modifications from webgpu standard <https://gpuweb.github.io/gpuweb/#valid-texture-copy-range>
/// Returns the mip level extent.
/// Returns the (virtual) mip level extent.
pub(crate) fn validate_texture_copy_range(
texture_copy_view: &ImageCopyTexture,
desc: &wgt::TextureDescriptor<()>,
@ -299,7 +299,7 @@ pub(crate) fn validate_texture_copy_range(
return Err(TransferError::UnalignedCopyHeight);
}
Ok(extent)
Ok(extent_virtual)
}
impl<G: GlobalIdentityHandlerFactory> Global<G> {

View File

@ -380,7 +380,7 @@ impl<A: HalApi> LifetimeTracker<A> {
resource::TextureViewSource::Native(source_id) => {
self.suspected_resources.textures.push(source_id.value);
}
resource::TextureViewSource::SwapChain { .. } => unreachable!(),
resource::TextureViewSource::SwapChain { .. } => {}
};
let submit_index = res.life_guard.submission_index.load(Ordering::Acquire);

View File

@ -202,6 +202,15 @@ impl<A: hal::Api> CommandAllocator<A> {
fn release_encoder(&mut self, encoder: A::CommandEncoder) {
self.free_encoders.push(encoder);
}
fn dispose(self, device: &A::Device) {
log::info!("Destroying {} command encoders", self.free_encoders.len());
for cmd_encoder in self.free_encoders {
unsafe {
device.destroy_command_encoder(cmd_encoder);
}
}
}
}
/// Structure describing a logical device. Some members are internally mutable,
@ -329,6 +338,17 @@ impl<A: HalApi> Device<A> {
Self::lock_life_internal(&self.life_tracker, token)
}
pub(crate) fn suspect_texture_view_for_destruction<'this, 'token: 'this>(
&'this self,
view_id: id::Valid<id::TextureViewId>,
token: &mut Token<'token, Self>,
) {
self.lock_life(token)
.suspected_resources
.texture_views
.push(view_id);
}
fn maintain<'this, 'token: 'this, G: GlobalIdentityHandlerFactory>(
&'this self,
hub: &Hub<A, G>,
@ -2299,10 +2319,12 @@ impl<A: hal::Api> Device<A> {
}
pub(crate) fn dispose(self) {
self.pending_writes.dispose(&self.raw);
self.command_allocator.into_inner().dispose(&self.raw);
unsafe {
self.raw.destroy_fence(self.fence);
self.raw.exit();
}
self.pending_writes.dispose(&self.raw);
}
}
@ -4239,7 +4261,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
// Adapter is only referenced by the device and itself.
// This isn't a robust way to destroy them, we should find a better one.
if device.adapter_id.ref_count.load() == 1 {
let (_adapter, _) = hub
let _ = hub
.adapters
.unregister(device.adapter_id.value.0, &mut token);
}

View File

@ -254,7 +254,7 @@ impl<A: hal::Api> Access<BindGroup<A>> for PipelineLayout<A> {}
impl<A: hal::Api> Access<BindGroup<A>> for CommandBuffer<A> {}
impl<A: hal::Api> Access<CommandBuffer<A>> for Root {}
impl<A: hal::Api> Access<CommandBuffer<A>> for Device<A> {}
impl<A: hal::Api> Access<CommandBuffer<A>> for SwapChain<A> {}
impl<A: hal::Api> Access<CommandBuffer<A>> for SwapChain<A> {} //TODO: remove this (only used in `submit()`)
impl<A: hal::Api> Access<RenderBundle> for Device<A> {}
impl<A: hal::Api> Access<RenderBundle> for CommandBuffer<A> {}
impl<A: hal::Api> Access<ComputePipeline<A>> for Device<A> {}
@ -281,7 +281,7 @@ impl<A: hal::Api> Access<Texture<A>> for Root {}
impl<A: hal::Api> Access<Texture<A>> for Device<A> {}
impl<A: hal::Api> Access<Texture<A>> for Buffer<A> {}
impl<A: hal::Api> Access<TextureView<A>> for Root {}
impl<A: hal::Api> Access<TextureView<A>> for SwapChain<A> {}
impl<A: hal::Api> Access<TextureView<A>> for SwapChain<A> {} //TODO: remove this (only used in `get_next_texture()`)
impl<A: hal::Api> Access<TextureView<A>> for Device<A> {}
impl<A: hal::Api> Access<TextureView<A>> for Texture<A> {}
impl<A: hal::Api> Access<Sampler<A>> for Root {}
@ -708,7 +708,9 @@ impl<A: HalApi, F: GlobalIdentityHandlerFactory> Hub<A, F> {
device.dispose();
}
}
if with_adapters {
drop(devices);
self.adapters.data.write().map.clear();
}
}

View File

@ -253,7 +253,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
.get_mut(swap_chain_id.to_surface_id())
.map_err(|_| SwapChainError::InvalidSurface)?;
let (mut device_guard, mut token) = hub.devices.write(&mut token);
let (mut swap_chain_guard, mut token) = hub.swap_chains.write(&mut token);
let (mut swap_chain_guard, _) = hub.swap_chains.write(&mut token);
let sc = swap_chain_guard
.get_mut(swap_chain_id)
.map_err(|_| SwapChainError::Invalid)?;
@ -269,12 +269,21 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
.acquired_texture
.take()
.ok_or(SwapChainError::AlreadyAcquired)?;
drop(swap_chain_guard);
device.suspect_texture_view_for_destruction(view_id.value, &mut token);
let (mut view_guard, _) = hub.texture_views.write(&mut token);
let view = &mut view_guard[view_id.value];
let _ = view.life_guard.ref_count.take();
/*
let (view_maybe, _) = hub.texture_views.unregister(view_id.value.0, &mut token);
drop(view_id); // contains the ref count
let view = view_maybe.ok_or(SwapChainError::Invalid)?;
if view.life_guard.ref_count.unwrap().load() != 1 {
return Err(SwapChainError::StillReferenced);
}
}*/
suf_texture
};

View File

@ -230,7 +230,7 @@ impl<S: ResourceState> ResourceTracker<S> {
}
/// Remove an id from the tracked map.
pub(crate) fn remove(&mut self, id: Valid<S::Id>) -> bool {
pub(crate) fn _remove(&mut self, id: Valid<S::Id>) -> bool {
let (index, epoch, backend) = id.0.unzip();
debug_assert_eq!(backend, self.backend);
match self.map.remove(&index) {

View File

@ -480,7 +480,7 @@ impl<A: hal::Api> Example<A> {
.destroy_bind_group_layout(self.global_group_layout);
self.device.destroy_pipeline_layout(self.pipeline_layout);
self.adapter.close(self.device);
self.device.exit();
self.instance.destroy_surface(self.surface);
}
}

View File

@ -77,7 +77,6 @@ impl crate::Adapter<Api> for Context {
unsafe fn open(&self, features: wgt::Features) -> DeviceResult<crate::OpenDevice<Api>> {
Err(crate::DeviceError::Lost)
}
unsafe fn close(&self, device: Context) {}
unsafe fn texture_format_capabilities(
&self,
format: wgt::TextureFormat,
@ -107,6 +106,7 @@ impl crate::Queue<Api> for Context {
}
impl crate::Device<Api> for Context {
unsafe fn exit(self) {}
unsafe fn create_buffer(&self, desc: &crate::BufferDescriptor) -> DeviceResult<Resource> {
Ok(Resource)
}

View File

@ -180,7 +180,6 @@ pub trait Surface<A: Api>: Send + Sync {
pub trait Adapter<A: Api>: Send + Sync {
unsafe fn open(&self, features: wgt::Features) -> Result<OpenDevice<A>, DeviceError>;
unsafe fn close(&self, device: A::Device);
/// Return the set of supported capabilities for a texture format.
unsafe fn texture_format_capabilities(
@ -195,6 +194,8 @@ pub trait Adapter<A: Api>: Send + Sync {
}
pub trait Device<A: Api>: Send + Sync {
/// Exit connection to this logical device.
unsafe fn exit(self);
/// Creates a new buffer.
///
/// The initial usage is `BufferUse::empty()`.

View File

@ -30,8 +30,6 @@ impl crate::Adapter<super::Api> for super::Adapter {
})
}
unsafe fn close(&self, _device: super::Device) {}
unsafe fn texture_format_capabilities(
&self,
format: wgt::TextureFormat,

View File

@ -132,6 +132,8 @@ impl super::Device {
}
impl crate::Device<super::Api> for super::Device {
unsafe fn exit(self) {}
unsafe fn create_buffer(&self, desc: &crate::BufferDescriptor) -> DeviceResult<super::Buffer> {
let map_read = desc.usage.contains(crate::BufferUse::MAP_READ);
let map_write = desc.usage.contains(crate::BufferUse::MAP_WRITE);

View File

@ -774,12 +774,6 @@ impl crate::Adapter<super::Api> for super::Adapter {
Ok(crate::OpenDevice { device, queue })
}
unsafe fn close(&self, device: super::Device) {
device.mem_allocator.lock().cleanup(&*device.shared);
device.desc_allocator.lock().cleanup(&*device.shared);
device.shared.free_resources();
}
unsafe fn texture_format_capabilities(
&self,
format: wgt::TextureFormat,

View File

@ -27,10 +27,9 @@ impl super::Texture {
conv::map_subresource_layers(&r.texture_base, dim, aspects, layer_count);
vk::BufferImageCopy {
buffer_offset: r.buffer_layout.offset,
buffer_row_length: r
.buffer_layout
.bytes_per_row
.map_or(0, |bpr| bpr.get() / fi.block_size as u32),
buffer_row_length: r.buffer_layout.bytes_per_row.map_or(0, |bpr| {
fi.block_dimensions.0 as u32 * (bpr.get() / fi.block_size as u32)
}),
buffer_image_height: r
.buffer_layout
.rows_per_image
@ -108,8 +107,9 @@ impl crate::CommandEncoder<super::Api> for super::CommandEncoder {
where
T: Iterator<Item = crate::BufferBarrier<'a, super::Api>>,
{
let mut src_stages = vk::PipelineStageFlags::empty();
let mut dst_stages = vk::PipelineStageFlags::empty();
//Note: this is done so that we never end up with empty stage flags
let mut src_stages = vk::PipelineStageFlags::TOP_OF_PIPE;
let mut dst_stages = vk::PipelineStageFlags::BOTTOM_OF_PIPE;
let vk_barriers = &mut self.temp.buffer_barriers;
vk_barriers.clear();
@ -130,9 +130,6 @@ impl crate::CommandEncoder<super::Api> for super::CommandEncoder {
}
if !vk_barriers.is_empty() {
if src_stages.is_empty() {
src_stages = vk::PipelineStageFlags::TOP_OF_PIPE;
}
self.device.raw.cmd_pipeline_barrier(
self.active,
src_stages,
@ -157,10 +154,10 @@ impl crate::CommandEncoder<super::Api> for super::CommandEncoder {
for bar in barriers {
let range = conv::map_subresource_range(&bar.range, bar.texture.aspects);
let (src_stage, src_access) = conv::map_texture_usage_to_barrier(bar.usage.start);
let src_layout = conv::derive_image_layout(bar.usage.start);
let src_layout = conv::derive_image_layout(bar.usage.start, bar.texture.aspects);
src_stages |= src_stage;
let (dst_stage, dst_access) = conv::map_texture_usage_to_barrier(bar.usage.end);
let dst_layout = conv::derive_image_layout(bar.usage.end);
let dst_layout = conv::derive_image_layout(bar.usage.end, bar.texture.aspects);
dst_stages |= dst_stage;
vk_barriers.push(
@ -228,7 +225,7 @@ impl crate::CommandEncoder<super::Api> for super::CommandEncoder {
) where
T: Iterator<Item = crate::TextureCopy>,
{
let src_layout = conv::derive_image_layout(src_usage);
let src_layout = conv::derive_image_layout(src_usage, src.aspects);
let vk_regions_iter = regions.map(|r| {
let (layer_count, extent) = conv::map_extent(r.size, src.dim);
@ -287,7 +284,7 @@ impl crate::CommandEncoder<super::Api> for super::CommandEncoder {
) where
T: Iterator<Item = crate::BufferTextureCopy>,
{
let src_layout = conv::derive_image_layout(src_usage);
let src_layout = conv::derive_image_layout(src_usage, src.aspects);
let vk_regions_iter = src.map_buffer_copies(regions);
inplace_or_alloc_from_iter(vk_regions_iter, |vk_regions| {
@ -734,7 +731,7 @@ impl crate::CommandEncoder<super::Api> for super::CommandEncoder {
#[test]
fn check_dst_image_layout() {
assert_eq!(
conv::derive_image_layout(crate::TextureUse::COPY_DST),
conv::derive_image_layout(crate::TextureUse::COPY_DST, crate::FormatAspect::empty()),
DST_IMAGE_LAYOUT
);
}

View File

@ -116,11 +116,12 @@ impl crate::Attachment<'_, super::Api> {
ops: crate::AttachmentOp,
caps: &super::PrivateCapabilities,
) -> super::AttachmentKey {
let aspects = self.view.aspects();
super::AttachmentKey {
format: caps.map_texture_format(self.view.attachment.view_format),
layout_pre: derive_image_layout(self.boundary_usage.start),
layout_in: derive_image_layout(self.usage),
layout_post: derive_image_layout(self.boundary_usage.end),
layout_pre: derive_image_layout(self.boundary_usage.start, aspects),
layout_in: derive_image_layout(self.usage, aspects),
layout_post: derive_image_layout(self.boundary_usage.end, aspects),
ops,
}
}
@ -152,21 +153,26 @@ impl crate::ColorAttachment<'_, super::Api> {
}
}
pub fn derive_image_layout(usage: crate::TextureUse) -> vk::ImageLayout {
pub fn derive_image_layout(
usage: crate::TextureUse,
aspects: crate::FormatAspect,
) -> vk::ImageLayout {
//Note: depth textures are always sampled with RODS layout
let is_color = aspects.contains(crate::FormatAspect::COLOR);
match usage {
crate::TextureUse::UNINITIALIZED => vk::ImageLayout::UNDEFINED,
crate::TextureUse::COPY_SRC => vk::ImageLayout::TRANSFER_SRC_OPTIMAL,
crate::TextureUse::COPY_DST => vk::ImageLayout::TRANSFER_DST_OPTIMAL,
crate::TextureUse::SAMPLED => vk::ImageLayout::SHADER_READ_ONLY_OPTIMAL,
crate::TextureUse::SAMPLED if is_color => vk::ImageLayout::SHADER_READ_ONLY_OPTIMAL,
crate::TextureUse::COLOR_TARGET => vk::ImageLayout::COLOR_ATTACHMENT_OPTIMAL,
crate::TextureUse::DEPTH_STENCIL_WRITE => vk::ImageLayout::DEPTH_STENCIL_ATTACHMENT_OPTIMAL,
_ => {
if usage.contains(crate::TextureUse::DEPTH_STENCIL_READ) {
vk::ImageLayout::DEPTH_STENCIL_READ_ONLY_OPTIMAL
} else if usage.is_empty() {
if usage.is_empty() {
vk::ImageLayout::PRESENT_SRC_KHR
} else {
} else if is_color {
vk::ImageLayout::GENERAL
} else {
vk::ImageLayout::DEPTH_STENCIL_READ_ONLY_OPTIMAL
}
}
}
@ -659,6 +665,14 @@ pub fn map_topology(topology: wgt::PrimitiveTopology) -> vk::PrimitiveTopology {
}
}
pub fn map_polygon_mode(mode: wgt::PolygonMode) -> vk::PolygonMode {
match mode {
wgt::PolygonMode::Fill => vk::PolygonMode::FILL,
wgt::PolygonMode::Line => vk::PolygonMode::LINE,
wgt::PolygonMode::Point => vk::PolygonMode::POINT,
}
}
pub fn map_front_face(front_face: wgt::FrontFace) -> vk::FrontFace {
match front_face {
wgt::FrontFace::Cw => vk::FrontFace::CLOCKWISE,

View File

@ -219,7 +219,7 @@ impl super::DeviceShared {
})
}
pub(super) unsafe fn free_resources(&self) {
unsafe fn free_resources(&self) {
for &raw in self.render_passes.lock().values() {
self.raw.destroy_render_pass(raw, None);
}
@ -518,6 +518,12 @@ impl super::Device {
}
impl crate::Device<super::Api> for super::Device {
unsafe fn exit(self) {
self.mem_allocator.into_inner().cleanup(&*self.shared);
self.desc_allocator.into_inner().cleanup(&*self.shared);
self.shared.free_resources();
}
unsafe fn create_buffer(
&self,
desc: &crate::BufferDescriptor,
@ -834,12 +840,16 @@ impl crate::Device<super::Api> for super::Device {
})
}
unsafe fn destroy_command_encoder(&self, cmd_encoder: super::CommandEncoder) {
self.shared
.raw
.free_command_buffers(cmd_encoder.raw, &cmd_encoder.free);
self.shared
.raw
.free_command_buffers(cmd_encoder.raw, &cmd_encoder.discarded);
if !cmd_encoder.free.is_empty() {
self.shared
.raw
.free_command_buffers(cmd_encoder.raw, &cmd_encoder.free);
}
if !cmd_encoder.discarded.is_empty() {
self.shared
.raw
.free_command_buffers(cmd_encoder.raw, &cmd_encoder.discarded);
}
self.shared.raw.destroy_command_pool(cmd_encoder.raw, None);
}
@ -1012,7 +1022,7 @@ impl crate::Device<super::Api> for super::Device {
vk::DescriptorType::SAMPLED_IMAGE | vk::DescriptorType::STORAGE_IMAGE => {
let index = image_infos.len();
let binding = &desc.textures[index];
let layout = conv::derive_image_layout(binding.usage);
let layout = conv::derive_image_layout(binding.usage, binding.view.aspects());
let vk_info = vk::DescriptorImageInfo::builder()
.image_view(binding.view.raw)
.image_layout(layout)
@ -1146,7 +1156,7 @@ impl crate::Device<super::Api> for super::Device {
let mut vk_rasterization = vk::PipelineRasterizationStateCreateInfo::builder()
.depth_clamp_enable(desc.primitive.clamp_depth)
.polygon_mode(vk::PolygonMode::FILL)
.polygon_mode(conv::map_polygon_mode(desc.primitive.polygon_mode))
.front_face(conv::map_front_face(desc.primitive.front_face))
.line_width(1.0);
if let Some(face) = desc.primitive.cull_mode {
@ -1163,7 +1173,7 @@ impl crate::Device<super::Api> for super::Device {
};
compatible_rp_key.depth_stencil = Some(super::DepthStencilAttachmentKey {
base: super::AttachmentKey::compatible(vk_format, vk_layout),
stencil_ops: crate::AttachmentOp::empty(),
stencil_ops: crate::AttachmentOp::all(),
});
if ds.is_depth_enabled() {

View File

@ -130,7 +130,7 @@ struct PrivateCapabilities {
texture_d24_s8: bool,
}
#[derive(Clone, Eq, Hash, PartialEq)]
#[derive(Clone, Debug, Eq, Hash, PartialEq)]
struct AttachmentKey {
format: vk::Format,
layout_pre: vk::ImageLayout,
@ -147,7 +147,7 @@ impl AttachmentKey {
layout_pre: vk::ImageLayout::GENERAL,
layout_in,
layout_post: vk::ImageLayout::GENERAL,
ops: crate::AttachmentOp::empty(),
ops: crate::AttachmentOp::all(),
}
}
}
@ -255,6 +255,12 @@ pub struct TextureView {
render_size: wgt::Extent3d,
}
impl TextureView {
fn aspects(&self) -> crate::FormatAspect {
self.attachment.view_format.into()
}
}
#[derive(Debug)]
pub struct Sampler {
raw: vk::Sampler,