Skip to content

Commit

Permalink
Merge #1717
Browse files Browse the repository at this point in the history
1717: Work Around Fastclear Bug for Web and Native GL r=cwfitzgerald,kvark a=zicklag

**Connections**
Resolves #1627 

**Description**
Works around Mesa fastclear bug by doing a manual shader clear on effected platforms

**Testing**
Tested on Mesa Intel(R) UHD Graphics (CML GT2) (Gl)


Co-authored-by: Zicklag <zicklag@katharostech.com>
  • Loading branch information
bors[bot] and zicklag authored Jul 26, 2021
2 parents dd1be1f + 0a66eef commit ee9e145
Show file tree
Hide file tree
Showing 8 changed files with 181 additions and 26 deletions.
76 changes: 72 additions & 4 deletions wgpu-hal/src/gles/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -161,17 +164,29 @@ impl super::Adapter {
}

pub(super) unsafe fn expose(gl: glow::Context) -> Option<crate::ExposedAdapter<super::Api>> {
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);
Expand Down Expand Up @@ -293,13 +308,30 @@ 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 {
adapter: super::Adapter {
shared: Arc::new(super::AdapterShared {
context: gl,
private_caps,
workarounds,
shading_language_version,
}),
},
Expand Down Expand Up @@ -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<super::Api> for super::Adapter {
Expand All @@ -349,6 +409,11 @@ impl crate::Adapter<super::Api> 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),
Expand All @@ -365,8 +430,11 @@ impl crate::Adapter<super::Api> 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,
},
})
}
Expand Down
9 changes: 5 additions & 4 deletions wgpu-hal/src/gles/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -444,10 +444,11 @@ impl crate::CommandEncoder<super::Api> 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,
Expand Down
7 changes: 5 additions & 2 deletions wgpu-hal/src/gles/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 _,
);
}
}
}
Expand Down Expand Up @@ -589,6 +591,7 @@ impl crate::Device<super::Api> 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) {}
Expand Down
5 changes: 0 additions & 5 deletions wgpu-hal/src/gles/egl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -422,11 +422,6 @@ impl crate::Instance<super::Api> 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::<egl::EGL1_5>())
{
Expand Down
44 changes: 35 additions & 9 deletions wgpu-hal/src/gles/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -148,6 +163,7 @@ struct TextureFormatDesc {
struct AdapterShared {
context: glow::Context,
private_caps: PrivateCapabilities,
workarounds: Workarounds,
shading_language_version: naga::back::glsl::Version,
}

Expand All @@ -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<u64>,
draw_buffer_count: u8,
}

#[derive(Debug)]
Expand Down Expand Up @@ -218,6 +239,7 @@ pub struct TextureView {
aspects: crate::FormatAspects,
mip_levels: Range<u32>,
array_layers: Range<u32>,
format: wgt::TextureFormat,
}

#[derive(Debug)]
Expand Down Expand Up @@ -471,7 +493,7 @@ struct PrimitiveState {
clamp_depth: bool,
}

type InvalidatedAttachments = arrayvec::ArrayVec<u32, { crate::MAX_COLOR_TARGETS + 2 }>;
type InvalidatedAttachments = ArrayVec<u32, { crate::MAX_COLOR_TARGETS + 2 }>;

#[derive(Debug)]
enum Command {
Expand Down Expand Up @@ -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),
Expand Down
48 changes: 46 additions & 2 deletions wgpu-hal/src/gles/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<ArrayVec<_, { crate::MAX_COLOR_TARGETS }>>();
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);
Expand Down Expand Up @@ -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::<ArrayVec<_, { crate::MAX_COLOR_TARGETS }>>();
Expand All @@ -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);
Expand Down
7 changes: 7 additions & 0 deletions wgpu-hal/src/gles/shader_clear.frag
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#version 300 es
precision lowp float;
uniform vec4 color;
out vec4 frag;
void main() {
frag = color;
}
11 changes: 11 additions & 0 deletions wgpu-hal/src/gles/shader_clear.vert
Original file line number Diff line number Diff line change
@@ -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);
}

0 comments on commit ee9e145

Please sign in to comment.