Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't create shader-clear program on GLES if it's not needed #5348

Merged
merged 2 commits into from
Mar 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ By @cwfitzgerald in [#5325](https://github.com/gfx-rs/wgpu/pull/5325).
#### GLES

- Fixes for being able to use an OpenGL 4.1 core context provided by macOS with wgpu. By @bes in [#5331](https://github.com/gfx-rs/wgpu/pull/5331).
- Don't create a program for shader-clearing if that workaround isn't required. By @Dinnerbone in [#5348](https://github.com/gfx-rs/wgpu/pull/5348).

## v0.19.0 (2024-01-17)

Expand Down
24 changes: 18 additions & 6 deletions wgpu-hal/src/gles/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::sync::{atomic::AtomicU8, Arc};
use wgt::AstcChannel;

use crate::auxil::db;
use crate::gles::ShaderClearProgram;

// https://webgl2fundamentals.org/webgl/lessons/webgl-data-textures.html

Expand Down Expand Up @@ -875,7 +876,7 @@ impl super::Adapter {
unsafe fn create_shader_clear_program(
gl: &glow::Context,
es: bool,
) -> Option<(glow::Program, glow::UniformLocation)> {
) -> Option<ShaderClearProgram> {
let program = unsafe { gl.create_program() }.expect("Could not create shader program");
let vertex = unsafe {
Self::compile_shader(
Expand Down Expand Up @@ -911,7 +912,10 @@ impl super::Adapter {
unsafe { gl.delete_shader(vertex) };
unsafe { gl.delete_shader(fragment) };

Some((program, color_uniform_location))
Some(ShaderClearProgram {
program,
color_uniform_location,
})
}
}

Expand All @@ -937,9 +941,18 @@ impl crate::Adapter<super::Api> for super::Adapter {
// 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) = unsafe {
Self::create_shader_clear_program(gl, self.shared.es)
.ok_or(crate::DeviceError::ResourceCreationFailed)?
let shader_clear_program = if self
.shared
.workarounds
.contains(super::Workarounds::MESA_I915_SRGB_SHADER_CLEAR)
{
Some(unsafe {
Self::create_shader_clear_program(gl, self.shared.es)
.ok_or(crate::DeviceError::ResourceCreationFailed)?
})
} else {
// If we don't need the workaround, don't waste time and resources compiling the clear program
None
};

Ok(crate::OpenDevice {
Expand All @@ -957,7 +970,6 @@ impl crate::Adapter<super::Api> for super::Adapter {
copy_fbo: unsafe { gl.create_framebuffer() }
.map_err(|_| crate::DeviceError::OutOfMemory)?,
shader_clear_program,
shader_clear_program_color_uniform_location,
zero_buffer,
temp_query_results: Mutex::new(Vec::new()),
draw_buffer_count: AtomicU8::new(1),
Expand Down
9 changes: 6 additions & 3 deletions wgpu-hal/src/gles/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,16 +264,19 @@ pub struct Device {
render_doc: crate::auxil::renderdoc::RenderDoc,
}

pub struct ShaderClearProgram {
pub program: glow::Program,
pub color_uniform_location: glow::UniformLocation,
}

pub struct Queue {
shared: Arc<AdapterShared>,
features: wgt::Features,
draw_fbo: glow::Framebuffer,
copy_fbo: glow::Framebuffer,
/// Shader program used to clear the screen for [`Workarounds::MESA_I915_SRGB_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,
shader_clear_program: Option<ShaderClearProgram>,
/// Keep a reasonably large buffer filled with zeroes, so that we can implement `ClearBuffer` of
/// zeroes by copying from it.
zero_buffer: glow::Buffer,
Expand Down
8 changes: 6 additions & 2 deletions wgpu-hal/src/gles/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,14 @@ fn get_z_offset(target: u32, base: &crate::TextureCopyBase) -> u32 {
impl super::Queue {
/// Performs a manual shader clear, used as a workaround for a clearing bug on mesa
unsafe fn perform_shader_clear(&self, gl: &glow::Context, draw_buffer: u32, color: [f32; 4]) {
unsafe { gl.use_program(Some(self.shader_clear_program)) };
let shader_clear = self
.shader_clear_program
.as_ref()
.expect("shader_clear_program should always be set if the workaround is enabled");
unsafe { gl.use_program(Some(shader_clear.program)) };
unsafe {
gl.uniform_4_f32(
Some(&self.shader_clear_program_color_uniform_location),
Some(&shader_clear.color_uniform_location),
color[0],
color[1],
color[2],
Expand Down
Loading