From 2c76b0e656864429e2d9f4807bb711d480071bf5 Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Thu, 8 Jul 2021 02:02:03 -0400 Subject: [PATCH] hal/dx12: clippy fixes, buffer-texture copies --- wgpu-hal/src/dx12/adapter.rs | 20 +++---- wgpu-hal/src/dx12/command.rs | 103 +++++++++++++++++++++++++++++----- wgpu-hal/src/dx12/device.rs | 25 +++++---- wgpu-hal/src/dx12/instance.rs | 16 +++--- wgpu-hal/src/dx12/mod.rs | 29 ++++++---- 5 files changed, 136 insertions(+), 57 deletions(-) diff --git a/wgpu-hal/src/dx12/adapter.rs b/wgpu-hal/src/dx12/adapter.rs index 00220b1619..17dc1a3910 100644 --- a/wgpu-hal/src/dx12/adapter.rs +++ b/wgpu-hal/src/dx12/adapter.rs @@ -21,7 +21,7 @@ impl super::Adapter { ) -> Option> { // Create the device so that we can get the capabilities. let device = match library.create_device(adapter, native::FeatureLevel::L11_0) { - Ok(pair) => match pair.to_result() { + Ok(pair) => match pair.into_result() { Ok(device) => device, Err(err) => { log::warn!("Device creation failed: {}", err); @@ -180,15 +180,11 @@ impl super::Adapter { .max_dynamic_storage_buffers_per_pipeline_layout, max_sampled_textures_per_shader_stage: match options.ResourceBindingTier { d3d12::D3D12_RESOURCE_BINDING_TIER_1 => 128, - d3d12::D3D12_RESOURCE_BINDING_TIER_2 - | d3d12::D3D12_RESOURCE_BINDING_TIER_3 - | _ => full_heap_count, + _ => full_heap_count, }, max_samplers_per_shader_stage: match options.ResourceBindingTier { d3d12::D3D12_RESOURCE_BINDING_TIER_1 => 16, - d3d12::D3D12_RESOURCE_BINDING_TIER_2 - | d3d12::D3D12_RESOURCE_BINDING_TIER_3 - | _ => d3d12::D3D12_MAX_SHADER_VISIBLE_SAMPLER_HEAP_SIZE, + _ => d3d12::D3D12_MAX_SHADER_VISIBLE_SAMPLER_HEAP_SIZE, }, // these both account towards `uav_count`, but we can't express the limit as as sum max_storage_buffers_per_shader_stage: base.max_storage_buffers_per_shader_stage, @@ -238,7 +234,7 @@ impl crate::Adapter for super::Adapter { native::CommandQueueFlags::empty(), 0, ) - .to_device_result("Queue creation")?; + .into_device_result("Queue creation")?; let device = super::Device::new(self.device, queue, self.private_caps, &self.library)?; Ok(crate::OpenDevice { @@ -328,7 +324,11 @@ impl crate::Adapter for super::Adapter { let mut present_modes = vec![wgt::PresentMode::Fifo]; #[allow(trivial_casts)] - if let Ok(factory5) = surface.factory.cast::().to_result() { + if let Ok(factory5) = surface + .factory + .cast::() + .into_result() + { let mut allow_tearing: minwindef::BOOL = minwindef::FALSE; let hr = factory5.CheckFeatureSupport( dxgi1_5::DXGI_FEATURE_PRESENT_ALLOW_TEARING, @@ -337,7 +337,7 @@ impl crate::Adapter for super::Adapter { ); factory5.destroy(); - match hr.to_result() { + match hr.into_result() { Err(err) => log::warn!("Unable to check for tearing support: {}", err), Ok(()) => present_modes.push(wgt::PresentMode::Immediate), } diff --git a/wgpu-hal/src/dx12/command.rs b/wgpu-hal/src/dx12/command.rs index 642e734bad..ecd3a89ead 100644 --- a/wgpu-hal/src/dx12/command.rs +++ b/wgpu-hal/src/dx12/command.rs @@ -2,6 +2,17 @@ use super::{conv, HResult as _, Resource}; use std::{mem, ops::Range}; use winapi::um::d3d12; +fn make_box(origin: &wgt::Origin3d, size: &crate::CopyExtent) -> d3d12::D3D12_BOX { + d3d12::D3D12_BOX { + left: origin.x, + top: origin.y, + right: origin.x + size.width, + bottom: origin.y + size.height, + front: origin.z, + back: origin.z + size.depth, + } +} + impl crate::CommandEncoder for super::CommandEncoder { unsafe fn begin_encoding(&mut self, label: crate::Label) -> Result<(), crate::DeviceError> { let list = match self.free_lists.pop() { @@ -17,7 +28,7 @@ impl crate::CommandEncoder for super::CommandEncoder { native::PipelineState::null(), 0, ) - .to_device_result("Create command list")?, + .into_device_result("Create command list")?, }; if let Some(label) = label { @@ -135,9 +146,9 @@ impl crate::CommandEncoder for super::CommandEncoder { barrier.range.base_array_layer + rel_array_layer, 0, ); + self.temp.barriers.push(raw); } } - self.temp.barriers.push(raw); } } else if barrier.usage.start == crate::TextureUses::STORAGE_STORE { let mut raw = d3d12::D3D12_RESOURCE_BARRIER { @@ -210,18 +221,10 @@ impl crate::CommandEncoder for super::CommandEncoder { }; for r in regions { - let src_box = d3d12::D3D12_BOX { - left: r.src_base.origin.x, - top: r.src_base.origin.y, - right: r.src_base.origin.x + r.size.width, - bottom: r.src_base.origin.y + r.size.height, - front: r.src_base.origin.z, - back: r.src_base.origin.z + r.size.depth, - }; - *src_location.u.SubresourceIndex_mut() = - src.calc_subresource(r.src_base.mip_level, r.src_base.array_layer, 0); - *dst_location.u.SubresourceIndex_mut() = - dst.calc_subresource(r.dst_base.mip_level, r.dst_base.array_layer, 0); + let src_box = make_box(&r.src_base.origin, &r.size); + *src_location.u.SubresourceIndex_mut() = src.calc_subresource_for_copy(&r.src_base); + *dst_location.u.SubresourceIndex_mut() = dst.calc_subresource_for_copy(&r.dst_base); + list.CopyTextureRegion( &dst_location, r.dst_base.origin.x, @@ -241,6 +244,45 @@ impl crate::CommandEncoder for super::CommandEncoder { ) where T: Iterator, { + let list = self.list.unwrap(); + let mut src_location = d3d12::D3D12_TEXTURE_COPY_LOCATION { + pResource: src.resource.as_mut_ptr(), + Type: d3d12::D3D12_TEXTURE_COPY_TYPE_PLACED_FOOTPRINT, + u: mem::zeroed(), + }; + let mut dst_location = d3d12::D3D12_TEXTURE_COPY_LOCATION { + pResource: dst.resource.as_mut_ptr(), + Type: d3d12::D3D12_TEXTURE_COPY_TYPE_SUBRESOURCE_INDEX, + u: mem::zeroed(), + }; + let raw_format = conv::map_texture_format(dst.format); + + for r in regions { + let src_box = make_box(&wgt::Origin3d::ZERO, &r.size); + *src_location.u.PlacedFootprint_mut() = d3d12::D3D12_PLACED_SUBRESOURCE_FOOTPRINT { + Offset: r.buffer_layout.offset, + Footprint: d3d12::D3D12_SUBRESOURCE_FOOTPRINT { + Format: raw_format, + Width: r.size.width, + Height: r + .buffer_layout + .rows_per_image + .map_or(r.size.height, |count| count.get()), + Depth: r.size.depth, + RowPitch: r.buffer_layout.bytes_per_row.map_or(0, |count| count.get()), + }, + }; + *dst_location.u.SubresourceIndex_mut() = dst.calc_subresource_for_copy(&r.texture_base); + + list.CopyTextureRegion( + &dst_location, + r.texture_base.origin.x, + r.texture_base.origin.y, + r.texture_base.origin.z, + &src_location, + &src_box, + ); + } } unsafe fn copy_texture_to_buffer( @@ -252,7 +294,38 @@ impl crate::CommandEncoder for super::CommandEncoder { ) where T: Iterator, { - for _r in regions {} + let list = self.list.unwrap(); + let mut src_location = d3d12::D3D12_TEXTURE_COPY_LOCATION { + pResource: src.resource.as_mut_ptr(), + Type: d3d12::D3D12_TEXTURE_COPY_TYPE_SUBRESOURCE_INDEX, + u: mem::zeroed(), + }; + let mut dst_location = d3d12::D3D12_TEXTURE_COPY_LOCATION { + pResource: dst.resource.as_mut_ptr(), + Type: d3d12::D3D12_TEXTURE_COPY_TYPE_PLACED_FOOTPRINT, + u: mem::zeroed(), + }; + let raw_format = conv::map_texture_format(src.format); + + for r in regions { + let dst_box = make_box(&r.texture_base.origin, &r.size); + *src_location.u.SubresourceIndex_mut() = src.calc_subresource_for_copy(&r.texture_base); + *dst_location.u.PlacedFootprint_mut() = d3d12::D3D12_PLACED_SUBRESOURCE_FOOTPRINT { + Offset: r.buffer_layout.offset, + Footprint: d3d12::D3D12_SUBRESOURCE_FOOTPRINT { + Format: raw_format, + Width: r.size.width, + Height: r + .buffer_layout + .rows_per_image + .map_or(r.size.height, |count| count.get()), + Depth: r.size.depth, + RowPitch: r.buffer_layout.bytes_per_row.map_or(0, |count| count.get()), + }, + }; + + list.CopyTextureRegion(&src_location, 0, 0, 0, &dst_location, &dst_box); + } } unsafe fn begin_query(&mut self, set: &super::QuerySet, index: u32) {} diff --git a/wgpu-hal/src/dx12/device.rs b/wgpu-hal/src/dx12/device.rs index f2747672b8..ab3b97671d 100644 --- a/wgpu-hal/src/dx12/device.rs +++ b/wgpu-hal/src/dx12/device.rs @@ -28,7 +28,7 @@ impl super::Device { idle_fence.mut_void(), ) }; - hr.to_device_result("Idle fence creation")?; + hr.into_device_result("Idle fence creation")?; Ok(super::Device { raw, @@ -66,7 +66,7 @@ impl super::Device { .idler .fence .set_event_on_completion(self.idler.event, value); - hr.to_device_result("Set event")?; + hr.into_device_result("Set event")?; synchapi::WaitForSingleObject(self.idler.event.0, winbase::INFINITE); Ok(()) } @@ -431,7 +431,7 @@ impl crate::Device for super::Device { if let Ok(debug_device) = self .raw .cast::() - .to_result() + .into_result() { debug_device.ReportLiveDeviceObjects(d3d12sdklayers::D3D12_RLDO_DETAIL); debug_device.destroy(); @@ -494,7 +494,7 @@ impl crate::Device for super::Device { resource.mut_void(), ); - hr.to_device_result("Buffer creation")?; + hr.into_device_result("Buffer creation")?; Ok(super::Buffer { resource }) } unsafe fn destroy_buffer(&self, buffer: super::Buffer) { @@ -507,7 +507,7 @@ impl crate::Device for super::Device { ) -> Result { let mut ptr = ptr::null_mut(); let hr = (*buffer.resource).Map(0, &d3d12::D3D12_RANGE { Begin: 0, End: 0 }, &mut ptr); - hr.to_device_result("Map buffer")?; + hr.into_device_result("Map buffer")?; Ok(crate::BufferMapping { ptr: ptr::NonNull::new(ptr.offset(range.start as isize) as *mut _).unwrap(), //TODO: double-check this. Documentation is a bit misleading - @@ -571,9 +571,10 @@ impl crate::Device for super::Device { resource.SetName(cwstr.as_ptr()); } - hr.to_device_result("Texture creation")?; + hr.into_device_result("Texture creation")?; Ok(super::Texture { resource, + format: desc.format, dimension: desc.dimension, size: desc.size, mip_level_count: desc.mip_level_count, @@ -696,7 +697,7 @@ impl crate::Device for super::Device { let allocator = self .raw .create_command_allocator(native::CmdListType::Direct) - .to_device_result("Command allocator creation")?; + .into_device_result("Command allocator creation")?; Ok(super::CommandEncoder { allocator, device: self.raw, @@ -915,7 +916,7 @@ impl crate::Device for super::Device { log::error!("Unable to find serialization function: {:?}", e); crate::DeviceError::Lost })? - .to_device_result("Root signature serialization")?; + .into_device_result("Root signature serialization")?; if !error.is_null() { log::error!( @@ -929,7 +930,7 @@ impl crate::Device for super::Device { let raw = self .raw .create_root_signature(blob, 0) - .to_device_result("Root signature creation")?; + .into_device_result("Root signature creation")?; blob.destroy(); Ok(super::PipelineLayout { @@ -987,7 +988,7 @@ impl crate::Device for super::Device { let raw = self .raw .create_query_heap(heap_ty, desc.count, 0) - .to_device_result("Query heap creation")?; + .into_device_result("Query heap creation")?; Ok(super::QuerySet { raw, ty: desc.ty }) } @@ -1003,7 +1004,7 @@ impl crate::Device for super::Device { &d3d12::ID3D12Fence::uuidof(), raw.mut_void(), ); - hr.to_device_result("Fence creation")?; + hr.into_device_result("Fence creation")?; Ok(super::Fence { raw }) } unsafe fn destroy_fence(&self, fence: super::Fence) { @@ -1025,7 +1026,7 @@ impl crate::Device for super::Device { return Ok(true); } let hr = fence.raw.set_event_on_completion(self.idler.event, value); - hr.to_device_result("Set event")?; + hr.into_device_result("Set event")?; match synchapi::WaitForSingleObject(self.idler.event.0, timeout_ms) { winbase::WAIT_ABANDONED | winbase::WAIT_FAILED => Err(crate::DeviceError::Lost), diff --git a/wgpu-hal/src/dx12/instance.rs b/wgpu-hal/src/dx12/instance.rs index 8d5ff2b84e..c493444fd8 100644 --- a/wgpu-hal/src/dx12/instance.rs +++ b/wgpu-hal/src/dx12/instance.rs @@ -41,7 +41,7 @@ unsafe extern "system" fn output_debug_string_handler( Some(msg) => { match MESSAGE_PREFIXES .iter() - .find(|&(prefix, level)| msg.starts_with(prefix)) + .find(|&&(prefix, level)| msg.starts_with(prefix)) { Some(&(prefix, level)) => (&msg[prefix.len() + 2..], level), None => (msg, log::Level::Debug), @@ -78,7 +78,7 @@ impl crate::Instance for super::Instance { if desc.flags.contains(crate::InstanceFlags::VALIDATION) { // Enable debug layer match lib_main.get_debug_interface() { - Ok(pair) => match pair.to_result() { + Ok(pair) => match pair.into_result() { Ok(debug_controller) => { debug_controller.enable_layer(); debug_controller.Release(); @@ -96,7 +96,7 @@ impl crate::Instance for super::Instance { // `CreateDXGIFactory2` if the debug interface is actually available. So // we check for whether it exists first. match lib_dxgi.get_debug_interface1() { - Ok(pair) => match pair.to_result() { + Ok(pair) => match pair.into_result() { Ok(debug_controller) => { debug_controller.destroy(); factory_flags |= native::FactoryCreationFlags::DEBUG; @@ -116,7 +116,7 @@ impl crate::Instance for super::Instance { // Create DXGI factory let factory = match lib_dxgi.create_factory2(factory_flags) { - Ok(pair) => match pair.to_result() { + Ok(pair) => match pair.into_result() { Ok(factory) => factory, Err(err) => { log::warn!("Failed to create DXGI factory: {}", err); @@ -155,7 +155,7 @@ impl crate::Instance for super::Instance { unsafe fn enumerate_adapters(&self) -> Vec> { // Try to use high performance order by default (returns None on Windows < 1803) - let factory6 = match self.factory.cast::().to_result() { + let factory6 = match self.factory.cast::().into_result() { Ok(f6) => { // It's okay to decrement the refcount here because we // have another reference to the factory already owned by `self`. @@ -184,7 +184,7 @@ impl crate::Instance for super::Instance { if hr == winerror::DXGI_ERROR_NOT_FOUND { break; } - if let Err(err) = hr.to_result() { + if let Err(err) = hr.into_result() { log::error!("Failed enumerating adapters: {}", err); break; } @@ -200,12 +200,12 @@ impl crate::Instance for super::Instance { if hr == winerror::DXGI_ERROR_NOT_FOUND { break; } - if let Err(err) = hr.to_result() { + if let Err(err) = hr.into_result() { log::error!("Failed enumerating adapters: {}", err); break; } - match adapter1.cast::().to_result() { + match adapter1.cast::().into_result() { Ok(adapter2) => { adapter1.destroy(); adapter2 diff --git a/wgpu-hal/src/dx12/mod.rs b/wgpu-hal/src/dx12/mod.rs index 542c845907..99764062a1 100644 --- a/wgpu-hal/src/dx12/mod.rs +++ b/wgpu-hal/src/dx12/mod.rs @@ -55,11 +55,11 @@ impl crate::Api for Api { } trait HResult { - fn to_result(self) -> Result>; - fn to_device_result(self, description: &str) -> Result; + fn into_result(self) -> Result>; + fn into_device_result(self, description: &str) -> Result; } impl HResult<()> for i32 { - fn to_result(self) -> Result<(), Cow<'static, str>> { + fn into_result(self) -> Result<(), Cow<'static, str>> { if self >= 0 { return Ok(()); } @@ -72,8 +72,8 @@ impl HResult<()> for i32 { }; Err(Cow::Borrowed(description)) } - fn to_device_result(self, description: &str) -> Result<(), crate::DeviceError> { - self.to_result().map_err(|err| { + fn into_device_result(self, description: &str) -> Result<(), crate::DeviceError> { + self.into_result().map_err(|err| { log::error!("{} failed: {}", description, err); if self == winerror::E_OUTOFMEMORY { crate::DeviceError::OutOfMemory @@ -85,11 +85,11 @@ impl HResult<()> for i32 { } impl HResult for (T, i32) { - fn to_result(self) -> Result> { - self.1.to_result().map(|()| self.0) + fn into_result(self) -> Result> { + self.1.into_result().map(|()| self.0) } - fn to_device_result(self, description: &str) -> Result { - self.1.to_device_result(description).map(|()| self.0) + fn into_device_result(self, description: &str) -> Result { + self.1.into_device_result(description).map(|()| self.0) } } @@ -220,6 +220,7 @@ unsafe impl Sync for Buffer {} #[derive(Debug)] pub struct Texture { resource: native::Resource, + format: wgt::TextureFormat, dimension: wgt::TextureDimension, size: wgt::Extent3d, mip_level_count: u32, @@ -242,6 +243,10 @@ impl Texture { fn calc_subresource(&self, mip_level: u32, array_layer: u32, plane: u32) -> u32 { mip_level + (array_layer + plane * self.array_layer_count()) * self.mip_level_count } + + fn calc_subresource_for_copy(&self, base: &crate::TextureCopyBase) -> u32 { + self.calc_subresource(base.mip_level, base.array_layer, 0) + } } #[derive(Debug)] @@ -367,7 +372,7 @@ impl crate::Surface for Surface { non_srgb_format, flags, ); - if let Err(err) = result.to_result() { + if let Err(err) = result.into_result() { log::error!("ResizeBuffers failed: {}", err); return Err(crate::SurfaceError::Other("window is in use")); } @@ -402,12 +407,12 @@ impl crate::Surface for Surface { swap_chain1.mut_void() as *mut *mut _, ); - if let Err(err) = hr.to_result() { + if let Err(err) = hr.into_result() { log::error!("SwapChain creation error: {}", err); return Err(crate::SurfaceError::Other("swap chain creation")); } - match swap_chain1.cast::().to_result() { + match swap_chain1.cast::().into_result() { Ok(swap_chain3) => { swap_chain1.destroy(); swap_chain3