From 74ca2a36691ee507ca0097a85a4b622210b55d93 Mon Sep 17 00:00:00 2001 From: Jinlei Li Date: Mon, 27 Jun 2022 21:13:15 +0800 Subject: [PATCH] Follow all suggestions --- wgpu-core/src/command/render.rs | 9 +++-- wgpu-core/src/device/mod.rs | 38 ++++++++++++-------- wgpu-hal/src/dx12/command.rs | 37 ++++++++++--------- wgpu-hal/src/dx12/device.rs | 21 +++++------ wgpu-hal/src/dx12/mod.rs | 4 +-- wgpu-hal/src/gles/command.rs | 2 +- wgpu-hal/src/vulkan/adapter.rs | 5 --- wgpu-hal/src/vulkan/command.rs | 1 - wgpu-hal/src/vulkan/device.rs | 63 ++++++++++++++++++--------------- 9 files changed, 97 insertions(+), 83 deletions(-) diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 810114dca89..0b2c0acd420 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -950,8 +950,13 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> { .collect(), resolves: color_attachments .iter() - .filter_map(|at| at.as_ref().map(|at| at.resolve_target)) - .filter_map(|attachment| attachment.as_ref().map(|at| view_guard.get(*at).unwrap())) + .filter_map(|at| match *at { + Some(RenderPassColorAttachment { + resolve_target: Some(resolve), + .. + }) => Some(view_guard.get(resolve).unwrap()), + _ => None, + }) .collect(), depth_stencil: depth_stencil_attachment.map(|at| view_guard.get(at.view).unwrap()), }; diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index 74f37b20073..f79c95038fa 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -20,7 +20,7 @@ use hal::{CommandEncoder as _, Device as _}; use parking_lot::{Mutex, MutexGuard}; use smallvec::SmallVec; use thiserror::Error; -use wgt::{BufferAddress, ColorTargetState, TextureFormat, TextureViewDimension}; +use wgt::{BufferAddress, TextureFormat, TextureViewDimension}; use std::{borrow::Cow, iter, mem, num::NonZeroU32, ops::Range, ptr}; @@ -2465,7 +2465,7 @@ impl Device { .map_or(&[][..], |fragment| &fragment.targets); let depth_stencil_state = desc.depth_stencil.as_ref(); - let cts: ArrayVec<&ColorTargetState, { hal::MAX_COLOR_ATTACHMENTS }> = + let cts: ArrayVec<_, { hal::MAX_COLOR_ATTACHMENTS }> = color_targets.iter().filter_map(|x| x.as_ref()).collect(); if !cts.is_empty() && { let first = &cts[0]; @@ -2760,23 +2760,31 @@ impl Device { if validated_stages.contains(wgt::ShaderStages::FRAGMENT) { for (i, output) in io.iter() { - if let Some(state) = color_targets.get(*i as usize) { - let format = if let Some(st) = state.as_ref() { - st.format - } else { + match color_targets.get(*i as usize) { + Some(&Some(ref state)) => { + validation::check_texture_format(state.format, &output.ty).map_err( + |pipeline| { + pipeline::CreateRenderPipelineError::ColorState( + *i as u8, + pipeline::ColorStateError::IncompatibleFormat { + pipeline, + shader: output.ty, + }, + ) + }, + )?; + } + Some(&None) => { return Err( pipeline::CreateRenderPipelineError::InvalidFragmentOutputLocation(*i), ); - }; - validation::check_texture_format(format, &output.ty).map_err(|pipeline| { - pipeline::CreateRenderPipelineError::ColorState( + } + _ => { + return Err(pipeline::CreateRenderPipelineError::ColorState( *i as u8, - pipeline::ColorStateError::IncompatibleFormat { - pipeline, - shader: output.ty, - }, - ) - })?; + pipeline::ColorStateError::Missing, + )); + } } } } diff --git a/wgpu-hal/src/dx12/command.rs b/wgpu-hal/src/dx12/command.rs index 20f26fc78dd..8e087f5ba46 100644 --- a/wgpu-hal/src/dx12/command.rs +++ b/wgpu-hal/src/dx12/command.rs @@ -585,26 +585,29 @@ impl crate::CommandEncoder for super::CommandEncoder { if let Some(cat) = cat.as_ref() { *rtv = cat.target.view.handle_rtv.unwrap().raw; } else { - if null_rtv_handle.is_none() { - let handle = self.rtv_pool.lock().alloc_handle(); - // A null pResource is used to initialize a null descriptor, - // which guarantees D3D11-like null binding behavior (reading 0s, writes are discarded) - self.device.create_render_target_view( - native::WeakPtr::null(), - &native::RenderTargetViewDesc::texture_2d( - winapi::shared::dxgiformat::DXGI_FORMAT_R8G8B8A8_UNORM, - 0, - 0, - ), - handle.raw, - ); - null_rtv_handle = Some(handle); - } - *rtv = null_rtv_handle.unwrap().raw; + *rtv = match null_rtv_handle { + Some(handle) => handle.raw, + None => { + let handle = self.shared.rtv_pool.lock().alloc_handle(); + // A null pResource is used to initialize a null descriptor, + // which guarantees D3D11-like null binding behavior (reading 0s, writes are discarded) + self.device.create_render_target_view( + native::WeakPtr::null(), + &native::RenderTargetViewDesc::texture_2d( + winapi::shared::dxgiformat::DXGI_FORMAT_R8G8B8A8_UNORM, + 0, + 0, + ), + handle.raw, + ); + null_rtv_handle = Some(handle); + handle.raw + } + }; } } if let Some(handle) = null_rtv_handle { - self.rtv_pool.lock().free_handle(handle); + self.shared.rtv_pool.lock().free_handle(handle); } let ds_view = match desc.depth_stencil_attachment { None => ptr::null(), diff --git a/wgpu-hal/src/dx12/device.rs b/wgpu-hal/src/dx12/device.rs index dd66c1ff78a..46a369b997c 100644 --- a/wgpu-hal/src/dx12/device.rs +++ b/wgpu-hal/src/dx12/device.rs @@ -123,6 +123,10 @@ impl super::Device { native::DescriptorHeapType::Sampler, capacity_samplers, )?, + rtv_pool: Mutex::new(descriptor::CpuPool::new( + raw, + native::DescriptorHeapType::Rtv, + )), }; Ok(super::Device { @@ -134,10 +138,6 @@ impl super::Device { }, private_caps, shared: Arc::new(shared), - rtv_pool: Arc::new(Mutex::new(descriptor::CpuPool::new( - raw, - native::DescriptorHeapType::Rtv, - ))), dsv_pool: Mutex::new(descriptor::CpuPool::new( raw, native::DescriptorHeapType::Dsv, @@ -306,15 +306,13 @@ impl super::Device { impl crate::Device for super::Device { unsafe fn exit(self, queue: super::Queue) { - Arc::try_unwrap(self.rtv_pool) - .ok() - .unwrap() - .into_inner() - .destroy(); self.dsv_pool.into_inner().destroy(); self.srv_uav_pool.into_inner().destroy(); self.sampler_pool.into_inner().destroy(); self.shared.destroy(); + Arc::try_unwrap(self.shared) + .ok() + .map(|shared| shared.rtv_pool.into_inner().destroy()); self.idler.destroy(); queue.raw.destroy(); } @@ -537,7 +535,7 @@ impl crate::Device for super::Device { }, handle_rtv: if desc.usage.intersects(crate::TextureUses::COLOR_TARGET) { let raw_desc = view_desc.to_rtv(); - let handle = self.rtv_pool.lock().alloc_handle(); + let handle = self.shared.rtv_pool.lock().alloc_handle(); self.raw.CreateRenderTargetView( texture.resource.as_mut_ptr(), &raw_desc, @@ -590,7 +588,7 @@ impl crate::Device for super::Device { } } if let Some(handle) = view.handle_rtv { - self.rtv_pool.lock().free_handle(handle); + self.shared.rtv_pool.lock().free_handle(handle); } if view.handle_dsv_ro.is_some() || view.handle_dsv_rw.is_some() { let mut pool = self.dsv_pool.lock(); @@ -662,7 +660,6 @@ impl crate::Device for super::Device { allocator, device: self.raw, shared: Arc::clone(&self.shared), - rtv_pool: Arc::clone(&self.rtv_pool), list: None, free_lists: Vec::new(), pass: super::PassState::new(), diff --git a/wgpu-hal/src/dx12/mod.rs b/wgpu-hal/src/dx12/mod.rs index 591ed16b9b5..9d73e9699c0 100644 --- a/wgpu-hal/src/dx12/mod.rs +++ b/wgpu-hal/src/dx12/mod.rs @@ -203,6 +203,7 @@ struct DeviceShared { cmd_signatures: CommandSignatures, heap_views: descriptor::GeneralHeap, heap_samplers: descriptor::GeneralHeap, + rtv_pool: Mutex, } impl DeviceShared { @@ -211,6 +212,7 @@ impl DeviceShared { self.cmd_signatures.destroy(); self.heap_views.raw.destroy(); self.heap_samplers.raw.destroy(); + // self.rtv_pool.into_inner().destroy(); } } @@ -221,7 +223,6 @@ pub struct Device { private_caps: PrivateCapabilities, shared: Arc, // CPU only pools - rtv_pool: Arc>, dsv_pool: Mutex, srv_uav_pool: Mutex, sampler_pool: Mutex, @@ -329,7 +330,6 @@ pub struct CommandEncoder { allocator: native::CommandAllocator, device: native::Device, shared: Arc, - rtv_pool: Arc>, list: Option, free_lists: Vec, pass: PassState, diff --git a/wgpu-hal/src/gles/command.rs b/wgpu-hal/src/gles/command.rs index 04958610e89..beaf600e6e4 100644 --- a/wgpu-hal/src/gles/command.rs +++ b/wgpu-hal/src/gles/command.rs @@ -430,7 +430,7 @@ impl crate::CommandEncoder for super::CommandEncoder { .color_attachments .first() .filter(|at| at.is_some()) - .map(|at| &at.as_ref().unwrap().target.view.inner) + .and_then(|at| at.as_ref().map(|at| &at.target.view.inner)) { // default framebuffer (provided externally) Some(&super::TextureInner::DefaultRenderbuffer) => { diff --git a/wgpu-hal/src/vulkan/adapter.rs b/wgpu-hal/src/vulkan/adapter.rs index 1be86d82110..878a5ed73cd 100644 --- a/wgpu-hal/src/vulkan/adapter.rs +++ b/wgpu-hal/src/vulkan/adapter.rs @@ -1063,7 +1063,6 @@ impl super::Instance { .imageless_framebuffer .map_or(false, |ext| ext.imageless_framebuffer != 0), }, - // imageless_framebuffers: false, image_view_usage: phd_capabilities.properties.api_version >= vk::API_VERSION_1_1 || phd_capabilities.supports_extension(vk::KhrMaintenance2Fn::name()), timeline_semaphores: match phd_features.vulkan_1_2 { @@ -1097,10 +1096,6 @@ impl super::Instance { .map_or(false, |ext| ext.robust_image_access != 0), }, }; - println!( - "caps.imageless_framebuffers {}", - private_caps.imageless_framebuffers - ); let capabilities = crate::Capabilities { limits: phd_capabilities.to_wgpu_limits(&phd_features), alignments: phd_capabilities.to_hal_alignments(), diff --git a/wgpu-hal/src/vulkan/command.rs b/wgpu-hal/src/vulkan/command.rs index 343fd1e7f0f..abfc3d6216f 100644 --- a/wgpu-hal/src/vulkan/command.rs +++ b/wgpu-hal/src/vulkan/command.rs @@ -379,7 +379,6 @@ impl crate::CommandEncoder for super::CommandEncoder { if let Some(ref at) = cat.resolve_target { vk_clear_values.push(mem::zeroed()); vk_image_views.push(at.view.raw); - println!("fb_key.attachments: {:?}", &at.view.attachment.raw); fb_key.attachments.push(at.view.attachment.clone()); } diff --git a/wgpu-hal/src/vulkan/device.rs b/wgpu-hal/src/vulkan/device.rs index 82567e67c00..6696dcca7bf 100644 --- a/wgpu-hal/src/vulkan/device.rs +++ b/wgpu-hal/src/vulkan/device.rs @@ -76,11 +76,11 @@ impl super::DeviceShared { layout: vk::ImageLayout::UNDEFINED, }; for cat in e.key().colors.iter() { - if let Some(cat) = cat.as_ref() { - color_refs.push(vk::AttachmentReference { + let (color_ref, resolve_ref) = if let Some(cat) = cat.as_ref() { + let color_ref = vk::AttachmentReference { attachment: vk_attachments.len() as u32, layout: cat.base.layout, - }); + }; vk_attachments.push({ let (load_op, store_op) = conv::map_attachment_ops(cat.base.ops); vk::AttachmentDescription::builder() @@ -92,11 +92,7 @@ impl super::DeviceShared { .final_layout(cat.base.layout) .build() }); - if let Some(ref rat) = cat.resolve { - resolve_refs.push(vk::AttachmentReference { - attachment: vk_attachments.len() as u32, - layout: rat.layout, - }); + let resolve_ref = if let Some(ref rat) = cat.resolve { let (load_op, store_op) = conv::map_attachment_ops(rat.ops); let vk_attachment = vk::AttachmentDescription::builder() .format(rat.format) @@ -107,13 +103,22 @@ impl super::DeviceShared { .final_layout(rat.layout) .build(); vk_attachments.push(vk_attachment); + + vk::AttachmentReference { + attachment: vk_attachments.len() as u32 - 1, + layout: rat.layout, + } } else { - resolve_refs.push(unused); + unused }; + + (color_ref, resolve_ref) } else { - color_refs.push(unused); - resolve_refs.push(unused); - } + (unused, unused) + }; + + color_refs.push(color_ref); + resolve_refs.push(resolve_ref); } if let Some(ref ds) = e.key().depth_stencil { @@ -1577,18 +1582,7 @@ impl crate::Device for super::Device { let mut vk_attachments = Vec::with_capacity(desc.color_targets.len()); for cat in desc.color_targets { - if let Some(cat) = cat.as_ref() { - let vk_format = self.shared.private_caps.map_texture_format(cat.format); - compatible_rp_key - .colors - .push(Some(super::ColorAttachmentKey { - base: super::AttachmentKey::compatible( - vk_format, - vk::ImageLayout::COLOR_ATTACHMENT_OPTIMAL, - ), - resolve: None, - })); - + let (key, attarchment) = if let Some(cat) = cat.as_ref() { let mut vk_attachment = vk::PipelineColorBlendAttachmentState::builder() .color_write_mask(vk::ColorComponentFlags::from_raw(cat.write_mask.bits())); if let Some(ref blend) = cat.blend { @@ -1603,11 +1597,24 @@ impl crate::Device for super::Device { .src_alpha_blend_factor(alpha_src) .dst_alpha_blend_factor(alpha_dst); } - vk_attachments.push(vk_attachment.build()); + + let vk_format = self.shared.private_caps.map_texture_format(cat.format); + ( + Some(super::ColorAttachmentKey { + base: super::AttachmentKey::compatible( + vk_format, + vk::ImageLayout::COLOR_ATTACHMENT_OPTIMAL, + ), + resolve: None, + }), + vk_attachment.build(), + ) } else { - vk_attachments.push(vk::PipelineColorBlendAttachmentState::default()); - compatible_rp_key.colors.push(None); - } + (None, vk::PipelineColorBlendAttachmentState::default()) + }; + + compatible_rp_key.colors.push(key); + vk_attachments.push(attarchment); } let vk_color_blend = vk::PipelineColorBlendStateCreateInfo::builder()