From 273c2c4b1a1c855b8ce7161c23440e20de3b1973 Mon Sep 17 00:00:00 2001 From: Fionn Langhans Date: Wed, 14 Aug 2024 18:48:54 +0200 Subject: [PATCH 001/141] Fixed spelling mistake in URL of d3d12 --- wgpu-hal/README.md | 2 +- wgpu-hal/src/lib.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/wgpu-hal/README.md b/wgpu-hal/README.md index bb5556b3d2..4f048eb1bc 100644 --- a/wgpu-hal/README.md +++ b/wgpu-hal/README.md @@ -89,7 +89,7 @@ platform graphics APIs: [`ash`]: https://crates.io/crates/ash [MoltenVK]: https://github.com/KhronosGroup/MoltenVK [`metal`]: https://crates.io/crates/metal -[`d3d12`]: ahttps://crates.io/crates/d3d12 +[`d3d12`]: https://crates.io/crates/d3d12 ## Secondary backends diff --git a/wgpu-hal/src/lib.rs b/wgpu-hal/src/lib.rs index f26b6925cc..8f7db88450 100644 --- a/wgpu-hal/src/lib.rs +++ b/wgpu-hal/src/lib.rs @@ -87,7 +87,7 @@ //! [`ash`]: https://crates.io/crates/ash //! [MoltenVK]: https://github.com/KhronosGroup/MoltenVK //! [`metal`]: https://crates.io/crates/metal -//! [`d3d12`]: ahttps://crates.io/crates/d3d12 +//! [`d3d12`]: https://crates.io/crates/d3d12 //! //! ## Secondary backends //! From a0c107f7c800730fc7a5968ca6542a7ac60e1ca8 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 13 Aug 2024 16:07:34 +0100 Subject: [PATCH 002/141] remove handling of error that is not documented to be returned by `vkAllocateMemory` --- wgpu-hal/src/vulkan/device.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/wgpu-hal/src/vulkan/device.rs b/wgpu-hal/src/vulkan/device.rs index c42cace857..fafd40291b 100644 --- a/wgpu-hal/src/vulkan/device.rs +++ b/wgpu-hal/src/vulkan/device.rs @@ -322,7 +322,6 @@ impl gpu_alloc::MemoryDevice for super::DeviceShared { Err(vk::Result::ERROR_OUT_OF_HOST_MEMORY) => { Err(gpu_alloc::OutOfMemory::OutOfHostMemory) } - Err(vk::Result::ERROR_TOO_MANY_OBJECTS) => panic!("Too many objects"), Err(err) => panic!("Unexpected Vulkan error: `{err}`"), } } From 7103520bd3cb1aac3a1ada65862f5d24de575657 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Thu, 15 Aug 2024 09:58:31 +0100 Subject: [PATCH 003/141] introduce a new function for hal usage errors --- wgpu-hal/src/vulkan/device.rs | 25 +++++++++++-------------- wgpu-hal/src/vulkan/mod.rs | 4 ++++ 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/wgpu-hal/src/vulkan/device.rs b/wgpu-hal/src/vulkan/device.rs index fafd40291b..dfb1ca2f52 100644 --- a/wgpu-hal/src/vulkan/device.rs +++ b/wgpu-hal/src/vulkan/device.rs @@ -948,13 +948,14 @@ impl crate::Device for super::Device { .contains(gpu_alloc::MemoryPropertyFlags::HOST_COHERENT); Ok(crate::BufferMapping { ptr, is_coherent }) } else { - Err(crate::DeviceError::OutOfMemory) + super::hal_usage_error("tried to map external buffer") } } unsafe fn unmap_buffer(&self, buffer: &super::Buffer) { - // We can only unmap the buffer if it was already mapped successfully. if let Some(ref block) = buffer.block { unsafe { block.lock().unmap(&*self.shared) }; + } else { + super::hal_usage_error("tried to unmap external buffer") } } @@ -2463,8 +2464,10 @@ impl super::DeviceShared { } } None => { - log::error!("No signals reached value {}", wait_value); - Err(crate::DeviceError::Lost) + super::hal_usage_error(format!( + "no signals reached value {}", + wait_value + )); } } } @@ -2477,11 +2480,8 @@ impl From for crate::DeviceError { fn from(error: gpu_alloc::AllocationError) -> Self { use gpu_alloc::AllocationError as Ae; match error { - Ae::OutOfDeviceMemory | Ae::OutOfHostMemory => Self::OutOfMemory, - _ => { - log::error!("memory allocation: {:?}", error); - Self::Lost - } + Ae::OutOfDeviceMemory | Ae::OutOfHostMemory | Ae::TooManyObjects => Self::OutOfMemory, + Ae::NoCompatibleMemoryTypes => super::hal_usage_error(error), } } } @@ -2489,11 +2489,8 @@ impl From for crate::DeviceError { fn from(error: gpu_alloc::MapError) -> Self { use gpu_alloc::MapError as Me; match error { - Me::OutOfDeviceMemory | Me::OutOfHostMemory => Self::OutOfMemory, - _ => { - log::error!("memory mapping: {:?}", error); - Self::Lost - } + Me::OutOfDeviceMemory | Me::OutOfHostMemory | Me::MapFailed => Self::OutOfMemory, + Me::NonHostVisible | Me::AlreadyMapped => super::hal_usage_error(error), } } } diff --git a/wgpu-hal/src/vulkan/mod.rs b/wgpu-hal/src/vulkan/mod.rs index 0b024b80a7..8757671b44 100644 --- a/wgpu-hal/src/vulkan/mod.rs +++ b/wgpu-hal/src/vulkan/mod.rs @@ -1199,3 +1199,7 @@ impl From for crate::DeviceError { } } } + +fn hal_usage_error(txt: T) -> ! { + panic!("wgpu-hal invariant was violated (usage error): {txt}") +} From 8b6450a9ce6fd7411fec71a8f944b4eb3698dfaf Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Thu, 15 Aug 2024 09:59:56 +0100 Subject: [PATCH 004/141] handle all variants of `gpu_descriptor::AllocationError` explicitly --- wgpu-hal/src/vulkan/device.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/wgpu-hal/src/vulkan/device.rs b/wgpu-hal/src/vulkan/device.rs index dfb1ca2f52..baf3726c75 100644 --- a/wgpu-hal/src/vulkan/device.rs +++ b/wgpu-hal/src/vulkan/device.rs @@ -2496,7 +2496,9 @@ impl From for crate::DeviceError { } impl From for crate::DeviceError { fn from(error: gpu_descriptor::AllocationError) -> Self { - log::error!("descriptor allocation: {:?}", error); - Self::OutOfMemory + use gpu_descriptor::AllocationError as Ae; + match error { + Ae::OutOfDeviceMemory | Ae::OutOfHostMemory | Ae::Fragmentation => Self::OutOfMemory, + } } } From e4c5b4760bb7df1fede8e323afaa71d09ab178c1 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Thu, 15 Aug 2024 10:05:07 +0100 Subject: [PATCH 005/141] introduce a new function that handles unexpected vulkan errors that can't be mapped to `DeviceError::Lost` --- wgpu-hal/src/vulkan/device.rs | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/wgpu-hal/src/vulkan/device.rs b/wgpu-hal/src/vulkan/device.rs index baf3726c75..67972b166d 100644 --- a/wgpu-hal/src/vulkan/device.rs +++ b/wgpu-hal/src/vulkan/device.rs @@ -322,7 +322,7 @@ impl gpu_alloc::MemoryDevice for super::DeviceShared { Err(vk::Result::ERROR_OUT_OF_HOST_MEMORY) => { Err(gpu_alloc::OutOfMemory::OutOfHostMemory) } - Err(err) => panic!("Unexpected Vulkan error: `{err}`"), + Err(err) => handle_unexpected(err), } } @@ -351,7 +351,7 @@ impl gpu_alloc::MemoryDevice for super::DeviceShared { Err(gpu_alloc::DeviceMapError::OutOfHostMemory) } Err(vk::Result::ERROR_MEMORY_MAP_FAILED) => Err(gpu_alloc::DeviceMapError::MapFailed), - Err(err) => panic!("Unexpected Vulkan error: `{err}`"), + Err(err) => handle_unexpected(err), } } @@ -450,10 +450,7 @@ impl Err(vk::Result::ERROR_FRAGMENTATION) => { Err(gpu_descriptor::CreatePoolError::Fragmentation) } - Err(other) => { - log::error!("create_descriptor_pool: {:?}", other); - Err(gpu_descriptor::CreatePoolError::OutOfHostMemory) - } + Err(err) => handle_unexpected(err), } } @@ -494,10 +491,7 @@ impl Err(vk::Result::ERROR_FRAGMENTED_POOL) => { Err(gpu_descriptor::DeviceAllocationError::FragmentedPool) } - Err(other) => { - log::error!("allocate_descriptor_sets: {:?}", other); - Err(gpu_descriptor::DeviceAllocationError::OutOfHostMemory) - } + Err(err) => handle_unexpected(err), } } @@ -514,7 +508,7 @@ impl }; match result { Ok(()) => {} - Err(err) => log::error!("free_descriptor_sets: {:?}", err), + Err(err) => handle_unexpected(err), } } } @@ -2502,3 +2496,13 @@ impl From for crate::DeviceError { } } } + +/// We usually map unexpected vulkan errors to the [`crate::DeviceError::Lost`] +/// variant to be more robust even in cases where the driver is not +/// complying with the spec. +/// +/// However, we implement a few Trait methods that don't have an equivalent +/// error variant. In those cases we use this function. +fn handle_unexpected(err: vk::Result) -> ! { + panic!("Unexpected Vulkan error: `{err}`") +} From 24134e049f5853f59105656cd11a66c1d4ab5668 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Thu, 15 Aug 2024 10:23:44 +0100 Subject: [PATCH 006/141] handle all vulkan error variants for each function explicitly --- wgpu-hal/src/vulkan/adapter.rs | 17 +++- wgpu-hal/src/vulkan/command.rs | 17 +++- wgpu-hal/src/vulkan/device.rs | 114 ++++++++++++++++++------ wgpu-hal/src/vulkan/instance.rs | 12 ++- wgpu-hal/src/vulkan/mod.rs | 152 ++++++++++++++++++++++++++------ 5 files changed, 251 insertions(+), 61 deletions(-) diff --git a/wgpu-hal/src/vulkan/adapter.rs b/wgpu-hal/src/vulkan/adapter.rs index 22b897f09b..0a4e73382a 100644 --- a/wgpu-hal/src/vulkan/adapter.rs +++ b/wgpu-hal/src/vulkan/adapter.rs @@ -1985,8 +1985,23 @@ impl crate::Adapter for super::Adapter { let info = enabled_phd_features.add_to_device_create(pre_info); let raw_device = { profiling::scope!("vkCreateDevice"); - unsafe { self.instance.raw.create_device(self.raw, &info, None)? } + unsafe { + self.instance + .raw + .create_device(self.raw, &info, None) + .map_err(map_err)? + } }; + fn map_err(err: vk::Result) -> crate::DeviceError { + match err { + vk::Result::ERROR_TOO_MANY_OBJECTS => crate::DeviceError::OutOfMemory, + vk::Result::ERROR_INITIALIZATION_FAILED => crate::DeviceError::Lost, + vk::Result::ERROR_EXTENSION_NOT_PRESENT | vk::Result::ERROR_FEATURE_NOT_PRESENT => { + super::hal_usage_error(err) + } + other => super::map_host_device_oom_and_lost_err(other), + } + } unsafe { self.device_from_raw( diff --git a/wgpu-hal/src/vulkan/command.rs b/wgpu-hal/src/vulkan/command.rs index 0c81321c93..dd89f7dae2 100644 --- a/wgpu-hal/src/vulkan/command.rs +++ b/wgpu-hal/src/vulkan/command.rs @@ -62,7 +62,12 @@ impl crate::CommandEncoder for super::CommandEncoder { let vk_info = vk::CommandBufferAllocateInfo::default() .command_pool(self.raw) .command_buffer_count(ALLOCATION_GRANULARITY); - let cmd_buf_vec = unsafe { self.device.raw.allocate_command_buffers(&vk_info)? }; + let cmd_buf_vec = unsafe { + self.device + .raw + .allocate_command_buffers(&vk_info) + .map_err(super::map_host_device_oom_err)? + }; self.free.extend(cmd_buf_vec); } let raw = self.free.pop().unwrap(); @@ -76,7 +81,8 @@ impl crate::CommandEncoder for super::CommandEncoder { let vk_info = vk::CommandBufferBeginInfo::default() .flags(vk::CommandBufferUsageFlags::ONE_TIME_SUBMIT); - unsafe { self.device.raw.begin_command_buffer(raw, &vk_info) }?; + unsafe { self.device.raw.begin_command_buffer(raw, &vk_info) } + .map_err(super::map_host_device_oom_err)?; self.active = raw; Ok(()) @@ -85,7 +91,12 @@ impl crate::CommandEncoder for super::CommandEncoder { unsafe fn end_encoding(&mut self) -> Result { let raw = self.active; self.active = vk::CommandBuffer::null(); - unsafe { self.device.raw.end_command_buffer(raw) }?; + unsafe { self.device.raw.end_command_buffer(raw) }.map_err(map_err)?; + fn map_err(err: vk::Result) -> crate::DeviceError { + // We don't use VK_KHR_video_encode_queue + // VK_ERROR_INVALID_VIDEO_STD_PARAMETERS_KHR + super::map_host_device_oom_err(err) + } Ok(super::CommandBuffer { raw }) } diff --git a/wgpu-hal/src/vulkan/device.rs b/wgpu-hal/src/vulkan/device.rs index 67972b166d..ef02899a1d 100644 --- a/wgpu-hal/src/vulkan/device.rs +++ b/wgpu-hal/src/vulkan/device.rs @@ -178,7 +178,11 @@ impl super::DeviceShared { vk_info = vk_info.push_next(&mut multiview_info); } - let raw = unsafe { self.raw.create_render_pass(&vk_info, None)? }; + let raw = unsafe { + self.raw + .create_render_pass(&vk_info, None) + .map_err(super::map_host_device_oom_err)? + }; *e.insert(raw) } @@ -322,6 +326,10 @@ impl gpu_alloc::MemoryDevice for super::DeviceShared { Err(vk::Result::ERROR_OUT_OF_HOST_MEMORY) => { Err(gpu_alloc::OutOfMemory::OutOfHostMemory) } + // We don't use VK_KHR_external_memory + // VK_ERROR_INVALID_EXTERNAL_HANDLE + // We don't use VK_KHR_buffer_device_address + // VK_ERROR_INVALID_OPAQUE_CAPTURE_ADDRESS_KHR Err(err) => handle_unexpected(err), } } @@ -598,17 +606,20 @@ impl super::Device { Ok(swapchain) => swapchain, Err(error) => { return Err(match error { - vk::Result::ERROR_SURFACE_LOST_KHR => crate::SurfaceError::Lost, + vk::Result::ERROR_SURFACE_LOST_KHR + | vk::Result::ERROR_INITIALIZATION_FAILED => crate::SurfaceError::Lost, vk::Result::ERROR_NATIVE_WINDOW_IN_USE_KHR => { crate::SurfaceError::Other("Native window is in use") } - other => crate::DeviceError::from(other).into(), - }) + // We don't use VK_EXT_image_compression_control + // VK_ERROR_COMPRESSION_EXHAUSTED_EXT + other => super::map_host_device_oom_and_lost_err(other).into(), + }); } }; let images = - unsafe { functor.get_swapchain_images(raw) }.map_err(crate::DeviceError::from)?; + unsafe { functor.get_swapchain_images(raw) }.map_err(super::map_host_device_oom_err)?; // NOTE: It's important that we define at least images.len() wait // semaphores, since we prospectively need to provide the call to @@ -695,8 +706,16 @@ impl super::Device { let raw = unsafe { profiling::scope!("vkCreateShaderModule"); - self.shared.raw.create_shader_module(&vk_info, None)? + self.shared + .raw + .create_shader_module(&vk_info, None) + .map_err(map_err)? }; + fn map_err(err: vk::Result) -> crate::DeviceError { + // We don't use VK_NV_glsl_shader + // VK_ERROR_INVALID_SHADER_NV + super::map_host_device_oom_err(err) + } Ok(raw) } @@ -852,7 +871,12 @@ impl crate::Device for super::Device { .usage(conv::map_buffer_usage(desc.usage)) .sharing_mode(vk::SharingMode::EXCLUSIVE); - let raw = unsafe { self.shared.raw.create_buffer(&vk_info, None)? }; + let raw = unsafe { + self.shared + .raw + .create_buffer(&vk_info, None) + .map_err(super::map_host_device_oom_and_ioca_err)? + }; let req = unsafe { self.shared.raw.get_buffer_memory_requirements(raw) }; let mut alloc_usage = if desc @@ -902,7 +926,8 @@ impl crate::Device for super::Device { unsafe { self.shared .raw - .bind_buffer_memory(raw, *block.memory(), block.offset())? + .bind_buffer_memory(raw, *block.memory(), block.offset()) + .map_err(super::map_host_device_oom_and_ioca_err)? }; if let Some(label) = desc.label { @@ -1035,7 +1060,17 @@ impl crate::Device for super::Device { vk_info = vk_info.push_next(&mut format_list_info); } - let raw = unsafe { self.shared.raw.create_image(&vk_info, None)? }; + let raw = unsafe { + self.shared + .raw + .create_image(&vk_info, None) + .map_err(map_err)? + }; + fn map_err(err: vk::Result) -> crate::DeviceError { + // We don't use VK_EXT_image_compression_control + // VK_ERROR_COMPRESSION_EXHAUSTED_EXT + super::map_host_device_oom_and_ioca_err(err) + } let req = unsafe { self.shared.raw.get_image_memory_requirements(raw) }; let block = unsafe { @@ -1055,7 +1090,8 @@ impl crate::Device for super::Device { unsafe { self.shared .raw - .bind_image_memory(raw, *block.memory(), block.offset())? + .bind_image_memory(raw, *block.memory(), block.offset()) + .map_err(super::map_host_device_oom_err)? }; if let Some(label) = desc.label { @@ -1113,7 +1149,8 @@ impl crate::Device for super::Device { texture.usage }; - let raw = unsafe { self.shared.raw.create_image_view(&vk_info, None) }?; + let raw = unsafe { self.shared.raw.create_image_view(&vk_info, None) } + .map_err(super::map_host_device_oom_and_ioca_err)?; if let Some(label) = desc.label { unsafe { self.shared.set_object_name(raw, label) }; @@ -1191,7 +1228,12 @@ impl crate::Device for super::Device { vk_info = vk_info.border_color(conv::map_border_color(color)); } - let raw = unsafe { self.shared.raw.create_sampler(&vk_info, None)? }; + let raw = unsafe { + self.shared + .raw + .create_sampler(&vk_info, None) + .map_err(super::map_host_device_oom_and_ioca_err)? + }; if let Some(label) = desc.label { unsafe { self.shared.set_object_name(raw, label) }; @@ -1214,7 +1256,13 @@ impl crate::Device for super::Device { let vk_info = vk::CommandPoolCreateInfo::default() .queue_family_index(desc.queue.family_index) .flags(vk::CommandPoolCreateFlags::TRANSIENT); - let raw = unsafe { self.shared.raw.create_command_pool(&vk_info, None)? }; + + let raw = unsafe { + self.shared + .raw + .create_command_pool(&vk_info, None) + .map_err(super::map_host_device_oom_err)? + }; self.counters.command_encoders.add(1); @@ -1353,7 +1401,8 @@ impl crate::Device for super::Device { let raw = unsafe { self.shared .raw - .create_descriptor_set_layout(&vk_info, None)? + .create_descriptor_set_layout(&vk_info, None) + .map_err(super::map_host_device_oom_err)? }; if let Some(label) = desc.label { @@ -1406,7 +1455,12 @@ impl crate::Device for super::Device { let raw = { profiling::scope!("vkCreatePipelineLayout"); - unsafe { self.shared.raw.create_pipeline_layout(&vk_info, None)? } + unsafe { + self.shared + .raw + .create_pipeline_layout(&vk_info, None) + .map_err(super::map_host_device_oom_err)? + } }; if let Some(label) = desc.label { @@ -1929,7 +1983,7 @@ impl crate::Device for super::Device { self.shared .raw .create_graphics_pipelines(pipeline_cache, &vk_infos, None) - .map_err(|(_, e)| crate::DeviceError::from(e)) + .map_err(|(_, e)| super::map_pipeline_err(e)) }? }; @@ -1991,7 +2045,7 @@ impl crate::Device for super::Device { self.shared .raw .create_compute_pipelines(pipeline_cache, &vk_infos, None) - .map_err(|(_, e)| crate::DeviceError::from(e)) + .map_err(|(_, e)| super::map_pipeline_err(e)) }? }; @@ -2025,7 +2079,7 @@ impl crate::Device for super::Device { } profiling::scope!("vkCreatePipelineCache"); let raw = unsafe { self.shared.raw.create_pipeline_cache(&info, None) } - .map_err(crate::DeviceError::from)?; + .map_err(super::map_host_device_oom_err)?; Ok(super::PipelineCache { raw }) } @@ -2059,7 +2113,8 @@ impl crate::Device for super::Device { .query_count(desc.count) .pipeline_statistics(pipeline_statistics); - let raw = unsafe { self.shared.raw.create_query_pool(&vk_info, None) }?; + let raw = unsafe { self.shared.raw.create_query_pool(&vk_info, None) } + .map_err(super::map_host_device_oom_err)?; if let Some(label) = desc.label { unsafe { self.shared.set_object_name(raw, label) }; } @@ -2082,7 +2137,8 @@ impl crate::Device for super::Device { let mut sem_type_info = vk::SemaphoreTypeCreateInfo::default().semaphore_type(vk::SemaphoreType::TIMELINE); let vk_info = vk::SemaphoreCreateInfo::default().push_next(&mut sem_type_info); - let raw = unsafe { self.shared.raw.create_semaphore(&vk_info, None) }?; + let raw = unsafe { self.shared.raw.create_semaphore(&vk_info, None) } + .map_err(super::map_host_device_oom_err)?; super::Fence::TimelineSemaphore(raw) } else { @@ -2326,7 +2382,11 @@ impl crate::Device for super::Device { .sharing_mode(vk::SharingMode::EXCLUSIVE); unsafe { - let raw_buffer = self.shared.raw.create_buffer(&vk_buffer_info, None)?; + let raw_buffer = self + .shared + .raw + .create_buffer(&vk_buffer_info, None) + .map_err(super::map_host_device_oom_and_ioca_err)?; let req = self.shared.raw.get_buffer_memory_requirements(raw_buffer); let block = self.mem_allocator.lock().alloc( @@ -2341,7 +2401,8 @@ impl crate::Device for super::Device { self.shared .raw - .bind_buffer_memory(raw_buffer, *block.memory(), block.offset())?; + .bind_buffer_memory(raw_buffer, *block.memory(), block.offset()) + .map_err(super::map_host_device_oom_and_ioca_err)?; if let Some(label) = desc.label { self.shared.set_object_name(raw_buffer, label); @@ -2355,7 +2416,8 @@ impl crate::Device for super::Device { let raw_acceleration_structure = ray_tracing_functions .acceleration_structure - .create_acceleration_structure(&vk_info, None)?; + .create_acceleration_structure(&vk_info, None) + .map_err(super::map_host_oom_and_ioca_err)?; if let Some(label) = desc.label { self.shared @@ -2408,7 +2470,7 @@ impl super::DeviceShared { unsafe { self.raw .create_semaphore(&vk::SemaphoreCreateInfo::default(), None) - .map_err(crate::DeviceError::from) + .map_err(super::map_host_device_oom_err) } } @@ -2438,7 +2500,7 @@ impl super::DeviceShared { match result { Ok(()) => Ok(true), Err(vk::Result::TIMEOUT) => Ok(false), - Err(other) => Err(other.into()), + Err(other) => Err(super::map_host_device_oom_and_lost_err(other)), } } super::Fence::FencePool { @@ -2454,7 +2516,7 @@ impl super::DeviceShared { match unsafe { self.raw.wait_for_fences(&[raw], true, timeout_ns) } { Ok(()) => Ok(true), Err(vk::Result::TIMEOUT) => Ok(false), - Err(other) => Err(other.into()), + Err(other) => Err(super::map_host_device_oom_and_lost_err(other)), } } None => { diff --git a/wgpu-hal/src/vulkan/instance.rs b/wgpu-hal/src/vulkan/instance.rs index 1d7386e623..832c74b030 100644 --- a/wgpu-hal/src/vulkan/instance.rs +++ b/wgpu-hal/src/vulkan/instance.rs @@ -161,7 +161,11 @@ impl super::Swapchain { 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() }; + let _ = unsafe { + device + .device_wait_idle() + .map_err(super::map_host_device_oom_and_lost_err) + }; }; // We cannot take this by value, as the function returns `self`. @@ -1046,8 +1050,10 @@ impl crate::Surface for super::Surface { Err(crate::SurfaceError::Outdated) } vk::Result::ERROR_SURFACE_LOST_KHR => Err(crate::SurfaceError::Lost), - other => Err(crate::DeviceError::from(other).into()), - } + // We don't use VK_EXT_full_screen_exclusive + // VK_ERROR_FULL_SCREEN_EXCLUSIVE_MODE_LOST_EXT + other => Err(super::map_host_device_oom_and_lost_err(other).into()), + }; } }; diff --git a/wgpu-hal/src/vulkan/mod.rs b/wgpu-hal/src/vulkan/mod.rs index 8757671b44..aa4b50f29c 100644 --- a/wgpu-hal/src/vulkan/mod.rs +++ b/wgpu-hal/src/vulkan/mod.rs @@ -940,7 +940,11 @@ impl Fence { ) -> Result { for &(value, raw) in active.iter() { unsafe { - if value > last_completed && device.get_fence_status(raw)? { + if value > last_completed + && device + .get_fence_status(raw) + .map_err(map_host_device_oom_and_lost_err)? + { last_completed = value; } } @@ -959,8 +963,12 @@ impl Fence { match *self { Self::TimelineSemaphore(raw) => unsafe { Ok(match *extension.unwrap() { - ExtensionFn::Extension(ref ext) => ext.get_semaphore_counter_value(raw)?, - ExtensionFn::Promoted => device.get_semaphore_counter_value(raw)?, + ExtensionFn::Extension(ref ext) => ext + .get_semaphore_counter_value(raw) + .map_err(map_host_device_oom_and_lost_err)?, + ExtensionFn::Promoted => device + .get_semaphore_counter_value(raw) + .map_err(map_host_device_oom_and_lost_err)?, }) }, Self::FencePool { @@ -1000,7 +1008,8 @@ impl Fence { } if free.len() != base_free { active.retain(|&(value, _)| value > latest); - unsafe { device.reset_fences(&free[base_free..]) }? + unsafe { device.reset_fences(&free[base_free..]) } + .map_err(map_device_oom_err)? } *last_completed = latest; } @@ -1095,7 +1104,8 @@ impl crate::Queue for Queue { None => unsafe { self.device .raw - .create_fence(&vk::FenceCreateInfo::default(), None)? + .create_fence(&vk::FenceCreateInfo::default(), None) + .map_err(map_host_device_oom_err)? }, }; active.push((signal_value, fence_raw)); @@ -1126,7 +1136,8 @@ impl crate::Queue for Queue { unsafe { self.device .raw - .queue_submit(self.raw, &[vk_info], fence_raw)? + .queue_submit(self.raw, &[vk_info], fence_raw) + .map_err(map_host_device_oom_and_lost_err)? }; Ok(()) } @@ -1153,7 +1164,9 @@ impl crate::Queue for Queue { match error { vk::Result::ERROR_OUT_OF_DATE_KHR => crate::SurfaceError::Outdated, vk::Result::ERROR_SURFACE_LOST_KHR => crate::SurfaceError::Lost, - _ => crate::DeviceError::from(error).into(), + // We don't use VK_EXT_full_screen_exclusive + // VK_ERROR_FULL_SCREEN_EXCLUSIVE_MODE_LOST_EXT + _ => map_host_device_oom_and_lost_err(error).into(), } })? }; @@ -1173,33 +1186,116 @@ impl crate::Queue for Queue { } } -impl From for crate::DeviceError { - fn from(result: vk::Result) -> Self { - #![allow(unreachable_code)] - match result { - vk::Result::ERROR_OUT_OF_HOST_MEMORY | vk::Result::ERROR_OUT_OF_DEVICE_MEMORY => { - #[cfg(feature = "oom_panic")] - panic!("Out of memory ({result:?})"); +/// Maps +/// +/// - VK_ERROR_OUT_OF_HOST_MEMORY +/// - VK_ERROR_OUT_OF_DEVICE_MEMORY +fn map_host_device_oom_err(err: vk::Result) -> crate::DeviceError { + match err { + vk::Result::ERROR_OUT_OF_HOST_MEMORY | vk::Result::ERROR_OUT_OF_DEVICE_MEMORY => { + get_oom_err(err) + } + e => get_unexpected_err(e), + } +} - Self::OutOfMemory - } - vk::Result::ERROR_DEVICE_LOST => { - #[cfg(feature = "device_lost_panic")] - panic!("Device lost"); +/// Maps +/// +/// - VK_ERROR_OUT_OF_HOST_MEMORY +/// - VK_ERROR_OUT_OF_DEVICE_MEMORY +/// - VK_ERROR_DEVICE_LOST +fn map_host_device_oom_and_lost_err(err: vk::Result) -> crate::DeviceError { + match err { + vk::Result::ERROR_DEVICE_LOST => get_lost_err(), + other => map_host_device_oom_err(other), + } +} - Self::Lost - } - _ => { - #[cfg(feature = "internal_error_panic")] - panic!("Internal error: {result:?}"); +/// Maps +/// +/// - VK_ERROR_OUT_OF_HOST_MEMORY +/// - VK_ERROR_OUT_OF_DEVICE_MEMORY +/// - VK_ERROR_INVALID_OPAQUE_CAPTURE_ADDRESS_KHR +fn map_host_device_oom_and_ioca_err(err: vk::Result) -> crate::DeviceError { + // We don't use VK_KHR_buffer_device_address + // VK_ERROR_INVALID_OPAQUE_CAPTURE_ADDRESS_KHR + map_host_device_oom_err(err) +} - log::warn!("Unrecognized device error {result:?}"); - Self::Lost - } - } +/// Maps +/// +/// - VK_ERROR_OUT_OF_HOST_MEMORY +fn map_host_oom_err(err: vk::Result) -> crate::DeviceError { + match err { + vk::Result::ERROR_OUT_OF_HOST_MEMORY => get_oom_err(err), + e => get_unexpected_err(e), } } +/// Maps +/// +/// - VK_ERROR_OUT_OF_DEVICE_MEMORY +fn map_device_oom_err(err: vk::Result) -> crate::DeviceError { + match err { + vk::Result::ERROR_OUT_OF_DEVICE_MEMORY => get_oom_err(err), + e => get_unexpected_err(e), + } +} + +/// Maps +/// +/// - VK_ERROR_OUT_OF_HOST_MEMORY +/// - VK_ERROR_INVALID_OPAQUE_CAPTURE_ADDRESS_KHR +fn map_host_oom_and_ioca_err(err: vk::Result) -> crate::DeviceError { + // We don't use VK_KHR_buffer_device_address + // VK_ERROR_INVALID_OPAQUE_CAPTURE_ADDRESS_KHR + map_host_oom_err(err) +} + +/// Maps +/// +/// - VK_ERROR_OUT_OF_HOST_MEMORY +/// - VK_ERROR_OUT_OF_DEVICE_MEMORY +/// - VK_PIPELINE_COMPILE_REQUIRED_EXT +/// - VK_ERROR_INVALID_SHADER_NV +fn map_pipeline_err(err: vk::Result) -> crate::DeviceError { + // We don't use VK_EXT_pipeline_creation_cache_control + // VK_PIPELINE_COMPILE_REQUIRED_EXT + // We don't use VK_NV_glsl_shader + // VK_ERROR_INVALID_SHADER_NV + map_host_device_oom_err(err) +} + +/// Returns [`crate::DeviceError::Lost`] or panics if the `internal_error_panic` +/// feature flag is enabled. +fn get_unexpected_err(_err: vk::Result) -> crate::DeviceError { + #[cfg(feature = "internal_error_panic")] + panic!("Unexpected Vulkan error: {_err:?}"); + + #[allow(unreachable_code)] + crate::DeviceError::Lost +} + +/// Returns [`crate::DeviceError::OutOfMemory`] or panics if the `oom_panic` +/// feature flag is enabled. +fn get_oom_err(_err: vk::Result) -> crate::DeviceError { + #[cfg(feature = "oom_panic")] + panic!("Out of memory ({_err:?})"); + + #[allow(unreachable_code)] + crate::DeviceError::OutOfMemory +} + +/// Returns [`crate::DeviceError::Lost`] or panics if the `device_lost_panic` +/// feature flag is enabled. +fn get_lost_err() -> crate::DeviceError { + #[cfg(feature = "device_lost_panic")] + panic!("Device lost"); + + #[allow(unreachable_code)] + crate::DeviceError::Lost +} + fn hal_usage_error(txt: T) -> ! { panic!("wgpu-hal invariant was violated (usage error): {txt}") } From 2c31414517c895a6225b1e72a2840484fe4d4a2a Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Thu, 15 Aug 2024 10:25:36 +0100 Subject: [PATCH 007/141] add an `Unexpected` variant to `DeviceError` --- wgpu-core/src/device/mod.rs | 1 + wgpu-core/src/instance.rs | 1 + wgpu-hal/src/lib.rs | 2 ++ wgpu-hal/src/vulkan/device.rs | 2 +- wgpu-hal/src/vulkan/mod.rs | 4 ++-- 5 files changed, 7 insertions(+), 3 deletions(-) diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index ac35ec7530..777dd262ab 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -404,6 +404,7 @@ impl From for DeviceError { hal::DeviceError::Lost => DeviceError::Lost, hal::DeviceError::OutOfMemory => DeviceError::OutOfMemory, hal::DeviceError::ResourceCreationFailed => DeviceError::ResourceCreationFailed, + hal::DeviceError::Unexpected => DeviceError::Lost, } } } diff --git a/wgpu-core/src/instance.rs b/wgpu-core/src/instance.rs index a71117cfe1..d23c6a65e8 100644 --- a/wgpu-core/src/instance.rs +++ b/wgpu-core/src/instance.rs @@ -342,6 +342,7 @@ impl Adapter { hal::DeviceError::Lost => RequestDeviceError::DeviceLost, hal::DeviceError::OutOfMemory => RequestDeviceError::OutOfMemory, hal::DeviceError::ResourceCreationFailed => RequestDeviceError::Internal, + hal::DeviceError::Unexpected => RequestDeviceError::DeviceLost, })?; self.create_device_and_queue_from_hal(open, desc, instance_flags, trace_path) diff --git a/wgpu-hal/src/lib.rs b/wgpu-hal/src/lib.rs index 8f7db88450..b62a6b5962 100644 --- a/wgpu-hal/src/lib.rs +++ b/wgpu-hal/src/lib.rs @@ -314,6 +314,8 @@ pub enum DeviceError { Lost, #[error("Creation of a resource failed for a reason other than running out of memory.")] ResourceCreationFailed, + #[error("Unexpected error variant (driver implementation is at fault)")] + Unexpected, } #[derive(Clone, Debug, Eq, PartialEq, Error)] diff --git a/wgpu-hal/src/vulkan/device.rs b/wgpu-hal/src/vulkan/device.rs index ef02899a1d..689bc1b3ee 100644 --- a/wgpu-hal/src/vulkan/device.rs +++ b/wgpu-hal/src/vulkan/device.rs @@ -2559,7 +2559,7 @@ impl From for crate::DeviceError { } } -/// We usually map unexpected vulkan errors to the [`crate::DeviceError::Lost`] +/// We usually map unexpected vulkan errors to the [`crate::DeviceError::Unexpected`] /// variant to be more robust even in cases where the driver is not /// complying with the spec. /// diff --git a/wgpu-hal/src/vulkan/mod.rs b/wgpu-hal/src/vulkan/mod.rs index aa4b50f29c..3f3bd557e4 100644 --- a/wgpu-hal/src/vulkan/mod.rs +++ b/wgpu-hal/src/vulkan/mod.rs @@ -1266,14 +1266,14 @@ fn map_pipeline_err(err: vk::Result) -> crate::DeviceError { map_host_device_oom_err(err) } -/// Returns [`crate::DeviceError::Lost`] or panics if the `internal_error_panic` +/// Returns [`crate::DeviceError::Unexpected`] or panics if the `internal_error_panic` /// feature flag is enabled. fn get_unexpected_err(_err: vk::Result) -> crate::DeviceError { #[cfg(feature = "internal_error_panic")] panic!("Unexpected Vulkan error: {_err:?}"); #[allow(unreachable_code)] - crate::DeviceError::Lost + crate::DeviceError::Unexpected } /// Returns [`crate::DeviceError::OutOfMemory`] or panics if the `oom_panic` From 89a64e911da44b0783cec965b07fcb057d2ad3dc Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Thu, 15 Aug 2024 12:01:39 +0100 Subject: [PATCH 008/141] refactor(hal): remove unneeded `trivial_casts` suppr. --- wgpu-hal/src/dx12/adapter.rs | 2 -- wgpu-hal/src/dx12/instance.rs | 1 - wgpu-hal/src/gles/egl.rs | 1 - wgpu-hal/src/vulkan/adapter.rs | 1 - 4 files changed, 5 deletions(-) diff --git a/wgpu-hal/src/dx12/adapter.rs b/wgpu-hal/src/dx12/adapter.rs index 72b9d04b71..f77c238d7c 100644 --- a/wgpu-hal/src/dx12/adapter.rs +++ b/wgpu-hal/src/dx12/adapter.rs @@ -47,7 +47,6 @@ impl super::Adapter { &self.raw } - #[allow(trivial_casts)] pub(super) fn expose( adapter: d3d12::DxgiAdapter, library: &Arc, @@ -554,7 +553,6 @@ impl crate::Adapter for super::Adapter { }) } - #[allow(trivial_casts)] unsafe fn texture_format_capabilities( &self, format: wgt::TextureFormat, diff --git a/wgpu-hal/src/dx12/instance.rs b/wgpu-hal/src/dx12/instance.rs index c9557355fb..880c567744 100644 --- a/wgpu-hal/src/dx12/instance.rs +++ b/wgpu-hal/src/dx12/instance.rs @@ -75,7 +75,6 @@ impl crate::Instance for super::Instance { }; let mut supports_allow_tearing = false; - #[allow(trivial_casts)] if let Some(factory5) = factory.as_factory5() { let mut allow_tearing: minwindef::BOOL = minwindef::FALSE; let hr = unsafe { diff --git a/wgpu-hal/src/gles/egl.rs b/wgpu-hal/src/gles/egl.rs index 9a8639d5a8..89baa52a6f 100644 --- a/wgpu-hal/src/gles/egl.rs +++ b/wgpu-hal/src/gles/egl.rs @@ -1241,7 +1241,6 @@ impl crate::Surface for Surface { None => { let mut wl_window = None; let (mut temp_xlib_handle, mut temp_xcb_handle); - #[allow(trivial_casts)] let native_window_ptr = match (self.wsi.kind, self.raw_window_handle) { (WindowKind::Unknown | WindowKind::X11, Rwh::Xlib(handle)) => { temp_xlib_handle = handle.window; diff --git a/wgpu-hal/src/vulkan/adapter.rs b/wgpu-hal/src/vulkan/adapter.rs index 0a4e73382a..f323456eaa 100644 --- a/wgpu-hal/src/vulkan/adapter.rs +++ b/wgpu-hal/src/vulkan/adapter.rs @@ -1099,7 +1099,6 @@ impl PhysicalDeviceProperties { } impl super::InstanceShared { - #[allow(trivial_casts)] // false positives fn inspect( &self, phd: vk::PhysicalDevice, From 23e78464005e98b07947ee37a14d8e5b29ccf35c Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Thu, 15 Aug 2024 12:01:39 +0100 Subject: [PATCH 009/141] refactor(hal): satisfy `trivial_casts` --- naga/src/back/msl/writer.rs | 8 ++++---- wgpu-hal/src/dynamic/device.rs | 26 ++++++++++++-------------- wgpu-hal/src/dynamic/instance.rs | 5 +---- wgpu-hal/src/metal/surface.rs | 5 ++--- wgpu-hal/src/vulkan/device.rs | 5 +---- 5 files changed, 20 insertions(+), 29 deletions(-) diff --git a/naga/src/back/msl/writer.rs b/naga/src/back/msl/writer.rs index 48f862f8ba..e0b3d31e84 100644 --- a/naga/src/back/msl/writer.rs +++ b/naga/src/back/msl/writer.rs @@ -6,6 +6,8 @@ use crate::{ proc::{self, NameKey, TypeResolution}, valid, FastHashMap, FastHashSet, }; +#[cfg(test)] +use std::ptr; use std::{ fmt::{Display, Error as FmtError, Formatter, Write}, iter, @@ -1411,9 +1413,8 @@ impl Writer { ) -> BackendResult { // Add to the set in order to track the stack size. #[cfg(test)] - #[allow(trivial_casts)] self.put_expression_stack_pointers - .insert(&expr_handle as *const _ as *const ()); + .insert(ptr::from_ref(&expr_handle).cast()); if let Some(name) = self.named_expressions.get(&expr_handle) { write!(self.out, "{name}")?; @@ -2792,9 +2793,8 @@ impl Writer { ) -> BackendResult { // Add to the set in order to track the stack size. #[cfg(test)] - #[allow(trivial_casts)] self.put_block_stack_pointers - .insert(&level as *const _ as *const ()); + .insert(ptr::from_ref(&level).cast()); for statement in statements { log::trace!("statement[{}] {:?}", level.0, statement); diff --git a/wgpu-hal/src/dynamic/device.rs b/wgpu-hal/src/dynamic/device.rs index c1baf5b76d..1386196d60 100644 --- a/wgpu-hal/src/dynamic/device.rs +++ b/wgpu-hal/src/dynamic/device.rs @@ -1,6 +1,3 @@ -// Box casts are needed, alternative would be a temporaries which are more verbose and not more expressive. -#![allow(trivial_casts)] - use crate::{ AccelerationStructureBuildSizes, AccelerationStructureDescriptor, Api, BindGroupDescriptor, BindGroupLayoutDescriptor, BufferDescriptor, BufferMapping, CommandEncoderDescriptor, @@ -261,7 +258,7 @@ impl DynDevice for D { queue: desc.queue.expect_downcast_ref(), }; unsafe { D::create_command_encoder(self, &desc) } - .map(|b| Box::new(b) as Box) + .map(|b| -> Box { Box::new(b) }) } unsafe fn destroy_command_encoder(&self, encoder: Box) { @@ -273,7 +270,7 @@ impl DynDevice for D { desc: &BindGroupLayoutDescriptor, ) -> Result, DeviceError> { unsafe { D::create_bind_group_layout(self, desc) } - .map(|b| Box::new(b) as Box) + .map(|b| -> Box { Box::new(b) }) } unsafe fn destroy_bind_group_layout(&self, bg_layout: Box) { @@ -297,7 +294,7 @@ impl DynDevice for D { }; unsafe { D::create_pipeline_layout(self, &desc) } - .map(|b| Box::new(b) as Box) + .map(|b| -> Box { Box::new(b) }) } unsafe fn destroy_pipeline_layout(&self, pipeline_layout: Box) { @@ -345,7 +342,8 @@ impl DynDevice for D { acceleration_structures: &acceleration_structures, }; - unsafe { D::create_bind_group(self, &desc) }.map(|b| Box::new(b) as Box) + unsafe { D::create_bind_group(self, &desc) } + .map(|b| -> Box { Box::new(b) }) } unsafe fn destroy_bind_group(&self, group: Box) { @@ -358,7 +356,7 @@ impl DynDevice for D { shader: ShaderInput, ) -> Result, ShaderError> { unsafe { D::create_shader_module(self, desc, shader) } - .map(|b| Box::new(b) as Box) + .map(|b| -> Box { Box::new(b) }) } unsafe fn destroy_shader_module(&self, module: Box) { @@ -388,7 +386,7 @@ impl DynDevice for D { }; unsafe { D::create_render_pipeline(self, &desc) } - .map(|b| Box::new(b) as Box) + .map(|b| -> Box { Box::new(b) }) } unsafe fn destroy_render_pipeline(&self, pipeline: Box) { @@ -411,7 +409,7 @@ impl DynDevice for D { }; unsafe { D::create_compute_pipeline(self, &desc) } - .map(|b| Box::new(b) as Box) + .map(|b| -> Box { Box::new(b) }) } unsafe fn destroy_compute_pipeline(&self, pipeline: Box) { @@ -423,7 +421,7 @@ impl DynDevice for D { desc: &PipelineCacheDescriptor<'_>, ) -> Result, PipelineCacheError> { unsafe { D::create_pipeline_cache(self, desc) } - .map(|b| Box::new(b) as Box) + .map(|b| -> Box { Box::new(b) }) } fn pipeline_cache_validation_key(&self) -> Option<[u8; 16]> { @@ -438,7 +436,7 @@ impl DynDevice for D { &self, desc: &wgt::QuerySetDescriptor