From 625afc3b4299c92c79e0b3881e22a0bb780678ed Mon Sep 17 00:00:00 2001 From: Rajesh Malviya Date: Thu, 31 Aug 2023 03:28:47 +0530 Subject: [PATCH 01/33] Drop texture `clear_view`s in surface_texture_discard (#4057) --- CHANGELOG.md | 1 + wgpu-core/src/present.rs | 16 +++++++--------- wgpu-core/src/resource.rs | 12 ++++++++++++ 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e168ebe856..fa3dba8f4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -89,6 +89,7 @@ By @Valaphee in [#3402](https://github.com/gfx-rs/wgpu/pull/3402) - Derive storage bindings via `naga::StorageAccess` instead of `naga::GlobalUse`. By @teoxoy in [#3985](https://github.com/gfx-rs/wgpu/pull/3985). - `Queue::on_submitted_work_done` callbacks will now always be called after all previous `BufferSlice::map_async` callbacks, even when there are no active submissions. By @cwfitzgerald in [#4036](https://github.com/gfx-rs/wgpu/pull/4036). +- Fix `clear` texture views being leaked when `wgpu::SurfaceTexture` is dropped before it is presented. By @rajveermalviya in [#4057](https://github.com/gfx-rs/wgpu/pull/4057). #### Vulkan - Fix enabling `wgpu::Features::PARTIALLY_BOUND_BINDING_ARRAY` not being actually enabled in vulkan backend. By @39ali in[#3772](https://github.com/gfx-rs/wgpu/pull/3772). diff --git a/wgpu-core/src/present.rs b/wgpu-core/src/present.rs index c9df46ad93..1303769d29 100644 --- a/wgpu-core/src/present.rs +++ b/wgpu-core/src/present.rs @@ -300,15 +300,7 @@ impl Global { let (texture, _) = hub.textures.unregister(texture_id.value.0, &mut token); if let Some(texture) = texture { - if let resource::TextureClearMode::RenderPass { clear_views, .. } = - texture.clear_mode - { - for clear_view in clear_views { - unsafe { - hal::Device::destroy_texture_view(&device.raw, clear_view); - } - } - } + texture.clear_mode.destroy_clear_views(&device.raw); let suf = A::get_surface_mut(surface); match texture.inner { @@ -386,10 +378,16 @@ impl Global { // The texture ID got added to the device tracker by `submit()`, // and now we are moving it away. + log::debug!( + "Removing swapchain texture {:?} from the device tracker", + texture_id.value + ); device.trackers.lock().textures.remove(texture_id.value); let (texture, _) = hub.textures.unregister(texture_id.value.0, &mut token); if let Some(texture) = texture { + texture.clear_mode.destroy_clear_views(&device.raw); + let suf = A::get_surface_mut(surface); match texture.inner { resource::TextureInner::Surface { diff --git a/wgpu-core/src/resource.rs b/wgpu-core/src/resource.rs index fe881c2d06..c0977b80ef 100644 --- a/wgpu-core/src/resource.rs +++ b/wgpu-core/src/resource.rs @@ -384,6 +384,18 @@ pub enum TextureClearMode { None, } +impl TextureClearMode { + pub(crate) fn destroy_clear_views(self, device: &A::Device) { + if let TextureClearMode::RenderPass { clear_views, .. } = self { + for clear_view in clear_views { + unsafe { + hal::Device::destroy_texture_view(device, clear_view); + } + } + } + } +} + #[derive(Debug)] pub struct Texture { pub(crate) inner: TextureInner, From 4a12ab73aeff562955b150a0b856e43956f477f2 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 31 Aug 2023 23:59:00 -0400 Subject: [PATCH 02/33] Bump profiling from 1.0.9 to 1.0.10 (#4102) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6890eeeee2..5a28ec8f7a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2127,9 +2127,9 @@ dependencies = [ [[package]] name = "profiling" -version = "1.0.9" +version = "1.0.10" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "46b2164ebdb1dfeec5e337be164292351e11daf63a05174c6776b2f47460f0c9" +checksum = "45f10e75d83c7aec79a6aa46f897075890e156b105eebe51cfa0abce51af025f" [[package]] name = "quote" From 41efabbd886d09163b7e340caee6bca84eae968d Mon Sep 17 00:00:00 2001 From: Patrik Buhring Date: Fri, 1 Sep 2023 01:46:00 -0400 Subject: [PATCH 03/33] Fix limits interface on web. (#4107) --- .cargo/config.toml | 7 --- CHANGELOG.md | 4 ++ wgpu/src/backend/web.rs | 135 +++++++++++++++++++++++++++++++--------- 3 files changed, 111 insertions(+), 35 deletions(-) delete mode 100644 .cargo/config.toml diff --git a/.cargo/config.toml b/.cargo/config.toml deleted file mode 100644 index 95d2a35175..0000000000 --- a/.cargo/config.toml +++ /dev/null @@ -1,7 +0,0 @@ -[alias] -xtask = "run --manifest-path xtask/Cargo.toml --" - -[build] -rustflags = [ -"--cfg=web_sys_unstable_apis" -] diff --git a/CHANGELOG.md b/CHANGELOG.md index fa3dba8f4f..c7c22fcb73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -107,6 +107,10 @@ By @Valaphee in [#3402](https://github.com/gfx-rs/wgpu/pull/3402) - Ensure that MTLCommandEncoder calls endEncoding before it is deallocated. By @bradwerth in [#4023](https://github.com/gfx-rs/wgpu/pull/4023) +#### WebGPU + +- Ensure that limit requests and reporting is done correctly. By @OptimisticPeach in [#4107](https://github.com/gfx-rs/wgpu/pull/4107) + ### Documentation - Add an overview of `RenderPass` and how render state works. By @kpreid in [#4055](https://github.com/gfx-rs/wgpu/pull/4055) diff --git a/wgpu/src/backend/web.rs b/wgpu/src/backend/web.rs index 97f5cb945d..1fc1c6683f 100644 --- a/wgpu/src/backend/web.rs +++ b/wgpu/src/backend/web.rs @@ -687,6 +687,99 @@ fn map_wgt_features(supported_features: web_sys::GpuSupportedFeatures) -> wgt::F features } +fn map_wgt_limits(limits: web_sys::GpuSupportedLimits) -> wgt::Limits { + wgt::Limits { + max_texture_dimension_1d: limits.max_texture_dimension_1d(), + max_texture_dimension_2d: limits.max_texture_dimension_2d(), + max_texture_dimension_3d: limits.max_texture_dimension_3d(), + max_texture_array_layers: limits.max_texture_array_layers(), + max_bind_groups: limits.max_bind_groups(), + max_bindings_per_bind_group: limits.max_bindings_per_bind_group(), + max_dynamic_uniform_buffers_per_pipeline_layout: limits + .max_dynamic_uniform_buffers_per_pipeline_layout(), + max_dynamic_storage_buffers_per_pipeline_layout: limits + .max_dynamic_storage_buffers_per_pipeline_layout(), + max_sampled_textures_per_shader_stage: limits.max_sampled_textures_per_shader_stage(), + max_samplers_per_shader_stage: limits.max_samplers_per_shader_stage(), + max_storage_buffers_per_shader_stage: limits.max_storage_buffers_per_shader_stage(), + max_storage_textures_per_shader_stage: limits.max_storage_textures_per_shader_stage(), + max_uniform_buffers_per_shader_stage: limits.max_uniform_buffers_per_shader_stage(), + max_uniform_buffer_binding_size: limits.max_uniform_buffer_binding_size() as u32, + max_storage_buffer_binding_size: limits.max_storage_buffer_binding_size() as u32, + max_vertex_buffers: limits.max_vertex_buffers(), + max_buffer_size: limits.max_buffer_size() as u64, + max_vertex_attributes: limits.max_vertex_attributes(), + max_vertex_buffer_array_stride: limits.max_vertex_buffer_array_stride(), + min_uniform_buffer_offset_alignment: limits.min_uniform_buffer_offset_alignment(), + min_storage_buffer_offset_alignment: limits.min_storage_buffer_offset_alignment(), + max_inter_stage_shader_components: limits.max_inter_stage_shader_components(), + max_compute_workgroup_storage_size: limits.max_compute_workgroup_storage_size(), + max_compute_invocations_per_workgroup: limits.max_compute_invocations_per_workgroup(), + max_compute_workgroup_size_x: limits.max_compute_workgroup_size_x(), + max_compute_workgroup_size_y: limits.max_compute_workgroup_size_y(), + max_compute_workgroup_size_z: limits.max_compute_workgroup_size_z(), + max_compute_workgroups_per_dimension: limits.max_compute_workgroups_per_dimension(), + // The following are not part of WebGPU + max_push_constant_size: wgt::Limits::default().max_push_constant_size, + max_non_sampler_bindings: wgt::Limits::default().max_non_sampler_bindings, + } +} + +fn map_js_sys_limits(limits: &wgt::Limits) -> js_sys::Object { + let object = js_sys::Object::new(); + + macro_rules! set_properties { + (($from:expr) => ($on:expr) : $(($js_ident:ident, $rs_ident:ident)),* $(,)?) => { + $( + ::js_sys::Reflect::set( + &$on, + &::wasm_bindgen::JsValue::from(stringify!($js_ident)), + // Numbers may be u64, however using `from` on a u64 yields + // errors on the wasm side, since it uses an unsupported api. + // Wasm sends us things that need to fit into u64s by sending + // us f64s instead. So we just send them f64s back. + &::wasm_bindgen::JsValue::from($from.$rs_ident as f64) + ) + .expect("Setting Object properties should never fail."); + )* + } + } + + set_properties![ + (limits) => (object): + (maxTextureDimension1D, max_texture_dimension_1d), + (maxTextureDimension2D, max_texture_dimension_2d), + (maxTextureDimension3D, max_texture_dimension_3d), + (maxTextureArrayLayers, max_texture_array_layers), + (maxBindGroups, max_bind_groups), + (maxBindingsPerBindGroup, max_bindings_per_bind_group), + (maxDynamicUniformBuffersPerPipelineLayout, max_dynamic_uniform_buffers_per_pipeline_layout), + (maxDynamicStorageBuffersPerPipelineLayout, max_dynamic_storage_buffers_per_pipeline_layout), + (maxSampledTexturesPerShaderStage, max_sampled_textures_per_shader_stage), + (maxSamplersPerShaderStage, max_samplers_per_shader_stage), + (maxStorageBuffersPerShaderStage, max_storage_buffers_per_shader_stage), + (maxStorageTexturesPerShaderStage, max_storage_textures_per_shader_stage), + (maxUniformBuffersPerShaderStage, max_uniform_buffers_per_shader_stage), + (maxUniformBufferBindingSize, max_uniform_buffer_binding_size), + (maxStorageBufferBindingSize, max_storage_buffer_binding_size), + (minUniformBufferOffsetAlignment, min_uniform_buffer_offset_alignment), + (minStorageBufferOffsetAlignment, min_storage_buffer_offset_alignment), + (maxVertexBuffers, max_vertex_buffers), + (maxBufferSize, max_buffer_size), + (maxVertexAttributes, max_vertex_attributes), + (maxVertexBufferArrayStride, max_vertex_buffer_array_stride), + (maxInterStageShaderComponents, max_inter_stage_shader_components), + (maxComputeWorkgroupStorageSize, max_compute_workgroup_storage_size), + (maxComputeInvocationsPerWorkgroup, max_compute_invocations_per_workgroup), + (maxComputeWorkgroupSizeX, max_compute_workgroup_size_x), + (maxComputeWorkgroupSizeY, max_compute_workgroup_size_y), + (maxComputeWorkgroupSizeZ, max_compute_workgroup_size_z), + (maxComputeWorkgroupsPerDimension, max_compute_workgroups_per_dimension), + ]; + + object +} + type JsFutureResult = Result; fn future_request_adapter( @@ -1014,9 +1107,19 @@ impl crate::context::Context for Context { //Error: Tracing isn't supported on the Web target } - // TODO: non-guaranteed limits let mut mapped_desc = web_sys::GpuDeviceDescriptor::new(); + // TODO: Migrate to a web_sys api. + // See https://github.com/rustwasm/wasm-bindgen/issues/3587 + let limits_object = map_js_sys_limits(&desc.limits); + + js_sys::Reflect::set( + &mapped_desc, + &JsValue::from("requiredLimits"), + &limits_object, + ) + .expect("Setting Object properties should never fail."); + let required_features = FEATURES_MAPPING .iter() .copied() @@ -1070,30 +1173,7 @@ impl crate::context::Context for Context { _adapter: &Self::AdapterId, adapter_data: &Self::AdapterData, ) -> wgt::Limits { - let limits = adapter_data.0.limits(); - wgt::Limits { - max_texture_dimension_1d: limits.max_texture_dimension_1d(), - max_texture_dimension_2d: limits.max_texture_dimension_2d(), - max_texture_dimension_3d: limits.max_texture_dimension_3d(), - max_texture_array_layers: limits.max_texture_array_layers(), - max_bind_groups: limits.max_bind_groups(), - max_bindings_per_bind_group: limits.max_bindings_per_bind_group(), - max_dynamic_uniform_buffers_per_pipeline_layout: limits - .max_dynamic_uniform_buffers_per_pipeline_layout(), - max_dynamic_storage_buffers_per_pipeline_layout: limits - .max_dynamic_storage_buffers_per_pipeline_layout(), - max_sampled_textures_per_shader_stage: limits.max_sampled_textures_per_shader_stage(), - max_samplers_per_shader_stage: limits.max_samplers_per_shader_stage(), - max_storage_buffers_per_shader_stage: limits.max_storage_buffers_per_shader_stage(), - max_storage_textures_per_shader_stage: limits.max_storage_textures_per_shader_stage(), - max_uniform_buffers_per_shader_stage: limits.max_uniform_buffers_per_shader_stage(), - max_uniform_buffer_binding_size: limits.max_uniform_buffer_binding_size() as u32, - max_storage_buffer_binding_size: limits.max_storage_buffer_binding_size() as u32, - max_vertex_buffers: limits.max_vertex_buffers(), - max_vertex_attributes: limits.max_vertex_attributes(), - max_vertex_buffer_array_stride: limits.max_vertex_buffer_array_stride(), - ..wgt::Limits::default() - } + map_wgt_limits(adapter_data.0.limits()) } fn adapter_downlevel_capabilities( @@ -1256,10 +1336,9 @@ impl crate::context::Context for Context { fn device_limits( &self, _device: &Self::DeviceId, - _device_data: &Self::DeviceData, + device_data: &Self::DeviceData, ) -> wgt::Limits { - // TODO - wgt::Limits::default() + map_wgt_limits(device_data.0.limits()) } fn device_downlevel_properties( From 332cd0325da52675432830870584ec9766679c34 Mon Sep 17 00:00:00 2001 From: Kevin Reid Date: Thu, 31 Aug 2023 22:48:31 -0700 Subject: [PATCH 04/33] Add details to `InstanceError` and `CreateSurfaceError`. (#4066) Co-authored-by: Connor Fitzgerald --- CHANGELOG.md | 3 ++ tests/src/lib.rs | 48 ++++++++--------- tests/tests/create_surface_error.rs | 28 ++++++++++ tests/tests/root.rs | 1 + wgpu-hal/examples/halmark/main.rs | 8 +-- wgpu-hal/src/auxil/dxgi/factory.rs | 42 ++++++++++----- wgpu-hal/src/dx11/instance.rs | 7 ++- wgpu-hal/src/dx12/instance.rs | 8 ++- wgpu-hal/src/gles/adapter.rs | 45 ++++++++-------- wgpu-hal/src/gles/egl.rs | 43 ++++++++++----- wgpu-hal/src/gles/web.rs | 16 +++--- wgpu-hal/src/lib.rs | 41 ++++++++++++-- wgpu-hal/src/metal/mod.rs | 4 +- wgpu-hal/src/vulkan/instance.rs | 67 +++++++++++++---------- wgpu/src/backend/direct.rs | 10 +--- wgpu/src/backend/web.rs | 17 ++++-- wgpu/src/lib.rs | 83 ++++++++++++++++++++++------- 17 files changed, 322 insertions(+), 149 deletions(-) create mode 100644 tests/tests/create_surface_error.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index c7c22fcb73..75e6554dcf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,9 +70,12 @@ By @Valaphee in [#3402](https://github.com/gfx-rs/wgpu/pull/3402) ### Changes +#### General + - Omit texture store bound checks since they are no-ops if out of bounds on all APIs. By @teoxoy in [#3975](https://github.com/gfx-rs/wgpu/pull/3975) - Validate `DownlevelFlags::READ_ONLY_DEPTH_STENCIL`. By @teoxoy in [#4031](https://github.com/gfx-rs/wgpu/pull/4031) - Add validation in accordance with WebGPU `setViewport` valid usage for `x`, `y` and `this.[[attachment_size]]`. By @James2022-rgb in [#4058](https://github.com/gfx-rs/wgpu/pull/4058) +- `wgpu::CreateSurfaceError` now gives details of the failure, but no longer implements `PartialEq`. By @kpreid in [#4066](https://github.com/gfx-rs/wgpu/pull/4066) - Make `WGPU_POWER_PREF=none` a valid value. By @fornwall in [4076](https://github.com/gfx-rs/wgpu/pull/4076) #### Vulkan diff --git a/tests/src/lib.rs b/tests/src/lib.rs index fb57d2a5a8..1d741f1812 100644 --- a/tests/src/lib.rs +++ b/tests/src/lib.rs @@ -312,7 +312,7 @@ pub fn initialize_test(parameters: TestParameters, test_function: impl FnOnce(Te if #[cfg(any(not(target_arch = "wasm32"), target_os = "emscripten"))] { let canary_set = wgpu::hal::VALIDATION_CANARY.get_and_reset(); } else { - let canary_set = _surface_guard.check_for_unreported_errors(); + let canary_set = _surface_guard.unwrap().check_for_unreported_errors(); } ); @@ -345,24 +345,18 @@ pub fn initialize_test(parameters: TestParameters, test_function: impl FnOnce(Te } } -fn initialize_adapter() -> (Adapter, SurfaceGuard) { - let backends = wgpu::util::backend_bits_from_env().unwrap_or_else(Backends::all); - let dx12_shader_compiler = wgpu::util::dx12_shader_compiler_from_env().unwrap_or_default(); - let gles_minor_version = wgpu::util::gles_minor_version_from_env().unwrap_or_default(); - let instance = Instance::new(wgpu::InstanceDescriptor { - backends, - dx12_shader_compiler, - gles_minor_version, - }); - let surface_guard; +fn initialize_adapter() -> (Adapter, Option) { + let instance = initialize_instance(); + let surface_guard: Option; let compatible_surface; + // Create a canvas iff we need a WebGL2RenderingContext to have a working device. #[cfg(not(all( target_arch = "wasm32", any(target_os = "emscripten", feature = "webgl") )))] { - surface_guard = SurfaceGuard {}; + surface_guard = None; compatible_surface = None; } #[cfg(all( @@ -398,7 +392,7 @@ fn initialize_adapter() -> (Adapter, SurfaceGuard) { .expect("could not create surface from canvas") }; - surface_guard = SurfaceGuard { canvas }; + surface_guard = Some(SurfaceGuard { canvas }); compatible_surface = Some(surface); } @@ -413,12 +407,21 @@ fn initialize_adapter() -> (Adapter, SurfaceGuard) { (adapter, surface_guard) } -struct SurfaceGuard { - #[cfg(all( - target_arch = "wasm32", - any(target_os = "emscripten", feature = "webgl") - ))] - canvas: web_sys::HtmlCanvasElement, +pub fn initialize_instance() -> Instance { + let backends = wgpu::util::backend_bits_from_env().unwrap_or_else(Backends::all); + let dx12_shader_compiler = wgpu::util::dx12_shader_compiler_from_env().unwrap_or_default(); + let gles_minor_version = wgpu::util::gles_minor_version_from_env().unwrap_or_default(); + Instance::new(wgpu::InstanceDescriptor { + backends, + dx12_shader_compiler, + gles_minor_version, + }) +} + +// Public because it is used by tests of interacting with canvas +pub struct SurfaceGuard { + #[cfg(target_arch = "wasm32")] + pub canvas: web_sys::HtmlCanvasElement, } impl SurfaceGuard { @@ -452,11 +455,8 @@ impl Drop for SurfaceGuard { } } -#[cfg(all( - target_arch = "wasm32", - any(target_os = "emscripten", feature = "webgl") -))] -fn create_html_canvas() -> web_sys::HtmlCanvasElement { +#[cfg(target_arch = "wasm32")] +pub fn create_html_canvas() -> web_sys::HtmlCanvasElement { use wasm_bindgen::JsCast; web_sys::window() diff --git a/tests/tests/create_surface_error.rs b/tests/tests/create_surface_error.rs new file mode 100644 index 0000000000..f8962697ce --- /dev/null +++ b/tests/tests/create_surface_error.rs @@ -0,0 +1,28 @@ +//! Test that `create_surface_*()` accurately reports those errors we can provoke. + +/// This test applies to those cfgs that have a `create_surface_from_canvas` method, which +/// include WebGL and WebGPU, but *not* Emscripten GLES. +#[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))] +#[wasm_bindgen_test::wasm_bindgen_test] +fn canvas_get_context_returned_null() { + // Not using initialize_test() because that goes straight to creating the canvas for us. + let instance = wgpu_test::initialize_instance(); + // Create canvas and cleanup on drop + let canvas_g = wgpu_test::SurfaceGuard { + canvas: wgpu_test::create_html_canvas(), + }; + // Using a context id that is not "webgl2" or "webgpu" will render the canvas unusable by wgpu. + canvas_g.canvas.get_context("2d").unwrap(); + + #[allow(clippy::redundant_clone)] // false positive — can't and shouldn't move out. + let error = instance + .create_surface_from_canvas(canvas_g.canvas.clone()) + .unwrap_err(); + + assert!( + error + .to_string() + .contains("canvas.getContext() returned null"), + "{error}" + ); +} diff --git a/tests/tests/root.rs b/tests/tests/root.rs index b376ab4981..25df8eda90 100644 --- a/tests/tests/root.rs +++ b/tests/tests/root.rs @@ -10,6 +10,7 @@ mod buffer; mod buffer_copy; mod buffer_usages; mod clear_texture; +mod create_surface_error; mod device; mod encoder; mod example_wgsl; diff --git a/wgpu-hal/examples/halmark/main.rs b/wgpu-hal/examples/halmark/main.rs index c6b739bf17..5518cdaf4b 100644 --- a/wgpu-hal/examples/halmark/main.rs +++ b/wgpu-hal/examples/halmark/main.rs @@ -86,7 +86,7 @@ struct Example { } impl Example { - fn init(window: &winit::window::Window) -> Result { + fn init(window: &winit::window::Window) -> Result> { let instance_desc = hal::InstanceDescriptor { name: "example", flags: if cfg!(debug_assertions) { @@ -108,13 +108,13 @@ impl Example { let (adapter, capabilities) = unsafe { let mut adapters = instance.enumerate_adapters(); if adapters.is_empty() { - return Err(hal::InstanceError); + return Err("no adapters found".into()); } let exposed = adapters.swap_remove(0); (exposed.adapter, exposed.capabilities) }; - let surface_caps = - unsafe { adapter.surface_capabilities(&surface) }.ok_or(hal::InstanceError)?; + let surface_caps = unsafe { adapter.surface_capabilities(&surface) } + .ok_or("failed to get surface capabilities")?; log::info!("Surface caps: {:#?}", surface_caps); let hal::OpenDevice { device, mut queue } = unsafe { diff --git a/wgpu-hal/src/auxil/dxgi/factory.rs b/wgpu-hal/src/auxil/dxgi/factory.rs index 123ca4933e..7ae6e745f0 100644 --- a/wgpu-hal/src/auxil/dxgi/factory.rs +++ b/wgpu-hal/src/auxil/dxgi/factory.rs @@ -96,7 +96,9 @@ pub fn create_factory( required_factory_type: DxgiFactoryType, instance_flags: crate::InstanceFlags, ) -> Result<(d3d12::DxgiLib, d3d12::DxgiFactory), crate::InstanceError> { - let lib_dxgi = d3d12::DxgiLib::new().map_err(|_| crate::InstanceError)?; + let lib_dxgi = d3d12::DxgiLib::new().map_err(|e| { + crate::InstanceError::with_source(String::from("failed to load dxgi.dll"), e) + })?; let mut factory_flags = d3d12::FactoryCreationFlags::empty(); @@ -128,18 +130,22 @@ pub fn create_factory( Ok(factory) => Some(factory), // We hard error here as we _should have_ been able to make a factory4 but couldn't. Err(err) => { - log::error!("Failed to create IDXGIFactory4: {}", err); - return Err(crate::InstanceError); + // err is a Cow, not an Error implementor + return Err(crate::InstanceError::new(format!( + "failed to create IDXGIFactory4: {err:?}" + ))); } }, // If we require factory4, hard error. Err(err) if required_factory_type == DxgiFactoryType::Factory4 => { - log::error!("IDXGIFactory1 creation function not found: {:?}", err); - return Err(crate::InstanceError); + return Err(crate::InstanceError::with_source( + String::from("IDXGIFactory1 creation function not found"), + err, + )); } // If we don't print it to info as all win7 will hit this case. Err(err) => { - log::info!("IDXGIFactory1 creation function not found: {:?}", err); + log::info!("IDXGIFactory1 creation function not found: {err:?}"); None } }; @@ -153,8 +159,10 @@ pub fn create_factory( } // If we require factory6, hard error. Err(err) if required_factory_type == DxgiFactoryType::Factory6 => { - log::warn!("Failed to cast IDXGIFactory4 to IDXGIFactory6: {:?}", err); - return Err(crate::InstanceError); + // err is a Cow, not an Error implementor + return Err(crate::InstanceError::new(format!( + "failed to cast IDXGIFactory4 to IDXGIFactory6: {err:?}" + ))); } // If we don't print it to info. Err(err) => { @@ -169,14 +177,18 @@ pub fn create_factory( Ok(pair) => match pair.into_result() { Ok(factory) => factory, Err(err) => { - log::error!("Failed to create IDXGIFactory1: {}", err); - return Err(crate::InstanceError); + // err is a Cow, not an Error implementor + return Err(crate::InstanceError::new(format!( + "failed to create IDXGIFactory1: {err:?}" + ))); } }, // We always require at least factory1, so hard error Err(err) => { - log::error!("IDXGIFactory1 creation function not found: {:?}", err); - return Err(crate::InstanceError); + return Err(crate::InstanceError::with_source( + String::from("IDXGIFactory1 creation function not found"), + err, + )); } }; @@ -188,8 +200,10 @@ pub fn create_factory( } // If we require factory2, hard error. Err(err) if required_factory_type == DxgiFactoryType::Factory2 => { - log::warn!("Failed to cast IDXGIFactory1 to IDXGIFactory2: {:?}", err); - return Err(crate::InstanceError); + // err is a Cow, not an Error implementor + return Err(crate::InstanceError::new(format!( + "failed to cast IDXGIFactory1 to IDXGIFactory2: {err:?}" + ))); } // If we don't print it to info. Err(err) => { diff --git a/wgpu-hal/src/dx11/instance.rs b/wgpu-hal/src/dx11/instance.rs index 1d8c2b51a2..e7a4e2e705 100644 --- a/wgpu-hal/src/dx11/instance.rs +++ b/wgpu-hal/src/dx11/instance.rs @@ -8,10 +8,13 @@ impl crate::Instance for super::Instance { }; if !enable_dx11 { - return Err(crate::InstanceError); + return Err(crate::InstanceError::new(String::from( + "DX11 support is unstable; set WGPU_UNSTABLE_DX11_BACKEND=1 to enable anyway", + ))); } - let lib_d3d11 = super::library::D3D11Lib::new().ok_or(crate::InstanceError)?; + let lib_d3d11 = super::library::D3D11Lib::new() + .ok_or_else(|| crate::InstanceError::new(String::from("failed to load d3d11.dll")))?; let (lib_dxgi, factory) = auxil::dxgi::factory::create_factory( auxil::dxgi::factory::DxgiFactoryType::Factory1, diff --git a/wgpu-hal/src/dx12/instance.rs b/wgpu-hal/src/dx12/instance.rs index 208d2179f7..32d6f1690c 100644 --- a/wgpu-hal/src/dx12/instance.rs +++ b/wgpu-hal/src/dx12/instance.rs @@ -12,7 +12,9 @@ impl Drop for super::Instance { impl crate::Instance for super::Instance { unsafe fn init(desc: &crate::InstanceDescriptor) -> Result { - let lib_main = d3d12::D3D12Lib::new().map_err(|_| crate::InstanceError)?; + let lib_main = d3d12::D3D12Lib::new().map_err(|e| { + crate::InstanceError::with_source(String::from("failed to load d3d12.dll"), e) + })?; if desc.flags.contains(crate::InstanceFlags::VALIDATION) { // Enable debug layer @@ -95,7 +97,9 @@ impl crate::Instance for super::Instance { supports_allow_tearing: self.supports_allow_tearing, swap_chain: None, }), - _ => Err(crate::InstanceError), + _ => Err(crate::InstanceError::new(format!( + "window handle {window_handle:?} is not a Win32 handle" + ))), } } unsafe fn destroy_surface(&self, _surface: super::Surface) { diff --git a/wgpu-hal/src/gles/adapter.rs b/wgpu-hal/src/gles/adapter.rs index 2c68961e39..348f62bc03 100644 --- a/wgpu-hal/src/gles/adapter.rs +++ b/wgpu-hal/src/gles/adapter.rs @@ -43,8 +43,9 @@ impl super::Adapter { src = &src[pos + es_sig.len()..]; } None => { - log::warn!("ES not found in '{}'", src); - return Err(crate::InstanceError); + return Err(crate::InstanceError::new(format!( + "OpenGL version {src:?} does not contain 'ES'" + ))); } } }; @@ -86,10 +87,9 @@ impl super::Adapter { }, minor, )), - _ => { - log::warn!("Unable to extract the version from '{}'", version); - Err(crate::InstanceError) - } + _ => Err(crate::InstanceError::new(format!( + "unable to extract OpenGL version from {version:?}" + ))), } } @@ -975,27 +975,30 @@ mod tests { #[test] fn test_version_parse() { - let error = Err(crate::InstanceError); - assert_eq!(Adapter::parse_version("1"), error); - assert_eq!(Adapter::parse_version("1."), error); - assert_eq!(Adapter::parse_version("1 h3l1o. W0rld"), error); - assert_eq!(Adapter::parse_version("1. h3l1o. W0rld"), error); - assert_eq!(Adapter::parse_version("1.2.3"), error); - assert_eq!(Adapter::parse_version("OpenGL ES 3.1"), Ok((3, 1))); + Adapter::parse_version("1").unwrap_err(); + Adapter::parse_version("1.").unwrap_err(); + Adapter::parse_version("1 h3l1o. W0rld").unwrap_err(); + Adapter::parse_version("1. h3l1o. W0rld").unwrap_err(); + Adapter::parse_version("1.2.3").unwrap_err(); + + assert_eq!(Adapter::parse_version("OpenGL ES 3.1").unwrap(), (3, 1)); + assert_eq!( + Adapter::parse_version("OpenGL ES 2.0 Google Nexus").unwrap(), + (2, 0) + ); + assert_eq!(Adapter::parse_version("GLSL ES 1.1").unwrap(), (1, 1)); assert_eq!( - Adapter::parse_version("OpenGL ES 2.0 Google Nexus"), - Ok((2, 0)) + Adapter::parse_version("OpenGL ES GLSL ES 3.20").unwrap(), + (3, 2) ); - assert_eq!(Adapter::parse_version("GLSL ES 1.1"), Ok((1, 1))); - assert_eq!(Adapter::parse_version("OpenGL ES GLSL ES 3.20"), Ok((3, 2))); assert_eq!( // WebGL 2.0 should parse as OpenGL ES 3.0 - Adapter::parse_version("WebGL 2.0 (OpenGL ES 3.0 Chromium)"), - Ok((3, 0)) + Adapter::parse_version("WebGL 2.0 (OpenGL ES 3.0 Chromium)").unwrap(), + (3, 0) ); assert_eq!( - Adapter::parse_version("WebGL GLSL ES 3.00 (OpenGL ES GLSL ES 3.0 Chromium)"), - Ok((3, 0)) + Adapter::parse_version("WebGL GLSL ES 3.00 (OpenGL ES GLSL ES 3.0 Chromium)").unwrap(), + (3, 0) ); } } diff --git a/wgpu-hal/src/gles/egl.rs b/wgpu-hal/src/gles/egl.rs index b904dffee9..d6d3d621f9 100644 --- a/wgpu-hal/src/gles/egl.rs +++ b/wgpu-hal/src/gles/egl.rs @@ -283,7 +283,10 @@ fn choose_config( } } - Err(crate::InstanceError) + // TODO: include diagnostic details that are currently logged + Err(crate::InstanceError::new(String::from( + "unable to find an acceptable EGL framebuffer configuration", + ))) } fn gl_debug_message_callback(source: u32, gltype: u32, id: u32, severity: u32, message: &str) { @@ -495,7 +498,12 @@ impl Inner { display: khronos_egl::Display, force_gles_minor_version: wgt::Gles3MinorVersion, ) -> Result { - let version = egl.initialize(display).map_err(|_| crate::InstanceError)?; + let version = egl.initialize(display).map_err(|e| { + crate::InstanceError::with_source( + String::from("failed to initialize EGL display connection"), + e, + ) + })?; let vendor = egl .query_string(Some(display), khronos_egl::VENDOR) .unwrap(); @@ -599,8 +607,10 @@ impl Inner { let context = match egl.create_context(display, config, None, &context_attributes) { Ok(context) => context, Err(e) => { - log::warn!("unable to create GLES 3.x context: {:?}", e); - return Err(crate::InstanceError); + return Err(crate::InstanceError::with_source( + String::from("unable to create GLES 3.x context"), + e, + )); } }; @@ -623,8 +633,10 @@ impl Inner { egl.create_pbuffer_surface(display, config, &attributes) .map(Some) .map_err(|e| { - log::warn!("Error in create_pbuffer_surface: {:?}", e); - crate::InstanceError + crate::InstanceError::with_source( + String::from("error in create_pbuffer_surface"), + e, + ) })? }; @@ -734,8 +746,10 @@ impl crate::Instance for Instance { let egl = match egl_result { Ok(egl) => Arc::new(egl), Err(e) => { - log::info!("Unable to open libEGL: {:?}", e); - return Err(crate::InstanceError); + return Err(crate::InstanceError::with_source( + String::from("unable to open libEGL"), + e, + )); } }; @@ -899,8 +913,9 @@ impl crate::Instance for Instance { }; if ret != 0 { - log::error!("Error returned from ANativeWindow_setBuffersGeometry"); - return Err(crate::InstanceError); + return Err(crate::InstanceError::new(format!( + "error {ret} returned from ANativeWindow_setBuffersGeometry", + ))); } } #[cfg(not(target_os = "emscripten"))] @@ -938,8 +953,7 @@ impl crate::Instance for Instance { Arc::clone(&inner.egl.instance), display, inner.force_gles_minor_version, - ) - .map_err(|_| crate::InstanceError)?; + )?; let old_inner = std::mem::replace(inner.deref_mut(), new_inner); inner.wl_display = Some(display_handle.display); @@ -950,8 +964,9 @@ impl crate::Instance for Instance { #[cfg(target_os = "emscripten")] (Rwh::Web(_), _) => {} other => { - log::error!("Unsupported window: {:?}", other); - return Err(crate::InstanceError); + return Err(crate::InstanceError::new(format!( + "unsupported window: {other:?}" + ))); } }; diff --git a/wgpu-hal/src/gles/web.rs b/wgpu-hal/src/gles/web.rs index 02cd6a3ecb..13bce85f84 100644 --- a/wgpu-hal/src/gles/web.rs +++ b/wgpu-hal/src/gles/web.rs @@ -66,14 +66,16 @@ impl Instance { // “not supported” could include “insufficient GPU resources” or “the GPU process // previously crashed”. So, we must return it as an `Err` since it could occur // for circumstances outside the application author's control. - return Err(crate::InstanceError); + return Err(crate::InstanceError::new(String::from( + "canvas.getContext() returned null; webgl2 not available or canvas already in use" + ))); } Err(js_error) => { // - // A thrown exception indicates misuse of the canvas state. Ideally we wouldn't - // panic in this case, but for now, `InstanceError` conveys no detail, so it - // is more informative to panic with a specific message. - panic!("canvas.getContext() threw {js_error:?}") + // A thrown exception indicates misuse of the canvas state. + return Err(crate::InstanceError::new(format!( + "canvas.getContext() threw exception {js_error:?}", + ))); } }; @@ -156,7 +158,9 @@ impl crate::Instance for Instance { self.create_surface_from_canvas(canvas) } else { - Err(crate::InstanceError) + Err(crate::InstanceError::new(format!( + "window handle {window_handle:?} is not a web handle" + ))) } } diff --git a/wgpu-hal/src/lib.rs b/wgpu-hal/src/lib.rs index 020c665709..4bff6b8d8f 100644 --- a/wgpu-hal/src/lib.rs +++ b/wgpu-hal/src/lib.rs @@ -90,7 +90,7 @@ use std::{ num::NonZeroU32, ops::{Range, RangeInclusive}, ptr::NonNull, - sync::atomic::AtomicBool, + sync::{atomic::AtomicBool, Arc}, }; use bitflags::bitflags; @@ -152,9 +152,42 @@ pub enum SurfaceError { Other(&'static str), } -#[derive(Clone, Debug, Eq, PartialEq, Error)] -#[error("Not supported")] -pub struct InstanceError; +/// Error occurring while trying to create an instance, or create a surface from an instance; +/// typically relating to the state of the underlying graphics API or hardware. +#[derive(Clone, Debug, Error)] +#[error("{message}")] +pub struct InstanceError { + /// These errors are very platform specific, so do not attempt to encode them as an enum. + /// + /// This message should describe the problem in sufficient detail to be useful for a + /// user-to-developer “why won't this work on my machine” bug report, and otherwise follow + /// . + message: String, + + /// Underlying error value, if any is available. + #[source] + source: Option>, +} + +impl InstanceError { + #[allow(dead_code)] // may be unused on some platforms + pub(crate) fn new(message: String) -> Self { + Self { + message, + source: None, + } + } + #[allow(dead_code)] // may be unused on some platforms + pub(crate) fn with_source( + message: String, + source: impl std::error::Error + Send + Sync + 'static, + ) -> Self { + Self { + message, + source: Some(Arc::new(source)), + } + } +} pub trait Api: Clone + Sized { type Instance: Instance; diff --git a/wgpu-hal/src/metal/mod.rs b/wgpu-hal/src/metal/mod.rs index 3a8ebc5570..76f57002ff 100644 --- a/wgpu-hal/src/metal/mod.rs +++ b/wgpu-hal/src/metal/mod.rs @@ -100,7 +100,9 @@ impl crate::Instance for Instance { raw_window_handle::RawWindowHandle::AppKit(handle) => Ok(unsafe { Surface::from_view(handle.ns_view, Some(&self.managed_metal_layer_delegate)) }), - _ => Err(crate::InstanceError), + _ => Err(crate::InstanceError::new(format!( + "window handle {window_handle:?} is not a Metal-compatible handle" + ))), } } diff --git a/wgpu-hal/src/vulkan/instance.rs b/wgpu-hal/src/vulkan/instance.rs index 4fa4a3e27d..18b141a070 100644 --- a/wgpu-hal/src/vulkan/instance.rs +++ b/wgpu-hal/src/vulkan/instance.rs @@ -194,8 +194,10 @@ impl super::Instance { let instance_extensions = entry .enumerate_instance_extension_properties(None) .map_err(|e| { - log::info!("enumerate_instance_extension_properties: {:?}", e); - crate::InstanceError + crate::InstanceError::with_source( + String::from("enumerate_instance_extension_properties() failed"), + e, + ) })?; // Check our extensions against the available extensions @@ -366,8 +368,9 @@ impl super::Instance { window: vk::Window, ) -> Result { if !self.shared.extensions.contains(&khr::XlibSurface::name()) { - log::warn!("Vulkan driver does not support VK_KHR_xlib_surface"); - return Err(crate::InstanceError); + return Err(crate::InstanceError::new(String::from( + "Vulkan driver does not support VK_KHR_xlib_surface", + ))); } let surface = { @@ -391,8 +394,9 @@ impl super::Instance { window: vk::xcb_window_t, ) -> Result { if !self.shared.extensions.contains(&khr::XcbSurface::name()) { - log::warn!("Vulkan driver does not support VK_KHR_xcb_surface"); - return Err(crate::InstanceError); + return Err(crate::InstanceError::new(String::from( + "Vulkan driver does not support VK_KHR_xcb_surface", + ))); } let surface = { @@ -420,8 +424,9 @@ impl super::Instance { .extensions .contains(&khr::WaylandSurface::name()) { - log::debug!("Vulkan driver does not support VK_KHR_wayland_surface"); - return Err(crate::InstanceError); + return Err(crate::InstanceError::new(String::from( + "Vulkan driver does not support VK_KHR_wayland_surface", + ))); } let surface = { @@ -447,8 +452,9 @@ impl super::Instance { .extensions .contains(&khr::AndroidSurface::name()) { - log::warn!("Vulkan driver does not support VK_KHR_android_surface"); - return Err(crate::InstanceError); + return Err(crate::InstanceError::new(String::from( + "Vulkan driver does not support VK_KHR_android_surface", + ))); } let surface = { @@ -470,8 +476,9 @@ impl super::Instance { hwnd: *mut c_void, ) -> Result { if !self.shared.extensions.contains(&khr::Win32Surface::name()) { - log::debug!("Vulkan driver does not support VK_KHR_win32_surface"); - return Err(crate::InstanceError); + return Err(crate::InstanceError::new(String::from( + "Vulkan driver does not support VK_KHR_win32_surface", + ))); } let surface = { @@ -496,8 +503,9 @@ impl super::Instance { view: *mut c_void, ) -> Result { if !self.shared.extensions.contains(&ext::MetalSurface::name()) { - log::warn!("Vulkan driver does not support VK_EXT_metal_surface"); - return Err(crate::InstanceError); + return Err(crate::InstanceError::new(String::from( + "Vulkan driver does not support VK_EXT_metal_surface", + ))); } let layer = unsafe { @@ -546,20 +554,18 @@ impl crate::Instance for super::Instance { unsafe fn init(desc: &crate::InstanceDescriptor) -> Result { use crate::auxil::cstr_from_bytes_until_nul; - let entry = match unsafe { ash::Entry::load() } { - Ok(entry) => entry, - Err(err) => { - log::info!("Missing Vulkan entry points: {:?}", err); - return Err(crate::InstanceError); - } - }; + let entry = unsafe { ash::Entry::load() }.map_err(|err| { + crate::InstanceError::with_source(String::from("missing Vulkan entry points"), err) + })?; let driver_api_version = match entry.try_enumerate_instance_version() { // Vulkan 1.1+ Ok(Some(version)) => version, Ok(None) => vk::API_VERSION_1_0, Err(err) => { - log::warn!("try_enumerate_instance_version: {:?}", err); - return Err(crate::InstanceError); + return Err(crate::InstanceError::with_source( + String::from("try_enumerate_instance_version() failed"), + err, + )); } }; @@ -590,7 +596,10 @@ impl crate::Instance for super::Instance { let instance_layers = entry.enumerate_instance_layer_properties().map_err(|e| { log::info!("enumerate_instance_layer_properties: {:?}", e); - crate::InstanceError + crate::InstanceError::with_source( + String::from("enumerate_instance_layer_properties() failed"), + e, + ) })?; fn find_layer<'layers>( @@ -682,8 +691,10 @@ impl crate::Instance for super::Instance { .enabled_extension_names(&str_pointers[layers.len()..]); unsafe { entry.create_instance(&create_info, None) }.map_err(|e| { - log::warn!("create_instance: {:?}", e); - crate::InstanceError + crate::InstanceError::with_source( + String::from("Entry::create_instance() failed"), + e, + ) })? }; @@ -739,7 +750,9 @@ impl crate::Instance for super::Instance { { self.create_surface_from_view(handle.ui_view) } - (_, _) => Err(crate::InstanceError), + (_, _) => Err(crate::InstanceError::new(format!( + "window handle {window_handle:?} is not a Vulkan-compatible handle" + ))), } } diff --git a/wgpu/src/backend/direct.rs b/wgpu/src/backend/direct.rs index fca1d80c3c..8eec9adad5 100644 --- a/wgpu/src/backend/direct.rs +++ b/wgpu/src/backend/direct.rs @@ -244,10 +244,7 @@ impl Context { &self, canvas: web_sys::HtmlCanvasElement, ) -> Result { - let id = self - .0 - .create_surface_webgl_canvas(canvas, ()) - .map_err(|hal::InstanceError| crate::CreateSurfaceError {})?; + let id = self.0.create_surface_webgl_canvas(canvas, ())?; Ok(Surface { id, configured_device: Mutex::default(), @@ -259,10 +256,7 @@ impl Context { &self, canvas: web_sys::OffscreenCanvas, ) -> Result { - let id = self - .0 - .create_surface_webgl_offscreen_canvas(canvas, ()) - .map_err(|hal::InstanceError| crate::CreateSurfaceError {})?; + let id = self.0.create_surface_webgl_offscreen_canvas(canvas, ())?; Ok(Surface { id, configured_device: Mutex::default(), diff --git a/wgpu/src/backend/web.rs b/wgpu/src/backend/web.rs index 1fc1c6683f..d64bd8bcb1 100644 --- a/wgpu/src/backend/web.rs +++ b/wgpu/src/backend/web.rs @@ -920,13 +920,22 @@ impl Context { // “not supported” could include “insufficient GPU resources” or “the GPU process // previously crashed”. So, we must return it as an `Err` since it could occur // for circumstances outside the application author's control. - return Err(crate::CreateSurfaceError {}); + return Err(crate::CreateSurfaceError { + inner: crate::CreateSurfaceErrorKind::Web( + String::from( + "canvas.getContext() returned null; webgpu not available or canvas already in use" + ) + ) + }); } Err(js_error) => { // - // A thrown exception indicates misuse of the canvas state. Ideally we wouldn't - // panic in this case ... TODO - panic!("canvas.getContext() threw {js_error:?}") + // A thrown exception indicates misuse of the canvas state. + return Err(crate::CreateSurfaceError { + inner: crate::CreateSurfaceErrorKind::Web(format!( + "canvas.getContext() threw exception {js_error:?}", + )), + }); } }; diff --git a/wgpu/src/lib.rs b/wgpu/src/lib.rs index 1c3e1a58b5..94345f1adb 100644 --- a/wgpu/src/lib.rs +++ b/wgpu/src/lib.rs @@ -15,8 +15,7 @@ mod macros; use std::{ any::Any, borrow::Cow, - error, - fmt::{Debug, Display}, + error, fmt, future::Future, marker::PhantomData, num::NonZeroU32, @@ -1700,8 +1699,8 @@ pub enum SurfaceError { } static_assertions::assert_impl_all!(SurfaceError: Send, Sync); -impl Display for SurfaceError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +impl fmt::Display for SurfaceError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}", match self { Self::Timeout => "A timeout was encountered while trying to acquire the next frame", Self::Outdated => "The underlying surface has changed, and therefore the swap chain must be updated", @@ -2744,8 +2743,8 @@ impl Drop for Device { pub struct RequestDeviceError; static_assertions::assert_impl_all!(RequestDeviceError: Send, Sync); -impl Display for RequestDeviceError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +impl fmt::Display for RequestDeviceError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "Requesting a device failed") } } @@ -2753,28 +2752,76 @@ impl Display for RequestDeviceError { impl error::Error for RequestDeviceError {} /// [`Instance::create_surface()`] or a related function failed. -#[derive(Clone, PartialEq, Eq, Debug)] +#[derive(Clone, Debug)] #[non_exhaustive] pub struct CreateSurfaceError { - // TODO: Report diagnostic clues + inner: CreateSurfaceErrorKind, +} +#[derive(Clone, Debug)] +enum CreateSurfaceErrorKind { + /// Error from [`wgpu_hal`]. + #[cfg(any( + not(target_arch = "wasm32"), + target_os = "emscripten", + feature = "webgl" + ))] + // must match dependency cfg + Hal(hal::InstanceError), + + /// Error from WebGPU surface creation. + #[allow(dead_code)] // may be unused depending on target and features + Web(String), } static_assertions::assert_impl_all!(CreateSurfaceError: Send, Sync); -impl Display for CreateSurfaceError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "Creating a surface failed") +impl fmt::Display for CreateSurfaceError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match &self.inner { + #[cfg(any( + not(target_arch = "wasm32"), + target_os = "emscripten", + feature = "webgl" + ))] + CreateSurfaceErrorKind::Hal(e) => e.fmt(f), + CreateSurfaceErrorKind::Web(e) => e.fmt(f), + } } } -impl error::Error for CreateSurfaceError {} +impl error::Error for CreateSurfaceError { + fn source(&self) -> Option<&(dyn error::Error + 'static)> { + match &self.inner { + #[cfg(any( + not(target_arch = "wasm32"), + target_os = "emscripten", + feature = "webgl" + ))] + CreateSurfaceErrorKind::Hal(e) => e.source(), + CreateSurfaceErrorKind::Web(_) => None, + } + } +} + +#[cfg(any( + not(target_arch = "wasm32"), + target_os = "emscripten", + feature = "webgl" +))] +impl From for CreateSurfaceError { + fn from(e: hal::InstanceError) -> Self { + Self { + inner: CreateSurfaceErrorKind::Hal(e), + } + } +} /// Error occurred when trying to async map a buffer. #[derive(Clone, PartialEq, Eq, Debug)] pub struct BufferAsyncError; static_assertions::assert_impl_all!(BufferAsyncError: Send, Sync); -impl Display for BufferAsyncError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +impl fmt::Display for BufferAsyncError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "Error occurred when trying to async map a buffer") } } @@ -4849,8 +4896,8 @@ impl Clone for Id { impl Copy for Id {} #[cfg(feature = "expose-ids")] -impl Debug for Id { - fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { +impl fmt::Debug for Id { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.debug_tuple("Id").field(&self.0).finish() } } @@ -5150,8 +5197,8 @@ impl error::Error for Error { } } -impl Display for Error { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Error::OutOfMemory { .. } => f.write_str("Out of Memory"), Error::Validation { description, .. } => f.write_str(description), From 54a7f0eac9b1531d2ebeec4cff3af842772b098e Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Sun, 3 Sep 2023 23:54:39 -0400 Subject: [PATCH 05/33] Skip `test_multithreaded_compute` on MoltenVK. (#4096) Co-authored-by: Connor Fitzgerald --- .github/workflows/ci.yml | 1 + CHANGELOG.md | 4 + examples/boids/src/main.rs | 2 +- examples/common/src/framework.rs | 2 +- examples/hello-compute/src/tests.rs | 23 +- examples/mipmap/src/main.rs | 4 +- examples/msaa-line/src/main.rs | 9 +- examples/shadow/src/main.rs | 10 +- examples/skybox/src/main.rs | 7 +- tests/src/image.rs | 25 +- tests/src/lib.rs | 396 +++++++++++------- tests/tests/clear_texture.rs | 30 +- tests/tests/device.rs | 40 +- tests/tests/encoder.rs | 5 +- tests/tests/poll.rs | 89 ++-- tests/tests/shader/struct_layout.rs | 6 +- tests/tests/shader/zero_init_workgroup_mem.rs | 19 +- tests/tests/shader_view_format/mod.rs | 9 +- tests/tests/vertex_indices/mod.rs | 6 +- tests/tests/write_texture.rs | 5 +- .../tests/zero_init_texture_after_discard.rs | 46 +- 21 files changed, 454 insertions(+), 284 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f4ed15c4a7..06ae299b77 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -284,6 +284,7 @@ jobs: done - uses: actions/upload-artifact@v3 + if: always() # We want artifacts even if the tests fail. with: name: comparison-images path: | diff --git a/CHANGELOG.md b/CHANGELOG.md index 75e6554dcf..0ca6f6350a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -114,6 +114,10 @@ By @Valaphee in [#3402](https://github.com/gfx-rs/wgpu/pull/3402) - Ensure that limit requests and reporting is done correctly. By @OptimisticPeach in [#4107](https://github.com/gfx-rs/wgpu/pull/4107) +#### Testing + +- Skip `test_multithreaded_compute` on MoltenVK. By @jimblandy in [#4096](https://github.com/gfx-rs/wgpu/pull/4096). + ### Documentation - Add an overview of `RenderPass` and how render state works. By @kpreid in [#4055](https://github.com/gfx-rs/wgpu/pull/4055) diff --git a/examples/boids/src/main.rs b/examples/boids/src/main.rs index 357792de4f..e8aa2f71fd 100644 --- a/examples/boids/src/main.rs +++ b/examples/boids/src/main.rs @@ -345,7 +345,7 @@ fn boids() { .downlevel_flags(wgpu::DownlevelFlags::COMPUTE_SHADERS) .limits(wgpu::Limits::downlevel_defaults()) // Lots of validation errors, maybe related to https://github.com/gfx-rs/wgpu/issues/3160 - .molten_vk_failure(), + .expect_fail(wgpu_test::FailureCase::molten_vk()), comparisons: &[wgpu_test::ComparisonType::Mean(0.005)], }); } diff --git a/examples/common/src/framework.rs b/examples/common/src/framework.rs index 06db6092f7..875d8544e7 100644 --- a/examples/common/src/framework.rs +++ b/examples/common/src/framework.rs @@ -625,7 +625,7 @@ pub fn test(mut params: FrameworkRefTest) { wgpu_test::image::compare_image_output( env!("CARGO_MANIFEST_DIR").to_string() + "/../../" + params.image_path, - ctx.adapter_info.backend, + &ctx.adapter_info, params.width, params.height, &bytes, diff --git a/examples/hello-compute/src/tests.rs b/examples/hello-compute/src/tests.rs index 54cddbe379..7f8649f72f 100644 --- a/examples/hello-compute/src/tests.rs +++ b/examples/hello-compute/src/tests.rs @@ -1,7 +1,7 @@ use std::sync::Arc; use super::*; -use wgpu_test::{initialize_test, TestParameters}; +use wgpu_test::{initialize_test, FailureCase, TestParameters}; wasm_bindgen_test::wasm_bindgen_test_configure!(run_in_browser); @@ -13,7 +13,7 @@ fn test_compute_1() { .downlevel_flags(wgpu::DownlevelFlags::COMPUTE_SHADERS) .limits(wgpu::Limits::downlevel_defaults()) .features(wgpu::Features::TIMESTAMP_QUERY) - .specific_failure(None, None, Some("V3D"), true), + .skip(FailureCase::adapter("V3D")), |ctx| { let input = &[1, 2, 3, 4]; @@ -35,7 +35,7 @@ fn test_compute_2() { .downlevel_flags(wgpu::DownlevelFlags::COMPUTE_SHADERS) .limits(wgpu::Limits::downlevel_defaults()) .features(wgpu::Features::TIMESTAMP_QUERY) - .specific_failure(None, None, Some("V3D"), true), + .skip(FailureCase::adapter("V3D")), |ctx| { let input = &[5, 23, 10, 9]; @@ -57,7 +57,7 @@ fn test_compute_overflow() { .downlevel_flags(wgpu::DownlevelFlags::COMPUTE_SHADERS) .limits(wgpu::Limits::downlevel_defaults()) .features(wgpu::Features::TIMESTAMP_QUERY) - .specific_failure(None, None, Some("V3D"), true), + .skip(FailureCase::adapter("V3D")), |ctx| { let input = &[77031, 837799, 8400511, 63728127]; pollster::block_on(assert_execute_gpu( @@ -78,16 +78,15 @@ fn test_multithreaded_compute() { .downlevel_flags(wgpu::DownlevelFlags::COMPUTE_SHADERS) .limits(wgpu::Limits::downlevel_defaults()) .features(wgpu::Features::TIMESTAMP_QUERY) - .specific_failure(None, None, Some("V3D"), true) + .skip(FailureCase::adapter("V3D")) // https://github.com/gfx-rs/wgpu/issues/3944 - .specific_failure( - Some(wgpu::Backends::VULKAN), - None, - Some("swiftshader"), - true, - ) + .skip(FailureCase::backend_adapter( + wgpu::Backends::VULKAN, + "swiftshader", + )) // https://github.com/gfx-rs/wgpu/issues/3250 - .specific_failure(Some(wgpu::Backends::GL), None, Some("llvmpipe"), true), + .skip(FailureCase::backend_adapter(wgpu::Backends::GL, "llvmpipe")) + .skip(FailureCase::molten_vk()), |ctx| { use std::{sync::mpsc, thread, time::Duration}; diff --git a/examples/mipmap/src/main.rs b/examples/mipmap/src/main.rs index d21f6c1e08..a85110ff14 100644 --- a/examples/mipmap/src/main.rs +++ b/examples/mipmap/src/main.rs @@ -521,7 +521,7 @@ fn mipmap() { height: 768, optional_features: wgpu::Features::default(), base_test_parameters: wgpu_test::TestParameters::default() - .backend_failure(wgpu::Backends::GL), + .expect_fail(wgpu_test::FailureCase::backend(wgpu::Backends::GL)), comparisons: &[wgpu_test::ComparisonType::Mean(0.02)], }); } @@ -535,7 +535,7 @@ fn mipmap_query() { height: 768, optional_features: QUERY_FEATURES, base_test_parameters: wgpu_test::TestParameters::default() - .backend_failure(wgpu::Backends::GL), + .expect_fail(wgpu_test::FailureCase::backend(wgpu::Backends::GL)), comparisons: &[wgpu_test::ComparisonType::Mean(0.02)], }); } diff --git a/examples/msaa-line/src/main.rs b/examples/msaa-line/src/main.rs index 2f42817765..aa7a277418 100644 --- a/examples/msaa-line/src/main.rs +++ b/examples/msaa-line/src/main.rs @@ -12,6 +12,9 @@ use std::{borrow::Cow, iter}; use bytemuck::{Pod, Zeroable}; use wgpu::util::DeviceExt; +#[cfg(test)] +use wgpu_test::FailureCase; + #[repr(C)] #[derive(Clone, Copy, Pod, Zeroable)] struct Vertex { @@ -326,7 +329,11 @@ fn msaa_line() { optional_features: wgpu::Features::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES, base_test_parameters: wgpu_test::TestParameters::default() // AMD seems to render nothing on DX12 https://github.com/gfx-rs/wgpu/issues/3838 - .specific_failure(Some(wgpu::Backends::DX12), Some(0x1002), None, false), + .expect_fail(FailureCase { + backends: Some(wgpu::Backends::DX12), + vendor: Some(0x1002), + ..FailureCase::default() + }), // There's a lot of natural variance so we check the weighted median too to differentiate // real failures from variance. comparisons: &[ diff --git a/examples/shadow/src/main.rs b/examples/shadow/src/main.rs index 09b0982ea9..3f963d0c53 100644 --- a/examples/shadow/src/main.rs +++ b/examples/shadow/src/main.rs @@ -857,9 +857,15 @@ fn shadow() { base_test_parameters: wgpu_test::TestParameters::default() .downlevel_flags(wgpu::DownlevelFlags::COMPARISON_SAMPLERS) // rpi4 on VK doesn't work: https://gitlab.freedesktop.org/mesa/mesa/-/issues/3916 - .specific_failure(Some(wgpu::Backends::VULKAN), None, Some("V3D"), false) + .expect_fail(wgpu_test::FailureCase::backend_adapter( + wgpu::Backends::VULKAN, + "V3D", + )) // llvmpipe versions in CI are flaky: https://github.com/gfx-rs/wgpu/issues/2594 - .specific_failure(Some(wgpu::Backends::VULKAN), None, Some("llvmpipe"), true), + .skip(wgpu_test::FailureCase::backend_adapter( + wgpu::Backends::VULKAN, + "llvmpipe", + )), comparisons: &[wgpu_test::ComparisonType::Mean(0.02)], }); } diff --git a/examples/skybox/src/main.rs b/examples/skybox/src/main.rs index 9873ac9c0b..d09622f53c 100644 --- a/examples/skybox/src/main.rs +++ b/examples/skybox/src/main.rs @@ -475,11 +475,8 @@ fn skybox() { width: 1024, height: 768, optional_features: wgpu::Features::default(), - base_test_parameters: wgpu_test::TestParameters::default().specific_failure( - Some(wgpu::Backends::GL), - None, - Some("ANGLE"), - false, + base_test_parameters: wgpu_test::TestParameters::default().expect_fail( + wgpu_test::FailureCase::backend_adapter(wgpu::Backends::GL, "ANGLE"), ), comparisons: &[wgpu_test::ComparisonType::Mean(0.015)], }); diff --git a/tests/src/image.rs b/tests/src/image.rs index 00aa78f660..e50fd43e7f 100644 --- a/tests/src/image.rs +++ b/tests/src/image.rs @@ -150,7 +150,7 @@ impl ComparisonType { pub fn compare_image_output( path: impl AsRef + AsRef, - backend: Backend, + adapter_info: &wgt::AdapterInfo, width: u32, height: u32, test_with_alpha: &[u8], @@ -205,17 +205,18 @@ pub fn compare_image_output( } let file_stem = reference_path.file_stem().unwrap().to_string_lossy(); + let renderer = format!( + "{}-{}-{}", + adapter_info.backend.to_str(), + sanitize_for_path(&adapter_info.name), + sanitize_for_path(&adapter_info.driver) + ); // Determine the paths to write out the various intermediate files let actual_path = Path::new(&path).with_file_name( - OsString::from_str(&format!("{}-{}-actual.png", file_stem, backend.to_str(),)).unwrap(), + OsString::from_str(&format!("{}-{}-actual.png", file_stem, renderer)).unwrap(), ); let difference_path = Path::new(&path).with_file_name( - OsString::from_str(&format!( - "{}-{}-difference.png", - file_stem, - backend.to_str(), - )) - .unwrap(), + OsString::from_str(&format!("{}-{}-difference.png", file_stem, renderer,)).unwrap(), ); // Convert the error values to a false color reprensentation @@ -246,10 +247,16 @@ pub fn compare_image_output( #[cfg(target_arch = "wasm32")] { - let _ = (path, backend, width, height, test_with_alpha, checks); + let _ = (path, adapter_info, width, height, test_with_alpha, checks); } } +fn sanitize_for_path(s: &str) -> String { + s.chars() + .map(|ch| if ch.is_ascii_alphanumeric() { ch } else { '_' }) + .collect() +} + fn copy_via_compute( device: &Device, encoder: &mut CommandEncoder, diff --git a/tests/src/lib.rs b/tests/src/lib.rs index 1d741f1812..236b353386 100644 --- a/tests/src/lib.rs +++ b/tests/src/lib.rs @@ -53,11 +53,195 @@ fn lowest_downlevel_properties() -> DownlevelCapabilities { } } +/// Conditions under which a test should fail or be skipped. +/// +/// By passing a `FailureCase` to [`TestParameters::expect_fail`], you can +/// mark a test as expected to fail under the indicated conditions. By +/// passing it to [`TestParameters::skip`], you can request that the +/// test be skipped altogether. +/// +/// If a field is `None`, then that field does not restrict matches. For +/// example: +/// +/// ``` +/// # use wgpu_test::FailureCase; +/// FailureCase { +/// backends: Some(wgpu::Backends::DX11 | wgpu::Backends::DX12), +/// vendor: None, +/// adapter: Some("RTX"), +/// driver: None, +/// } +/// # ; +/// ``` +/// +/// This applies to all cards with `"RTX'` in their name on either +/// Direct3D backend, no matter the vendor ID or driver name. +/// +/// The strings given here need only appear as a substring in the +/// corresponding [`AdapterInfo`] fields. The comparison is +/// case-insensitive. +/// +/// The default value of `FailureCase` applies to any test case. That +/// is, there are no criteria to constrain the match. +/// +/// [`AdapterInfo`]: wgt::AdapterInfo +#[derive(Default)] pub struct FailureCase { - backends: Option, - vendor: Option, - adapter: Option, - skip: bool, + /// Backends expected to fail, or `None` for any backend. + /// + /// If this is `None`, or if the test is using one of the backends + /// in `backends`, then this `FailureCase` applies. + pub backends: Option, + + /// Vendor expected to fail, or `None` for any vendor. + /// + /// If `Some`, this must match [`AdapterInfo::device`], which is + /// usually the PCI device id. Otherwise, this `FailureCase` + /// applies regardless of vendor. + /// + /// [`AdapterInfo::device`]: wgt::AdapterInfo::device + pub vendor: Option, + + /// Name of adaper expected to fail, or `None` for any adapter name. + /// + /// If this is `Some(s)` and `s` is a substring of + /// [`AdapterInfo::name`], then this `FailureCase` applies. If + /// this is `None`, the adapter name isn't considered. + /// + /// [`AdapterInfo::name`]: wgt::AdapterInfo::name + pub adapter: Option<&'static str>, + + /// Name of driver expected to fail, or `None` for any driver name. + /// + /// If this is `Some(s)` and `s` is a substring of + /// [`AdapterInfo::driver`], then this `FailureCase` applies. If + /// this is `None`, the driver name isn't considered. + /// + /// [`AdapterInfo::driver`]: wgt::AdapterInfo::driver + pub driver: Option<&'static str>, +} + +impl FailureCase { + /// This case applies to all tests. + pub fn always() -> Self { + FailureCase::default() + } + + /// This case applies to no tests. + pub fn never() -> Self { + FailureCase { + backends: Some(wgpu::Backends::empty()), + ..FailureCase::default() + } + } + + /// Tests running on any of the given backends. + pub fn backend(backends: wgpu::Backends) -> Self { + FailureCase { + backends: Some(backends), + ..FailureCase::default() + } + } + + /// Tests running on `adapter`. + /// + /// For this case to apply, the `adapter` string must appear as a substring + /// of the adapter's [`AdapterInfo::name`]. The comparison is + /// case-insensitive. + /// + /// [`AdapterInfo::name`]: wgt::AdapterInfo::name + pub fn adapter(adapter: &'static str) -> Self { + FailureCase { + adapter: Some(adapter), + ..FailureCase::default() + } + } + + /// Tests running on `backend` and `adapter`. + /// + /// For this case to apply, the test must be using an adapter for one of the + /// given `backend` bits, and `adapter` string must appear as a substring of + /// the adapter's [`AdapterInfo::name`]. The string comparison is + /// case-insensitive. + /// + /// [`AdapterInfo::name`]: wgt::AdapterInfo::name + pub fn backend_adapter(backends: wgpu::Backends, adapter: &'static str) -> Self { + FailureCase { + backends: Some(backends), + adapter: Some(adapter), + ..FailureCase::default() + } + } + + /// Tests running under WebGL. + /// + /// Because of wasm's limited ability to recover from errors, we + /// usually need to skip the test altogether if it's not + /// supported, so this should be usually used with + /// [`TestParameters::skip`]. + pub fn webgl2() -> Self { + #[cfg(target_arch = "wasm32")] + let case = FailureCase::backend(wgpu::Backends::GL); + #[cfg(not(target_arch = "wasm32"))] + let case = FailureCase::never(); + case + } + + /// Tests running on the MoltenVK Vulkan driver on macOS. + pub fn molten_vk() -> Self { + FailureCase { + backends: Some(wgpu::Backends::VULKAN), + driver: Some("MoltenVK"), + ..FailureCase::default() + } + } + + /// Test whether `self` applies to `info`. + /// + /// If it does, return a `FailureReasons` whose set bits indicate + /// why. If it doesn't, return `None`. + /// + /// The caller is responsible for converting the string-valued + /// fields of `info` to lower case, to ensure case-insensitive + /// matching. + fn applies_to(&self, info: &wgt::AdapterInfo) -> Option { + let mut reasons = FailureReasons::empty(); + + if let Some(backends) = self.backends { + if !backends.contains(wgpu::Backends::from(info.backend)) { + return None; + } + reasons.set(FailureReasons::BACKEND, true); + } + if let Some(vendor) = self.vendor { + if vendor != info.vendor { + return None; + } + reasons.set(FailureReasons::VENDOR, true); + } + if let Some(adapter) = self.adapter { + let adapter = adapter.to_lowercase(); + if !info.name.contains(&adapter) { + return None; + } + reasons.set(FailureReasons::ADAPTER, true); + } + if let Some(driver) = self.driver { + let driver = driver.to_lowercase(); + if !info.driver.contains(&driver) { + return None; + } + reasons.set(FailureReasons::DRIVER, true); + } + + // If we got this far but no specific reasons were triggered, then this + // must be a wildcard. + if reasons.is_empty() { + Some(FailureReasons::ALWAYS) + } else { + Some(reasons) + } + } } // This information determines if a test should run. @@ -65,7 +249,11 @@ pub struct TestParameters { pub required_features: Features, pub required_downlevel_properties: DownlevelCapabilities, pub required_limits: Limits, - // Backends where test should fail. + + /// Conditions under which this test should be skipped. + pub skips: Vec, + + /// Conditions under which this test should be run, but is expected to fail. pub failures: Vec, } @@ -75,6 +263,7 @@ impl Default for TestParameters { required_features: Features::empty(), required_downlevel_properties: lowest_downlevel_properties(), required_limits: Limits::downlevel_webgl2_defaults(), + skips: Vec::new(), failures: Vec::new(), } } @@ -86,7 +275,8 @@ bitflags::bitflags! { const BACKEND = 1 << 0; const VENDOR = 1 << 1; const ADAPTER = 1 << 2; - const ALWAYS = 1 << 3; + const DRIVER = 1 << 3; + const ALWAYS = 1 << 4; } } @@ -115,87 +305,17 @@ impl TestParameters { self } - /// Mark the test as always failing, equivalent to specific_failure(None, None, None) - pub fn failure(mut self) -> Self { - self.failures.push(FailureCase { - backends: None, - vendor: None, - adapter: None, - skip: false, - }); - self - } - - /// Mark the test as always failing and needing to be skipped, equivalent to specific_failure(None, None, None) - pub fn skip(mut self) -> Self { - self.failures.push(FailureCase { - backends: None, - vendor: None, - adapter: None, - skip: true, - }); - self - } - - /// Mark the test as always failing on a specific backend, equivalent to specific_failure(backend, None, None) - pub fn backend_failure(mut self, backends: wgpu::Backends) -> Self { - self.failures.push(FailureCase { - backends: Some(backends), - vendor: None, - adapter: None, - skip: false, - }); - self - } - - /// Mark the test as always failing on WebGL. Because limited ability of wasm to recover from errors, we need to wholesale - /// skip the test if it's not supported. - pub fn webgl2_failure(mut self) -> Self { - let _ = &mut self; - #[cfg(target_arch = "wasm32")] - self.failures.push(FailureCase { - backends: Some(wgpu::Backends::GL), - vendor: None, - adapter: None, - skip: true, - }); + /// Mark the test as always failing, but not to be skipped. + pub fn expect_fail(mut self, when: FailureCase) -> Self { + self.failures.push(when); self } - /// Determines if a test should fail under a particular set of conditions. If any of these are None, that means that it will match anything in that field. - /// - /// ex. - /// `specific_failure(Some(wgpu::Backends::DX11 | wgpu::Backends::DX12), None, Some("RTX"), false)` - /// means that this test will fail on all cards with RTX in their name on either D3D backend, no matter the vendor ID. - /// - /// If segfault is set to true, the test won't be run at all due to avoid segfaults. - pub fn specific_failure( - mut self, - backends: Option, - vendor: Option, - device: Option<&'static str>, - skip: bool, - ) -> Self { - self.failures.push(FailureCase { - backends, - vendor, - adapter: device.as_ref().map(AsRef::as_ref).map(str::to_lowercase), - skip, - }); + /// Mark the test as always failing, and needing to be skipped. + pub fn skip(mut self, when: FailureCase) -> Self { + self.skips.push(when); self } - - /// Mark the test as failing on vulkan on mac only - pub fn molten_vk_failure(self) -> Self { - #[cfg(any(target_os = "macos", target_os = "ios"))] - { - self.specific_failure(Some(wgpu::Backends::VULKAN), None, None, false) - } - #[cfg(not(any(target_os = "macos", target_os = "ios")))] - { - self - } - } } pub fn initialize_test(parameters: TestParameters, test_function: impl FnOnce(TestingContext)) { @@ -210,7 +330,15 @@ pub fn initialize_test(parameters: TestParameters, test_function: impl FnOnce(Te let (adapter, _surface_guard) = initialize_adapter(); let adapter_info = adapter.get_info(); - let adapter_lowercase_name = adapter_info.name.to_lowercase(); + + // Produce a lower-case version of the adapter info, for comparison against + // `parameters.skips` and `parameters.failures`. + let adapter_lowercase_info = wgt::AdapterInfo { + name: adapter_info.name.to_lowercase(), + driver: adapter_info.driver.to_lowercase(), + ..adapter_info.clone() + }; + let adapter_features = adapter.features(); let adapter_limits = adapter.limits(); let adapter_downlevel_capabilities = adapter.get_downlevel_capabilities(); @@ -254,7 +382,7 @@ pub fn initialize_test(parameters: TestParameters, test_function: impl FnOnce(Te let context = TestingContext { adapter, - adapter_info: adapter_info.clone(), + adapter_info, adapter_downlevel_capabilities, device, device_features: parameters.required_features, @@ -262,52 +390,26 @@ pub fn initialize_test(parameters: TestParameters, test_function: impl FnOnce(Te queue, }; - let expected_failure_reason = parameters.failures.iter().find_map(|failure| { - let always = - failure.backends.is_none() && failure.vendor.is_none() && failure.adapter.is_none(); - - let expect_failure_backend = failure - .backends - .map(|f| f.contains(wgpu::Backends::from(adapter_info.backend))); - let expect_failure_vendor = failure.vendor.map(|v| v == adapter_info.vendor); - let expect_failure_adapter = failure - .adapter - .as_deref() - .map(|f| adapter_lowercase_name.contains(f)); - - if expect_failure_backend.unwrap_or(true) - && expect_failure_vendor.unwrap_or(true) - && expect_failure_adapter.unwrap_or(true) - { - if always { - Some((FailureReasons::ALWAYS, failure.skip)) - } else { - let mut reason = FailureReasons::empty(); - reason.set( - FailureReasons::BACKEND, - expect_failure_backend.unwrap_or(false), - ); - reason.set( - FailureReasons::VENDOR, - expect_failure_vendor.unwrap_or(false), - ); - reason.set( - FailureReasons::ADAPTER, - expect_failure_adapter.unwrap_or(false), - ); - Some((reason, failure.skip)) - } - } else { - None - } - }); - - if let Some((reason, true)) = expected_failure_reason { - log::info!("EXPECTED TEST FAILURE SKIPPED: {:?}", reason); + // Check if we should skip the test altogether. + if let Some(skip_reason) = parameters + .skips + .iter() + .find_map(|case| case.applies_to(&adapter_lowercase_info)) + { + log::info!("EXPECTED TEST FAILURE SKIPPED: {:?}", skip_reason); return; } + // Determine if we expect this test to fail, and if so, why. + let expected_failure_reason = parameters + .failures + .iter() + .find_map(|case| case.applies_to(&adapter_lowercase_info)); + + // Run the test, and catch panics (possibly due to failed assertions). let panicked = catch_unwind(AssertUnwindSafe(|| test_function(context))).is_err(); + + // Check whether any validation errors were reported during the test run. cfg_if::cfg_if!( if #[cfg(any(not(target_arch = "wasm32"), target_os = "emscripten"))] { let canary_set = wgpu::hal::VALIDATION_CANARY.get_and_reset(); @@ -316,32 +418,34 @@ pub fn initialize_test(parameters: TestParameters, test_function: impl FnOnce(Te } ); - let failed = panicked || canary_set; - + // Summarize reasons for actual failure, if any. let failure_cause = match (panicked, canary_set) { - (true, true) => "PANIC AND VALIDATION ERROR", - (true, false) => "PANIC", - (false, true) => "VALIDATION ERROR", - (false, false) => "", + (true, true) => Some("PANIC AND VALIDATION ERROR"), + (true, false) => Some("PANIC"), + (false, true) => Some("VALIDATION ERROR"), + (false, false) => None, }; - let expect_failure = expected_failure_reason.is_some(); - - if failed == expect_failure { - // We got the conditions we expected - if let Some((expected_reason, _)) = expected_failure_reason { - // Print out reason for the failure + // Compare actual results against expectations. + match (failure_cause, expected_failure_reason) { + // The test passed, as expected. + (None, None) => {} + // The test failed unexpectedly. + (Some(cause), None) => { + panic!("UNEXPECTED TEST FAILURE DUE TO {cause}") + } + // The test passed unexpectedly. + (None, Some(reason)) => { + panic!("UNEXPECTED TEST PASS: {reason:?}"); + } + // The test failed, as expected. + (Some(cause), Some(reason_expected)) => { log::info!( - "GOT EXPECTED TEST FAILURE DUE TO {}: {:?}", - failure_cause, - expected_reason + "EXPECTED FAILURE DUE TO {} (expected because of {:?})", + cause, + reason_expected ); } - } else if let Some((reason, _)) = expected_failure_reason { - // We expected to fail, but things passed - panic!("UNEXPECTED TEST PASS: {reason:?}"); - } else { - panic!("UNEXPECTED TEST FAILURE DUE TO {failure_cause}") } } diff --git a/tests/tests/clear_texture.rs b/tests/tests/clear_texture.rs index 7b2024c64c..36f48af359 100644 --- a/tests/tests/clear_texture.rs +++ b/tests/tests/clear_texture.rs @@ -1,5 +1,7 @@ use wasm_bindgen_test::*; -use wgpu_test::{image::ReadbackBuffers, initialize_test, TestParameters, TestingContext}; +use wgpu_test::{ + image::ReadbackBuffers, initialize_test, FailureCase, TestParameters, TestingContext, +}; static TEXTURE_FORMATS_UNCOMPRESSED_GLES_COMPAT: &[wgpu::TextureFormat] = &[ wgpu::TextureFormat::R8Unorm, @@ -328,7 +330,7 @@ fn clear_texture_tests(ctx: &TestingContext, formats: &[wgpu::TextureFormat]) { fn clear_texture_uncompressed_gles_compat() { initialize_test( TestParameters::default() - .webgl2_failure() + .skip(FailureCase::webgl2()) .features(wgpu::Features::CLEAR_TEXTURE), |ctx| { clear_texture_tests(&ctx, TEXTURE_FORMATS_UNCOMPRESSED_GLES_COMPAT); @@ -341,8 +343,8 @@ fn clear_texture_uncompressed_gles_compat() { fn clear_texture_uncompressed() { initialize_test( TestParameters::default() - .webgl2_failure() - .backend_failure(wgpu::Backends::GL) + .skip(FailureCase::webgl2()) + .expect_fail(FailureCase::backend(wgpu::Backends::GL)) .features(wgpu::Features::CLEAR_TEXTURE), |ctx| { clear_texture_tests(&ctx, TEXTURE_FORMATS_UNCOMPRESSED); @@ -355,7 +357,7 @@ fn clear_texture_uncompressed() { fn clear_texture_depth() { initialize_test( TestParameters::default() - .webgl2_failure() + .skip(FailureCase::webgl2()) .downlevel_flags( wgpu::DownlevelFlags::DEPTH_TEXTURE_AND_BUFFER_COPIES | wgpu::DownlevelFlags::COMPUTE_SHADERS, @@ -385,8 +387,10 @@ fn clear_texture_bc() { initialize_test( TestParameters::default() .features(wgpu::Features::CLEAR_TEXTURE | wgpu::Features::TEXTURE_COMPRESSION_BC) - .specific_failure(Some(wgpu::Backends::GL), None, Some("ANGLE"), false) // https://bugs.chromium.org/p/angleproject/issues/detail?id=7056 - .backend_failure(wgpu::Backends::GL), // compressed texture copy to buffer not yet implemented + // https://bugs.chromium.org/p/angleproject/issues/detail?id=7056 + .expect_fail(FailureCase::backend_adapter(wgpu::Backends::GL, "ANGLE")) + // compressed texture copy to buffer not yet implemented + .expect_fail(FailureCase::backend(wgpu::Backends::GL)), |ctx| { clear_texture_tests(&ctx, TEXTURE_FORMATS_BC); }, @@ -402,8 +406,10 @@ fn clear_texture_astc() { max_texture_dimension_2d: wgpu::COPY_BYTES_PER_ROW_ALIGNMENT * 12, ..wgpu::Limits::downlevel_defaults() }) - .specific_failure(Some(wgpu::Backends::GL), None, Some("ANGLE"), false) // https://bugs.chromium.org/p/angleproject/issues/detail?id=7056 - .backend_failure(wgpu::Backends::GL), // compressed texture copy to buffer not yet implemented + // https://bugs.chromium.org/p/angleproject/issues/detail?id=7056 + .expect_fail(FailureCase::backend_adapter(wgpu::Backends::GL, "ANGLE")) + // compressed texture copy to buffer not yet implemented + .expect_fail(FailureCase::backend(wgpu::Backends::GL)), |ctx| { clear_texture_tests(&ctx, TEXTURE_FORMATS_ASTC); }, @@ -415,8 +421,10 @@ fn clear_texture_etc2() { initialize_test( TestParameters::default() .features(wgpu::Features::CLEAR_TEXTURE | wgpu::Features::TEXTURE_COMPRESSION_ETC2) - .specific_failure(Some(wgpu::Backends::GL), None, Some("ANGLE"), false) // https://bugs.chromium.org/p/angleproject/issues/detail?id=7056 - .backend_failure(wgpu::Backends::GL), // compressed texture copy to buffer not yet implemented + // https://bugs.chromium.org/p/angleproject/issues/detail?id=7056 + .expect_fail(FailureCase::backend_adapter(wgpu::Backends::GL, "ANGLE")) + // compressed texture copy to buffer not yet implemented + .expect_fail(FailureCase::backend(wgpu::Backends::GL)), |ctx| { clear_texture_tests(&ctx, TEXTURE_FORMATS_ETC2); }, diff --git a/tests/tests/device.rs b/tests/tests/device.rs index 945d5476d7..f43791f86e 100644 --- a/tests/tests/device.rs +++ b/tests/tests/device.rs @@ -1,6 +1,6 @@ use wasm_bindgen_test::*; -use wgpu_test::{initialize_test, TestParameters}; +use wgpu_test::{initialize_test, FailureCase, TestParameters}; #[test] #[wasm_bindgen_test] @@ -13,26 +13,30 @@ fn device_initialization() { #[test] #[ignore] fn device_mismatch() { - initialize_test(TestParameters::default().failure(), |ctx| { - // Create a bind group uisng a lyaout from another device. This should be a validation - // error but currently crashes. - let (device2, _) = - pollster::block_on(ctx.adapter.request_device(&Default::default(), None)).unwrap(); + initialize_test( + // https://github.com/gfx-rs/wgpu/issues/3927 + TestParameters::default().expect_fail(FailureCase::always()), + |ctx| { + // Create a bind group uisng a lyaout from another device. This should be a validation + // error but currently crashes. + let (device2, _) = + pollster::block_on(ctx.adapter.request_device(&Default::default(), None)).unwrap(); - { - let bind_group_layout = - device2.create_bind_group_layout(&wgpu::BindGroupLayoutDescriptor { + { + let bind_group_layout = + device2.create_bind_group_layout(&wgpu::BindGroupLayoutDescriptor { + label: None, + entries: &[], + }); + + let _bind_group = ctx.device.create_bind_group(&wgpu::BindGroupDescriptor { label: None, + layout: &bind_group_layout, entries: &[], }); + } - let _bind_group = ctx.device.create_bind_group(&wgpu::BindGroupDescriptor { - label: None, - layout: &bind_group_layout, - entries: &[], - }); - } - - ctx.device.poll(wgpu::Maintain::Poll); - }); + ctx.device.poll(wgpu::Maintain::Poll); + }, + ); } diff --git a/tests/tests/encoder.rs b/tests/tests/encoder.rs index eada44ec45..5914cd22da 100644 --- a/tests/tests/encoder.rs +++ b/tests/tests/encoder.rs @@ -1,6 +1,6 @@ use wasm_bindgen_test::*; use wgpu::RenderPassDescriptor; -use wgpu_test::{fail, initialize_test, TestParameters}; +use wgpu_test::{fail, initialize_test, FailureCase, TestParameters}; #[test] #[wasm_bindgen_test] @@ -22,7 +22,8 @@ fn drop_encoder_after_error() { // #543: COMMAND_ALLOCATOR_CANNOT_RESET] // // For now, we mark the test as failing on DX12. - let parameters = TestParameters::default().backend_failure(wgpu::Backends::DX12); + let parameters = + TestParameters::default().expect_fail(FailureCase::backend(wgpu::Backends::DX12)); initialize_test(parameters, |ctx| { let mut encoder = ctx .device diff --git a/tests/tests/poll.rs b/tests/tests/poll.rs index 7409dad093..e27a47a42c 100644 --- a/tests/tests/poll.rs +++ b/tests/tests/poll.rs @@ -7,7 +7,7 @@ use wgpu::{ }; use wasm_bindgen_test::*; -use wgpu_test::{initialize_test, TestParameters, TestingContext}; +use wgpu_test::{initialize_test, FailureCase, TestParameters, TestingContext}; fn generate_dummy_work(ctx: &TestingContext) -> CommandBuffer { let buffer = ctx.device.create_buffer(&BufferDescriptor { @@ -56,60 +56,75 @@ fn generate_dummy_work(ctx: &TestingContext) -> CommandBuffer { #[test] #[wasm_bindgen_test] fn wait() { - initialize_test(TestParameters::default().skip(), |ctx| { - let cmd_buf = generate_dummy_work(&ctx); - - ctx.queue.submit(Some(cmd_buf)); - ctx.device.poll(Maintain::Wait); - }) + initialize_test( + TestParameters::default().skip(FailureCase::always()), + |ctx| { + let cmd_buf = generate_dummy_work(&ctx); + + ctx.queue.submit(Some(cmd_buf)); + ctx.device.poll(Maintain::Wait); + }, + ) } #[test] #[wasm_bindgen_test] fn double_wait() { - initialize_test(TestParameters::default().skip(), |ctx| { - let cmd_buf = generate_dummy_work(&ctx); - - ctx.queue.submit(Some(cmd_buf)); - ctx.device.poll(Maintain::Wait); - ctx.device.poll(Maintain::Wait); - }) + initialize_test( + TestParameters::default().skip(FailureCase::always()), + |ctx| { + let cmd_buf = generate_dummy_work(&ctx); + + ctx.queue.submit(Some(cmd_buf)); + ctx.device.poll(Maintain::Wait); + ctx.device.poll(Maintain::Wait); + }, + ) } #[test] #[wasm_bindgen_test] fn wait_on_submission() { - initialize_test(TestParameters::default().skip(), |ctx| { - let cmd_buf = generate_dummy_work(&ctx); - - let index = ctx.queue.submit(Some(cmd_buf)); - ctx.device.poll(Maintain::WaitForSubmissionIndex(index)); - }) + initialize_test( + TestParameters::default().skip(FailureCase::always()), + |ctx| { + let cmd_buf = generate_dummy_work(&ctx); + + let index = ctx.queue.submit(Some(cmd_buf)); + ctx.device.poll(Maintain::WaitForSubmissionIndex(index)); + }, + ) } #[test] #[wasm_bindgen_test] fn double_wait_on_submission() { - initialize_test(TestParameters::default().skip(), |ctx| { - let cmd_buf = generate_dummy_work(&ctx); - - let index = ctx.queue.submit(Some(cmd_buf)); - ctx.device - .poll(Maintain::WaitForSubmissionIndex(index.clone())); - ctx.device.poll(Maintain::WaitForSubmissionIndex(index)); - }) + initialize_test( + TestParameters::default().skip(FailureCase::always()), + |ctx| { + let cmd_buf = generate_dummy_work(&ctx); + + let index = ctx.queue.submit(Some(cmd_buf)); + ctx.device + .poll(Maintain::WaitForSubmissionIndex(index.clone())); + ctx.device.poll(Maintain::WaitForSubmissionIndex(index)); + }, + ) } #[test] #[wasm_bindgen_test] fn wait_out_of_order() { - initialize_test(TestParameters::default().skip(), |ctx| { - let cmd_buf1 = generate_dummy_work(&ctx); - let cmd_buf2 = generate_dummy_work(&ctx); - - let index1 = ctx.queue.submit(Some(cmd_buf1)); - let index2 = ctx.queue.submit(Some(cmd_buf2)); - ctx.device.poll(Maintain::WaitForSubmissionIndex(index2)); - ctx.device.poll(Maintain::WaitForSubmissionIndex(index1)); - }) + initialize_test( + TestParameters::default().skip(FailureCase::always()), + |ctx| { + let cmd_buf1 = generate_dummy_work(&ctx); + let cmd_buf2 = generate_dummy_work(&ctx); + + let index1 = ctx.queue.submit(Some(cmd_buf1)); + let index2 = ctx.queue.submit(Some(cmd_buf2)); + ctx.device.poll(Maintain::WaitForSubmissionIndex(index2)); + ctx.device.poll(Maintain::WaitForSubmissionIndex(index1)); + }, + ) } diff --git a/tests/tests/shader/struct_layout.rs b/tests/tests/shader/struct_layout.rs index bc433b5820..7da8cfeef8 100644 --- a/tests/tests/shader/struct_layout.rs +++ b/tests/tests/shader/struct_layout.rs @@ -4,7 +4,7 @@ use wasm_bindgen_test::*; use wgpu::{Backends, DownlevelFlags, Features, Limits}; use crate::shader::{shader_input_output_test, InputStorageType, ShaderTest, MAX_BUFFER_SIZE}; -use wgpu_test::{initialize_test, TestParameters}; +use wgpu_test::{initialize_test, FailureCase, TestParameters}; fn create_struct_layout_tests(storage_type: InputStorageType) -> Vec { let input_values: Vec<_> = (0..(MAX_BUFFER_SIZE as u32 / 4)).collect(); @@ -182,7 +182,7 @@ fn uniform_input() { TestParameters::default() .downlevel_flags(DownlevelFlags::COMPUTE_SHADERS) // Validation errors thrown by the SPIR-V validator https://github.com/gfx-rs/naga/issues/2034 - .specific_failure(Some(wgpu::Backends::VULKAN), None, None, false) + .expect_fail(FailureCase::backend(wgpu::Backends::VULKAN)) .limits(Limits::downlevel_defaults()), |ctx| { shader_input_output_test( @@ -222,7 +222,7 @@ fn push_constant_input() { max_push_constant_size: MAX_BUFFER_SIZE as u32, ..Limits::downlevel_defaults() }) - .backend_failure(Backends::GL), + .expect_fail(FailureCase::backend(Backends::GL)), |ctx| { shader_input_output_test( ctx, diff --git a/tests/tests/shader/zero_init_workgroup_mem.rs b/tests/tests/shader/zero_init_workgroup_mem.rs index a666d2aa28..cbd1b3e561 100644 --- a/tests/tests/shader/zero_init_workgroup_mem.rs +++ b/tests/tests/shader/zero_init_workgroup_mem.rs @@ -8,7 +8,7 @@ use wgpu::{ ShaderStages, }; -use wgpu_test::{initialize_test, TestParameters, TestingContext}; +use wgpu_test::{initialize_test, FailureCase, TestParameters, TestingContext}; #[test] fn zero_init_workgroup_mem() { @@ -18,13 +18,16 @@ fn zero_init_workgroup_mem() { .limits(Limits::downlevel_defaults()) // remove both of these once we get to https://github.com/gfx-rs/wgpu/issues/3193 or // https://github.com/gfx-rs/wgpu/issues/3160 - .specific_failure( - Some(Backends::DX12), - Some(5140), - Some("Microsoft Basic Render Driver"), - true, - ) - .specific_failure(Some(Backends::VULKAN), None, Some("swiftshader"), true), + .skip(FailureCase { + backends: Some(Backends::DX12), + vendor: Some(5140), + adapter: Some("Microsoft Basic Render Driver"), + ..FailureCase::default() + }) + .skip(FailureCase::backend_adapter( + Backends::VULKAN, + "swiftshader", + )), zero_init_workgroup_mem_impl, ); } diff --git a/tests/tests/shader_view_format/mod.rs b/tests/tests/shader_view_format/mod.rs index 1d7dd2630d..46741b4ea8 100644 --- a/tests/tests/shader_view_format/mod.rs +++ b/tests/tests/shader_view_format/mod.rs @@ -1,12 +1,17 @@ use wgpu::{util::DeviceExt, DownlevelFlags, Limits, TextureFormat}; -use wgpu_test::{image::calc_difference, initialize_test, TestParameters, TestingContext}; +use wgpu_test::{ + image::calc_difference, initialize_test, FailureCase, TestParameters, TestingContext, +}; #[test] fn reinterpret_srgb_ness() { let parameters = TestParameters::default() .downlevel_flags(DownlevelFlags::VIEW_FORMATS) .limits(Limits::downlevel_defaults()) - .specific_failure(Some(wgpu::Backends::GL), None, None, true); + .skip(FailureCase { + backends: Some(wgpu::Backends::GL), + ..FailureCase::default() + }); initialize_test(parameters, |ctx| { let unorm_data: [[u8; 4]; 4] = [ [180, 0, 0, 255], diff --git a/tests/tests/vertex_indices/mod.rs b/tests/tests/vertex_indices/mod.rs index 136876017f..edd4f7b057 100644 --- a/tests/tests/vertex_indices/mod.rs +++ b/tests/tests/vertex_indices/mod.rs @@ -3,7 +3,7 @@ use std::num::NonZeroU64; use wasm_bindgen_test::*; use wgpu::util::DeviceExt; -use wgpu_test::{initialize_test, TestParameters, TestingContext}; +use wgpu_test::{initialize_test, FailureCase, TestParameters, TestingContext}; fn pulling_common( ctx: TestingContext, @@ -150,7 +150,7 @@ fn draw_vertex_offset() { initialize_test( TestParameters::default() .test_features_limits() - .backend_failure(wgpu::Backends::DX11), + .expect_fail(FailureCase::backend(wgpu::Backends::DX11)), |ctx| { pulling_common(ctx, &[0, 1, 2, 3, 4, 5], |cmb| { cmb.draw(0..3, 0..1); @@ -176,7 +176,7 @@ fn draw_instanced_offset() { initialize_test( TestParameters::default() .test_features_limits() - .backend_failure(wgpu::Backends::DX11), + .expect_fail(FailureCase::backend(wgpu::Backends::DX11)), |ctx| { pulling_common(ctx, &[0, 1, 2, 3, 4, 5], |cmb| { cmb.draw(0..3, 0..1); diff --git a/tests/tests/write_texture.rs b/tests/tests/write_texture.rs index 0578c60352..8b33cae7f5 100644 --- a/tests/tests/write_texture.rs +++ b/tests/tests/write_texture.rs @@ -1,6 +1,6 @@ //! Tests for texture copy -use wgpu_test::{initialize_test, TestParameters}; +use wgpu_test::{initialize_test, FailureCase, TestParameters}; use wasm_bindgen_test::*; @@ -8,7 +8,8 @@ use wasm_bindgen_test::*; #[wasm_bindgen_test] fn write_texture_subset_2d() { let size = 256; - let parameters = TestParameters::default().backend_failure(wgpu::Backends::DX12); + let parameters = + TestParameters::default().expect_fail(FailureCase::backend(wgpu::Backends::DX12)); initialize_test(parameters, |ctx| { let tex = ctx.device.create_texture(&wgpu::TextureDescriptor { label: None, diff --git a/tests/tests/zero_init_texture_after_discard.rs b/tests/tests/zero_init_texture_after_discard.rs index 4d508f8280..2b757e069a 100644 --- a/tests/tests/zero_init_texture_after_discard.rs +++ b/tests/tests/zero_init_texture_after_discard.rs @@ -1,38 +1,46 @@ use wasm_bindgen_test::*; use wgpu::*; -use wgpu_test::{image::ReadbackBuffers, initialize_test, TestParameters, TestingContext}; +use wgpu_test::{ + image::ReadbackBuffers, initialize_test, FailureCase, TestParameters, TestingContext, +}; // Checks if discarding a color target resets its init state, causing a zero read of this texture when copied in after submit of the encoder. #[test] #[wasm_bindgen_test] fn discarding_color_target_resets_texture_init_state_check_visible_on_copy_after_submit() { - initialize_test(TestParameters::default().webgl2_failure(), |mut ctx| { - let mut case = TestCase::new(&mut ctx, TextureFormat::Rgba8UnormSrgb); - case.create_command_encoder(); - case.discard(); - case.submit_command_encoder(); + initialize_test( + TestParameters::default().skip(FailureCase::webgl2()), + |mut ctx| { + let mut case = TestCase::new(&mut ctx, TextureFormat::Rgba8UnormSrgb); + case.create_command_encoder(); + case.discard(); + case.submit_command_encoder(); - case.create_command_encoder(); - case.copy_texture_to_buffer(); - case.submit_command_encoder(); + case.create_command_encoder(); + case.copy_texture_to_buffer(); + case.submit_command_encoder(); - case.assert_buffers_are_zero(); - }); + case.assert_buffers_are_zero(); + }, + ); } // Checks if discarding a color target resets its init state, causing a zero read of this texture when copied in the same encoder to a buffer. #[test] #[wasm_bindgen_test] fn discarding_color_target_resets_texture_init_state_check_visible_on_copy_in_same_encoder() { - initialize_test(TestParameters::default().webgl2_failure(), |mut ctx| { - let mut case = TestCase::new(&mut ctx, TextureFormat::Rgba8UnormSrgb); - case.create_command_encoder(); - case.discard(); - case.copy_texture_to_buffer(); - case.submit_command_encoder(); + initialize_test( + TestParameters::default().skip(FailureCase::webgl2()), + |mut ctx| { + let mut case = TestCase::new(&mut ctx, TextureFormat::Rgba8UnormSrgb); + case.create_command_encoder(); + case.discard(); + case.copy_texture_to_buffer(); + case.submit_command_encoder(); - case.assert_buffers_are_zero(); - }); + case.assert_buffers_are_zero(); + }, + ); } #[test] From e45119550091398403966c16ccfc7c1cd1172924 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 5 Sep 2023 15:11:05 +0200 Subject: [PATCH 06/33] Bump thiserror from 1.0.47 to 1.0.48 (#4112) Bumps [thiserror](https://github.com/dtolnay/thiserror) from 1.0.47 to 1.0.48. - [Release notes](https://github.com/dtolnay/thiserror/releases) - [Commits](https://github.com/dtolnay/thiserror/compare/1.0.47...1.0.48) --- updated-dependencies: - dependency-name: thiserror dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Cargo.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5a28ec8f7a..6a47c810ab 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2642,18 +2642,18 @@ dependencies = [ [[package]] name = "thiserror" -version = "1.0.47" +version = "1.0.48" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "97a802ec30afc17eee47b2855fc72e0c4cd62be9b4efe6591edde0ec5bd68d8f" +checksum = "9d6d7a740b8a666a7e828dd00da9c0dc290dff53154ea77ac109281de90589b7" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.47" +version = "1.0.48" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6bb623b56e39ab7dcd4b1b98bb6c8f8d907ed255b18de254088016b27a8ee19b" +checksum = "49922ecae66cc8a249b77e68d1d0623c1b2c514f0060c27cdc68bd62a1219d35" dependencies = [ "proc-macro2", "quote", From e8e53fb31c6f8bce07e7c84eb3ded6d0ab94691f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 5 Sep 2023 15:20:04 +0200 Subject: [PATCH 07/33] Bump serde from 1.0.186 to 1.0.188 (#4091) Bumps [serde](https://github.com/serde-rs/serde) from 1.0.186 to 1.0.188. - [Release notes](https://github.com/serde-rs/serde/releases) - [Commits](https://github.com/serde-rs/serde/compare/v1.0.186...v1.0.188) --- updated-dependencies: - dependency-name: serde dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Cargo.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6a47c810ab..3921a045a7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2400,9 +2400,9 @@ checksum = "388a1df253eca08550bef6c72392cfe7c30914bf41df5269b68cbd6ff8f570a3" [[package]] name = "serde" -version = "1.0.186" +version = "1.0.188" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9f5db24220c009de9bd45e69fb2938f4b6d2df856aa9304ce377b3180f83b7c1" +checksum = "cf9e0fcba69a370eed61bcf2b728575f726b50b55cba78064753d708ddc7549e" dependencies = [ "serde_derive", ] @@ -2418,9 +2418,9 @@ dependencies = [ [[package]] name = "serde_derive" -version = "1.0.186" +version = "1.0.188" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5ad697f7e0b65af4983a4ce8f56ed5b357e8d3c36651bf6a7e13639c17b8e670" +checksum = "4eca7ac642d82aa35b60049a6eccb4be6be75e599bd2e9adb5f875a737654af2" dependencies = [ "proc-macro2", "quote", From 9591505af16007ace9d6b0659a313a319c299854 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 5 Sep 2023 15:27:38 +0200 Subject: [PATCH 08/33] Bump actions/checkout from 3 to 4 (#4117) Bumps [actions/checkout](https://github.com/actions/checkout) from 3 to 4. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/checkout/compare/v3...v4) --- updated-dependencies: - dependency-name: actions/checkout dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/ci.yml | 16 ++++++++-------- .github/workflows/cts.yml | 2 +- .github/workflows/docs.yml | 2 +- .github/workflows/publish.yml | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 06ae299b77..981fcd3498 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -93,7 +93,7 @@ jobs: steps: - name: checkout repo - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Install MSRV toolchain run: | @@ -181,7 +181,7 @@ jobs: runs-on: ubuntu-latest steps: - name: checkout repo - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Install wasm-pack uses: taiki-e/install-action@v2 @@ -219,7 +219,7 @@ jobs: steps: - name: checkout repo - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Install cargo-nextest and cargo-llvm-cov uses: taiki-e/install-action@v2 @@ -309,7 +309,7 @@ jobs: steps: - name: checkout repo - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: disable debug shell: bash @@ -336,7 +336,7 @@ jobs: runs-on: ubuntu-latest steps: - name: checkout repo - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: run rustfmt run: | @@ -347,7 +347,7 @@ jobs: runs-on: ubuntu-latest steps: - name: checkout repo - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Install MSRV toolchain run: | @@ -376,7 +376,7 @@ jobs: runs-on: ubuntu-latest steps: - name: checkout repo - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Run `cargo deny check` uses: EmbarkStudios/cargo-deny-action@v1 @@ -390,7 +390,7 @@ jobs: runs-on: ubuntu-latest steps: - name: checkout repo - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Run `cargo deny check` uses: EmbarkStudios/cargo-deny-action@v1 diff --git a/.github/workflows/cts.yml b/.github/workflows/cts.yml index 70479533cf..e4bb20e7b1 100644 --- a/.github/workflows/cts.yml +++ b/.github/workflows/cts.yml @@ -39,7 +39,7 @@ jobs: steps: - name: checkout repo - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: path: wgpu diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 11d8d9e962..396a93ef04 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -16,7 +16,7 @@ jobs: steps: - name: Checkout the code - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: persist-credentials: false diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 81a2a7b407..f0aa086961 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -17,7 +17,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout the code - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: persist-credentials: false From ff807295da214a975cb2255601320352942ef5c5 Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Tue, 5 Sep 2023 09:41:46 -0400 Subject: [PATCH 09/33] hal/vulkan: `Instance::required_extensions` -> `desired_extensions` (#4115) Rename `wgpu_hal::vulkan::Instance::required_extensions` to `desired_extensions`, to match its behavior. Document the function to clarify its role. --- CHANGELOG.md | 2 ++ wgpu-hal/src/vulkan/instance.rs | 19 ++++++++++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ca6f6350a..8940f627a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -80,6 +80,8 @@ By @Valaphee in [#3402](https://github.com/gfx-rs/wgpu/pull/3402) #### Vulkan +- Rename `wgpu_hal::vulkan::Instance::required_extensions` to `desired_extensions`. By @jimblandy in [#4115](https://github.com/gfx-rs/wgpu/pull/4115) + - Don't bother calling `vkFreeCommandBuffers` when `vkDestroyCommandPool` will take care of that for us. By @jimblandy in [#4059](https://github.com/gfx-rs/wgpu/pull/4059) diff --git a/wgpu-hal/src/vulkan/instance.rs b/wgpu-hal/src/vulkan/instance.rs index 18b141a070..0fcee254af 100644 --- a/wgpu-hal/src/vulkan/instance.rs +++ b/wgpu-hal/src/vulkan/instance.rs @@ -186,7 +186,20 @@ impl super::Instance { &self.shared } - pub fn required_extensions( + /// Return the instance extension names wgpu would like to enable. + /// + /// Return a vector of the names of instance extensions actually available + /// on `entry` that wgpu would like to enable. + /// + /// The `driver_api_version` argument should be the instance's Vulkan API + /// version, as obtained from `vkEnumerateInstanceVersion`. This is the same + /// space of values as the `VK_API_VERSION` constants. + /// + /// Note that wgpu can function without many of these extensions (for + /// example, `VK_KHR_wayland_surface` is certainly not going to be available + /// everywhere), but if one of these extensions is available at all, wgpu + /// assumes that it has been enabled. + pub fn desired_extensions( entry: &ash::Entry, _driver_api_version: u32, flags: crate::InstanceFlags, @@ -265,7 +278,7 @@ impl super::Instance { /// /// - `raw_instance` must be created from `entry` /// - `raw_instance` must be created respecting `driver_api_version`, `extensions` and `flags` - /// - `extensions` must be a superset of `required_extensions()` and must be created from the + /// - `extensions` must be a superset of `desired_extensions()` and must be created from the /// same entry, driver_api_version and flags. /// - `android_sdk_version` is ignored and can be `0` for all platforms besides Android /// @@ -592,7 +605,7 @@ impl crate::Instance for super::Instance { }, ); - let extensions = Self::required_extensions(&entry, driver_api_version, desc.flags)?; + let extensions = Self::desired_extensions(&entry, driver_api_version, desc.flags)?; let instance_layers = entry.enumerate_instance_layer_properties().map_err(|e| { log::info!("enumerate_instance_layer_properties: {:?}", e); From d17165f08b46628360b2400975ea2770c40826a1 Mon Sep 17 00:00:00 2001 From: Luke Jones Date: Wed, 6 Sep 2023 02:20:04 +1200 Subject: [PATCH 10/33] Enable vulkan presentation on Intel Mesa >= v21.2 (#4110) Due to an issue with Mesa versions less than 21.2 presentation on Vulkan was forced to Nvidia only. This in itself brought new issues around the Nvidia driver specfic format modifers. As of Mesa 21.2 the Intel vulkan issue is fixed. This commit enables presentation on versions 21.2 and above for Intel. References: - https://github.com/NVIDIA/egl-wayland/issues/72 Closes [#4101](https://github.com/gfx-rs/wgpu/issues/4101) --- CHANGELOG.md | 2 ++ wgpu-hal/src/vulkan/instance.rs | 23 ++++++++++++++++------- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8940f627a8..db91b89718 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -103,6 +103,8 @@ By @Valaphee in [#3402](https://github.com/gfx-rs/wgpu/pull/3402) - Enhancement of [#4038], using ash's definition instead of hard-coded c_str. By @hybcloud in[#4044](https://github.com/gfx-rs/wgpu/pull/4044). +- Enable vulkan presentation on (Linux) Intel Mesa >= v21.2. By @flukejones in[#4110](https://github.com/gfx-rs/wgpu/pull/4110) + #### DX12 - DX12 doesn't support `Features::POLYGON_MODE_POINT``. By @teoxoy in [#4032](https://github.com/gfx-rs/wgpu/pull/4032). diff --git a/wgpu-hal/src/vulkan/instance.rs b/wgpu-hal/src/vulkan/instance.rs index 0fcee254af..34a2c4f23c 100644 --- a/wgpu-hal/src/vulkan/instance.rs +++ b/wgpu-hal/src/vulkan/instance.rs @@ -799,13 +799,22 @@ impl crate::Instance for super::Instance { if exposed.info.device_type == wgt::DeviceType::IntegratedGpu && exposed.info.vendor == db::intel::VENDOR { - // See https://gitlab.freedesktop.org/mesa/mesa/-/issues/4688 - log::warn!( - "Disabling presentation on '{}' (id {:?}) because of NV Optimus (on Linux)", - exposed.info.name, - exposed.adapter.raw - ); - exposed.adapter.private_caps.can_present = false; + // Check if mesa driver and version less than 21.2 + if let Some(version) = exposed.info.driver_info.split_once("Mesa ").map(|s| { + s.1.rsplit_once('.') + .map(|v| v.0.parse::().unwrap_or_default()) + .unwrap_or_default() + }) { + if version < 21.2 { + // See https://gitlab.freedesktop.org/mesa/mesa/-/issues/4688 + log::warn!( + "Disabling presentation on '{}' (id {:?}) due to NV Optimus and Intel Mesa < v21.2", + exposed.info.name, + exposed.adapter.raw + ); + exposed.adapter.private_caps.can_present = false; + } + } } } } From 7634ae6112923f8bc97580b933e1170fb7898c69 Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Mon, 4 Sep 2023 14:05:21 -0700 Subject: [PATCH 11/33] wgpu_core: Add logging to Instance::new. For each backend `blah`, log::debug/trace whether we were able to populate `Instance::blah`. --- wgpu-core/src/instance.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/wgpu-core/src/instance.rs b/wgpu-core/src/instance.rs index ae1a395d85..0aee56ac6e 100644 --- a/wgpu-core/src/instance.rs +++ b/wgpu-core/src/instance.rs @@ -84,8 +84,22 @@ impl Instance { dx12_shader_compiler: instance_desc.dx12_shader_compiler.clone(), gles_minor_version: instance_desc.gles_minor_version, }; - unsafe { hal::Instance::init(&hal_desc).ok() } + match unsafe { hal::Instance::init(&hal_desc) } { + Ok(instance) => { + log::debug!("Instance::new: created {:?} backend", A::VARIANT); + Some(instance) + } + Err(err) => { + log::debug!( + "Instance::new: failed to create {:?} backend: {:?}", + A::VARIANT, + err + ); + None + } + } } else { + log::trace!("Instance::new: backend {:?} not requested", A::VARIANT); None } } From 4235b0dd1cd2fd6ef2f3be8a3bc1b4785a7e299a Mon Sep 17 00:00:00 2001 From: Connor Fitzgerald Date: Tue, 5 Sep 2023 14:06:33 -0400 Subject: [PATCH 12/33] Fix D3D12 Surface Leak (#4106) --- wgpu-core/src/device/global.rs | 26 +++++++++++++++++++------- wgpu-core/src/device/mod.rs | 3 +-- wgpu-core/src/present.rs | 14 +++++++++++++- wgpu-hal/src/dx12/device.rs | 5 ++++- wgpu-hal/src/dx12/mod.rs | 33 ++++++++++++++++++++++++--------- wgpu-hal/src/lib.rs | 16 ++++++++++++++++ wgpu-hal/src/vulkan/device.rs | 2 +- wgpu-hal/src/vulkan/instance.rs | 9 +++++---- 8 files changed, 83 insertions(+), 25 deletions(-) diff --git a/wgpu-core/src/device/global.rs b/wgpu-core/src/device/global.rs index 8fe5a6fcc9..632c83e37f 100644 --- a/wgpu-core/src/device/global.rs +++ b/wgpu-core/src/device/global.rs @@ -2134,7 +2134,7 @@ impl Global { let (mut surface_guard, mut token) = self.surfaces.write(&mut token); let (adapter_guard, mut token) = hub.adapters.read(&mut token); - let (device_guard, _token) = hub.devices.read(&mut token); + let (device_guard, mut token) = hub.devices.read(&mut token); let error = 'outer: loop { let device = match device_guard.get(device_id) { @@ -2207,6 +2207,24 @@ impl Global { break error; } + // Wait for all work to finish before configuring the surface. + if let Err(e) = device.maintain(hub, wgt::Maintain::Wait, &mut token) { + break e.into(); + } + + // All textures must be destroyed before the surface can be re-configured. + if let Some(present) = surface.presentation.take() { + if present.acquired_texture.is_some() { + break E::PreviousOutputExists; + } + } + + // TODO: Texture views may still be alive that point to the texture. + // this will allow the user to render to the surface texture, long after + // it has been removed. + // + // https://github.com/gfx-rs/wgpu/issues/4105 + match unsafe { A::get_surface_mut(surface) .unwrap() @@ -2226,12 +2244,6 @@ impl Global { } } - if let Some(present) = surface.presentation.take() { - if present.acquired_texture.is_some() { - break E::PreviousOutputExists; - } - } - surface.presentation = Some(present::Presentation { device_id: Stored { value: id::Valid(device_id), diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index 0ae6d7a2dd..9a77bf9536 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -1,6 +1,5 @@ use crate::{ binding_model, - device::life::WaitIdleError, hal_api::HalApi, hub::Hub, id, @@ -24,7 +23,7 @@ pub mod queue; pub mod resource; #[cfg(any(feature = "trace", feature = "replay"))] pub mod trace; -pub use resource::Device; +pub use {life::WaitIdleError, resource::Device}; pub const SHADER_STAGE_COUNT: usize = 3; // Should be large enough for the largest possible texture row. This diff --git a/wgpu-core/src/present.rs b/wgpu-core/src/present.rs index 1303769d29..7366934d27 100644 --- a/wgpu-core/src/present.rs +++ b/wgpu-core/src/present.rs @@ -15,7 +15,7 @@ use std::borrow::Borrow; use crate::device::trace::Action; use crate::{ conv, - device::{DeviceError, MissingDownlevelFlags}, + device::{DeviceError, MissingDownlevelFlags, WaitIdleError}, global::Global, hal_api::HalApi, hub::Token, @@ -96,6 +96,18 @@ pub enum ConfigureSurfaceError { }, #[error("Requested usage is not supported")] UnsupportedUsage, + #[error("Gpu got stuck :(")] + StuckGpu, +} + +impl From for ConfigureSurfaceError { + fn from(e: WaitIdleError) -> Self { + match e { + WaitIdleError::Device(d) => ConfigureSurfaceError::Device(d), + WaitIdleError::WrongSubmissionIndex(..) => unreachable!(), + WaitIdleError::StuckGpu => ConfigureSurfaceError::StuckGpu, + } + } } #[repr(C)] diff --git a/wgpu-hal/src/dx12/device.rs b/wgpu-hal/src/dx12/device.rs index 1a44d98f0d..4ad43cc165 100644 --- a/wgpu-hal/src/dx12/device.rs +++ b/wgpu-hal/src/dx12/device.rs @@ -181,7 +181,10 @@ impl super::Device { }) } - pub(super) unsafe fn wait_idle(&self) -> Result<(), crate::DeviceError> { + // Blocks until the dedicated present queue is finished with all of its work. + // + // Once this method completes, the surface is able to be resized or deleted. + pub(super) unsafe fn wait_for_present_queue_idle(&self) -> Result<(), crate::DeviceError> { let cur_value = self.idler.fence.get_value(); if cur_value == !0 { return Err(crate::DeviceError::Lost); diff --git a/wgpu-hal/src/dx12/mod.rs b/wgpu-hal/src/dx12/mod.rs index 564bc349c6..a231619512 100644 --- a/wgpu-hal/src/dx12/mod.rs +++ b/wgpu-hal/src/dx12/mod.rs @@ -613,19 +613,23 @@ impl crate::Surface for Surface { let mut flags = dxgi::DXGI_SWAP_CHAIN_FLAG_FRAME_LATENCY_WAITABLE_OBJECT; // We always set ALLOW_TEARING on the swapchain no matter // what kind of swapchain we want because ResizeBuffers - // cannot change if ALLOW_TEARING is applied to the swapchain. + // cannot change the swapchain's ALLOW_TEARING flag. + // + // This does not change the behavior of the swapchain, just + // allow present calls to use tearing. if self.supports_allow_tearing { flags |= dxgi::DXGI_SWAP_CHAIN_FLAG_ALLOW_TEARING; } + // While `configure`s contract ensures that no work on the GPU's main queues + // are in flight, we still need to wait for the present queue to be idle. + unsafe { device.wait_for_present_queue_idle() }?; + let non_srgb_format = auxil::dxgi::conv::map_texture_format_nosrgb(config.format); let swap_chain = match self.swap_chain.take() { //Note: this path doesn't properly re-initialize all of the things Some(sc) => { - // can't have image resources in flight used by GPU - let _ = unsafe { device.wait_idle() }; - let raw = unsafe { sc.release_resources() }; let result = unsafe { raw.ResizeBuffers( @@ -773,12 +777,16 @@ impl crate::Surface for Surface { } unsafe fn unconfigure(&mut self, device: &Device) { - if let Some(mut sc) = self.swap_chain.take() { + if let Some(sc) = self.swap_chain.take() { unsafe { - let _ = sc.wait(None); - //TODO: this shouldn't be needed, - // but it complains that the queue is still used otherwise - let _ = device.wait_idle(); + // While `unconfigure`s contract ensures that no work on the GPU's main queues + // are in flight, we still need to wait for the present queue to be idle. + + // The major failure mode of this function is device loss, + // which if we have lost the device, we should just continue + // cleaning up, without error. + let _ = device.wait_for_present_queue_idle(); + let _raw = sc.release_resources(); } } @@ -837,6 +845,13 @@ impl crate::Queue for Queue { .signal(&fence.raw, value) .into_device_result("Signal fence")?; } + + // Note the lack of synchronization here between the main Direct queue + // and the dedicated presentation queue. This is automatically handled + // by the D3D runtime by detecting uses of resources derived from the + // swapchain. This automatic detection is why you cannot use a swapchain + // as an UAV in D3D12. + Ok(()) } unsafe fn present( diff --git a/wgpu-hal/src/lib.rs b/wgpu-hal/src/lib.rs index 4bff6b8d8f..f1f4b2109e 100644 --- a/wgpu-hal/src/lib.rs +++ b/wgpu-hal/src/lib.rs @@ -227,12 +227,28 @@ pub trait Instance: Sized + WasmNotSend + WasmNotSync { } pub trait Surface: WasmNotSend + WasmNotSync { + /// Configures the surface to use the given device. + /// + /// # Safety + /// + /// - All gpu work that uses the surface must have been completed. + /// - All [`AcquiredSurfaceTexture`]s must have been destroyed. + /// - All [`Api::TextureView`]s derived from the [`AcquiredSurfaceTexture`]s must have been destroyed. + /// - All surfaces created using other devices must have been unconfigured before this call. unsafe fn configure( &mut self, device: &A::Device, config: &SurfaceConfiguration, ) -> Result<(), SurfaceError>; + /// Unconfigures the surface on the given device. + /// + /// # Safety + /// + /// - All gpu work that uses the surface must have been completed. + /// - All [`AcquiredSurfaceTexture`]s must have been destroyed. + /// - All [`Api::TextureView`]s derived from the [`AcquiredSurfaceTexture`]s must have been destroyed. + /// - The surface must have been configured on the given device. unsafe fn unconfigure(&mut self, device: &A::Device); /// Returns the next texture to be presented by the swapchain for drawing diff --git a/wgpu-hal/src/vulkan/device.rs b/wgpu-hal/src/vulkan/device.rs index 4f2a0feb8a..cb955e8318 100644 --- a/wgpu-hal/src/vulkan/device.rs +++ b/wgpu-hal/src/vulkan/device.rs @@ -1143,7 +1143,7 @@ impl crate::Device for super::Device { } if desc.anisotropy_clamp != 1 { - // We only enable anisotropy if it is supported, and wgpu-hal interface guarentees + // We only enable anisotropy if it is supported, and wgpu-hal interface guarantees // the clamp is in the range [1, 16] which is always supported if anisotropy is. vk_info = vk_info .anisotropy_enable(true) diff --git a/wgpu-hal/src/vulkan/instance.rs b/wgpu-hal/src/vulkan/instance.rs index 34a2c4f23c..18269fff77 100644 --- a/wgpu-hal/src/vulkan/instance.rs +++ b/wgpu-hal/src/vulkan/instance.rs @@ -152,12 +152,11 @@ unsafe extern "system" fn debug_utils_messenger_callback( } impl super::Swapchain { + /// # Safety + /// + /// - The device must have been made idle before calling this function. unsafe fn release_resources(self, device: &ash::Device) -> Self { profiling::scope!("Swapchain::release_resources"); - { - profiling::scope!("vkDeviceWaitIdle"); - let _ = unsafe { device.device_wait_idle() }; - }; unsafe { device.destroy_fence(self.fence, None) }; self } @@ -829,6 +828,7 @@ impl crate::Surface for super::Surface { device: &super::Device, config: &crate::SurfaceConfiguration, ) -> Result<(), crate::SurfaceError> { + // Safety: `configure`'s contract guarantees there are no resources derived from the swapchain in use. let old = self .swapchain .take() @@ -842,6 +842,7 @@ impl crate::Surface for super::Surface { unsafe fn unconfigure(&mut self, device: &super::Device) { if let Some(sc) = self.swapchain.take() { + // Safety: `unconfigure`'s contract guarantees there are no resources derived from the swapchain in use. let swapchain = unsafe { sc.release_resources(&device.shared.raw) }; unsafe { swapchain.functor.destroy_swapchain(swapchain.raw, None) }; } From 9a91953537de46ff27a66480273e1cbe968bd98b Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 6 Sep 2023 09:29:52 -0400 Subject: [PATCH 13/33] Bump bytemuck from 1.13.1 to 1.14.0 (#4123) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3921a045a7..c0a529700b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -262,9 +262,9 @@ checksum = "a3e2c3daef883ecc1b5d58c15adae93470a91d425f3532ba1695849656af3fc1" [[package]] name = "bytemuck" -version = "1.13.1" +version = "1.14.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "17febce684fd15d89027105661fec94afb475cb995fbc59d2865198446ba2eea" +checksum = "374d28ec25809ee0e23827c2ab573d729e293f281dfe393500e7ad618baa61c6" dependencies = [ "bytemuck_derive", ] diff --git a/Cargo.toml b/Cargo.toml index 9455290b3d..c3676fada3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -60,7 +60,7 @@ arrayvec = "0.7" async-executor = "1" bitflags = "2" bit-vec = "0.6" -bytemuck = { version = "1.13", features = ["derive"] } +bytemuck = { version = "1.14", features = ["derive"] } cfg_aliases = "0.1" cfg-if = "1" codespan-reporting = "0.11" From 012304ea111a06b574fcd7863946acef917581f8 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Mon, 11 Sep 2023 15:57:43 +0200 Subject: [PATCH 14/33] Update `naga` to 0.13.0@git:cc87b8f9eb30bb55d0735b89d3df3e099e1a6e7c (#4130) Co-authored-by: Nicolas Silva --- Cargo.lock | 2 +- Cargo.toml | 2 +- wgpu-core/Cargo.toml | 2 +- wgpu-core/src/validation.rs | 1 + wgpu-hal/Cargo.toml | 4 ++-- 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c0a529700b..ced70abf1e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1603,7 +1603,7 @@ dependencies = [ [[package]] name = "naga" version = "0.13.0" -source = "git+https://github.com/gfx-rs/naga?rev=7a19f3af909202c7eafd36633b5584bfbb353ecb#7a19f3af909202c7eafd36633b5584bfbb353ecb" +source = "git+https://github.com/gfx-rs/naga?rev=cc87b8f9eb30bb55d0735b89d3df3e099e1a6e7c#cc87b8f9eb30bb55d0735b89d3df3e099e1a6e7c" dependencies = [ "bit-set", "bitflags 2.4.0", diff --git a/Cargo.toml b/Cargo.toml index c3676fada3..2975a79aa3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -51,7 +51,7 @@ version = "0.17" [workspace.dependencies.naga] git = "https://github.com/gfx-rs/naga" -rev = "7a19f3af909202c7eafd36633b5584bfbb353ecb" +rev = "cc87b8f9eb30bb55d0735b89d3df3e099e1a6e7c" version = "0.13.0" [workspace.dependencies] diff --git a/wgpu-core/Cargo.toml b/wgpu-core/Cargo.toml index 5cebd9fdca..5487a8bdc0 100644 --- a/wgpu-core/Cargo.toml +++ b/wgpu-core/Cargo.toml @@ -72,7 +72,7 @@ thiserror = "1" [dependencies.naga] git = "https://github.com/gfx-rs/naga" -rev = "7a19f3af909202c7eafd36633b5584bfbb353ecb" +rev = "cc87b8f9eb30bb55d0735b89d3df3e099e1a6e7c" version = "0.13.0" features = ["clone", "span", "validate"] diff --git a/wgpu-core/src/validation.rs b/wgpu-core/src/validation.rs index 84e1e71691..e3ecb916d3 100644 --- a/wgpu-core/src/validation.rs +++ b/wgpu-core/src/validation.rs @@ -812,6 +812,7 @@ impl Interface { location, interpolation, sampling, + .. // second_blend_source }) => Varying::Local { location, iv: InterfaceVar { diff --git a/wgpu-hal/Cargo.toml b/wgpu-hal/Cargo.toml index 53b2816435..225f18256a 100644 --- a/wgpu-hal/Cargo.toml +++ b/wgpu-hal/Cargo.toml @@ -120,14 +120,14 @@ android_system_properties = "0.1.1" [dependencies.naga] git = "https://github.com/gfx-rs/naga" -rev = "7a19f3af909202c7eafd36633b5584bfbb353ecb" +rev = "cc87b8f9eb30bb55d0735b89d3df3e099e1a6e7c" version = "0.13.0" features = ["clone"] # DEV dependencies [dev-dependencies.naga] git = "https://github.com/gfx-rs/naga" -rev = "7a19f3af909202c7eafd36633b5584bfbb353ecb" +rev = "cc87b8f9eb30bb55d0735b89d3df3e099e1a6e7c" version = "0.13.0" features = ["wgsl-in"] From 7fea9e934efd8d5dc03b9aa3e06b775c1ac4a23e Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 11 Sep 2023 22:12:43 +0200 Subject: [PATCH 15/33] Bump serde_json from 1.0.105 to 1.0.106 (#4129) Bumps [serde_json](https://github.com/serde-rs/json) from 1.0.105 to 1.0.106. - [Release notes](https://github.com/serde-rs/json/releases) - [Commits](https://github.com/serde-rs/json/compare/v1.0.105...v1.0.106) --- updated-dependencies: - dependency-name: serde_json dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- wgpu-types/Cargo.toml | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ced70abf1e..b249da1315 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2429,9 +2429,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.105" +version = "1.0.106" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "693151e1ac27563d6dbcec9dee9fbd5da8539b20fa14ad3752b2e6d363ace360" +checksum = "2cc66a619ed80bf7a0f6b17dd063a84b88f6dea1813737cf469aef1d081142c2" dependencies = [ "indexmap 2.0.0", "itoa", diff --git a/Cargo.toml b/Cargo.toml index 2975a79aa3..fc2790b546 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -89,7 +89,7 @@ raw-window-handle = "0.5" renderdoc-sys = "1.0.0" ron = "0.8" serde = "1" -serde_json = "1.0.105" +serde_json = "1.0.106" smallvec = "1" static_assertions = "1.1.0" thiserror = "1" diff --git a/wgpu-types/Cargo.toml b/wgpu-types/Cargo.toml index 4ef59398d0..93e447a0bc 100644 --- a/wgpu-types/Cargo.toml +++ b/wgpu-types/Cargo.toml @@ -42,4 +42,4 @@ web-sys = { version = "0.3.64", features = [ [dev-dependencies] serde = { version = "1", features = ["serde_derive"] } -serde_json = "1.0.105" +serde_json = "1.0.106" From 90b022d437657de4758b25b655d9d4cc85e6a123 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Thu, 14 Sep 2023 18:12:35 +0200 Subject: [PATCH 16/33] Print errors in a more readable format in the player. (#4137) Co-authored-by: Nicolas Silva --- player/src/lib.rs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/player/src/lib.rs b/player/src/lib.rs index a4be0b1c81..fbfb2697d1 100644 --- a/player/src/lib.rs +++ b/player/src/lib.rs @@ -158,7 +158,7 @@ impl GlobalPlay for wgc::global::Global { let (cmd_buf, error) = self .command_encoder_finish::(encoder, &wgt::CommandBufferDescriptor { label: None }); if let Some(e) = error { - panic!("{:?}", e); + panic!("{e}"); } cmd_buf } @@ -186,7 +186,7 @@ impl GlobalPlay for wgc::global::Global { self.device_maintain_ids::(device).unwrap(); let (_, error) = self.device_create_buffer::(device, &desc, id); if let Some(e) = error { - panic!("{:?}", e); + panic!("{e}"); } } Action::FreeBuffer(id) => { @@ -199,7 +199,7 @@ impl GlobalPlay for wgc::global::Global { self.device_maintain_ids::(device).unwrap(); let (_, error) = self.device_create_texture::(device, &desc, id); if let Some(e) = error { - panic!("{:?}", e); + panic!("{e}"); } } Action::FreeTexture(id) => { @@ -216,7 +216,7 @@ impl GlobalPlay for wgc::global::Global { self.device_maintain_ids::(device).unwrap(); let (_, error) = self.texture_create_view::(parent_id, &desc, id); if let Some(e) = error { - panic!("{:?}", e); + panic!("{e}"); } } Action::DestroyTextureView(id) => { @@ -226,7 +226,7 @@ impl GlobalPlay for wgc::global::Global { self.device_maintain_ids::(device).unwrap(); let (_, error) = self.device_create_sampler::(device, &desc, id); if let Some(e) = error { - panic!("{:?}", e); + panic!("{e}"); } } Action::DestroySampler(id) => { @@ -242,7 +242,7 @@ impl GlobalPlay for wgc::global::Global { Action::CreateBindGroupLayout(id, desc) => { let (_, error) = self.device_create_bind_group_layout::(device, &desc, id); if let Some(e) = error { - panic!("{:?}", e); + panic!("{e}"); } } Action::DestroyBindGroupLayout(id) => { @@ -252,7 +252,7 @@ impl GlobalPlay for wgc::global::Global { self.device_maintain_ids::(device).unwrap(); let (_, error) = self.device_create_pipeline_layout::(device, &desc, id); if let Some(e) = error { - panic!("{:?}", e); + panic!("{e}"); } } Action::DestroyPipelineLayout(id) => { @@ -262,7 +262,7 @@ impl GlobalPlay for wgc::global::Global { self.device_maintain_ids::(device).unwrap(); let (_, error) = self.device_create_bind_group::(device, &desc, id); if let Some(e) = error { - panic!("{:?}", e); + panic!("{e}"); } } Action::DestroyBindGroup(id) => { @@ -272,7 +272,7 @@ impl GlobalPlay for wgc::global::Global { log::info!("Creating shader from {}", data); let code = fs::read_to_string(dir.join(&data)).unwrap(); let source = if data.ends_with(".wgsl") { - wgc::pipeline::ShaderModuleSource::Wgsl(Cow::Owned(code)) + wgc::pipeline::ShaderModuleSource::Wgsl(Cow::Owned(code.clone())) } else if data.ends_with(".ron") { let module = ron::de::from_str(&code).unwrap(); wgc::pipeline::ShaderModuleSource::Naga(module) @@ -281,7 +281,7 @@ impl GlobalPlay for wgc::global::Global { }; let (_, error) = self.device_create_shader_module::(device, &desc, source, id); if let Some(e) = error { - panic!("{:?}", e); + println!("shader compilation error:\n---{code}\n---\n{e}"); } } Action::DestroyShaderModule(id) => { @@ -303,7 +303,7 @@ impl GlobalPlay for wgc::global::Global { let (_, error) = self.device_create_compute_pipeline::(device, &desc, id, implicit_ids); if let Some(e) = error { - panic!("{:?}", e); + panic!("{e}"); } } Action::DestroyComputePipeline(id) => { @@ -325,7 +325,7 @@ impl GlobalPlay for wgc::global::Global { let (_, error) = self.device_create_render_pipeline::(device, &desc, id, implicit_ids); if let Some(e) = error { - panic!("{:?}", e); + panic!("{e}"); } } Action::DestroyRenderPipeline(id) => { @@ -340,7 +340,7 @@ impl GlobalPlay for wgc::global::Global { id, ); if let Some(e) = error { - panic!("{:?}", e); + panic!("{e}"); } } Action::DestroyRenderBundle(id) => { @@ -350,7 +350,7 @@ impl GlobalPlay for wgc::global::Global { self.device_maintain_ids::(device).unwrap(); let (_, error) = self.device_create_query_set::(device, &desc, id); if let Some(e) = error { - panic!("{:?}", e); + panic!("{e}"); } } Action::DestroyQuerySet(id) => { @@ -393,7 +393,7 @@ impl GlobalPlay for wgc::global::Global { comb_manager.alloc(device.backend()), ); if let Some(e) = error { - panic!("{:?}", e); + panic!("{e}"); } let cmdbuf = self.encode_commands::(encoder, commands); self.queue_submit::(device, &[cmdbuf]).unwrap(); From b488e03d9fec114e145a89faee976ede32a7d774 Mon Sep 17 00:00:00 2001 From: Connor Fitzgerald Date: Thu, 14 Sep 2023 16:38:21 -0400 Subject: [PATCH 17/33] Workaround NV bug (#4132) --- tests/tests/regression/issue_4122.rs | 110 +++++++++++++++++++++++++++ tests/tests/root.rs | 1 + wgpu-hal/src/vulkan/adapter.rs | 4 + wgpu-hal/src/vulkan/command.rs | 47 +++++++++--- wgpu-hal/src/vulkan/mod.rs | 22 ++++++ 5 files changed, 175 insertions(+), 9 deletions(-) create mode 100644 tests/tests/regression/issue_4122.rs diff --git a/tests/tests/regression/issue_4122.rs b/tests/tests/regression/issue_4122.rs new file mode 100644 index 0000000000..41b9cd4231 --- /dev/null +++ b/tests/tests/regression/issue_4122.rs @@ -0,0 +1,110 @@ +use std::{num::NonZeroU64, ops::Range}; + +use wasm_bindgen_test::wasm_bindgen_test; +use wgpu_test::{initialize_test, TestParameters, TestingContext}; + +fn fill_test(ctx: &TestingContext, range: Range, size: u64) -> bool { + let gpu_buffer = ctx.device.create_buffer(&wgpu::BufferDescriptor { + label: Some("gpu_buffer"), + size, + usage: wgpu::BufferUsages::COPY_DST | wgpu::BufferUsages::COPY_SRC, + mapped_at_creation: false, + }); + + let cpu_buffer = ctx.device.create_buffer(&wgpu::BufferDescriptor { + label: Some("cpu_buffer"), + size, + usage: wgpu::BufferUsages::COPY_DST | wgpu::BufferUsages::MAP_READ, + mapped_at_creation: false, + }); + + // Initialize the whole buffer with values. + let buffer_contents = vec![0xFF_u8; size as usize]; + ctx.queue.write_buffer(&gpu_buffer, 0, &buffer_contents); + + let mut encoder = ctx + .device + .create_command_encoder(&wgpu::CommandEncoderDescriptor { + label: Some("encoder"), + }); + + encoder.clear_buffer( + &gpu_buffer, + range.start, + NonZeroU64::new(range.end - range.start), + ); + encoder.copy_buffer_to_buffer(&gpu_buffer, 0, &cpu_buffer, 0, size); + + ctx.queue.submit(Some(encoder.finish())); + cpu_buffer.slice(..).map_async(wgpu::MapMode::Read, |_| ()); + ctx.device.poll(wgpu::Maintain::Wait); + + let buffer_slice = cpu_buffer.slice(..); + let buffer_data = buffer_slice.get_mapped_range(); + + let first_clear_byte = buffer_data + .iter() + .enumerate() + .find_map(|(index, byte)| (*byte == 0x00).then_some(index)) + .expect("No clear happened at all"); + + let first_dirty_byte = buffer_data + .iter() + .enumerate() + .skip(first_clear_byte) + .find_map(|(index, byte)| (*byte != 0x00).then_some(index)) + .unwrap_or(size as usize); + + let second_clear_byte = buffer_data + .iter() + .enumerate() + .skip(first_dirty_byte) + .find_map(|(index, byte)| (*byte == 0x00).then_some(index)); + + if second_clear_byte.is_some() { + eprintln!("Found multiple cleared ranges instead of a single clear range of {}..{} on a buffer of size {}.", range.start, range.end, size); + return false; + } + + let cleared_range = first_clear_byte as u64..first_dirty_byte as u64; + + if cleared_range != range { + eprintln!( + "Cleared range is {}..{}, but the clear range is {}..{} on a buffer of size {}.", + cleared_range.start, cleared_range.end, range.start, range.end, size + ); + return false; + } + + eprintln!( + "Cleared range is {}..{} on a buffer of size {}.", + cleared_range.start, cleared_range.end, size + ); + + true +} + +/// Nvidia has a bug in vkCmdFillBuffer where the clear range is not properly respected under +/// certain conditions. See https://github.com/gfx-rs/wgpu/issues/4122 for more information. +/// +/// This test will fail on nvidia if the bug is not properly worked around. +#[wasm_bindgen_test] +#[test] +fn clear_buffer_bug() { + initialize_test(TestParameters::default(), |ctx| { + // This hits most of the cases in nvidia's clear buffer bug + let mut succeeded = true; + for power in 4..14 { + let size = 1 << power; + for start_offset in (0..=36).step_by(4) { + for size_offset in (0..=36).step_by(4) { + let range = start_offset..size + size_offset + start_offset; + let result = fill_test(&ctx, range, 1 << 16); + + succeeded &= result; + } + } + } + assert!(succeeded); + }); +} diff --git a/tests/tests/root.rs b/tests/tests/root.rs index 25df8eda90..85901ae491 100644 --- a/tests/tests/root.rs +++ b/tests/tests/root.rs @@ -3,6 +3,7 @@ use wasm_bindgen_test::wasm_bindgen_test_configure; mod regression { mod issue_3457; mod issue_4024; + mod issue_4122; } mod bind_group_layout_dedup; diff --git a/wgpu-hal/src/vulkan/adapter.rs b/wgpu-hal/src/vulkan/adapter.rs index 4a7ccf9535..bcbab85084 100644 --- a/wgpu-hal/src/vulkan/adapter.rs +++ b/wgpu-hal/src/vulkan/adapter.rs @@ -984,6 +984,10 @@ impl super::Instance { super::Workarounds::EMPTY_RESOLVE_ATTACHMENT_LISTS, phd_capabilities.properties.vendor_id == db::qualcomm::VENDOR, ); + workarounds.set( + super::Workarounds::FORCE_FILL_BUFFER_WITH_SIZE_GREATER_4096_ALIGNED_OFFSET_16, + phd_capabilities.properties.vendor_id == db::nvidia::VENDOR, + ); }; if phd_capabilities.effective_api_version == vk::API_VERSION_1_0 diff --git a/wgpu-hal/src/vulkan/command.rs b/wgpu-hal/src/vulkan/command.rs index c2e7afe3f1..391b754d33 100644 --- a/wgpu-hal/src/vulkan/command.rs +++ b/wgpu-hal/src/vulkan/command.rs @@ -212,15 +212,44 @@ impl crate::CommandEncoder for super::CommandEncoder { } unsafe fn clear_buffer(&mut self, buffer: &super::Buffer, range: crate::MemoryRange) { - unsafe { - self.device.raw.cmd_fill_buffer( - self.active, - buffer.raw, - range.start, - range.end - range.start, - 0, - ) - }; + let range_size = range.end - range.start; + if self.device.workarounds.contains( + super::Workarounds::FORCE_FILL_BUFFER_WITH_SIZE_GREATER_4096_ALIGNED_OFFSET_16, + ) && range_size >= 4096 + && range.start % 16 != 0 + { + let rounded_start = wgt::math::align_to(range.start, 16); + let prefix_size = rounded_start - range.start; + + unsafe { + self.device.raw.cmd_fill_buffer( + self.active, + buffer.raw, + range.start, + prefix_size, + 0, + ) + }; + + // This will never be zero, as rounding can only add up to 12 bytes, and the total size is 4096. + let suffix_size = range.end - rounded_start; + + unsafe { + self.device.raw.cmd_fill_buffer( + self.active, + buffer.raw, + rounded_start, + suffix_size, + 0, + ) + }; + } else { + unsafe { + self.device + .raw + .cmd_fill_buffer(self.active, buffer.raw, range.start, range_size, 0) + }; + } } unsafe fn copy_buffer_to_buffer( diff --git a/wgpu-hal/src/vulkan/mod.rs b/wgpu-hal/src/vulkan/mod.rs index c2165e1dd8..fe2ee914cd 100644 --- a/wgpu-hal/src/vulkan/mod.rs +++ b/wgpu-hal/src/vulkan/mod.rs @@ -207,6 +207,28 @@ bitflags::bitflags!( /// Qualcomm OOMs when there are zero color attachments but a non-null pointer /// to a subpass resolve attachment array. This nulls out that pointer in that case. const EMPTY_RESOLVE_ATTACHMENT_LISTS = 0x2; + /// If the following code returns false, then nvidia will end up filling the wrong range. + /// + /// ```skip + /// fn nvidia_succeeds() -> bool { + /// # let (copy_length, start_offset) = (0, 0); + /// if copy_length >= 4096 { + /// if start_offset % 16 != 0 { + /// if copy_length == 4096 { + /// return true; + /// } + /// if copy_length % 16 == 0 { + /// return false; + /// } + /// } + /// } + /// true + /// } + /// ``` + /// + /// As such, we need to make sure all calls to vkCmdFillBuffer are aligned to 16 bytes + /// if they cover a range of 4096 bytes or more. + const FORCE_FILL_BUFFER_WITH_SIZE_GREATER_4096_ALIGNED_OFFSET_16 = 0x4; } ); From 4bc7d8788afc7e95b0c3ff00b85c8c4551f1c7b3 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 14 Sep 2023 16:39:39 -0400 Subject: [PATCH 18/33] Bump serde_json from 1.0.106 to 1.0.107 (#4133) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- wgpu-types/Cargo.toml | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b249da1315..10b7ee144a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2429,9 +2429,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.106" +version = "1.0.107" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2cc66a619ed80bf7a0f6b17dd063a84b88f6dea1813737cf469aef1d081142c2" +checksum = "6b420ce6e3d8bd882e9b243c6eed35dbc9a6110c9769e74b584e0d68d1f20c65" dependencies = [ "indexmap 2.0.0", "itoa", diff --git a/Cargo.toml b/Cargo.toml index fc2790b546..55c6048b86 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -89,7 +89,7 @@ raw-window-handle = "0.5" renderdoc-sys = "1.0.0" ron = "0.8" serde = "1" -serde_json = "1.0.106" +serde_json = "1.0.107" smallvec = "1" static_assertions = "1.1.0" thiserror = "1" diff --git a/wgpu-types/Cargo.toml b/wgpu-types/Cargo.toml index 93e447a0bc..fd0abb0dc9 100644 --- a/wgpu-types/Cargo.toml +++ b/wgpu-types/Cargo.toml @@ -42,4 +42,4 @@ web-sys = { version = "0.3.64", features = [ [dev-dependencies] serde = { version = "1", features = ["serde_derive"] } -serde_json = "1.0.106" +serde_json = "1.0.107" From 40cc2ee88a5c9bf281395547375e279ca30aedc0 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 14 Sep 2023 16:39:48 -0400 Subject: [PATCH 19/33] Bump libc from 0.2.147 to 0.2.148 (#4134) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 10b7ee144a..ba1a403628 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1466,9 +1466,9 @@ checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646" [[package]] name = "libc" -version = "0.2.147" +version = "0.2.148" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b4668fb0ea861c1df094127ac5f1da3409a82116a4ba74fca2e58ef927159bb3" +checksum = "9cdc71e17332e86d2e1d38c1f99edcb6288ee11b815fb1a4b049eaa2114d369b" [[package]] name = "libloading" From 7c575a0b40b4cda2977d10ccc2c007ca2f77f3aa Mon Sep 17 00:00:00 2001 From: Kevin Reid Date: Fri, 15 Sep 2023 21:16:49 -0700 Subject: [PATCH 20/33] Add details to `RequestDeviceError`. (#4145) --- CHANGELOG.md | 2 +- tests/src/lib.rs | 2 +- tests/tests/device.rs | 51 ++++++++++++++++++++ wgpu/src/backend/direct.rs | 3 +- wgpu/src/backend/web.rs | 4 +- wgpu/src/lib.rs | 95 ++++++++++++++++++++++++++++++++++++-- 6 files changed, 147 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db91b89718..ad4c81d076 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -75,7 +75,7 @@ By @Valaphee in [#3402](https://github.com/gfx-rs/wgpu/pull/3402) - Omit texture store bound checks since they are no-ops if out of bounds on all APIs. By @teoxoy in [#3975](https://github.com/gfx-rs/wgpu/pull/3975) - Validate `DownlevelFlags::READ_ONLY_DEPTH_STENCIL`. By @teoxoy in [#4031](https://github.com/gfx-rs/wgpu/pull/4031) - Add validation in accordance with WebGPU `setViewport` valid usage for `x`, `y` and `this.[[attachment_size]]`. By @James2022-rgb in [#4058](https://github.com/gfx-rs/wgpu/pull/4058) -- `wgpu::CreateSurfaceError` now gives details of the failure, but no longer implements `PartialEq`. By @kpreid in [#4066](https://github.com/gfx-rs/wgpu/pull/4066) +- `wgpu::CreateSurfaceError` and `wgpu::RequestDeviceError` now give details of the failure, but no longer implement `PartialEq` and cannot be constructed. By @kpreid in [#4066](https://github.com/gfx-rs/wgpu/pull/4066) and [#4145](https://github.com/gfx-rs/wgpu/pull/4145) - Make `WGPU_POWER_PREF=none` a valid value. By @fornwall in [4076](https://github.com/gfx-rs/wgpu/pull/4076) #### Vulkan diff --git a/tests/src/lib.rs b/tests/src/lib.rs index 236b353386..c506126708 100644 --- a/tests/src/lib.rs +++ b/tests/src/lib.rs @@ -449,7 +449,7 @@ pub fn initialize_test(parameters: TestParameters, test_function: impl FnOnce(Te } } -fn initialize_adapter() -> (Adapter, Option) { +pub fn initialize_adapter() -> (Adapter, Option) { let instance = initialize_instance(); let surface_guard: Option; let compatible_surface; diff --git a/tests/tests/device.rs b/tests/tests/device.rs index f43791f86e..7964f2afdb 100644 --- a/tests/tests/device.rs +++ b/tests/tests/device.rs @@ -40,3 +40,54 @@ fn device_mismatch() { }, ); } + +#[cfg(not(all(target_arch = "wasm32", not(target_os = "emscripten"))))] +#[test] +fn request_device_error_on_native() { + pollster::block_on(request_device_error_message()); +} + +/// Check that `RequestDeviceError`s produced have some diagnostic information. +/// +/// Note: this is a wasm *and* native test. On wasm it is run directly; on native, indirectly +#[wasm_bindgen_test::wasm_bindgen_test] +async fn request_device_error_message() { + // Not using initialize_test() because that doesn't let us catch the error + // nor .await anything + let (adapter, _surface_guard) = wgpu_test::initialize_adapter(); + + let device_error = adapter + .request_device( + &wgpu::DeviceDescriptor { + // Force a failure by requesting absurd limits. + features: wgpu::Features::all(), + limits: wgpu::Limits { + max_texture_dimension_1d: u32::MAX, + max_texture_dimension_2d: u32::MAX, + max_texture_dimension_3d: u32::MAX, + max_bind_groups: u32::MAX, + max_push_constant_size: u32::MAX, + ..Default::default() + }, + ..Default::default() + }, + None, + ) + .await + .unwrap_err(); + + let device_error = device_error.to_string(); + cfg_if::cfg_if! { + if #[cfg(all(target_arch = "wasm32", not(feature = "webgl")))] { + // On WebGPU, so the error we get will be from the browser WebGPU API. + // Per the WebGPU specification this should be a `TypeError` when features are not + // available, , + // and the stringification it goes through for Rust should put that in the message. + let expected = "TypeError"; + } else { + // This message appears whenever wgpu-core is used as the implementation. + let expected = "Unsupported features were requested: Features("; + } + } + assert!(device_error.contains(expected), "{device_error}"); +} diff --git a/wgpu/src/backend/direct.rs b/wgpu/src/backend/direct.rs index 8eec9adad5..2e15e295e8 100644 --- a/wgpu/src/backend/direct.rs +++ b/wgpu/src/backend/direct.rs @@ -622,8 +622,7 @@ impl crate::Context for Context { () )); if let Some(err) = error { - log::error!("Error in Adapter::request_device: {}", err); - return ready(Err(crate::RequestDeviceError)); + return ready(Err(err.into())); } let error_sink = Arc::new(Mutex::new(ErrorSinkRaw::new())); let device = Device { diff --git a/wgpu/src/backend/web.rs b/wgpu/src/backend/web.rs index d64bd8bcb1..2f83d50c55 100644 --- a/wgpu/src/backend/web.rs +++ b/wgpu/src/backend/web.rs @@ -812,7 +812,9 @@ fn future_request_device( (device_id, device_data, queue_id, queue_data) }) - .map_err(|_| crate::RequestDeviceError) + .map_err(|error_value| crate::RequestDeviceError { + inner: crate::RequestDeviceErrorKind::Web(error_value), + }) } fn future_pop_error_scope(result: JsFutureResult) -> Option { diff --git a/wgpu/src/lib.rs b/wgpu/src/lib.rs index 94345f1adb..19dc20120d 100644 --- a/wgpu/src/lib.rs +++ b/wgpu/src/lib.rs @@ -2738,18 +2738,103 @@ impl Drop for Device { } } -/// Requesting a device failed. -#[derive(Clone, PartialEq, Eq, Debug)] -pub struct RequestDeviceError; +/// Requesting a device from an [`Adapter`] failed. +#[derive(Clone, Debug)] +pub struct RequestDeviceError { + inner: RequestDeviceErrorKind, +} +#[derive(Clone, Debug)] +enum RequestDeviceErrorKind { + /// Error from [`wgpu_core`]. + // must match dependency cfg + #[cfg(any( + not(target_arch = "wasm32"), + feature = "webgl", + target_os = "emscripten" + ))] + Core(core::instance::RequestDeviceError), + + /// Error from web API that was called by `wgpu` to request a device. + /// + /// (This is currently never used by the webgl backend, but it could be.) + #[cfg(all( + target_arch = "wasm32", + not(any(target_os = "emscripten", feature = "webgl")) + ))] + Web(wasm_bindgen::JsValue), +} + +#[cfg(all( + feature = "fragile-send-sync-non-atomic-wasm", + not(target_feature = "atomics") +))] +unsafe impl Send for RequestDeviceErrorKind {} +#[cfg(all( + feature = "fragile-send-sync-non-atomic-wasm", + not(target_feature = "atomics") +))] +unsafe impl Sync for RequestDeviceErrorKind {} + +#[cfg(any( + not(target_arch = "wasm32"), + all( + feature = "fragile-send-sync-non-atomic-wasm", + not(target_feature = "atomics") + ) +))] static_assertions::assert_impl_all!(RequestDeviceError: Send, Sync); impl fmt::Display for RequestDeviceError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "Requesting a device failed") + match &self.inner { + #[cfg(any( + not(target_arch = "wasm32"), + feature = "webgl", + target_os = "emscripten" + ))] + RequestDeviceErrorKind::Core(error) => error.fmt(f), + #[cfg(all( + target_arch = "wasm32", + not(any(target_os = "emscripten", feature = "webgl")) + ))] + RequestDeviceErrorKind::Web(error_js_value) => { + // wasm-bindgen provides a reasonable error stringification via `Debug` impl + write!(f, "{error_js_value:?}") + } + } + } +} + +impl error::Error for RequestDeviceError { + fn source(&self) -> Option<&(dyn error::Error + 'static)> { + match &self.inner { + #[cfg(any( + not(target_arch = "wasm32"), + feature = "webgl", + target_os = "emscripten" + ))] + RequestDeviceErrorKind::Core(error) => error.source(), + #[cfg(all( + target_arch = "wasm32", + not(any(target_os = "emscripten", feature = "webgl")) + ))] + RequestDeviceErrorKind::Web(_) => None, + } } } -impl error::Error for RequestDeviceError {} +#[cfg(any( + not(target_arch = "wasm32"), + feature = "webgl", + target_os = "emscripten" +))] +impl From for RequestDeviceError { + fn from(error: core::instance::RequestDeviceError) -> Self { + Self { + inner: RequestDeviceErrorKind::Core(error), + } + } +} /// [`Instance::create_surface()`] or a related function failed. #[derive(Clone, Debug)] From 0ffdae31a1f75e1041ed4472eb0552c487831efe Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sat, 16 Sep 2023 22:01:46 +0200 Subject: [PATCH 21/33] Metal encoder & pass timestamp support (#4008) Implements timer queries via write_timestamp on Metal for encoders (whenever timer queries are available) and passes (for Intel/AMD GPUs, where we should advertise TIMESTAMP_QUERY_INSIDE_PASSES now). Due to some bugs in Metal this was a lot harder than expected. I believe the solution is close to optimal with the current restrictions in place. For details see code comments. --- .deny.toml | 1 + CHANGELOG.md | 4 + Cargo.lock | 3 +- Cargo.toml | 2 + examples/timestamp-queries/src/main.rs | 18 +-- wgpu-hal/src/metal/adapter.rs | 46 ++++-- wgpu-hal/src/metal/command.rs | 213 +++++++++++++++++++++---- wgpu-hal/src/metal/mod.rs | 27 +++- wgpu-types/src/lib.rs | 7 +- 9 files changed, 260 insertions(+), 61 deletions(-) diff --git a/.deny.toml b/.deny.toml index 5c214bbc28..f7c233c5d4 100644 --- a/.deny.toml +++ b/.deny.toml @@ -27,6 +27,7 @@ allow = [ [sources] allow-git = [ "https://github.com/grovesNL/glow", + "https://github.com/gfx-rs/metal-rs", ] unknown-registry = "deny" unknown-git = "deny" diff --git a/CHANGELOG.md b/CHANGELOG.md index ad4c81d076..039bce54ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -88,6 +88,10 @@ By @Valaphee in [#3402](https://github.com/gfx-rs/wgpu/pull/3402) ### Documentation - Use WGSL for VertexFormat example types. By @ScanMountGoat in [#4305](https://github.com/gfx-rs/wgpu/pull/4035) +#### Metal + +- Support for timestamp queries on encoders and passes. By @wumpf in [#4008](https://github.com/gfx-rs/wgpu/pull/4008) + ### Bug Fixes #### General diff --git a/Cargo.lock b/Cargo.lock index ba1a403628..07ed8c2c66 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1551,8 +1551,7 @@ dependencies = [ [[package]] name = "metal" version = "0.26.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "623b5e6cefd76e58f774bd3cc0c6f5c7615c58c03a97815245a25c3c9bdee318" +source = "git+https://github.com/gfx-rs/metal-rs/?rev=d24f1a4#d24f1a4ae92470bf87a0c65ecfe78c9299835505" dependencies = [ "bitflags 2.4.0", "block", diff --git a/Cargo.toml b/Cargo.toml index 55c6048b86..22f79b73b1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -158,6 +158,8 @@ termcolor = "1.2.0" #glow = { path = "../glow" } #d3d12 = { path = "../d3d12-rs" } #metal = { path = "../metal-rs" } +#metal = { path = "../metal-rs" } +metal = { git = "https://github.com/gfx-rs/metal-rs/", rev = "d24f1a4" } # More timer support via https://github.com/gfx-rs/metal-rs/pull/280 #web-sys = { path = "../wasm-bindgen/crates/web-sys" } #js-sys = { path = "../wasm-bindgen/crates/js-sys" } #wasm-bindgen = { path = "../wasm-bindgen" } diff --git a/examples/timestamp-queries/src/main.rs b/examples/timestamp-queries/src/main.rs index 3479122c79..f8c524f03c 100644 --- a/examples/timestamp-queries/src/main.rs +++ b/examples/timestamp-queries/src/main.rs @@ -47,6 +47,7 @@ impl QueryResults { // * compute end const NUM_QUERIES: u64 = 8; + #[allow(clippy::redundant_closure)] // False positive fn from_raw_results(timestamps: Vec, timestamps_inside_passes: bool) -> Self { assert_eq!(timestamps.len(), Self::NUM_QUERIES as usize); @@ -60,9 +61,9 @@ impl QueryResults { let mut encoder_timestamps = [0, 0]; encoder_timestamps[0] = get_next_slot(); let render_start_end_timestamps = [get_next_slot(), get_next_slot()]; - let render_inside_timestamp = timestamps_inside_passes.then_some(get_next_slot()); + let render_inside_timestamp = timestamps_inside_passes.then(|| get_next_slot()); let compute_start_end_timestamps = [get_next_slot(), get_next_slot()]; - let compute_inside_timestamp = timestamps_inside_passes.then_some(get_next_slot()); + let compute_inside_timestamp = timestamps_inside_passes.then(|| get_next_slot()); encoder_timestamps[1] = get_next_slot(); QueryResults { @@ -79,8 +80,8 @@ impl QueryResults { let elapsed_us = |start, end: u64| end.wrapping_sub(start) as f64 * period as f64 / 1000.0; println!( - "Elapsed time render + compute: {:.2} μs", - elapsed_us(self.encoder_timestamps[0], self.encoder_timestamps[1]) + "Elapsed time before render until after compute: {:.2} μs", + elapsed_us(self.encoder_timestamps[0], self.encoder_timestamps[1]), ); println!( "Elapsed time render pass: {:.2} μs", @@ -464,13 +465,10 @@ mod tests { render_start_end_timestamps[1].wrapping_sub(render_start_end_timestamps[0]); let compute_delta = compute_start_end_timestamps[1].wrapping_sub(compute_start_end_timestamps[0]); + let encoder_delta = encoder_timestamps[1].wrapping_sub(encoder_timestamps[0]); - // TODO: Metal encoder timestamps aren't implemented yet. - if ctx.adapter.get_info().backend != wgpu::Backend::Metal { - let encoder_delta = encoder_timestamps[1].wrapping_sub(encoder_timestamps[0]); - assert!(encoder_delta > 0); - assert!(encoder_delta >= render_delta + compute_delta); - } + assert!(encoder_delta > 0); + assert!(encoder_delta >= render_delta + compute_delta); if let Some(render_inside_timestamp) = render_inside_timestamp { assert!(render_inside_timestamp >= render_start_end_timestamps[0]); diff --git a/wgpu-hal/src/metal/adapter.rs b/wgpu-hal/src/metal/adapter.rs index bc90954b35..126741d257 100644 --- a/wgpu-hal/src/metal/adapter.rs +++ b/wgpu-hal/src/metal/adapter.rs @@ -5,6 +5,8 @@ use wgt::{AstcBlock, AstcChannel}; use std::{sync::Arc, thread}; +use super::TimestampQuerySupport; + const MAX_COMMAND_BUFFERS: u64 = 2048; unsafe impl Send for super::Adapter {} @@ -536,6 +538,26 @@ impl super::PrivateCapabilities { MTLReadWriteTextureTier::TierNone }; + let mut timestamp_query_support = TimestampQuerySupport::empty(); + if version.at_least((11, 0), (14, 0), os_is_mac) + && device.supports_counter_sampling(metal::MTLCounterSamplingPoint::AtStageBoundary) + { + // If we don't support at stage boundary, don't support anything else. + timestamp_query_support.insert(TimestampQuerySupport::STAGE_BOUNDARIES); + + if device.supports_counter_sampling(metal::MTLCounterSamplingPoint::AtDrawBoundary) { + timestamp_query_support.insert(TimestampQuerySupport::ON_RENDER_ENCODER); + } + if device.supports_counter_sampling(metal::MTLCounterSamplingPoint::AtDispatchBoundary) + { + timestamp_query_support.insert(TimestampQuerySupport::ON_COMPUTE_ENCODER); + } + if device.supports_counter_sampling(metal::MTLCounterSamplingPoint::AtBlitBoundary) { + timestamp_query_support.insert(TimestampQuerySupport::ON_BLIT_ENCODER); + } + // `TimestampQuerySupport::INSIDE_WGPU_PASSES` emerges from the other flags. + } + Self { family_check, msl_version: if os_is_xr || version.at_least((12, 0), (15, 0), os_is_mac) { @@ -773,13 +795,7 @@ impl super::PrivateCapabilities { } else { None }, - support_timestamp_query: version.at_least((11, 0), (14, 0), os_is_mac) - && device - .supports_counter_sampling(metal::MTLCounterSamplingPoint::AtStageBoundary), - support_timestamp_query_in_passes: version.at_least((11, 0), (14, 0), os_is_mac) - && device.supports_counter_sampling(metal::MTLCounterSamplingPoint::AtDrawBoundary) - && device - .supports_counter_sampling(metal::MTLCounterSamplingPoint::AtDispatchBoundary), + timestamp_query_support, } } @@ -807,12 +823,16 @@ impl super::PrivateCapabilities { | F::DEPTH32FLOAT_STENCIL8 | F::MULTI_DRAW_INDIRECT; - features.set(F::TIMESTAMP_QUERY, self.support_timestamp_query); - // TODO: Not yet implemented. - // features.set( - // F::TIMESTAMP_QUERY_INSIDE_PASSES, - // self.support_timestamp_query_in_passes, - // ); + features.set( + F::TIMESTAMP_QUERY, + self.timestamp_query_support + .contains(TimestampQuerySupport::STAGE_BOUNDARIES), + ); + features.set( + F::TIMESTAMP_QUERY_INSIDE_PASSES, + self.timestamp_query_support + .contains(TimestampQuerySupport::INSIDE_WGPU_PASSES), + ); features.set(F::TEXTURE_COMPRESSION_ASTC, self.format_astc); features.set(F::TEXTURE_COMPRESSION_ASTC_HDR, self.format_astc_hdr); features.set(F::TEXTURE_COMPRESSION_BC, self.format_bc); diff --git a/wgpu-hal/src/metal/command.rs b/wgpu-hal/src/metal/command.rs index cc737fd228..c4b37f9932 100644 --- a/wgpu-hal/src/metal/command.rs +++ b/wgpu-hal/src/metal/command.rs @@ -1,4 +1,4 @@ -use super::{conv, AsNative}; +use super::{conv, AsNative, TimestampQuerySupport}; use crate::CommandEncoder as _; use std::{borrow::Cow, mem, ops::Range}; @@ -18,6 +18,7 @@ impl Default for super::CommandState { storage_buffer_length_map: Default::default(), work_group_memory_sizes: Vec::new(), push_constants: Vec::new(), + pending_timer_queries: Vec::new(), } } } @@ -26,10 +27,85 @@ impl super::CommandEncoder { fn enter_blit(&mut self) -> &metal::BlitCommandEncoderRef { if self.state.blit.is_none() { debug_assert!(self.state.render.is_none() && self.state.compute.is_none()); + let cmd_buf = self.raw_cmd_buf.as_ref().unwrap(); + + // Take care of pending timer queries. + // If we can't use `sample_counters_in_buffer` we have to create a dummy blit encoder! + // + // There is a known bug in Metal where blit encoders won't write timestamps if they don't have a blit operation. + // See https://github.com/gpuweb/gpuweb/issues/2046#issuecomment-1205793680 & https://source.chromium.org/chromium/chromium/src/+/006c4eb70c96229834bbaf271290f40418144cd3:third_party/dawn/src/dawn/native/metal/BackendMTL.mm;l=350 + // + // To make things worse: + // * what counts as a blit operation is a bit unclear, experimenting seemed to indicate that resolve_counters doesn't count. + // * in some cases (when?) using `set_start_of_encoder_sample_index` doesn't work, so we have to use `set_end_of_encoder_sample_index` instead + // + // All this means that pretty much the only *reliable* thing as of writing is to: + // * create a dummy blit encoder using set_end_of_encoder_sample_index + // * do a dummy write that is known to be not optimized out. + // * close the encoder since we used set_end_of_encoder_sample_index and don't want to get any extra stuff in there. + // * create another encoder for whatever we actually had in mind. + let supports_sample_counters_in_buffer = self + .shared + .private_caps + .timestamp_query_support + .contains(TimestampQuerySupport::ON_BLIT_ENCODER); + + if !self.state.pending_timer_queries.is_empty() && !supports_sample_counters_in_buffer { + objc::rc::autoreleasepool(|| { + let descriptor = metal::BlitPassDescriptor::new(); + let mut last_query = None; + for (i, (set, index)) in self.state.pending_timer_queries.drain(..).enumerate() + { + let sba_descriptor = descriptor + .sample_buffer_attachments() + .object_at(i as _) + .unwrap(); + sba_descriptor + .set_sample_buffer(set.counter_sample_buffer.as_ref().unwrap()); + + // Here be dragons: + // As mentioned above, for some reasons using the start of the encoder won't yield any results sometimes! + sba_descriptor + .set_start_of_encoder_sample_index(metal::COUNTER_DONT_SAMPLE); + sba_descriptor.set_end_of_encoder_sample_index(index as _); + + last_query = Some((set, index)); + } + let encoder = cmd_buf.blit_command_encoder_with_descriptor(descriptor); + + // As explained above, we need to do some write: + // Conveniently, we have a buffer with every query set, that we can use for this for a dummy write, + // since we know that it is going to be overwritten again on timer resolve and HAL doesn't define its state before that. + let raw_range = metal::NSRange { + location: last_query.as_ref().unwrap().1 as u64 * crate::QUERY_SIZE, + length: 1, + }; + encoder.fill_buffer( + &last_query.as_ref().unwrap().0.raw_buffer, + raw_range, + 255, // Don't write 0, so it's easier to identify if something went wrong. + ); + + encoder.end_encoding(); + }); + } + objc::rc::autoreleasepool(|| { - let cmd_buf = self.raw_cmd_buf.as_ref().unwrap(); self.state.blit = Some(cmd_buf.new_blit_command_encoder().to_owned()); }); + + let encoder = self.state.blit.as_ref().unwrap(); + + // UNTESTED: + // If the above described issue with empty blit encoder applies to `sample_counters_in_buffer` as well, we should use the same workaround instead! + for (set, index) in self.state.pending_timer_queries.drain(..) { + debug_assert!(supports_sample_counters_in_buffer); + encoder.sample_counters_in_buffer( + set.counter_sample_buffer.as_ref().unwrap(), + index as _, + true, + ) + } } self.state.blit.as_ref().unwrap() } @@ -40,7 +116,7 @@ impl super::CommandEncoder { } } - fn enter_any(&mut self) -> Option<&metal::CommandEncoderRef> { + fn active_encoder(&mut self) -> Option<&metal::CommandEncoderRef> { if let Some(ref encoder) = self.state.render { Some(encoder) } else if let Some(ref encoder) = self.state.compute { @@ -127,9 +203,17 @@ impl crate::CommandEncoder for super::CommandEncoder { } unsafe fn end_encoding(&mut self) -> Result { + // Handle pending timer query if any. + if !self.state.pending_timer_queries.is_empty() { + self.leave_blit(); + self.enter_blit(); + } + self.leave_blit(); debug_assert!(self.state.render.is_none()); debug_assert!(self.state.compute.is_none()); + debug_assert!(self.state.pending_timer_queries.is_empty()); + Ok(super::CommandBuffer { raw: self.raw_cmd_buf.take().unwrap(), }) @@ -322,16 +406,43 @@ impl crate::CommandEncoder for super::CommandEncoder { _ => {} } } - unsafe fn write_timestamp(&mut self, _set: &super::QuerySet, _index: u32) { - // TODO: If MTLCounterSamplingPoint::AtDrawBoundary/AtBlitBoundary/AtDispatchBoundary is supported, - // we don't need to insert a new encoder, but can instead use respective current one. - //let encoder = self.enter_any().unwrap_or_else(|| self.enter_blit()); + unsafe fn write_timestamp(&mut self, set: &super::QuerySet, index: u32) { + let support = self.shared.private_caps.timestamp_query_support; + debug_assert!( + support.contains(TimestampQuerySupport::STAGE_BOUNDARIES), + "Timestamp queries are not supported" + ); + let sample_buffer = set.counter_sample_buffer.as_ref().unwrap(); + let with_barrier = true; + + // Try to use an existing encoder for timestamp query if possible. + // This works only if it's supported for the active encoder. + if let (true, Some(encoder)) = ( + support.contains(TimestampQuerySupport::ON_BLIT_ENCODER), + self.state.blit.as_ref(), + ) { + encoder.sample_counters_in_buffer(sample_buffer, index as _, with_barrier); + } else if let (true, Some(encoder)) = ( + support.contains(TimestampQuerySupport::ON_RENDER_ENCODER), + self.state.render.as_ref(), + ) { + encoder.sample_counters_in_buffer(sample_buffer, index as _, with_barrier); + } else if let (true, Some(encoder)) = ( + support.contains(TimestampQuerySupport::ON_COMPUTE_ENCODER), + self.state.compute.as_ref(), + ) { + encoder.sample_counters_in_buffer(sample_buffer, index as _, with_barrier); + } else { + // If we're here it means we either have no encoder open, or it's not supported to sample within them. + // If this happens with render/compute open, this is an invalid usage! + debug_assert!(self.state.render.is_none() && self.state.compute.is_none()); - // TODO: Otherwise, we need to create a new blit command encoder with a descriptor that inserts the timestamps. - // Note that as of writing creating a new encoder is not exposed by the metal crate. - // https://developer.apple.com/documentation/metal/mtlcommandbuffer/3564431-makeblitcommandencoder + // But otherwise it means we'll put defer this to the next created encoder. + self.state.pending_timer_queries.push((set.clone(), index)); - // TODO: Enable respective test in `examples/timestamp-queries/src/tests.rs`. + // Ensure we didn't already have a blit open. + self.leave_blit(); + }; } unsafe fn reset_queries(&mut self, set: &super::QuerySet, range: Range) { @@ -342,6 +453,7 @@ impl crate::CommandEncoder for super::CommandEncoder { }; encoder.fill_buffer(&set.raw_buffer, raw_range, 0); } + unsafe fn copy_query_results( &mut self, set: &super::QuerySet, @@ -454,8 +566,29 @@ impl crate::CommandEncoder for super::CommandEncoder { } } + let mut sba_index = 0; + let mut next_sba_descriptor = || { + let sba_descriptor = descriptor + .sample_buffer_attachments() + .object_at(sba_index) + .unwrap(); + + sba_descriptor.set_end_of_vertex_sample_index(metal::COUNTER_DONT_SAMPLE); + sba_descriptor.set_start_of_fragment_sample_index(metal::COUNTER_DONT_SAMPLE); + + sba_index += 1; + sba_descriptor + }; + + for (set, index) in self.state.pending_timer_queries.drain(..) { + let sba_descriptor = next_sba_descriptor(); + sba_descriptor.set_sample_buffer(set.counter_sample_buffer.as_ref().unwrap()); + sba_descriptor.set_start_of_vertex_sample_index(index as _); + sba_descriptor.set_end_of_fragment_sample_index(metal::COUNTER_DONT_SAMPLE); + } + if let Some(ref timestamp_writes) = desc.timestamp_writes { - let sba_descriptor = descriptor.sample_buffer_attachments().object_at(0).unwrap(); + let sba_descriptor = next_sba_descriptor(); sba_descriptor.set_sample_buffer( timestamp_writes .query_set @@ -464,12 +597,16 @@ impl crate::CommandEncoder for super::CommandEncoder { .unwrap(), ); - if let Some(start_index) = timestamp_writes.beginning_of_pass_write_index { - sba_descriptor.set_start_of_vertex_sample_index(start_index as _); - } - if let Some(end_index) = timestamp_writes.end_of_pass_write_index { - sba_descriptor.set_end_of_fragment_sample_index(end_index as _); - } + sba_descriptor.set_start_of_vertex_sample_index( + timestamp_writes + .beginning_of_pass_write_index + .map_or(metal::COUNTER_DONT_SAMPLE, |i| i as _), + ); + sba_descriptor.set_end_of_fragment_sample_index( + timestamp_writes + .end_of_pass_write_index + .map_or(metal::COUNTER_DONT_SAMPLE, |i| i as _), + ); } if let Some(occlusion_query_set) = desc.occlusion_query_set { @@ -697,19 +834,19 @@ impl crate::CommandEncoder for super::CommandEncoder { } unsafe fn insert_debug_marker(&mut self, label: &str) { - if let Some(encoder) = self.enter_any() { + if let Some(encoder) = self.active_encoder() { encoder.insert_debug_signpost(label); } } unsafe fn begin_debug_marker(&mut self, group_label: &str) { - if let Some(encoder) = self.enter_any() { + if let Some(encoder) = self.active_encoder() { encoder.push_debug_group(group_label); } else if let Some(ref buf) = self.raw_cmd_buf { buf.push_debug_group(group_label); } } unsafe fn end_debug_marker(&mut self) { - if let Some(encoder) = self.enter_any() { + if let Some(encoder) = self.active_encoder() { encoder.pop_debug_group(); } else if let Some(ref buf) = self.raw_cmd_buf { buf.pop_debug_group(); @@ -969,11 +1106,25 @@ impl crate::CommandEncoder for super::CommandEncoder { objc::rc::autoreleasepool(|| { let descriptor = metal::ComputePassDescriptor::new(); - if let Some(timestamp_writes) = desc.timestamp_writes.as_ref() { + let mut sba_index = 0; + let mut next_sba_descriptor = || { let sba_descriptor = descriptor .sample_buffer_attachments() - .object_at(0 as _) + .object_at(sba_index) .unwrap(); + sba_index += 1; + sba_descriptor + }; + + for (set, index) in self.state.pending_timer_queries.drain(..) { + let sba_descriptor = next_sba_descriptor(); + sba_descriptor.set_sample_buffer(set.counter_sample_buffer.as_ref().unwrap()); + sba_descriptor.set_start_of_encoder_sample_index(index as _); + sba_descriptor.set_end_of_encoder_sample_index(metal::COUNTER_DONT_SAMPLE); + } + + if let Some(timestamp_writes) = desc.timestamp_writes.as_ref() { + let sba_descriptor = next_sba_descriptor(); sba_descriptor.set_sample_buffer( timestamp_writes .query_set @@ -982,12 +1133,16 @@ impl crate::CommandEncoder for super::CommandEncoder { .unwrap(), ); - if let Some(start_index) = timestamp_writes.beginning_of_pass_write_index { - sba_descriptor.set_start_of_encoder_sample_index(start_index as _); - } - if let Some(end_index) = timestamp_writes.end_of_pass_write_index { - sba_descriptor.set_end_of_encoder_sample_index(end_index as _); - } + sba_descriptor.set_start_of_encoder_sample_index( + timestamp_writes + .beginning_of_pass_write_index + .map_or(metal::COUNTER_DONT_SAMPLE, |i| i as _), + ); + sba_descriptor.set_end_of_encoder_sample_index( + timestamp_writes + .end_of_pass_write_index + .map_or(metal::COUNTER_DONT_SAMPLE, |i| i as _), + ); } let encoder = raw.compute_command_encoder_with_descriptor(descriptor); diff --git a/wgpu-hal/src/metal/mod.rs b/wgpu-hal/src/metal/mod.rs index 76f57002ff..c6b91a4f3c 100644 --- a/wgpu-hal/src/metal/mod.rs +++ b/wgpu-hal/src/metal/mod.rs @@ -33,6 +33,7 @@ use std::{ }; use arrayvec::ArrayVec; +use bitflags::bitflags; use metal::foreign_types::ForeignTypeRef as _; use parking_lot::Mutex; @@ -143,6 +144,24 @@ impl crate::Instance for Instance { } } +bitflags!( + /// Similar to `MTLCounterSamplingPoint`, but a bit higher abstracted for our purposes. + #[derive(Debug, Copy, Clone)] + pub struct TimestampQuerySupport: u32 { + /// On creating Metal encoders. + const STAGE_BOUNDARIES = 1 << 1; + /// Within existing draw encoders. + const ON_RENDER_ENCODER = Self::STAGE_BOUNDARIES.bits() | (1 << 2); + /// Within existing dispatch encoders. + const ON_COMPUTE_ENCODER = Self::STAGE_BOUNDARIES.bits() | (1 << 3); + /// Within existing blit encoders. + const ON_BLIT_ENCODER = Self::STAGE_BOUNDARIES.bits() | (1 << 4); + + /// Within any wgpu render/compute pass. + const INSIDE_WGPU_PASSES = Self::ON_RENDER_ENCODER.bits() | Self::ON_COMPUTE_ENCODER.bits(); + } +); + #[allow(dead_code)] #[derive(Clone, Debug)] struct PrivateCapabilities { @@ -239,8 +258,7 @@ struct PrivateCapabilities { supports_preserve_invariance: bool, supports_shader_primitive_index: bool, has_unified_memory: Option, - support_timestamp_query: bool, - support_timestamp_query_in_passes: bool, + timestamp_query_support: TimestampQuerySupport, } #[derive(Clone, Debug)] @@ -704,7 +722,7 @@ pub struct ComputePipeline { unsafe impl Send for ComputePipeline {} unsafe impl Sync for ComputePipeline {} -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct QuerySet { raw_buffer: metal::Buffer, //Metal has a custom buffer for counters. @@ -787,6 +805,9 @@ struct CommandState { work_group_memory_sizes: Vec, push_constants: Vec, + + /// Timer query that should be executed when the next pass starts. + pending_timer_queries: Vec<(QuerySet, u32)>, } pub struct CommandEncoder { diff --git a/wgpu-types/src/lib.rs b/wgpu-types/src/lib.rs index c892874afa..9f61e2e490 100644 --- a/wgpu-types/src/lib.rs +++ b/wgpu-types/src/lib.rs @@ -270,7 +270,7 @@ bitflags::bitflags! { /// Supported Platforms: /// - Vulkan /// - DX12 - /// - Metal - TODO: Not yet supported on command encoder. + /// - Metal /// /// This is a web and native feature. const TIMESTAMP_QUERY = 1 << 1; @@ -458,10 +458,9 @@ bitflags::bitflags! { /// Supported platforms: /// - Vulkan /// - DX12 + /// - Metal (AMD & Intel, not Apple GPUs) /// - /// This is currently unimplemented on Metal. - /// When implemented, it will be supported on Metal on AMD and Intel GPUs, but not Apple GPUs. - /// (This is a common limitation of tile-based rasterization GPUs) + /// This is generally not available on tile-based rasterization GPUs. /// /// This is a native only feature with a [proposal](https://github.com/gpuweb/gpuweb/blob/0008bd30da2366af88180b511a5d0d0c1dffbc36/proposals/timestamp-query-inside-passes.md) for the web. const TIMESTAMP_QUERY_INSIDE_PASSES = 1 << 33; From f2bd5571863ef967bd730e8713efd69e3293ebdc Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Sun, 17 Sep 2023 11:08:42 -0700 Subject: [PATCH 22/33] Tests for wgpu#4139. (#4148) --- tests/tests/query_set.rs | 26 ++++++++++++++++++++++++++ tests/tests/root.rs | 1 + 2 files changed, 27 insertions(+) create mode 100644 tests/tests/query_set.rs diff --git a/tests/tests/query_set.rs b/tests/tests/query_set.rs new file mode 100644 index 0000000000..16e5094089 --- /dev/null +++ b/tests/tests/query_set.rs @@ -0,0 +1,26 @@ +use wgpu_test::{initialize_test, FailureCase, TestParameters}; + +#[test] +fn drop_failed_timestamp_query_set() { + let parameters = TestParameters::default() + // https://github.com/gfx-rs/wgpu/issues/4139 + .expect_fail(FailureCase::always()); + initialize_test(parameters, |ctx| { + // Enter an error scope, so the validation catch-all doesn't + // report the error too early. + ctx.device.push_error_scope(wgpu::ErrorFilter::Validation); + + // Creating this query set should fail, since we didn't include + // TIMESTAMP_QUERY in our required features. + let bad_query_set = ctx.device.create_query_set(&wgpu::QuerySetDescriptor { + label: Some("doomed query set"), + ty: wgpu::QueryType::Timestamp, + count: 1, + }); + + // Dropping this should not panic. + drop(bad_query_set); + + assert!(pollster::block_on(ctx.device.pop_error_scope()).is_some()); + }); +} diff --git a/tests/tests/root.rs b/tests/tests/root.rs index 85901ae491..b2695fd827 100644 --- a/tests/tests/root.rs +++ b/tests/tests/root.rs @@ -20,6 +20,7 @@ mod instance; mod occlusion_query; mod partially_bounded_arrays; mod poll; +mod query_set; mod queue_transfer; mod resource_descriptor_accessor; mod resource_error; From 471229a48f184d9c095c52f7390816f4b4d30e92 Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Sun, 17 Sep 2023 12:39:16 -0700 Subject: [PATCH 23/33] Update Naga to df8107b7 (2023-9-15). (#4149) --- CHANGELOG.md | 1 + Cargo.lock | 2 +- Cargo.toml | 2 +- wgpu-core/Cargo.toml | 2 +- wgpu-hal/Cargo.toml | 4 ++-- 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 039bce54ab..d0eb15b5ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,6 +72,7 @@ By @Valaphee in [#3402](https://github.com/gfx-rs/wgpu/pull/3402) #### General +- Update Naga to df8107b7 (2023-9-15). By @jimblandy in [#4149](https://github.com/gfx-rs/wgpu/pull/4149) - Omit texture store bound checks since they are no-ops if out of bounds on all APIs. By @teoxoy in [#3975](https://github.com/gfx-rs/wgpu/pull/3975) - Validate `DownlevelFlags::READ_ONLY_DEPTH_STENCIL`. By @teoxoy in [#4031](https://github.com/gfx-rs/wgpu/pull/4031) - Add validation in accordance with WebGPU `setViewport` valid usage for `x`, `y` and `this.[[attachment_size]]`. By @James2022-rgb in [#4058](https://github.com/gfx-rs/wgpu/pull/4058) diff --git a/Cargo.lock b/Cargo.lock index 07ed8c2c66..303e94dbd8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1602,7 +1602,7 @@ dependencies = [ [[package]] name = "naga" version = "0.13.0" -source = "git+https://github.com/gfx-rs/naga?rev=cc87b8f9eb30bb55d0735b89d3df3e099e1a6e7c#cc87b8f9eb30bb55d0735b89d3df3e099e1a6e7c" +source = "git+https://github.com/gfx-rs/naga?rev=df8107b7#df8107b78812cc2b1e3d5de35279cedc1f0da3fb" dependencies = [ "bit-set", "bitflags 2.4.0", diff --git a/Cargo.toml b/Cargo.toml index 22f79b73b1..40b8c46c96 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -51,7 +51,7 @@ version = "0.17" [workspace.dependencies.naga] git = "https://github.com/gfx-rs/naga" -rev = "cc87b8f9eb30bb55d0735b89d3df3e099e1a6e7c" +rev = "df8107b7" version = "0.13.0" [workspace.dependencies] diff --git a/wgpu-core/Cargo.toml b/wgpu-core/Cargo.toml index 5487a8bdc0..19bc3ad64d 100644 --- a/wgpu-core/Cargo.toml +++ b/wgpu-core/Cargo.toml @@ -72,7 +72,7 @@ thiserror = "1" [dependencies.naga] git = "https://github.com/gfx-rs/naga" -rev = "cc87b8f9eb30bb55d0735b89d3df3e099e1a6e7c" +rev = "df8107b7" version = "0.13.0" features = ["clone", "span", "validate"] diff --git a/wgpu-hal/Cargo.toml b/wgpu-hal/Cargo.toml index 225f18256a..3db7363616 100644 --- a/wgpu-hal/Cargo.toml +++ b/wgpu-hal/Cargo.toml @@ -120,14 +120,14 @@ android_system_properties = "0.1.1" [dependencies.naga] git = "https://github.com/gfx-rs/naga" -rev = "cc87b8f9eb30bb55d0735b89d3df3e099e1a6e7c" +rev = "df8107b7" version = "0.13.0" features = ["clone"] # DEV dependencies [dev-dependencies.naga] git = "https://github.com/gfx-rs/naga" -rev = "cc87b8f9eb30bb55d0735b89d3df3e099e1a6e7c" +rev = "df8107b7" version = "0.13.0" features = ["wgsl-in"] From 8adab259053eb0a9ada89815f3680736f4a1f17b Mon Sep 17 00:00:00 2001 From: Teodor Tanasoaia <28601907+teoxoy@users.noreply.github.com> Date: Mon, 18 Sep 2023 16:43:11 +0200 Subject: [PATCH 24/33] [d3d12] Document `map_blend_factor` (#4151) --- wgpu-hal/src/dx12/conv.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/wgpu-hal/src/dx12/conv.rs b/wgpu-hal/src/dx12/conv.rs index 8b44ae9c4b..908944567a 100644 --- a/wgpu-hal/src/dx12/conv.rs +++ b/wgpu-hal/src/dx12/conv.rs @@ -222,6 +222,10 @@ pub fn map_polygon_mode(mode: wgt::PolygonMode) -> d3d12_ty::D3D12_FILL_MODE { } } +/// D3D12 doesn't support passing factors ending in `_COLOR` for alpha blending +/// (see https://learn.microsoft.com/en-us/windows/win32/api/d3d12/ns-d3d12-d3d12_render_target_blend_desc). +/// Therefore this function takes an additional `is_alpha` argument +/// which if set will return an equivalent `_ALPHA` factor. fn map_blend_factor(factor: wgt::BlendFactor, is_alpha: bool) -> d3d12_ty::D3D12_BLEND { use wgt::BlendFactor as Bf; match factor { From 507101987baced0f75978486c4db941113409d40 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Mon, 18 Sep 2023 20:58:41 +0200 Subject: [PATCH 25/33] Make `StoreOp` an enum instead of a bool (#4147) Co-authored-by: Connor Fitzgerald --- CHANGELOG.md | 22 ++++++++ examples/boids/src/main.rs | 2 +- examples/bunnymark/src/main.rs | 2 +- examples/capture/src/main.rs | 2 +- examples/conservative-raster/src/main.rs | 4 +- examples/cube/src/main.rs | 2 +- examples/hello-triangle/src/main.rs | 2 +- examples/hello-windows/src/main.rs | 2 +- examples/mipmap/src/main.rs | 4 +- examples/msaa-line/src/main.rs | 4 +- examples/shadow/src/main.rs | 6 +-- examples/skybox/src/main.rs | 4 +- examples/stencil-triangles/src/main.rs | 4 +- examples/texture-arrays/src/main.rs | 2 +- examples/timestamp-queries/src/main.rs | 2 +- examples/water/src/main.rs | 10 ++-- tests/tests/occlusion_query/mod.rs | 2 +- tests/tests/regression/issue_3457.rs | 4 +- tests/tests/scissor_tests/mod.rs | 2 +- tests/tests/shader_primitive_index/mod.rs | 2 +- .../tests/zero_init_texture_after_discard.rs | 18 +++---- wgpu/src/backend/direct.rs | 21 ++++---- wgpu/src/backend/web.rs | 9 ++-- wgpu/src/lib.rs | 52 ++++++++++++++++--- 24 files changed, 120 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d0eb15b5ae..32f90b3d90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,28 @@ let render_pass = encoder.begin_render_pass(&wgpu::RenderPassDescriptor { By @Valaphee in [#3402](https://github.com/gfx-rs/wgpu/pull/3402) + +#### Render pass store operation is now an enum + +`wgpu::Operations::store` used to be an underdocumented boolean value, +causing misunderstandings of the effect of setting it to `false`. + +The API now more closely resembles WebGPU which distinguishes between `store` and `discard`, +see [WebGPU spec on GPUStoreOp](https://gpuweb.github.io/gpuweb/#enumdef-gpustoreop). + +```diff +// ... +depth_ops: Some(wgpu::Operations { + load: wgpu::LoadOp::Clear(1.0), +- store: false, ++ store: wgpu::StoreOp::Discard, +}), +// ... +``` + +By @wumpf in [#4147](https://github.com/gfx-rs/wgpu/pull/4147) + + ### Added/New Features - Add `gles_minor_version` field to `wgpu::InstanceDescriptor`. By @PJB3005 in [#3998](https://github.com/gfx-rs/wgpu/pull/3998) diff --git a/examples/boids/src/main.rs b/examples/boids/src/main.rs index e8aa2f71fd..eb5146f8bd 100644 --- a/examples/boids/src/main.rs +++ b/examples/boids/src/main.rs @@ -276,7 +276,7 @@ impl wgpu_example::framework::Example for Example { // Not clearing here in order to test wgpu's zero texture initialization on a surface texture. // Users should avoid loading uninitialized memory since this can cause additional overhead. load: wgpu::LoadOp::Load, - store: true, + store: wgpu::StoreOp::Store, }, })]; let render_pass_descriptor = wgpu::RenderPassDescriptor { diff --git a/examples/bunnymark/src/main.rs b/examples/bunnymark/src/main.rs index 256083eebb..ca8dbbee06 100644 --- a/examples/bunnymark/src/main.rs +++ b/examples/bunnymark/src/main.rs @@ -332,7 +332,7 @@ impl wgpu_example::framework::Example for Example { resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(clear_color), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: None, diff --git a/examples/capture/src/main.rs b/examples/capture/src/main.rs index b783b3af80..47a453de6b 100644 --- a/examples/capture/src/main.rs +++ b/examples/capture/src/main.rs @@ -101,7 +101,7 @@ async fn create_red_image_with_dimensions( resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(wgpu::Color::RED), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: None, diff --git a/examples/conservative-raster/src/main.rs b/examples/conservative-raster/src/main.rs index e5cfb4d775..093740a206 100644 --- a/examples/conservative-raster/src/main.rs +++ b/examples/conservative-raster/src/main.rs @@ -269,7 +269,7 @@ impl wgpu_example::framework::Example for Example { resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(wgpu::Color::BLACK), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: None, @@ -290,7 +290,7 @@ impl wgpu_example::framework::Example for Example { resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(wgpu::Color::BLACK), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: None, diff --git a/examples/cube/src/main.rs b/examples/cube/src/main.rs index a10dfd0fd0..b031e1004c 100644 --- a/examples/cube/src/main.rs +++ b/examples/cube/src/main.rs @@ -375,7 +375,7 @@ impl wgpu_example::framework::Example for Example { b: 0.3, a: 1.0, }), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: None, diff --git a/examples/hello-triangle/src/main.rs b/examples/hello-triangle/src/main.rs index c5432acd07..ebb8f6b736 100644 --- a/examples/hello-triangle/src/main.rs +++ b/examples/hello-triangle/src/main.rs @@ -118,7 +118,7 @@ async fn run(event_loop: EventLoop<()>, window: Window) { resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(wgpu::Color::GREEN), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: None, diff --git a/examples/hello-windows/src/main.rs b/examples/hello-windows/src/main.rs index f368804c36..ba28341395 100644 --- a/examples/hello-windows/src/main.rs +++ b/examples/hello-windows/src/main.rs @@ -131,7 +131,7 @@ async fn run(event_loop: EventLoop<()>, viewports: Vec<(Window, wgpu::Color)>) { resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(viewport.desc.background), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: None, diff --git a/examples/mipmap/src/main.rs b/examples/mipmap/src/main.rs index a85110ff14..5536579b0b 100644 --- a/examples/mipmap/src/main.rs +++ b/examples/mipmap/src/main.rs @@ -163,7 +163,7 @@ impl Example { resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(wgpu::Color::WHITE), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: None, @@ -490,7 +490,7 @@ impl wgpu_example::framework::Example for Example { resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(clear_color), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: None, diff --git a/examples/msaa-line/src/main.rs b/examples/msaa-line/src/main.rs index aa7a277418..07cc4eaf57 100644 --- a/examples/msaa-line/src/main.rs +++ b/examples/msaa-line/src/main.rs @@ -282,7 +282,7 @@ impl wgpu_example::framework::Example for Example { resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(wgpu::Color::BLACK), - store: true, + store: wgpu::StoreOp::Store, }, } } else { @@ -293,7 +293,7 @@ impl wgpu_example::framework::Example for Example { load: wgpu::LoadOp::Clear(wgpu::Color::BLACK), // Storing pre-resolve MSAA data is unnecessary if it isn't used later. // On tile-based GPU, avoid store can reduce your app's memory footprint. - store: false, + store: wgpu::StoreOp::Discard, }, } }; diff --git a/examples/shadow/src/main.rs b/examples/shadow/src/main.rs index 3f963d0c53..c63076e6ac 100644 --- a/examples/shadow/src/main.rs +++ b/examples/shadow/src/main.rs @@ -773,7 +773,7 @@ impl wgpu_example::framework::Example for Example { view: &light.target_view, depth_ops: Some(wgpu::Operations { load: wgpu::LoadOp::Clear(1.0), - store: true, + store: wgpu::StoreOp::Store, }), stencil_ops: None, }), @@ -810,14 +810,14 @@ impl wgpu_example::framework::Example for Example { b: 0.3, a: 1.0, }), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: Some(wgpu::RenderPassDepthStencilAttachment { view: &self.forward_depth, depth_ops: Some(wgpu::Operations { load: wgpu::LoadOp::Clear(1.0), - store: false, + store: wgpu::StoreOp::Discard, }), stencil_ops: None, }), diff --git a/examples/skybox/src/main.rs b/examples/skybox/src/main.rs index d09622f53c..5d91c62865 100644 --- a/examples/skybox/src/main.rs +++ b/examples/skybox/src/main.rs @@ -428,14 +428,14 @@ impl wgpu_example::framework::Example for Skybox { b: 0.3, a: 1.0, }), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: Some(wgpu::RenderPassDepthStencilAttachment { view: &self.depth_view, depth_ops: Some(wgpu::Operations { load: wgpu::LoadOp::Clear(1.0), - store: false, + store: wgpu::StoreOp::Discard, }), stencil_ops: None, }), diff --git a/examples/stencil-triangles/src/main.rs b/examples/stencil-triangles/src/main.rs index 55aad9c9ba..9d918500d5 100644 --- a/examples/stencil-triangles/src/main.rs +++ b/examples/stencil-triangles/src/main.rs @@ -200,7 +200,7 @@ impl wgpu_example::framework::Example for Triangles { b: 0.3, a: 1.0, }), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: Some(wgpu::RenderPassDepthStencilAttachment { @@ -208,7 +208,7 @@ impl wgpu_example::framework::Example for Triangles { depth_ops: None, stencil_ops: Some(wgpu::Operations { load: wgpu::LoadOp::Clear(0), - store: true, + store: wgpu::StoreOp::Store, }), }), timestamp_writes: None, diff --git a/examples/texture-arrays/src/main.rs b/examples/texture-arrays/src/main.rs index 373c2396ae..af9cfa56ff 100644 --- a/examples/texture-arrays/src/main.rs +++ b/examples/texture-arrays/src/main.rs @@ -379,7 +379,7 @@ impl wgpu_example::framework::Example for Example { resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(wgpu::Color::BLACK), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: None, diff --git a/examples/timestamp-queries/src/main.rs b/examples/timestamp-queries/src/main.rs index f8c524f03c..d463ea6579 100644 --- a/examples/timestamp-queries/src/main.rs +++ b/examples/timestamp-queries/src/main.rs @@ -375,7 +375,7 @@ fn render_pass( resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(wgpu::Color::GREEN), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: None, diff --git a/examples/water/src/main.rs b/examples/water/src/main.rs index 5d5daa1f59..da9f1238ab 100644 --- a/examples/water/src/main.rs +++ b/examples/water/src/main.rs @@ -739,7 +739,7 @@ impl wgpu_example::framework::Example for Example { resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(back_color), - store: true, + store: wgpu::StoreOp::Store, }, })], // We still need to use the depth buffer here @@ -748,7 +748,7 @@ impl wgpu_example::framework::Example for Example { view: &self.depth_buffer, depth_ops: Some(wgpu::Operations { load: wgpu::LoadOp::Clear(1.0), - store: true, + store: wgpu::StoreOp::Store, }), stencil_ops: None, }), @@ -768,14 +768,14 @@ impl wgpu_example::framework::Example for Example { resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(back_color), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: Some(wgpu::RenderPassDepthStencilAttachment { view: &self.depth_buffer, depth_ops: Some(wgpu::Operations { load: wgpu::LoadOp::Clear(1.0), - store: true, + store: wgpu::StoreOp::Store, }), stencil_ops: None, }), @@ -797,7 +797,7 @@ impl wgpu_example::framework::Example for Example { resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Load, - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: Some(wgpu::RenderPassDepthStencilAttachment { diff --git a/tests/tests/occlusion_query/mod.rs b/tests/tests/occlusion_query/mod.rs index eab0828e41..7747eaa624 100644 --- a/tests/tests/occlusion_query/mod.rs +++ b/tests/tests/occlusion_query/mod.rs @@ -69,7 +69,7 @@ fn occlusion_query() { view: &depth_texture_view, depth_ops: Some(wgpu::Operations { load: wgpu::LoadOp::Clear(1.0), - store: true, + store: wgpu::StoreOp::Store, }), stencil_ops: None, }), diff --git a/tests/tests/regression/issue_3457.rs b/tests/tests/regression/issue_3457.rs index 2dccd3d427..0d2c086ed5 100644 --- a/tests/tests/regression/issue_3457.rs +++ b/tests/tests/regression/issue_3457.rs @@ -140,7 +140,7 @@ fn pass_reset_vertex_buffer() { resolve_target: None, ops: Operations { load: LoadOp::Clear(Color::BLACK), - store: false, + store: StoreOp::Discard, }, })], depth_stencil_attachment: None, @@ -175,7 +175,7 @@ fn pass_reset_vertex_buffer() { resolve_target: None, ops: Operations { load: LoadOp::Clear(Color::BLACK), - store: false, + store: StoreOp::Discard, }, })], depth_stencil_attachment: None, diff --git a/tests/tests/scissor_tests/mod.rs b/tests/tests/scissor_tests/mod.rs index da050cb61f..a921827e0d 100644 --- a/tests/tests/scissor_tests/mod.rs +++ b/tests/tests/scissor_tests/mod.rs @@ -75,7 +75,7 @@ fn scissor_test_impl(ctx: &TestingContext, scissor_rect: Rect, expected_data: [u b: 0.0, a: 0.0, }), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: None, diff --git a/tests/tests/shader_primitive_index/mod.rs b/tests/tests/shader_primitive_index/mod.rs index a05d1cd5f0..2739b2e77d 100644 --- a/tests/tests/shader_primitive_index/mod.rs +++ b/tests/tests/shader_primitive_index/mod.rs @@ -180,7 +180,7 @@ fn pulling_common( color_attachments: &[Some(wgpu::RenderPassColorAttachment { ops: wgpu::Operations { load: wgpu::LoadOp::Clear(wgpu::Color::WHITE), - store: true, + store: wgpu::StoreOp::Store, }, resolve_target: None, view: &color_view, diff --git a/tests/tests/zero_init_texture_after_discard.rs b/tests/tests/zero_init_texture_after_discard.rs index 2b757e069a..e47f1aa0fa 100644 --- a/tests/tests/zero_init_texture_after_discard.rs +++ b/tests/tests/zero_init_texture_after_discard.rs @@ -155,11 +155,11 @@ impl<'ctx> TestCase<'ctx> { view: &texture.create_view(&TextureViewDescriptor::default()), depth_ops: format.has_depth_aspect().then_some(Operations { load: LoadOp::Clear(1.0), - store: true, + store: StoreOp::Store, }), stencil_ops: format.has_stencil_aspect().then_some(Operations { load: LoadOp::Clear(0xFFFFFFFF), - store: true, + store: StoreOp::Store, }), }), timestamp_writes: None, @@ -230,7 +230,7 @@ impl<'ctx> TestCase<'ctx> { resolve_target: None, ops: Operations { load: LoadOp::Load, - store: false, // discard! + store: StoreOp::Discard, }, }, )], @@ -239,11 +239,11 @@ impl<'ctx> TestCase<'ctx> { view: &self.texture.create_view(&TextureViewDescriptor::default()), depth_ops: self.format.has_depth_aspect().then_some(Operations { load: LoadOp::Load, - store: false, // discard! + store: StoreOp::Discard, }), stencil_ops: self.format.has_stencil_aspect().then_some(Operations { load: LoadOp::Load, - store: false, // discard! + store: StoreOp::Discard, }), }, ), @@ -264,11 +264,11 @@ impl<'ctx> TestCase<'ctx> { view: &self.texture.create_view(&TextureViewDescriptor::default()), depth_ops: Some(Operations { load: LoadOp::Load, - store: false, // discard! + store: StoreOp::Discard, }), stencil_ops: self.format.has_stencil_aspect().then_some(Operations { load: LoadOp::Clear(0), - store: true, + store: StoreOp::Store, }), }, ), @@ -289,11 +289,11 @@ impl<'ctx> TestCase<'ctx> { view: &self.texture.create_view(&TextureViewDescriptor::default()), depth_ops: self.format.has_depth_aspect().then_some(Operations { load: LoadOp::Clear(0.0), - store: true, + store: StoreOp::Store, }), stencil_ops: Some(Operations { load: LoadOp::Load, - store: false, // discard! + store: StoreOp::Discard, }), }, ), diff --git a/wgpu/src/backend/direct.rs b/wgpu/src/backend/direct.rs index 2e15e295e8..3d3028d334 100644 --- a/wgpu/src/backend/direct.rs +++ b/wgpu/src/backend/direct.rs @@ -4,7 +4,7 @@ use crate::{ BufferDescriptor, CommandEncoderDescriptor, ComputePassDescriptor, ComputePipelineDescriptor, DownlevelCapabilities, Features, Label, Limits, LoadOp, MapMode, Operations, PipelineLayoutDescriptor, RenderBundleEncoderDescriptor, RenderPipelineDescriptor, - SamplerDescriptor, ShaderModuleDescriptor, ShaderModuleDescriptorSpirV, ShaderSource, + SamplerDescriptor, ShaderModuleDescriptor, ShaderModuleDescriptorSpirV, ShaderSource, StoreOp, SurfaceStatus, TextureDescriptor, TextureViewDescriptor, UncapturedErrorHandler, }; @@ -392,6 +392,13 @@ fn map_texture_tagged_copy_view( } } +fn map_store_op(op: StoreOp) -> wgc::command::StoreOp { + match op { + StoreOp::Store => wgc::command::StoreOp::Store, + StoreOp::Discard => wgc::command::StoreOp::Discard, + } +} + fn map_pass_channel( ops: Option<&Operations>, ) -> wgc::command::PassChannel { @@ -401,11 +408,7 @@ fn map_pass_channel( store, }) => wgc::command::PassChannel { load_op: wgc::command::LoadOp::Clear, - store_op: if store { - wgc::command::StoreOp::Store - } else { - wgc::command::StoreOp::Discard - }, + store_op: map_store_op(store), clear_value, read_only: false, }, @@ -414,11 +417,7 @@ fn map_pass_channel( store, }) => wgc::command::PassChannel { load_op: wgc::command::LoadOp::Load, - store_op: if store { - wgc::command::StoreOp::Store - } else { - wgc::command::StoreOp::Discard - }, + store_op: map_store_op(store), clear_value: V::default(), read_only: false, }, diff --git a/wgpu/src/backend/web.rs b/wgpu/src/backend/web.rs index 2f83d50c55..d457681e57 100644 --- a/wgpu/src/backend/web.rs +++ b/wgpu/src/backend/web.rs @@ -621,11 +621,10 @@ fn map_color(color: wgt::Color) -> web_sys::GpuColorDict { web_sys::GpuColorDict::new(color.a, color.b, color.g, color.r) } -fn map_store_op(store: bool) -> web_sys::GpuStoreOp { - if store { - web_sys::GpuStoreOp::Store - } else { - web_sys::GpuStoreOp::Discard +fn map_store_op(store: crate::StoreOp) -> web_sys::GpuStoreOp { + match store { + crate::StoreOp::Store => web_sys::GpuStoreOp::Store, + crate::StoreOp::Discard => web_sys::GpuStoreOp::Discard, } } diff --git a/wgpu/src/lib.rs b/wgpu/src/lib.rs index 19dc20120d..88b852eaa4 100644 --- a/wgpu/src/lib.rs +++ b/wgpu/src/lib.rs @@ -1006,16 +1006,24 @@ static_assertions::assert_impl_all!(BufferBinding: Send, Sync); /// Operation to perform to the output attachment at the start of a render pass. /// -/// The render target must be cleared at least once before its content is loaded. -/// -/// Corresponds to [WebGPU `GPULoadOp`](https://gpuweb.github.io/gpuweb/#enumdef-gpuloadop). +/// Corresponds to [WebGPU `GPULoadOp`](https://gpuweb.github.io/gpuweb/#enumdef-gpuloadop), +/// plus the corresponding clearValue. #[derive(Copy, Clone, Debug, Hash, Eq, PartialEq)] #[cfg_attr(feature = "trace", derive(serde::Serialize))] #[cfg_attr(feature = "replay", derive(serde::Deserialize))] pub enum LoadOp { - /// Clear with a specified value. + /// Loads the specified value for this attachment into the render pass. + /// + /// On some GPU hardware (primarily mobile), "clear" is significantly cheaper + /// because it avoids loading data from main memory into tile-local memory. + /// + /// On other GPU hardware, there isn’t a significant difference. + /// + /// As a result, it is recommended to use "clear" rather than "load" in cases + /// where the initial value doesn’t matter + /// (e.g. the render target will be cleared using a skybox). Clear(V), - /// Load from memory. + /// Loads the existing value for this attachment into the render pass. Load, } @@ -1025,6 +1033,28 @@ impl Default for LoadOp { } } +/// Operation to perform to the output attachment at the end of a render pass. +/// +/// Corresponds to [WebGPU `GPUStoreOp`](https://gpuweb.github.io/gpuweb/#enumdef-gpustoreop). +#[derive(Copy, Clone, Debug, Hash, Eq, PartialEq, Default)] +#[cfg_attr(feature = "trace", derive(serde::Serialize))] +#[cfg_attr(feature = "replay", derive(serde::Deserialize))] +pub enum StoreOp { + /// Stores the resulting value of the render pass for this attachment. + #[default] + Store, + /// Discards the resulting value of the render pass for this attachment. + /// + /// The attachment will be treated as uninitialized afterwards. + /// (If only either Depth or Stencil texture-aspects is set to `Discard`, + /// the respective other texture-aspect will be preserved.) + /// + /// This can be significantly faster on tile-based render hardware. + /// + /// Prefer this if the attachment is not read by subsequent passes. + Discard, +} + /// Pair of load and store operations for an attachment aspect. /// /// This type is unique to the Rust API of `wgpu`. In the WebGPU specification, @@ -1036,14 +1066,18 @@ pub struct Operations { /// How data should be read through this attachment. pub load: LoadOp, /// Whether data will be written to through this attachment. - pub store: bool, + /// + /// Note that resolve textures (if specified) are always written to, + /// regardless of this setting. + pub store: StoreOp, } impl Default for Operations { + #[inline] fn default() -> Self { Self { - load: Default::default(), - store: true, + load: LoadOp::::default(), + store: StoreOp::default(), } } } @@ -1084,6 +1118,8 @@ pub struct RenderPassColorAttachment<'tex> { /// The view to use as an attachment. pub view: &'tex TextureView, /// The view that will receive the resolved output if multisampling is used. + /// + /// If set, it is always written to, regardless of how [`Self::ops`] is configured. pub resolve_target: Option<&'tex TextureView>, /// What operations will be performed on this color attachment. pub ops: Operations, From 87a0cd0e69dabd66da4d542b1cec0c71765cfc04 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 19 Sep 2023 09:35:02 +0200 Subject: [PATCH 26/33] Bump profiling from 1.0.10 to 1.0.11 (#4153) Bumps [profiling](https://github.com/aclysma/profiling) from 1.0.10 to 1.0.11. - [Changelog](https://github.com/aclysma/profiling/blob/master/CHANGELOG.md) - [Commits](https://github.com/aclysma/profiling/commits) --- updated-dependencies: - dependency-name: profiling dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 303e94dbd8..f966a215f0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2126,9 +2126,9 @@ dependencies = [ [[package]] name = "profiling" -version = "1.0.10" +version = "1.0.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "45f10e75d83c7aec79a6aa46f897075890e156b105eebe51cfa0abce51af025f" +checksum = "f89dff0959d98c9758c88826cc002e2c3d0b9dfac4139711d1f30de442f1139b" [[package]] name = "quote" From 5c26841d66b7fb0675ba6b8509b483e6ac2c30ee Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 19 Sep 2023 09:35:12 +0200 Subject: [PATCH 27/33] Bump termcolor from 1.2.0 to 1.3.0 (#4152) Bumps [termcolor](https://github.com/BurntSushi/termcolor) from 1.2.0 to 1.3.0. - [Commits](https://github.com/BurntSushi/termcolor/compare/1.2.0...1.3.0) --- updated-dependencies: - dependency-name: termcolor dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f966a215f0..9978941db4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2632,9 +2632,9 @@ dependencies = [ [[package]] name = "termcolor" -version = "1.2.0" +version = "1.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "be55cf8942feac5c765c2c993422806843c9a9a45d4d5c407ad6dd2ea95eb9b6" +checksum = "6093bad37da69aab9d123a8091e4be0aa4a03e4d601ec641c327398315f62b64" dependencies = [ "winapi-util", ] diff --git a/Cargo.toml b/Cargo.toml index 40b8c46c96..10378fc231 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -142,7 +142,7 @@ deno_web = "0.137.0" deno_webidl = "0.106.0" deno_webgpu = { path = "./deno_webgpu" } tokio = "1.32.0" -termcolor = "1.2.0" +termcolor = "1.3.0" [patch."https://github.com/gfx-rs/naga"] #naga = { path = "../naga" } From dc5beac8c96536d8c8a2ca98c22021dff14e53cf Mon Sep 17 00:00:00 2001 From: Frederik Magnus Johansen Vestre Date: Tue, 19 Sep 2023 13:26:30 +0200 Subject: [PATCH 28/33] Support dual source blending (#4022) Co-authored-by: Teodor Tanasoaia <28601907+teoxoy@users.noreply.github.com> --- CHANGELOG.md | 1 + wgpu-core/src/device/resource.rs | 46 +++++++++++++++++++++++++++++++- wgpu-core/src/pipeline.rs | 9 +++++++ wgpu-core/src/validation.rs | 14 +++++++++- wgpu-hal/src/dx12/adapter.rs | 4 ++- wgpu-hal/src/dx12/conv.rs | 12 ++++----- wgpu-hal/src/gles/adapter.rs | 4 +++ wgpu-hal/src/gles/conv.rs | 4 +++ wgpu-hal/src/metal/adapter.rs | 4 +++ wgpu-hal/src/metal/conv.rs | 10 +++---- wgpu-hal/src/vulkan/adapter.rs | 2 ++ wgpu-hal/src/vulkan/conv.rs | 4 +++ wgpu-types/src/lib.rs | 37 ++++++++++++++++++++++++- wgpu/src/backend/web.rs | 9 +++++++ 14 files changed, 144 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 32f90b3d90..d60e0cd84f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -100,6 +100,7 @@ By @wumpf in [#4147](https://github.com/gfx-rs/wgpu/pull/4147) - Add validation in accordance with WebGPU `setViewport` valid usage for `x`, `y` and `this.[[attachment_size]]`. By @James2022-rgb in [#4058](https://github.com/gfx-rs/wgpu/pull/4058) - `wgpu::CreateSurfaceError` and `wgpu::RequestDeviceError` now give details of the failure, but no longer implement `PartialEq` and cannot be constructed. By @kpreid in [#4066](https://github.com/gfx-rs/wgpu/pull/4066) and [#4145](https://github.com/gfx-rs/wgpu/pull/4145) - Make `WGPU_POWER_PREF=none` a valid value. By @fornwall in [4076](https://github.com/gfx-rs/wgpu/pull/4076) +- Support dual source blending in OpenGL ES, Metal, Vulkan & DX12. By @freqmod in [4022](https://github.com/gfx-rs/wgpu/pull/4022) #### Vulkan diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index 73f1887e10..ba7006d3d5 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -1276,6 +1276,10 @@ impl Device { .flags .contains(wgt::DownlevelFlags::MULTISAMPLED_SHADING), ); + caps.set( + Caps::DUAL_SOURCE_BLENDING, + self.features.contains(wgt::Features::DUAL_SOURCE_BLENDING), + ); let info = naga::valid::Validator::new(naga::valid::ValidationFlags::all(), caps) .validate(&module) @@ -2560,6 +2564,8 @@ impl Device { let mut vertex_steps = Vec::with_capacity(desc.vertex.buffers.len()); let mut vertex_buffers = Vec::with_capacity(desc.vertex.buffers.len()); let mut total_attributes = 0; + let mut shader_expects_dual_source_blending = false; + let mut pipeline_expects_dual_source_blending = false; for (i, vb_state) in desc.vertex.buffers.iter().enumerate() { vertex_steps.push(pipeline::VertexStep { stride: vb_state.array_stride, @@ -2700,7 +2706,25 @@ impl Device { { break Some(pipeline::ColorStateError::FormatNotMultisampled(cs.format)); } - + if let Some(blend_mode) = cs.blend { + for factor in [ + blend_mode.color.src_factor, + blend_mode.color.dst_factor, + blend_mode.alpha.src_factor, + blend_mode.alpha.dst_factor, + ] { + if factor.ref_second_blend_source() { + self.require_features(wgt::Features::DUAL_SOURCE_BLENDING)?; + if i == 0 { + pipeline_expects_dual_source_blending = true; + break; + } else { + return Err(crate::pipeline::CreateRenderPipelineError + ::BlendFactorOnUnsupportedTarget { factor, target: i as u32 }); + } + } + } + } break None; }; if let Some(e) = error { @@ -2857,6 +2881,15 @@ impl Device { } } + if let Some(ref interface) = shader_module.interface { + shader_expects_dual_source_blending = interface + .fragment_uses_dual_source_blending(&fragment.stage.entry_point) + .map_err(|error| pipeline::CreateRenderPipelineError::Stage { + stage: flag, + error, + })?; + } + Some(hal::ProgrammableStage { module: &shader_module.raw, entry_point: fragment.stage.entry_point.as_ref(), @@ -2865,6 +2898,17 @@ impl Device { None => None, }; + if !pipeline_expects_dual_source_blending && shader_expects_dual_source_blending { + return Err( + pipeline::CreateRenderPipelineError::ShaderExpectsPipelineToUseDualSourceBlending, + ); + } + if pipeline_expects_dual_source_blending && !shader_expects_dual_source_blending { + return Err( + pipeline::CreateRenderPipelineError::PipelineExpectsShaderToUseDualSourceBlending, + ); + } + if validated_stages.contains(wgt::ShaderStages::FRAGMENT) { for (i, output) in io.iter() { match color_targets.get(*i as usize) { diff --git a/wgpu-core/src/pipeline.rs b/wgpu-core/src/pipeline.rs index da06b652ea..c78a79820d 100644 --- a/wgpu-core/src/pipeline.rs +++ b/wgpu-core/src/pipeline.rs @@ -384,6 +384,15 @@ pub enum CreateRenderPipelineError { }, #[error("In the provided shader, the type given for group {group} binding {binding} has a size of {size}. As the device does not support `DownlevelFlags::BUFFER_BINDINGS_NOT_16_BYTE_ALIGNED`, the type must have a size that is a multiple of 16 bytes.")] UnalignedShader { group: u32, binding: u32, size: u64 }, + #[error("Using the blend factor {factor:?} for render target {target} is not possible. Only the first render target may be used when dual-source blending.")] + BlendFactorOnUnsupportedTarget { + factor: wgt::BlendFactor, + target: u32, + }, + #[error("Pipeline expects the shader entry point to make use of dual-source blending.")] + PipelineExpectsShaderToUseDualSourceBlending, + #[error("Shader entry point expects the pipeline to make use of dual-source blending.")] + ShaderExpectsPipelineToUseDualSourceBlending, } bitflags::bitflags! { diff --git a/wgpu-core/src/validation.rs b/wgpu-core/src/validation.rs index e3ecb916d3..778cc26cd5 100644 --- a/wgpu-core/src/validation.rs +++ b/wgpu-core/src/validation.rs @@ -116,6 +116,7 @@ struct EntryPoint { spec_constants: Vec, sampling_pairs: FastHashSet<(naga::Handle, naga::Handle)>, workgroup_size: [u32; 3], + dual_source_blending: bool, } #[derive(Debug)] @@ -903,7 +904,7 @@ impl Interface { ep.sampling_pairs .insert((resource_mapping[&key.image], resource_mapping[&key.sampler])); } - + ep.dual_source_blending = info.dual_source_blending; ep.workgroup_size = entry_point.workgroup_size; entry_points.insert((entry_point.stage, entry_point.name.clone()), ep); @@ -1177,4 +1178,15 @@ impl Interface { .collect(); Ok(outputs) } + + pub fn fragment_uses_dual_source_blending( + &self, + entry_point_name: &str, + ) -> Result { + let pair = (naga::ShaderStage::Fragment, entry_point_name.to_string()); + self.entry_points + .get(&pair) + .ok_or(StageError::MissingEntryPoint(pair.1)) + .map(|ep| ep.dual_source_blending) + } } diff --git a/wgpu-hal/src/dx12/adapter.rs b/wgpu-hal/src/dx12/adapter.rs index 02cde913ca..3959deeccd 100644 --- a/wgpu-hal/src/dx12/adapter.rs +++ b/wgpu-hal/src/dx12/adapter.rs @@ -250,7 +250,9 @@ impl super::Adapter { | wgt::Features::TEXTURE_FORMAT_16BIT_NORM | wgt::Features::PUSH_CONSTANTS | wgt::Features::SHADER_PRIMITIVE_INDEX - | wgt::Features::RG11B10UFLOAT_RENDERABLE; + | wgt::Features::RG11B10UFLOAT_RENDERABLE + | wgt::Features::DUAL_SOURCE_BLENDING; + //TODO: in order to expose this, we need to run a compute shader // that extract the necessary statistics out of the D3D12 result. // Alternatively, we could allocate a buffer for the query set, diff --git a/wgpu-hal/src/dx12/conv.rs b/wgpu-hal/src/dx12/conv.rs index 908944567a..f484d1a9e2 100644 --- a/wgpu-hal/src/dx12/conv.rs +++ b/wgpu-hal/src/dx12/conv.rs @@ -246,12 +246,12 @@ fn map_blend_factor(factor: wgt::BlendFactor, is_alpha: bool) -> d3d12_ty::D3D12 Bf::Constant => d3d12_ty::D3D12_BLEND_BLEND_FACTOR, Bf::OneMinusConstant => d3d12_ty::D3D12_BLEND_INV_BLEND_FACTOR, Bf::SrcAlphaSaturated => d3d12_ty::D3D12_BLEND_SRC_ALPHA_SAT, - //Bf::Src1Color if is_alpha => d3d12_ty::D3D12_BLEND_SRC1_ALPHA, - //Bf::Src1Color => d3d12_ty::D3D12_BLEND_SRC1_COLOR, - //Bf::OneMinusSrc1Color if is_alpha => d3d12_ty::D3D12_BLEND_INV_SRC1_ALPHA, - //Bf::OneMinusSrc1Color => d3d12_ty::D3D12_BLEND_INV_SRC1_COLOR, - //Bf::Src1Alpha => d3d12_ty::D3D12_BLEND_SRC1_ALPHA, - //Bf::OneMinusSrc1Alpha => d3d12_ty::D3D12_BLEND_INV_SRC1_ALPHA, + Bf::Src1 if is_alpha => d3d12_ty::D3D12_BLEND_SRC1_ALPHA, + Bf::Src1 => d3d12_ty::D3D12_BLEND_SRC1_COLOR, + Bf::OneMinusSrc1 if is_alpha => d3d12_ty::D3D12_BLEND_INV_SRC1_ALPHA, + Bf::OneMinusSrc1 => d3d12_ty::D3D12_BLEND_INV_SRC1_COLOR, + Bf::Src1Alpha => d3d12_ty::D3D12_BLEND_SRC1_ALPHA, + Bf::OneMinusSrc1Alpha => d3d12_ty::D3D12_BLEND_INV_SRC1_ALPHA, } } diff --git a/wgpu-hal/src/gles/adapter.rs b/wgpu-hal/src/gles/adapter.rs index 348f62bc03..3dae58b7c4 100644 --- a/wgpu-hal/src/gles/adapter.rs +++ b/wgpu-hal/src/gles/adapter.rs @@ -363,6 +363,10 @@ impl super::Adapter { wgt::Features::MULTIVIEW, extensions.contains("OVR_multiview2"), ); + features.set( + wgt::Features::DUAL_SOURCE_BLENDING, + extensions.contains("GL_EXT_blend_func_extended"), + ); features.set( wgt::Features::SHADER_PRIMITIVE_INDEX, ver >= (3, 2) || extensions.contains("OES_geometry_shader"), diff --git a/wgpu-hal/src/gles/conv.rs b/wgpu-hal/src/gles/conv.rs index dd5d764c6a..9bfac022a1 100644 --- a/wgpu-hal/src/gles/conv.rs +++ b/wgpu-hal/src/gles/conv.rs @@ -376,6 +376,10 @@ fn map_blend_factor(factor: wgt::BlendFactor) -> u32 { Bf::Constant => glow::CONSTANT_COLOR, Bf::OneMinusConstant => glow::ONE_MINUS_CONSTANT_COLOR, Bf::SrcAlphaSaturated => glow::SRC_ALPHA_SATURATE, + Bf::Src1 => glow::SRC1_COLOR, + Bf::OneMinusSrc1 => glow::ONE_MINUS_SRC1_COLOR, + Bf::Src1Alpha => glow::SRC1_ALPHA, + Bf::OneMinusSrc1Alpha => glow::ONE_MINUS_SRC1_ALPHA, } } diff --git a/wgpu-hal/src/metal/adapter.rs b/wgpu-hal/src/metal/adapter.rs index 126741d257..da254442bc 100644 --- a/wgpu-hal/src/metal/adapter.rs +++ b/wgpu-hal/src/metal/adapter.rs @@ -833,6 +833,10 @@ impl super::PrivateCapabilities { self.timestamp_query_support .contains(TimestampQuerySupport::INSIDE_WGPU_PASSES), ); + features.set( + F::DUAL_SOURCE_BLENDING, + self.msl_version >= MTLLanguageVersion::V1_2 && self.dual_source_blending, + ); features.set(F::TEXTURE_COMPRESSION_ASTC, self.format_astc); features.set(F::TEXTURE_COMPRESSION_ASTC_HDR, self.format_astc_hdr); features.set(F::TEXTURE_COMPRESSION_BC, self.format_bc); diff --git a/wgpu-hal/src/metal/conv.rs b/wgpu-hal/src/metal/conv.rs index a1ceb287ab..8f6439b50b 100644 --- a/wgpu-hal/src/metal/conv.rs +++ b/wgpu-hal/src/metal/conv.rs @@ -152,13 +152,11 @@ pub fn map_blend_factor(factor: wgt::BlendFactor) -> metal::MTLBlendFactor { Bf::OneMinusDstAlpha => OneMinusDestinationAlpha, Bf::Constant => BlendColor, Bf::OneMinusConstant => OneMinusBlendColor, - //Bf::ConstantAlpha => BlendAlpha, - //Bf::OneMinusConstantAlpha => OneMinusBlendAlpha, Bf::SrcAlphaSaturated => SourceAlphaSaturated, - //Bf::Src1 => Source1Color, - //Bf::OneMinusSrc1 => OneMinusSource1Color, - //Bf::Src1Alpha => Source1Alpha, - //Bf::OneMinusSrc1Alpha => OneMinusSource1Alpha, + Bf::Src1 => Source1Color, + Bf::OneMinusSrc1 => OneMinusSource1Color, + Bf::Src1Alpha => Source1Alpha, + Bf::OneMinusSrc1Alpha => OneMinusSource1Alpha, } } diff --git a/wgpu-hal/src/vulkan/adapter.rs b/wgpu-hal/src/vulkan/adapter.rs index bcbab85084..78aceeeeef 100644 --- a/wgpu-hal/src/vulkan/adapter.rs +++ b/wgpu-hal/src/vulkan/adapter.rs @@ -177,6 +177,7 @@ impl PhysicalDeviceFeatures { //.shader_resource_residency(requested_features.contains(wgt::Features::SHADER_RESOURCE_RESIDENCY)) .geometry_shader(requested_features.contains(wgt::Features::SHADER_PRIMITIVE_INDEX)) .depth_clamp(requested_features.contains(wgt::Features::DEPTH_CLIP_CONTROL)) + .dual_src_blend(requested_features.contains(wgt::Features::DUAL_SOURCE_BLENDING)) .build(), descriptor_indexing: if requested_features.intersects(indexing_features()) { Some( @@ -460,6 +461,7 @@ impl PhysicalDeviceFeatures { } features.set(F::DEPTH_CLIP_CONTROL, self.core.depth_clamp != 0); + features.set(F::DUAL_SOURCE_BLENDING, self.core.dual_src_blend != 0); if let Some(ref multiview) = self.multiview { features.set(F::MULTIVIEW, multiview.multiview != 0); diff --git a/wgpu-hal/src/vulkan/conv.rs b/wgpu-hal/src/vulkan/conv.rs index e2398c2689..459b7f858f 100644 --- a/wgpu-hal/src/vulkan/conv.rs +++ b/wgpu-hal/src/vulkan/conv.rs @@ -792,6 +792,10 @@ fn map_blend_factor(factor: wgt::BlendFactor) -> vk::BlendFactor { Bf::SrcAlphaSaturated => vk::BlendFactor::SRC_ALPHA_SATURATE, Bf::Constant => vk::BlendFactor::CONSTANT_COLOR, Bf::OneMinusConstant => vk::BlendFactor::ONE_MINUS_CONSTANT_COLOR, + Bf::Src1 => vk::BlendFactor::SRC1_COLOR, + Bf::OneMinusSrc1 => vk::BlendFactor::ONE_MINUS_SRC1_COLOR, + Bf::Src1Alpha => vk::BlendFactor::SRC1_ALPHA, + Bf::OneMinusSrc1Alpha => vk::BlendFactor::ONE_MINUS_SRC1_ALPHA, } } diff --git a/wgpu-types/src/lib.rs b/wgpu-types/src/lib.rs index 9f61e2e490..e08b802094 100644 --- a/wgpu-types/src/lib.rs +++ b/wgpu-types/src/lib.rs @@ -781,7 +781,17 @@ bitflags::bitflags! { /// This is a native only feature. const SHADER_EARLY_DEPTH_TEST = 1 << 62; - // 62..64 available + /// Allows two outputs from a shader to be used for blending. + /// Note that dual-source blending doesn't support multiple render targets. + /// + /// For more info see the OpenGL ES extension GL_EXT_blend_func_extended. + /// + /// Supported platforms: + /// - OpenGL ES (with GL_EXT_blend_func_extended) + /// - Metal (with MSL 1.2+) + /// - Vulkan (with dualSrcBlend) + /// - DX12 + const DUAL_SOURCE_BLENDING = 1 << 63; } } @@ -1549,6 +1559,8 @@ impl TextureViewDimension { /// /// Corresponds to [WebGPU `GPUBlendFactor`]( /// https://gpuweb.github.io/gpuweb/#enumdef-gpublendfactor). +/// Values using S1 requires [`Features::DUAL_SOURCE_BLENDING`] and can only be +/// used with the first render target. #[repr(C)] #[derive(Copy, Clone, Debug, Hash, Eq, PartialEq)] #[cfg_attr(feature = "trace", derive(Serialize))] @@ -1581,6 +1593,29 @@ pub enum BlendFactor { Constant = 11, /// 1.0 - Constant OneMinusConstant = 12, + /// S1.component + Src1 = 13, + /// 1.0 - S1.component + OneMinusSrc1 = 14, + /// S1.alpha + Src1Alpha = 15, + /// 1.0 - S1.alpha + OneMinusSrc1Alpha = 16, +} + +impl BlendFactor { + /// Returns `true` if the blend factor references the second blend source. + /// + /// Note that the usage of those blend factors require [`Features::DUAL_SOURCE_BLENDING`]. + pub fn ref_second_blend_source(&self) -> bool { + match self { + BlendFactor::Src1 + | BlendFactor::OneMinusSrc1 + | BlendFactor::Src1Alpha + | BlendFactor::OneMinusSrc1Alpha => true, + _ => false, + } + } } /// Alpha blend operation. diff --git a/wgpu/src/backend/web.rs b/wgpu/src/backend/web.rs index d457681e57..b649d41ebb 100644 --- a/wgpu/src/backend/web.rs +++ b/wgpu/src/backend/web.rs @@ -421,6 +421,15 @@ fn map_blend_factor(factor: wgt::BlendFactor) -> web_sys::GpuBlendFactor { BlendFactor::SrcAlphaSaturated => bf::SrcAlphaSaturated, BlendFactor::Constant => bf::Constant, BlendFactor::OneMinusConstant => bf::OneMinusConstant, + BlendFactor::Src1 + | BlendFactor::OneMinusSrc1 + | BlendFactor::Src1Alpha + | BlendFactor::OneMinusSrc1Alpha => { + panic!( + "{:?} is not enabled for this backend", + wgt::Features::DUAL_SOURCE_BLENDING + ) + } } } From 82f0cd9ee655fc14af04693e9d30c5e3130161ce Mon Sep 17 00:00:00 2001 From: Alphyr <47725341+a1phyr@users.noreply.github.com> Date: Wed, 20 Sep 2023 20:45:09 +0200 Subject: [PATCH 29/33] Fix `Features::TEXTURE_COMPRESSION_ASTC*` doc (#4157) Co-authored-by: Connor Fitzgerald --- CHANGELOG.md | 3 ++- wgpu-types/src/lib.rs | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d60e0cd84f..4a54939a25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -110,7 +110,8 @@ By @wumpf in [#4147](https://github.com/gfx-rs/wgpu/pull/4147) ### Documentation -- Use WGSL for VertexFormat example types. By @ScanMountGoat in [#4305](https://github.com/gfx-rs/wgpu/pull/4035) +- Use WGSL for VertexFormat example types. By @ScanMountGoat in [#4035](https://github.com/gfx-rs/wgpu/pull/4035) +- Fix description of `Features::TEXTURE_COMPRESSION_ASTC_HDR` in [#4157](https://github.com/gfx-rs/wgpu/pull/4157) #### Metal diff --git a/wgpu-types/src/lib.rs b/wgpu-types/src/lib.rs index e08b802094..59c32ec3b8 100644 --- a/wgpu-types/src/lib.rs +++ b/wgpu-types/src/lib.rs @@ -371,7 +371,7 @@ bitflags::bitflags! { /// Compressed textures sacrifice some quality in exchange for significantly reduced /// bandwidth usage. /// - /// Support for this feature guarantees availability of [`TextureUsages::COPY_SRC | TextureUsages::COPY_DST | TextureUsages::TEXTURE_BINDING`] for ASTC formats. + /// Support for this feature guarantees availability of [`TextureUsages::COPY_SRC | TextureUsages::COPY_DST | TextureUsages::TEXTURE_BINDING`] for ASTC formats with Unorm/UnormSrgb channel type. /// [`Features::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES`] may enable additional usages. /// /// Supported Platforms: @@ -409,7 +409,7 @@ bitflags::bitflags! { /// Compressed textures sacrifice some quality in exchange for significantly reduced /// bandwidth usage. /// - /// Support for this feature guarantees availability of [`TextureUsages::COPY_SRC | TextureUsages::COPY_DST | TextureUsages::TEXTURE_BINDING`] for BCn formats. + /// Support for this feature guarantees availability of [`TextureUsages::COPY_SRC | TextureUsages::COPY_DST | TextureUsages::TEXTURE_BINDING`] for ASTC formats with the HDR channel type. /// [`Features::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES`] may enable additional usages. /// /// Supported Platforms: From 3c37c2afe525d9ea28c3b525b1f5a927a1ce13fb Mon Sep 17 00:00:00 2001 From: Neil Sarkar Date: Wed, 20 Sep 2023 12:07:40 -0700 Subject: [PATCH 30/33] Bring back xtask alias (#4160) --- .cargo/config.toml | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .cargo/config.toml diff --git a/.cargo/config.toml b/.cargo/config.toml new file mode 100644 index 0000000000..95d2a35175 --- /dev/null +++ b/.cargo/config.toml @@ -0,0 +1,7 @@ +[alias] +xtask = "run --manifest-path xtask/Cargo.toml --" + +[build] +rustflags = [ +"--cfg=web_sys_unstable_apis" +] From 2b4a8b318fb3717473b7da20b028dbc1f58ee9a2 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 20 Sep 2023 21:02:37 -0400 Subject: [PATCH 31/33] wgpu-core: Only produce StageError::InputNotConsumed on DX11/DX12 (#4116) * wgpu-core: Only produce StageError::InputNotConsumed on DX11/DX12 This error only exists due to an issue with naga's HLSL support: https://github.com/gfx-rs/naga/issues/1945 The WGPU spec itself allows vertex shader outputs that are not consumed by the fragment shader. Until the issue is fixed, we can allow unconsumed outputs on all platforms other than DX11/DX12. * Add Features::SHADER_UNUSED_VERTEX_OUTPUT to allow disabling check * Pick an unused feature id --- CHANGELOG.md | 1 + deno_webgpu/lib.rs | 7 +++++++ wgpu-core/src/device/resource.rs | 3 ++- wgpu-core/src/validation.rs | 16 ++++++++++++++-- wgpu-hal/src/gles/adapter.rs | 1 + wgpu-hal/src/metal/adapter.rs | 1 + wgpu-hal/src/vulkan/adapter.rs | 1 + wgpu-types/src/lib.rs | 9 +++++++++ 8 files changed, 36 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a54939a25..08d20c09fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -124,6 +124,7 @@ By @wumpf in [#4147](https://github.com/gfx-rs/wgpu/pull/4147) - Derive storage bindings via `naga::StorageAccess` instead of `naga::GlobalUse`. By @teoxoy in [#3985](https://github.com/gfx-rs/wgpu/pull/3985). - `Queue::on_submitted_work_done` callbacks will now always be called after all previous `BufferSlice::map_async` callbacks, even when there are no active submissions. By @cwfitzgerald in [#4036](https://github.com/gfx-rs/wgpu/pull/4036). - Fix `clear` texture views being leaked when `wgpu::SurfaceTexture` is dropped before it is presented. By @rajveermalviya in [#4057](https://github.com/gfx-rs/wgpu/pull/4057). +- Add `Feature::SHADER_UNUSED_VERTEX_OUTPUT` to allow unused vertex shader outputs. By @Aaron1011 in [#4116](https://github.com/gfx-rs/wgpu/pull/4116). #### Vulkan - Fix enabling `wgpu::Features::PARTIALLY_BOUND_BINDING_ARRAY` not being actually enabled in vulkan backend. By @39ali in[#3772](https://github.com/gfx-rs/wgpu/pull/3772). diff --git a/deno_webgpu/lib.rs b/deno_webgpu/lib.rs index 92a6a51334..bf61e42517 100644 --- a/deno_webgpu/lib.rs +++ b/deno_webgpu/lib.rs @@ -365,6 +365,9 @@ fn deserialize_features(features: &wgpu_types::Features) -> Vec<&'static str> { if features.contains(wgpu_types::Features::SHADER_EARLY_DEPTH_TEST) { return_features.push("shader-early-depth-test"); } + if features.contains(wgpu_types::Features::SHADER_UNUSED_VERTEX_OUTPUT) { + return_features.push("shader-unused-vertex-output"); + } return_features } @@ -625,6 +628,10 @@ impl From for wgpu_types::Features { wgpu_types::Features::SHADER_EARLY_DEPTH_TEST, required_features.0.contains("shader-early-depth-test"), ); + features.set( + wgpu_types::Features::SHADER_UNUSED_VERTEX_OUTPUT, + required_features.0.contains("shader-unused-vertex-output"), + ); features } diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index ba7006d3d5..8acc34acf4 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -1290,7 +1290,8 @@ impl Device { inner: Box::new(inner), }) })?; - let interface = validation::Interface::new(&module, &info, self.limits.clone()); + let interface = + validation::Interface::new(&module, &info, self.limits.clone(), self.features); let hal_shader = hal::ShaderInput::Naga(hal::NagaShader { module, info }); let hal_desc = hal::ShaderModuleDescriptor { diff --git a/wgpu-core/src/validation.rs b/wgpu-core/src/validation.rs index 778cc26cd5..ef5c65ed00 100644 --- a/wgpu-core/src/validation.rs +++ b/wgpu-core/src/validation.rs @@ -122,6 +122,7 @@ struct EntryPoint { #[derive(Debug)] pub struct Interface { limits: wgt::Limits, + features: wgt::Features, resources: naga::Arena, entry_points: FastHashMap<(naga::ShaderStage, String), EntryPoint>, } @@ -831,7 +832,12 @@ impl Interface { list.push(varying); } - pub fn new(module: &naga::Module, info: &naga::valid::ModuleInfo, limits: wgt::Limits) -> Self { + pub fn new( + module: &naga::Module, + info: &naga::valid::ModuleInfo, + limits: wgt::Limits, + features: wgt::Features, + ) -> Self { let mut resources = naga::Arena::new(); let mut resource_mapping = FastHashMap::default(); for (var_handle, var) in module.global_variables.iter() { @@ -912,6 +918,7 @@ impl Interface { Self { limits, + features, resources, entry_points, } @@ -1121,7 +1128,12 @@ impl Interface { } // Check all vertex outputs and make sure the fragment shader consumes them. - if shader_stage == naga::ShaderStage::Fragment { + // This requirement is removed if the `SHADER_UNUSED_VERTEX_OUTPUT` feature is enabled. + if shader_stage == naga::ShaderStage::Fragment + && !self + .features + .contains(wgt::Features::SHADER_UNUSED_VERTEX_OUTPUT) + { for &index in inputs.keys() { // This is a linear scan, but the count should be low enough // that this should be fine. diff --git a/wgpu-hal/src/gles/adapter.rs b/wgpu-hal/src/gles/adapter.rs index 3dae58b7c4..cbbcf7399e 100644 --- a/wgpu-hal/src/gles/adapter.rs +++ b/wgpu-hal/src/gles/adapter.rs @@ -372,6 +372,7 @@ impl super::Adapter { ver >= (3, 2) || extensions.contains("OES_geometry_shader"), ); features.set(wgt::Features::SHADER_EARLY_DEPTH_TEST, ver >= (3, 1)); + features.set(wgt::Features::SHADER_UNUSED_VERTEX_OUTPUT, true); let gles_bcn_exts = [ "GL_EXT_texture_compression_s3tc_srgb", "GL_EXT_texture_compression_rgtc", diff --git a/wgpu-hal/src/metal/adapter.rs b/wgpu-hal/src/metal/adapter.rs index da254442bc..c4617deaa0 100644 --- a/wgpu-hal/src/metal/adapter.rs +++ b/wgpu-hal/src/metal/adapter.rs @@ -871,6 +871,7 @@ impl super::PrivateCapabilities { features.set(F::ADDRESS_MODE_CLAMP_TO_ZERO, true); features.set(F::RG11B10UFLOAT_RENDERABLE, self.format_rg11b10_all); + features.set(F::SHADER_UNUSED_VERTEX_OUTPUT, true); features } diff --git a/wgpu-hal/src/vulkan/adapter.rs b/wgpu-hal/src/vulkan/adapter.rs index 78aceeeeef..b515628726 100644 --- a/wgpu-hal/src/vulkan/adapter.rs +++ b/wgpu-hal/src/vulkan/adapter.rs @@ -522,6 +522,7 @@ impl PhysicalDeviceFeatures { | vk::FormatFeatureFlags::COLOR_ATTACHMENT_BLEND, ); features.set(F::RG11B10UFLOAT_RENDERABLE, rg11b10ufloat_renderable); + features.set(F::SHADER_UNUSED_VERTEX_OUTPUT, true); (features, dl_flags) } diff --git a/wgpu-types/src/lib.rs b/wgpu-types/src/lib.rs index 59c32ec3b8..13603fc03f 100644 --- a/wgpu-types/src/lib.rs +++ b/wgpu-types/src/lib.rs @@ -737,6 +737,15 @@ bitflags::bitflags! { /// This is a native only feature. const VERTEX_ATTRIBUTE_64BIT = 1 << 53; + /// Allows vertex shaders to have outputs which are not consumed + /// by the fragment shader. + /// + /// Supported platforms: + /// - Vulkan + /// - Metal + /// - OpenGL + const SHADER_UNUSED_VERTEX_OUTPUT = 1 << 54; + // 54..59 available // Shader: From 3ff04c31e6b9b5f8b0589e6dfa55aecb91d1ce8b Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 21 Sep 2023 02:36:45 +0000 Subject: [PATCH 32/33] Bump smallvec from 1.11.0 to 1.11.1 (#4161) Bumps [smallvec](https://github.com/servo/rust-smallvec) from 1.11.0 to 1.11.1. - [Release notes](https://github.com/servo/rust-smallvec/releases) - [Commits](https://github.com/servo/rust-smallvec/compare/v1.11.0...v1.11.1) --- updated-dependencies: - dependency-name: smallvec dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9978941db4..942cf10970 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2531,9 +2531,9 @@ dependencies = [ [[package]] name = "smallvec" -version = "1.11.0" +version = "1.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "62bb4feee49fdd9f707ef802e22365a35de4b7b299de4763d44bfea899442ff9" +checksum = "942b4a808e05215192e39f4ab80813e599068285906cc91aa64f923db842bd5a" [[package]] name = "smithay-client-toolkit" From 855fefc10e14caa7c262c9a21a1db25a821f3c96 Mon Sep 17 00:00:00 2001 From: Connor Fitzgerald Date: Thu, 21 Sep 2023 02:01:35 -0400 Subject: [PATCH 33/33] Update CODEOWNERS with new wgpu team (#4162) --- .github/CODEOWNERS | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 7fefad320c..1d6e147803 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,2 +1,4 @@ -/cts_runner/ @crowlKats -/deno_webgpu/ @crowlKats +* @gfx-rs/wgpu + +/cts_runner/ @gfx-rs/deno @gfx-rs/wgpu +/deno_webgpu/ @gfx-rs/deno @gfx-rs/wgpu