Clear color values are now explicitely sent to the rendering backend as-is. (#2666)

* Clear color values are not explicitely sent to the rendering backend as-is.
Previously, converting from Color32 to Rgba caused an srgb->linear conversion. This conversion is incorrect if the backbuffer doesn't perform automatic conversion from linear->srgb (lack of this conversion is generally what egui assumes!).

* fill in pr numbers in changelog

* Epi comment fix

Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>

* Color32 comment fix

Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>

* move changelog line

* rename fix

* use backticks in doc

---------

Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
This commit is contained in:
Andreas Reich 2023-02-04 11:02:15 +01:00 committed by GitHub
parent a6b60d5d58
commit 8aa07e9d43
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 52 additions and 42 deletions

View file

@ -198,4 +198,19 @@ impl Color32 {
// we need a somewhat expensive conversion to linear space and back. // we need a somewhat expensive conversion to linear space and back.
Rgba::from(self).multiply(factor).into() Rgba::from(self).multiply(factor).into()
} }
/// Converts to floating point values in the range 0-1 without any gamma space conversion.
///
/// Use this with great care! In almost all cases, you want to convert to [`crate::Rgba`] instead
/// in order to obtain linear space color values.
#[inline]
pub fn to_normalized_gamma_f32(self) -> [f32; 4] {
let Self([r, g, b, a]) = self;
[
r as f32 / 255.0,
g as f32 / 255.0,
b as f32 / 255.0,
a as f32 / 255.0,
]
}
} }

View file

@ -5,6 +5,7 @@ NOTE: [`egui-winit`](../egui-winit/CHANGELOG.md), [`egui_glium`](../egui_glium/C
## Unreleased ## Unreleased
* ⚠️ BREAKING: `App::clear_color` now expects you to return a raw float array ([#2666](https://github.com/emilk/egui/pull/2666)):
#### Desktop/Native: #### Desktop/Native:
* `eframe::run_native` now returns a `Result` ([#2433](https://github.com/emilk/egui/pull/2433)). * `eframe::run_native` now returns a `Result` ([#2433](https://github.com/emilk/egui/pull/2433)).

View file

@ -149,13 +149,21 @@ pub trait App {
egui::Vec2::INFINITY egui::Vec2::INFINITY
} }
/// Background color for the app, e.g. what is sent to `gl.clearColor`. /// Background color values for the app, e.g. what is sent to `gl.clearColor`.
///
/// This is the background of your windows if you don't set a central panel. /// This is the background of your windows if you don't set a central panel.
fn clear_color(&self, _visuals: &egui::Visuals) -> egui::Rgba { ///
/// ATTENTION:
/// Since these float values go to the render as-is, any color space conversion as done
/// e.g. by converting from [`egui::Color32`] to [`egui::Rgba`] may cause incorrect results.
/// egui recommends that rendering backends use a normal "gamma-space" (non-sRGB-aware) blending,
/// which means the values you return here should also be in `sRGB` gamma-space in the 0-1 range.
/// You can use [`egui::Color32::to_normalized_gamma_f32`] for this.
fn clear_color(&self, _visuals: &egui::Visuals) -> [f32; 4] {
// NOTE: a bright gray makes the shadows of the windows look weird. // NOTE: a bright gray makes the shadows of the windows look weird.
// We use a bit of transparency so that if the user switches on the // We use a bit of transparency so that if the user switches on the
// `transparent()` option they get immediate results. // `transparent()` option they get immediate results.
egui::Color32::from_rgba_unmultiplied(12, 12, 12, 180).into() egui::Color32::from_rgba_unmultiplied(12, 12, 12, 180).to_normalized_gamma_f32()
// _visuals.window_fill() would also be a natural choice // _visuals.window_fill() would also be a natural choice
} }

View file

@ -1,4 +1,3 @@
use egui::Rgba;
use wasm_bindgen::JsValue; use wasm_bindgen::JsValue;
/// Renderer for a browser canvas. /// Renderer for a browser canvas.
@ -19,7 +18,7 @@ pub(crate) trait WebPainter {
/// Update all internal textures and paint gui. /// Update all internal textures and paint gui.
fn paint_and_update_textures( fn paint_and_update_textures(
&mut self, &mut self,
clear_color: Rgba, clear_color: [f32; 4],
clipped_primitives: &[egui::ClippedPrimitive], clipped_primitives: &[egui::ClippedPrimitive],
pixels_per_point: f32, pixels_per_point: f32,
textures_delta: &egui::TexturesDelta, textures_delta: &egui::TexturesDelta,

View file

@ -2,7 +2,6 @@ use wasm_bindgen::JsCast;
use wasm_bindgen::JsValue; use wasm_bindgen::JsValue;
use web_sys::HtmlCanvasElement; use web_sys::HtmlCanvasElement;
use egui::Rgba;
use egui_glow::glow; use egui_glow::glow;
use crate::{WebGlContextOption, WebOptions}; use crate::{WebGlContextOption, WebOptions};
@ -49,7 +48,7 @@ impl WebPainter for WebPainterGlow {
fn paint_and_update_textures( fn paint_and_update_textures(
&mut self, &mut self,
clear_color: Rgba, clear_color: [f32; 4],
clipped_primitives: &[egui::ClippedPrimitive], clipped_primitives: &[egui::ClippedPrimitive],
pixels_per_point: f32, pixels_per_point: f32,
textures_delta: &egui::TexturesDelta, textures_delta: &egui::TexturesDelta,

View file

@ -3,7 +3,7 @@ use std::sync::Arc;
use wasm_bindgen::JsValue; use wasm_bindgen::JsValue;
use web_sys::HtmlCanvasElement; use web_sys::HtmlCanvasElement;
use egui::{mutex::RwLock, Rgba}; use egui::mutex::RwLock;
use egui_wgpu::{renderer::ScreenDescriptor, RenderState, SurfaceErrorAction}; use egui_wgpu::{renderer::ScreenDescriptor, RenderState, SurfaceErrorAction};
use crate::WebOptions; use crate::WebOptions;
@ -135,7 +135,7 @@ impl WebPainter for WebPainterWgpu {
fn paint_and_update_textures( fn paint_and_update_textures(
&mut self, &mut self,
clear_color: Rgba, clear_color: [f32; 4],
clipped_primitives: &[egui::ClippedPrimitive], clipped_primitives: &[egui::ClippedPrimitive],
pixels_per_point: f32, pixels_per_point: f32,
textures_delta: &egui::TexturesDelta, textures_delta: &egui::TexturesDelta,
@ -228,10 +228,10 @@ impl WebPainter for WebPainterWgpu {
resolve_target: None, resolve_target: None,
ops: wgpu::Operations { ops: wgpu::Operations {
load: wgpu::LoadOp::Clear(wgpu::Color { load: wgpu::LoadOp::Clear(wgpu::Color {
r: clear_color.r() as f64, r: clear_color[0] as f64,
g: clear_color.g() as f64, g: clear_color[1] as f64,
b: clear_color.b() as f64, b: clear_color[2] as f64,
a: clear_color.a() as f64, a: clear_color[3] as f64,
}), }),
store: true, store: true,
}, },

View file

@ -255,7 +255,7 @@ impl Painter {
pub fn paint_and_update_textures( pub fn paint_and_update_textures(
&mut self, &mut self,
pixels_per_point: f32, pixels_per_point: f32,
clear_color: epaint::Rgba, clear_color: [f32; 4],
clipped_primitives: &[epaint::ClippedPrimitive], clipped_primitives: &[epaint::ClippedPrimitive],
textures_delta: &epaint::textures::TexturesDelta, textures_delta: &epaint::textures::TexturesDelta,
) { ) {
@ -335,10 +335,10 @@ impl Painter {
resolve_target: None, resolve_target: None,
ops: wgpu::Operations { ops: wgpu::Operations {
load: wgpu::LoadOp::Clear(wgpu::Color { load: wgpu::LoadOp::Clear(wgpu::Color {
r: clear_color.r() as f64, r: clear_color[0] as f64,
g: clear_color.g() as f64, g: clear_color[1] as f64,
b: clear_color.b() as f64, b: clear_color[2] as f64,
a: clear_color.a() as f64, a: clear_color[3] as f64,
}), }),
store: true, store: true,
}, },

View file

@ -175,8 +175,8 @@ impl eframe::App for WrapApp {
eframe::set_value(storage, eframe::APP_KEY, &self.state); eframe::set_value(storage, eframe::APP_KEY, &self.state);
} }
fn clear_color(&self, visuals: &egui::Visuals) -> egui::Rgba { fn clear_color(&self, visuals: &egui::Visuals) -> [f32; 4] {
visuals.panel_fill.into() visuals.panel_fill.to_normalized_gamma_f32()
} }
fn update(&mut self, ctx: &egui::Context, frame: &mut eframe::Frame) { fn update(&mut self, ctx: &egui::Context, frame: &mut eframe::Frame) {

View file

@ -5,7 +5,7 @@ use std::{collections::HashMap, sync::Arc};
use egui::{ use egui::{
emath::Rect, emath::Rect,
epaint::{Color32, Mesh, PaintCallbackInfo, Primitive, Vertex}, epaint::{Mesh, PaintCallbackInfo, Primitive, Vertex},
}; };
use glow::HasContext as _; use glow::HasContext as _;
use memoffset::offset_of; use memoffset::offset_of;
@ -665,7 +665,7 @@ impl Painter {
} }
} }
pub fn clear(gl: &glow::Context, screen_size_in_pixels: [u32; 2], clear_color: egui::Rgba) { pub fn clear(gl: &glow::Context, screen_size_in_pixels: [u32; 2], clear_color: [f32; 4]) {
crate::profile_function!(); crate::profile_function!();
unsafe { unsafe {
gl.disable(glow::SCISSOR_TEST); gl.disable(glow::SCISSOR_TEST);
@ -676,24 +676,12 @@ pub fn clear(gl: &glow::Context, screen_size_in_pixels: [u32; 2], clear_color: e
screen_size_in_pixels[0] as i32, screen_size_in_pixels[0] as i32,
screen_size_in_pixels[1] as i32, screen_size_in_pixels[1] as i32,
); );
gl.clear_color(
if true { clear_color[0],
// verified to be correct on eframe native (on Mac). clear_color[1],
gl.clear_color( clear_color[2],
clear_color[0], clear_color[3],
clear_color[1], );
clear_color[2],
clear_color[3],
);
} else {
let clear_color: Color32 = clear_color.into();
gl.clear_color(
clear_color[0] as f32 / 255.0,
clear_color[1] as f32 / 255.0,
clear_color[2] as f32 / 255.0,
clear_color[3] as f32 / 255.0,
);
}
gl.clear(glow::COLOR_BUFFER_BIT); gl.clear(glow::COLOR_BUFFER_BIT);
} }
} }

View file

@ -25,8 +25,8 @@ fn main() -> Result<(), eframe::Error> {
struct MyApp {} struct MyApp {}
impl eframe::App for MyApp { impl eframe::App for MyApp {
fn clear_color(&self, _visuals: &egui::Visuals) -> egui::Rgba { fn clear_color(&self, _visuals: &egui::Visuals) -> [f32; 4] {
egui::Rgba::TRANSPARENT // Make sure we don't paint anything behind the rounded corners egui::Rgba::TRANSPARENT.to_array() // Make sure we don't paint anything behind the rounded corners
} }
fn update(&mut self, ctx: &egui::Context, frame: &mut eframe::Frame) { fn update(&mut self, ctx: &egui::Context, frame: &mut eframe::Frame) {