From 8ded21862e0aea88ccbd8eda1b3623864ff254f0 Mon Sep 17 00:00:00 2001 From: Connor Fitzgerald Date: Mon, 5 Jul 2021 22:28:36 -0400 Subject: [PATCH] Fix downlevel vertex stage storage buffer check --- wgpu-core/src/device/mod.rs | 55 +++++++++++++++++++++--------------- wgpu-hal/src/gles/adapter.rs | 49 +++++++++++++++++++------------- wgpu-types/src/lib.rs | 2 -- wgpu/examples/shadow/main.rs | 2 +- 4 files changed, 64 insertions(+), 44 deletions(-) diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index 47be0230e8..ee3b2d5fc7 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -976,22 +976,33 @@ impl Device { label: Option<&str>, entry_map: binding_model::BindEntryMap, ) -> Result, binding_model::CreateBindGroupLayoutError> { + #[derive(PartialEq)] + enum WritableStorage { + Yes, + No, + } + for entry in entry_map.values() { use wgt::BindingType as Bt; let mut required_features = wgt::Features::empty(); - let mut required_downlevel_flags = wgt::DownlevelFlags::empty(); - let (array_feature, is_writable_storage) = match entry.ty { + let (array_feature, writable_storage) = match entry.ty { Bt::Buffer { ty: wgt::BufferBindingType::Uniform, has_dynamic_offset: false, min_binding_size: _, - } => (Some(wgt::Features::BUFFER_BINDING_ARRAY), false), + } => ( + Some(wgt::Features::BUFFER_BINDING_ARRAY), + WritableStorage::No, + ), Bt::Buffer { ty: wgt::BufferBindingType::Uniform, has_dynamic_offset: true, min_binding_size: _, - } => (Some(wgt::Features::BUFFER_BINDING_ARRAY), false), + } => ( + Some(wgt::Features::BUFFER_BINDING_ARRAY), + WritableStorage::No, + ), Bt::Buffer { ty: wgt::BufferBindingType::Storage { read_only }, .. @@ -1000,22 +1011,28 @@ impl Device { wgt::Features::BUFFER_BINDING_ARRAY | wgt::Features::STORAGE_RESOURCE_BINDING_ARRAY, ), - !read_only, + match read_only { + true => WritableStorage::No, + false => WritableStorage::Yes, + }, + ), + Bt::Sampler { .. } => (None, WritableStorage::No), + Bt::Texture { .. } => ( + Some(wgt::Features::TEXTURE_BINDING_ARRAY), + WritableStorage::No, ), - Bt::Sampler { .. } => (None, false), - Bt::Texture { .. } => (Some(wgt::Features::TEXTURE_BINDING_ARRAY), false), Bt::StorageTexture { access, .. } => ( Some( wgt::Features::TEXTURE_BINDING_ARRAY | wgt::Features::STORAGE_RESOURCE_BINDING_ARRAY, ), match access { - wgt::StorageTextureAccess::ReadOnly => false, - wgt::StorageTextureAccess::WriteOnly => true, + wgt::StorageTextureAccess::ReadOnly => WritableStorage::No, + wgt::StorageTextureAccess::WriteOnly => WritableStorage::Yes, wgt::StorageTextureAccess::ReadWrite => { required_features |= wgt::Features::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES; - true + WritableStorage::Yes } }, ), @@ -1030,13 +1047,14 @@ impl Device { error, })?; } - if entry.visibility.contains(wgt::ShaderStages::VERTEX) { - required_downlevel_flags |= wgt::DownlevelFlags::VERTEX_ACCESSABLE_STORAGE_BUFFERS; - } - if is_writable_storage && entry.visibility.contains(wgt::ShaderStages::VERTEX) { + if writable_storage == WritableStorage::Yes + && entry.visibility.contains(wgt::ShaderStages::VERTEX) + { required_features |= wgt::Features::VERTEX_WRITABLE_STORAGE; } - if is_writable_storage && entry.visibility.contains(wgt::ShaderStages::FRAGMENT) { + if writable_storage == WritableStorage::Yes + && entry.visibility.contains(wgt::ShaderStages::FRAGMENT) + { self.require_downlevel_flags(wgt::DownlevelFlags::FRAGMENT_WRITABLE_STORAGE) .map_err(binding_model::BindGroupLayoutEntryError::MissingDownlevelFlags) .map_err(|error| binding_model::CreateBindGroupLayoutError::Entry { @@ -1051,13 +1069,6 @@ impl Device { binding: entry.binding, error, })?; - - self.require_downlevel_flags(required_downlevel_flags) - .map_err(binding_model::BindGroupLayoutEntryError::MissingDownlevelFlags) - .map_err(|error| binding_model::CreateBindGroupLayoutError::Entry { - binding: entry.binding, - error, - })?; } let mut hal_bindings = entry_map.values().cloned().collect::>(); diff --git a/wgpu-hal/src/gles/adapter.rs b/wgpu-hal/src/gles/adapter.rs index 60c9139ffd..00f5d7ea0d 100644 --- a/wgpu-hal/src/gles/adapter.rs +++ b/wgpu-hal/src/gles/adapter.rs @@ -180,17 +180,39 @@ impl super::Adapter { naga::back::glsl::Version::Embedded(value) }; + let vertex_shader_storage_blocks = + gl.get_parameter_i32(glow::MAX_VERTEX_SHADER_STORAGE_BLOCKS) as u32; + let fragment_shader_storage_blocks = + gl.get_parameter_i32(glow::MAX_FRAGMENT_SHADER_STORAGE_BLOCKS) as u32; + + let vertex_shader_storage_textures = + gl.get_parameter_i32(glow::MAX_VERTEX_IMAGE_UNIFORMS) as u32; + let fragment_shader_storage_textures = + gl.get_parameter_i32(glow::MAX_FRAGMENT_IMAGE_UNIFORMS) as u32; + + // WORKAROUND: + // In order to work around an issue with GL on RPI4 and similar, we ignore a zero vertex ssbo count if there are vertex sstos. (more info: https://github.com/gfx-rs/wgpu/pull/1607#issuecomment-874938961) + // The hardware does not want us to write to these SSBOs, but GLES cannot express that. We detect this case and disable writing to SSBOs. + let vertex_ssbo_false_zero = + vertex_shader_storage_blocks == 0 && vertex_shader_storage_textures != 0; + + let max_storage_buffers_per_shader_stage = if vertex_ssbo_false_zero { + // We only care about fragment here as the 0 is a lie. + log::warn!("Max vertex shader SSBO == 0 and SSTO != 0. Interpreting as false zero."); + fragment_shader_storage_blocks + } else { + vertex_shader_storage_blocks.min(fragment_shader_storage_blocks) + }; + let mut features = wgt::Features::empty() | wgt::Features::TEXTURE_COMPRESSION_ETC2; features.set( wgt::Features::DEPTH_CLAMPING, extensions.contains("GL_EXT_depth_clamp"), ); - features.set(wgt::Features::VERTEX_WRITABLE_STORAGE, ver >= (3, 1)); - - let vertex_shader_storage_blocks = - gl.get_parameter_i32(glow::MAX_VERTEX_SHADER_STORAGE_BLOCKS); - let fragment_shader_storage_blocks = - gl.get_parameter_i32(glow::MAX_FRAGMENT_SHADER_STORAGE_BLOCKS); + features.set( + wgt::Features::VERTEX_WRITABLE_STORAGE, + ver >= (3, 1) && !vertex_ssbo_false_zero, + ); let mut downlevel_flags = wgt::DownlevelFlags::empty() | wgt::DownlevelFlags::DEVICE_LOCAL_IMAGE_COPIES @@ -210,10 +232,6 @@ impl super::Adapter { wgt::DownlevelFlags::INDEPENDENT_BLENDING, ver >= (3, 2) || extensions.contains("GL_EXT_draw_buffers_indexed"), ); - downlevel_flags.set( - wgt::DownlevelFlags::VERTEX_ACCESSABLE_STORAGE_BUFFERS, - vertex_shader_storage_blocks > 0, - ); let max_texture_size = gl.get_parameter_i32(glow::MAX_TEXTURE_SIZE) as u32; let max_texture_3d_size = gl.get_parameter_i32(glow::MAX_3D_TEXTURE_SIZE) as u32; @@ -228,14 +246,6 @@ impl super::Adapter { let max_uniform_buffers_per_shader_stage = gl.get_parameter_i32(glow::MAX_VERTEX_UNIFORM_BLOCKS) .min(gl.get_parameter_i32(glow::MAX_FRAGMENT_UNIFORM_BLOCKS)) as u32; - let max_storage_buffers_per_shader_stage = if vertex_shader_storage_blocks > 0 { - vertex_shader_storage_blocks.min(fragment_shader_storage_blocks) - } else { - fragment_shader_storage_blocks - } as u32; - - let max_storage_textures_per_shader_stage = - gl.get_parameter_i32(glow::MAX_FRAGMENT_IMAGE_UNIFORMS) as u32; let limits = wgt::Limits { max_texture_dimension_1d: max_texture_size, @@ -248,7 +258,8 @@ impl super::Adapter { max_sampled_textures_per_shader_stage: super::MAX_TEXTURE_SLOTS as u32, max_samplers_per_shader_stage: super::MAX_SAMPLERS as u32, max_storage_buffers_per_shader_stage, - max_storage_textures_per_shader_stage, + max_storage_textures_per_shader_stage: vertex_shader_storage_textures + .min(fragment_shader_storage_textures), max_uniform_buffers_per_shader_stage, max_uniform_buffer_binding_size: gl.get_parameter_i32(glow::MAX_UNIFORM_BLOCK_SIZE) as u32, diff --git a/wgpu-types/src/lib.rs b/wgpu-types/src/lib.rs index 6b9cfc9736..f267b6496a 100644 --- a/wgpu-types/src/lib.rs +++ b/wgpu-types/src/lib.rs @@ -702,8 +702,6 @@ bitflags::bitflags! { const COMPARISON_SAMPLERS = 0x0000_0100; /// Supports different blending modes per color target. const INDEPENDENT_BLENDING = 0x0000_0200; - /// Supports attaching storage buffers to vertex shaders. - const VERTEX_ACCESSABLE_STORAGE_BUFFERS = 0x0000_0400; /// Supports samplers with anisotropic filtering. Note this isn't actually required by WebGPU, diff --git a/wgpu/examples/shadow/main.rs b/wgpu/examples/shadow/main.rs index b78a6dee79..49beff0bb4 100644 --- a/wgpu/examples/shadow/main.rs +++ b/wgpu/examples/shadow/main.rs @@ -542,7 +542,7 @@ impl framework::Example for Example { }, wgpu::BindGroupLayoutEntry { binding: 1, // lights - visibility: wgpu::ShaderStages::VERTEX | wgpu::ShaderStages::FRAGMENT, + visibility: wgpu::ShaderStages::FRAGMENT, ty: wgpu::BindingType::Buffer { ty: wgpu::BufferBindingType::Storage { read_only: true }, has_dynamic_offset: false,