From 0a66eef8221a71e38584deee410ed121b7317b13 Mon Sep 17 00:00:00 2001 From: Zicklag Date: Thu, 22 Jul 2021 18:51:45 -0500 Subject: [PATCH] Work Around Fastclear Bug for Web and Native GL --- wgpu-hal/src/gles/adapter.rs | 76 +++++++++++++++++++++++++++-- wgpu-hal/src/gles/command.rs | 9 ++-- wgpu-hal/src/gles/device.rs | 7 ++- wgpu-hal/src/gles/egl.rs | 5 -- wgpu-hal/src/gles/mod.rs | 44 +++++++++++++---- wgpu-hal/src/gles/queue.rs | 48 +++++++++++++++++- wgpu-hal/src/gles/shader_clear.frag | 7 +++ wgpu-hal/src/gles/shader_clear.vert | 11 +++++ 8 files changed, 181 insertions(+), 26 deletions(-) create mode 100644 wgpu-hal/src/gles/shader_clear.frag create mode 100644 wgpu-hal/src/gles/shader_clear.vert diff --git a/wgpu-hal/src/gles/adapter.rs b/wgpu-hal/src/gles/adapter.rs index 99066dbff4..94f8884c24 100644 --- a/wgpu-hal/src/gles/adapter.rs +++ b/wgpu-hal/src/gles/adapter.rs @@ -3,6 +3,9 @@ use std::sync::Arc; // https://webgl2fundamentals.org/webgl/lessons/webgl-data-textures.html +const GL_UNMASKED_VENDOR_WEBGL: u32 = 0x9245; +const GL_UNMASKED_RENDERER_WEBGL: u32 = 0x9246; + impl super::Adapter { /// According to the OpenGL specification, the version information is /// expected to follow the following syntax: @@ -161,17 +164,29 @@ impl super::Adapter { } pub(super) unsafe fn expose(gl: glow::Context) -> Option> { - let vendor = gl.get_parameter_string(glow::VENDOR); - let renderer = gl.get_parameter_string(glow::RENDERER); + let extensions = gl.supported_extensions(); + + let (vendor_const, renderer_const) = if extensions.contains("WEBGL_debug_renderer_info") { + (GL_UNMASKED_VENDOR_WEBGL, GL_UNMASKED_RENDERER_WEBGL) + } else { + (glow::VENDOR, glow::RENDERER) + }; + let (vendor, renderer) = { + let vendor = gl.get_parameter_string(vendor_const); + let renderer = gl.get_parameter_string(renderer_const); + + (vendor, renderer) + }; let version = gl.get_parameter_string(glow::VERSION); + log::info!("Vendor: {}", vendor); log::info!("Renderer: {}", renderer); log::info!("Version: {}", version); - let ver = Self::parse_version(&version).ok()?; - let extensions = gl.supported_extensions(); log::debug!("Extensions: {:#?}", extensions); + let ver = Self::parse_version(&version).ok()?; + let shading_language_version = { let sl_version = gl.get_parameter_string(glow::SHADING_LANGUAGE_VERSION); log::info!("SL version: {}", sl_version); @@ -293,6 +308,22 @@ impl super::Adapter { ver >= (3, 1), ); + let mut workarounds = super::Workarounds::empty(); + let r = renderer.to_lowercase(); + // Check for Mesa sRGB clear bug. See + // [`super::PrivateCapabilities::MESA_I915_SRGB_SHADER_CLEAR`]. + if r.contains("mesa") + && r.contains("intel") + && r.split(&[' ', '(', ')'][..]) + .any(|substr| substr.len() == 3 && substr.chars().nth(2) == Some('l')) + { + log::warn!( + "Detected skylake derivative running on mesa i915. Clears to srgb textures will \ + use manual shader clears." + ); + workarounds.set(super::Workarounds::MESA_I915_SRGB_SHADER_CLEAR, true); + } + let downlevel_defaults = wgt::DownlevelLimits {}; Some(crate::ExposedAdapter { @@ -300,6 +331,7 @@ impl super::Adapter { shared: Arc::new(super::AdapterShared { context: gl, private_caps, + workarounds, shading_language_version, }), }, @@ -327,6 +359,34 @@ impl super::Adapter { }, }) } + + unsafe fn create_shader_clear_program( + gl: &glow::Context, + ) -> (glow::Program, glow::UniformLocation) { + let program = gl + .create_program() + .expect("Could not create shader program"); + let vertex = gl + .create_shader(glow::VERTEX_SHADER) + .expect("Could not create shader"); + gl.shader_source(vertex, include_str!("./shader_clear.vert")); + gl.compile_shader(vertex); + let fragment = gl + .create_shader(glow::FRAGMENT_SHADER) + .expect("Could not create shader"); + gl.shader_source(fragment, include_str!("./shader_clear.frag")); + gl.compile_shader(fragment); + gl.attach_shader(program, vertex); + gl.attach_shader(program, fragment); + gl.link_program(program); + let color_uniform_location = gl + .get_uniform_location(program, "color") + .expect("Could not find color uniform in shader clear shader"); + gl.delete_shader(vertex); + gl.delete_shader(fragment); + + (program, color_uniform_location) + } } impl crate::Adapter for super::Adapter { @@ -349,6 +409,11 @@ impl crate::Adapter for super::Adapter { let zeroes = vec![0u8; super::ZERO_BUFFER_SIZE]; gl.buffer_data_u8_slice(glow::COPY_READ_BUFFER, &zeroes, glow::STATIC_DRAW); + // Compile the shader program we use for doing manual clears to work around Mesa fastclear + // bug. + let (shader_clear_program, shader_clear_program_color_uniform_location) = + Self::create_shader_clear_program(gl); + Ok(crate::OpenDevice { device: super::Device { shared: Arc::clone(&self.shared), @@ -365,8 +430,11 @@ impl crate::Adapter for super::Adapter { copy_fbo: gl .create_framebuffer() .map_err(|_| crate::DeviceError::OutOfMemory)?, + shader_clear_program, + shader_clear_program_color_uniform_location, zero_buffer, temp_query_results: Vec::new(), + draw_buffer_count: 1, }, }) } diff --git a/wgpu-hal/src/gles/command.rs b/wgpu-hal/src/gles/command.rs index 42a6b15dac..b5020554b4 100644 --- a/wgpu-hal/src/gles/command.rs +++ b/wgpu-hal/src/gles/command.rs @@ -444,10 +444,11 @@ impl crate::CommandEncoder for super::CommandEncoder { self.cmd_buffer .commands .push(match cat.target.view.sample_type { - wgt::TextureSampleType::Float { .. } => C::ClearColorF( - i as u32, - [c.r as f32, c.g as f32, c.b as f32, c.a as f32], - ), + wgt::TextureSampleType::Float { .. } => C::ClearColorF { + draw_buffer: i as u32, + color: [c.r as f32, c.g as f32, c.b as f32, c.a as f32], + is_srgb: cat.target.view.format.describe().srgb, + }, wgt::TextureSampleType::Depth => unimplemented!(), wgt::TextureSampleType::Uint => C::ClearColorU( i as u32, diff --git a/wgpu-hal/src/gles/device.rs b/wgpu-hal/src/gles/device.rs index 9b6b378909..1450c11842 100644 --- a/wgpu-hal/src/gles/device.rs +++ b/wgpu-hal/src/gles/device.rs @@ -248,8 +248,10 @@ impl super::Device { return Err(crate::DeviceError::Lost.into()); } super::BindingRegister::Textures | super::BindingRegister::Images => { - let loc = gl.get_uniform_location(program, name).unwrap(); - gl.uniform_1_i32(Some(&loc), slot as _); + gl.uniform_1_i32( + gl.get_uniform_location(program, name).as_ref(), + slot as _, + ); } } } @@ -589,6 +591,7 @@ impl crate::Device for super::Device { & crate::FormatAspects::from(desc.range.aspect), mip_levels: desc.range.base_mip_level..end_mip_level, array_layers: desc.range.base_array_layer..end_array_layer, + format: texture.format, }) } unsafe fn destroy_texture_view(&self, _view: super::TextureView) {} diff --git a/wgpu-hal/src/gles/egl.rs b/wgpu-hal/src/gles/egl.rs index 1108816c2c..f8686a19e3 100644 --- a/wgpu-hal/src/gles/egl.rs +++ b/wgpu-hal/src/gles/egl.rs @@ -422,11 +422,6 @@ impl crate::Instance for Instance { None }; - // Workaround Mesa driver bug on Intel cards by disabling fastclear: - // https://gitlab.freedesktop.org/mesa/mesa/-/issues/2565 - // https://github.com/gfx-rs/wgpu/issues/1627#issuecomment-877854185 - std::env::set_var("INTEL_DEBUG", "nofc"); - let display = if let (Some(library), Some(egl)) = (wayland_library, egl.upcast::()) { diff --git a/wgpu-hal/src/gles/mod.rs b/wgpu-hal/src/gles/mod.rs index 3c881fb61d..8374505ba4 100644 --- a/wgpu-hal/src/gles/mod.rs +++ b/wgpu-hal/src/gles/mod.rs @@ -68,6 +68,8 @@ mod queue; #[cfg(not(target_arch = "wasm32"))] use self::egl::{Instance, Surface}; +use arrayvec::ArrayVec; + use glow::HasContext; use std::{ops::Range, sync::Arc}; @@ -113,13 +115,26 @@ bitflags::bitflags! { /// change the exposed feature set. struct PrivateCapabilities: u32 { /// Support explicit layouts in shader. - const SHADER_BINDING_LAYOUT = 0x0001; + const SHADER_BINDING_LAYOUT = 1 << 0; /// Support extended shadow sampling instructions. - const SHADER_TEXTURE_SHADOW_LOD = 0x0002; + const SHADER_TEXTURE_SHADOW_LOD = 1 << 1; /// Support memory barriers. - const MEMORY_BARRIERS = 0x0004; + const MEMORY_BARRIERS = 1 << 2; /// Vertex buffer layouts separate from the data. - const VERTEX_BUFFER_LAYOUT = 0x0008; + const VERTEX_BUFFER_LAYOUT = 1 << 3; + } +} + +bitflags::bitflags! { + /// Flags that indicate necessary workarounds for specific devices or driver bugs + struct Workarounds: u32 { + // Needs workaround for Intel Mesa bug: + // https://gitlab.freedesktop.org/mesa/mesa/-/issues/2565. + // + // This comment + // (https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4972/diffs?diff_id=75888#22f5d1004713c9bbf857988c7efb81631ab88f99_323_327) + // seems to indicate all skylake models are effected. + const MESA_I915_SRGB_SHADER_CLEAR = 1 << 0; } } @@ -148,6 +163,7 @@ struct TextureFormatDesc { struct AdapterShared { context: glow::Context, private_caps: PrivateCapabilities, + workarounds: Workarounds, shading_language_version: naga::back::glsl::Version, } @@ -167,11 +183,16 @@ pub struct Queue { features: wgt::Features, draw_fbo: glow::Framebuffer, copy_fbo: glow::Framebuffer, - /// Keep a reasonably large buffer filled with zeroes, - /// so that we can implement `FillBuffer` of zeroes - /// by copying from it. + /// Shader program used to clear the screen for [`PrivateCapabilities::REQUIRES_SHADER_CLEAR`] + /// devices. + shader_clear_program: glow::Program, + /// The uniform location of the color uniform in the shader clear program + shader_clear_program_color_uniform_location: glow::UniformLocation, + /// Keep a reasonably large buffer filled with zeroes, so that we can implement `FillBuffer` of + /// zeroes by copying from it. zero_buffer: glow::Buffer, temp_query_results: Vec, + draw_buffer_count: u8, } #[derive(Debug)] @@ -218,6 +239,7 @@ pub struct TextureView { aspects: crate::FormatAspects, mip_levels: Range, array_layers: Range, + format: wgt::TextureFormat, } #[derive(Debug)] @@ -471,7 +493,7 @@ struct PrimitiveState { clamp_depth: bool, } -type InvalidatedAttachments = arrayvec::ArrayVec; +type InvalidatedAttachments = ArrayVec; #[derive(Debug)] enum Command { @@ -562,7 +584,11 @@ enum Command { }, InvalidateAttachments(InvalidatedAttachments), SetDrawColorBuffers(u8), - ClearColorF(u32, [f32; 4]), + ClearColorF { + draw_buffer: u32, + color: [f32; 4], + is_srgb: bool, + }, ClearColorU(u32, [u32; 4]), ClearColorI(u32, [i32; 4]), ClearDepth(f32), diff --git a/wgpu-hal/src/gles/queue.rs b/wgpu-hal/src/gles/queue.rs index 33bb7c68b1..0f94052360 100644 --- a/wgpu-hal/src/gles/queue.rs +++ b/wgpu-hal/src/gles/queue.rs @@ -26,6 +26,36 @@ fn is_3d_target(target: super::BindTarget) -> bool { } impl super::Queue { + /// Performs a manual shader clear, used as a workaround for a clearing bug on mesa + unsafe fn perform_shader_clear(&self, draw_buffer: u32, color: [f32; 4]) { + let gl = &self.shared.context; + + gl.use_program(Some(self.shader_clear_program)); + gl.uniform_4_f32( + Some(&self.shader_clear_program_color_uniform_location), + color[0], + color[1], + color[2], + color[3], + ); + gl.disable(glow::DEPTH_TEST); + gl.disable(glow::STENCIL_TEST); + gl.disable(glow::SCISSOR_TEST); + gl.disable(glow::BLEND); + gl.disable(glow::CULL_FACE); + gl.draw_buffers(&[glow::COLOR_ATTACHMENT0 + draw_buffer]); + gl.draw_arrays(glow::TRIANGLES, 0, 3); + + // Reset the draw buffers to what they were before the clear + let indices = (0..self.draw_buffer_count as u32) + .map(|i| glow::COLOR_ATTACHMENT0 + i) + .collect::>(); + gl.draw_buffers(&indices); + for draw_buffer in 0..self.draw_buffer_count as u32 { + gl.disable_draw_buffer(glow::BLEND, draw_buffer); + } + } + unsafe fn reset_state(&self) { let gl = &self.shared.context; gl.use_program(None); @@ -551,6 +581,7 @@ impl super::Queue { gl.invalidate_framebuffer(glow::DRAW_FRAMEBUFFER, list); } C::SetDrawColorBuffers(count) => { + self.draw_buffer_count = count; let indices = (0..count as u32) .map(|i| glow::COLOR_ATTACHMENT0 + i) .collect::>(); @@ -559,8 +590,21 @@ impl super::Queue { gl.disable_draw_buffer(glow::BLEND, draw_buffer); } } - C::ClearColorF(draw_buffer, ref color) => { - gl.clear_buffer_f32_slice(glow::COLOR, draw_buffer, color); + C::ClearColorF { + draw_buffer, + ref color, + is_srgb, + } => { + if self + .shared + .workarounds + .contains(super::Workarounds::MESA_I915_SRGB_SHADER_CLEAR) + && is_srgb + { + self.perform_shader_clear(draw_buffer, *color); + } else { + gl.clear_buffer_f32_slice(glow::COLOR, draw_buffer, color); + } } C::ClearColorU(draw_buffer, ref color) => { gl.clear_buffer_u32_slice(glow::COLOR, draw_buffer, color); diff --git a/wgpu-hal/src/gles/shader_clear.frag b/wgpu-hal/src/gles/shader_clear.frag new file mode 100644 index 0000000000..25a9ee96d4 --- /dev/null +++ b/wgpu-hal/src/gles/shader_clear.frag @@ -0,0 +1,7 @@ +#version 300 es +precision lowp float; +uniform vec4 color; +out vec4 frag; +void main() { + frag = color; +} \ No newline at end of file diff --git a/wgpu-hal/src/gles/shader_clear.vert b/wgpu-hal/src/gles/shader_clear.vert new file mode 100644 index 0000000000..ac655e7f31 --- /dev/null +++ b/wgpu-hal/src/gles/shader_clear.vert @@ -0,0 +1,11 @@ +#version 300 es +precision lowp float; +// A triangle that fills the whole screen +const vec2[3] TRIANGLE_POS = vec2[]( + vec2( 0.0, -3.0), + vec2(-3.0, 1.0), + vec2( 3.0, 1.0) +); +void main() { + gl_Position = vec4(TRIANGLE_POS[gl_VertexID], 0.0, 1.0); +} \ No newline at end of file