From 990a8c8b44ad8d5145a13fbe74fe1f17ada74976 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 21 Sep 2022 21:30:02 +0200 Subject: [PATCH] Fix sRGB blending and cleanup the relevant code (#2070) * Fix sRGBA blending on Mac * Clean up handling of sRGB support * Assume sRGB support if any extension has sRGB in it * improve logging very slightly --- crates/egui_glow/src/painter.rs | 89 +++++++++++++------------- crates/egui_glow/src/shader_version.rs | 13 ++++ 2 files changed, 56 insertions(+), 46 deletions(-) diff --git a/crates/egui_glow/src/painter.rs b/crates/egui_glow/src/painter.rs index dd5e6013a31..4f6ce975539 100644 --- a/crates/egui_glow/src/painter.rs +++ b/crates/egui_glow/src/painter.rs @@ -1,3 +1,4 @@ +#![allow(clippy::collapsible_else_if)] #![allow(unsafe_code)] use std::{collections::HashMap, sync::Arc}; @@ -53,7 +54,7 @@ pub struct Painter { is_webgl_1: bool, is_embedded: bool, vao: crate::vao::VertexArrayObject, - srgb_support: bool, + srgb_textures: bool, post_process: Option, vbo: glow::Buffer, element_array_buffer: glow::Buffer, @@ -112,45 +113,41 @@ impl Painter { crate::check_for_gl_error_even_in_release!(&gl, "before Painter::new"); let max_texture_side = unsafe { gl.get_parameter_i32(glow::MAX_TEXTURE_SIZE) } as usize; - let shader = shader_version.unwrap_or_else(|| ShaderVersion::get(&gl)); - let is_webgl_1 = shader == ShaderVersion::Es100; - let header = shader.version_declaration(); + let shader_version = shader_version.unwrap_or_else(|| ShaderVersion::get(&gl)); + let is_webgl_1 = shader_version == ShaderVersion::Es100; + let header = shader_version.version_declaration(); tracing::debug!("Shader header: {:?}.", header); - // Previously checking srgb_support on WebGL only, now we have to check on other GL | ES as well. - let srgb_support = gl.supported_extensions().contains("EXT_sRGB") - || gl.supported_extensions().contains("GL_EXT_sRGB") - || gl - .supported_extensions() - .contains("GL_ARB_framebuffer_sRGB"); - tracing::debug!("SRGB Support: {:?}.", srgb_support); - - let (post_process, srgb_support_define) = match (shader, srgb_support) { - // WebGL2 support sRGB default - (ShaderVersion::Es300, _) | (ShaderVersion::Es100, true) => unsafe { - // Add sRGB support marker for fragment shader - if let Some(size) = pp_fb_extent { - tracing::debug!("WebGL with sRGB enabled. Turning on post processing for linear framebuffer blending."); - // install post process to correct sRGB color: - ( - Some(PostProcess::new( - gl.clone(), - shader_prefix, - is_webgl_1, - size, - )?), - "#define SRGB_SUPPORTED", - ) - } else { - tracing::debug!("WebGL or OpenGL ES detected but PostProcess disabled because dimension is None"); - (None, "") - } - }, - - // WebGL1 without sRGB support disable postprocess and use fallback shader - (ShaderVersion::Es100, false) => (None, ""), - // OpenGL 2.1 or above always support sRGB so add sRGB support marker - _ => (None, "#define SRGB_SUPPORTED"), + let supported_extensions = gl.supported_extensions(); + tracing::trace!("OpenGL extensions: {supported_extensions:?}"); + let srgb_textures = shader_version == ShaderVersion::Es300 // WebGL2 always support sRGB + || supported_extensions.iter().any(|extension| { + // EXT_sRGB, GL_ARB_framebuffer_sRGB, GL_EXT_sRGB, GL_EXT_texture_sRGB_decode, … + extension.contains("sRGB") + }); + tracing::debug!("SRGB Support: {:?}", srgb_textures); + + let (post_process, srgb_support_define) = if shader_version.is_embedded() { + // WebGL doesn't support linear framebuffer blending… but maybe we can emulate it with `PostProcess`? + + if let Some(size) = pp_fb_extent { + tracing::debug!("WebGL with sRGB support. Turning on post processing for linear framebuffer blending."); + // install post process to correct sRGB color: + ( + Some(unsafe { PostProcess::new(gl.clone(), shader_prefix, is_webgl_1, size)? }), + "#define SRGB_SUPPORTED", + ) + } else { + tracing::warn!("WebGL or OpenGL ES detected but PostProcess disabled because dimension is None. Linear framebuffer blending will not be supported."); + (None, "") + } + } else { + if srgb_textures { + (None, "#define SRGB_SUPPORTED") + } else { + tracing::warn!("sRGB aware texture filtering not available"); + (None, "") + } }; unsafe { @@ -161,7 +158,7 @@ impl Painter { "{}\n{}\n{}\n{}", header, shader_prefix, - if shader.is_new_shader_interface() { + if shader_version.is_new_shader_interface() { "#define NEW_SHADER_INTERFACE\n" } else { "" @@ -177,7 +174,7 @@ impl Painter { header, shader_prefix, srgb_support_define, - if shader.is_new_shader_interface() { + if shader_version.is_new_shader_interface() { "#define NEW_SHADER_INTERFACE\n" } else { "" @@ -239,9 +236,9 @@ impl Painter { u_screen_size, u_sampler, is_webgl_1, - is_embedded: matches!(shader, ShaderVersion::Es100 | ShaderVersion::Es300), + is_embedded: shader_version.is_embedded(), vao, - srgb_support, + srgb_textures, post_process, vbo, element_array_buffer, @@ -591,16 +588,16 @@ impl Painter { check_for_gl_error!(&self.gl, "tex_parameter"); let (internal_format, src_format) = if self.is_webgl_1 { - let format = if self.srgb_support { + let format = if self.srgb_textures { glow::SRGB_ALPHA } else { glow::RGBA }; (format, format) - } else if !self.srgb_support { - (glow::RGBA8, glow::RGBA) - } else { + } else if self.srgb_textures { (glow::SRGB8_ALPHA8, glow::RGBA) + } else { + (glow::RGBA8, glow::RGBA) }; self.gl.pixel_store_i32(glow::UNPACK_ALIGNMENT, 1); diff --git a/crates/egui_glow/src/shader_version.rs b/crates/egui_glow/src/shader_version.rs index b17a511edd0..c066962a09c 100644 --- a/crates/egui_glow/src/shader_version.rs +++ b/crates/egui_glow/src/shader_version.rs @@ -7,8 +7,14 @@ use std::convert::TryInto; #[allow(dead_code)] pub enum ShaderVersion { Gl120, + + /// OpenGL 1.4 or later Gl140, + + /// e.g. WebGL1 Es100, + + /// e.g. WebGL2 Es300, } @@ -70,6 +76,13 @@ impl ShaderVersion { Self::Es300 | Self::Gl140 => true, } } + + pub fn is_embedded(&self) -> bool { + match self { + Self::Gl120 | Self::Gl140 => false, + Self::Es100 | Self::Es300 => true, + } + } } #[test]