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

Wumpf · emilk · lictex · commit d9ece3eeb00f · 2023-02-05T21:48:13.000+08:00
* Clear color values are not explicitely sent to the rendering backend as-is.
Previously, converting from Color32 to Rgba caused an srgb-&gt;linear conversion. This conversion is incorrect if the backbuffer doesn't perform automatic conversion from linear-&gt;srgb (lack of this conversion is generally what egui assumes!).

* fill in pr numbers in changelog

* Epi comment fix

Co-authored-by: Emil Ernerfeldt &lt;emil.ernerfeldt@gmail.com&gt;

* Color32 comment fix

Co-authored-by: Emil Ernerfeldt &lt;emil.ernerfeldt@gmail.com&gt;

* move changelog line

* rename fix

* use backticks in doc

---------

Co-authored-by: Emil Ernerfeldt &lt;emil.ernerfeldt@gmail.com&gt;
diff --git a/crates/ecolor/src/color32.rs b/crates/ecolor/src/color32.rs
@@ -198,4 +198,19 @@ impl Color32 {
         // we need a somewhat expensive conversion to linear space and back.
         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,
+        ]
+    }
 }
diff --git a/crates/eframe/CHANGELOG.md b/crates/eframe/CHANGELOG.md
@@ -5,6 +5,7 @@ NOTE: [`egui-winit`](../egui-winit/CHANGELOG.md), [`egui_glium`](../egui_glium/C
 
 
 ## Unreleased
+* ⚠️ BREAKING: `App::clear_color` now expects you to return a raw float array ([#2666](https://github.com/emilk/egui/pull/2666)):
 
 #### Desktop/Native:
 * `eframe::run_native` now returns a `Result` ([#2433](https://github.com/emilk/egui/pull/2433)).
diff --git a/crates/eframe/src/epi.rs b/crates/eframe/src/epi.rs
@@ -149,13 +149,21 @@ pub trait App {
         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.
-    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.
         // We use a bit of transparency so that if the user switches on the
         // `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
     }
diff --git a/crates/eframe/src/web/web_painter.rs b/crates/eframe/src/web/web_painter.rs
@@ -1,4 +1,3 @@
-use egui::Rgba;
 use wasm_bindgen::JsValue;
 
 /// Renderer for a browser canvas.
@@ -19,7 +18,7 @@ pub(crate) trait WebPainter {
     /// Update all internal textures and paint gui.
     fn paint_and_update_textures(
         &mut self,
-        clear_color: Rgba,
+        clear_color: [f32; 4],
         clipped_primitives: &[egui::ClippedPrimitive],
         pixels_per_point: f32,
         textures_delta: &egui::TexturesDelta,
diff --git a/crates/eframe/src/web/web_painter_glow.rs b/crates/eframe/src/web/web_painter_glow.rs
@@ -2,7 +2,6 @@ use wasm_bindgen::JsCast;
 use wasm_bindgen::JsValue;
 use web_sys::HtmlCanvasElement;
 
-use egui::Rgba;
 use egui_glow::glow;
 
 use crate::{WebGlContextOption, WebOptions};
@@ -49,7 +48,7 @@ impl WebPainter for WebPainterGlow {
 
     fn paint_and_update_textures(
         &mut self,
-        clear_color: Rgba,
+        clear_color: [f32; 4],
         clipped_primitives: &[egui::ClippedPrimitive],
         pixels_per_point: f32,
         textures_delta: &egui::TexturesDelta,
diff --git a/crates/eframe/src/web/web_painter_wgpu.rs b/crates/eframe/src/web/web_painter_wgpu.rs
@@ -3,7 +3,7 @@ use std::sync::Arc;
 use wasm_bindgen::JsValue;
 use web_sys::HtmlCanvasElement;
 
-use egui::{mutex::RwLock, Rgba};
+use egui::mutex::RwLock;
 use egui_wgpu::{renderer::ScreenDescriptor, RenderState, SurfaceErrorAction};
 
 use crate::WebOptions;
@@ -135,7 +135,7 @@ impl WebPainter for WebPainterWgpu {
 
     fn paint_and_update_textures(
         &mut self,
-        clear_color: Rgba,
+        clear_color: [f32; 4],
         clipped_primitives: &[egui::ClippedPrimitive],
         pixels_per_point: f32,
         textures_delta: &egui::TexturesDelta,
@@ -228,10 +228,10 @@ impl WebPainter for WebPainterWgpu {
                         resolve_target: None,
                         ops: wgpu::Operations {
                             load: wgpu::LoadOp::Clear(wgpu::Color {
-                                r: clear_color.r() as f64,
-                                g: clear_color.g() as f64,
-                                b: clear_color.b() as f64,
-                                a: clear_color.a() as f64,
+                                r: clear_color[0] as f64,
+                                g: clear_color[1] as f64,
+                                b: clear_color[2] as f64,
+                                a: clear_color[3] as f64,
                             }),
                             store: true,
                         },
diff --git a/crates/egui-wgpu/src/winit.rs b/crates/egui-wgpu/src/winit.rs
@@ -255,7 +255,7 @@ impl Painter {
     pub fn paint_and_update_textures(
         &mut self,
         pixels_per_point: f32,
-        clear_color: epaint::Rgba,
+        clear_color: [f32; 4],
         clipped_primitives: &[epaint::ClippedPrimitive],
         textures_delta: &epaint::textures::TexturesDelta,
     ) {
@@ -335,10 +335,10 @@ impl Painter {
                     resolve_target: None,
                     ops: wgpu::Operations {
                         load: wgpu::LoadOp::Clear(wgpu::Color {
-                            r: clear_color.r() as f64,
-                            g: clear_color.g() as f64,
-                            b: clear_color.b() as f64,
-                            a: clear_color.a() as f64,
+                            r: clear_color[0] as f64,
+                            g: clear_color[1] as f64,
+                            b: clear_color[2] as f64,
+                            a: clear_color[3] as f64,
                         }),
                         store: true,
                     },
diff --git a/crates/egui_demo_app/src/wrap_app.rs b/crates/egui_demo_app/src/wrap_app.rs
@@ -175,8 +175,8 @@ impl eframe::App for WrapApp {
         eframe::set_value(storage, eframe::APP_KEY, &self.state);
     }
 
-    fn clear_color(&self, visuals: &egui::Visuals) -> egui::Rgba {
-        visuals.panel_fill.into()
+    fn clear_color(&self, visuals: &egui::Visuals) -> [f32; 4] {
+        visuals.panel_fill.to_normalized_gamma_f32()
     }
 
     fn update(&mut self, ctx: &egui::Context, frame: &mut eframe::Frame) {
diff --git a/crates/egui_glow/src/painter.rs b/crates/egui_glow/src/painter.rs
@@ -5,7 +5,7 @@ use std::{collections::HashMap, sync::Arc};
 
 use egui::{
     emath::Rect,
-    epaint::{Color32, Mesh, PaintCallbackInfo, Primitive, Vertex},
+    epaint::{Mesh, PaintCallbackInfo, Primitive, Vertex},
 };
 use glow::HasContext as _;
 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!();
     unsafe {
         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[1] as i32,
         );
-
-        if true {
-            // verified to be correct on eframe native (on Mac).
-            gl.clear_color(
-                clear_color[0],
-                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_color(
+            clear_color[0],
+            clear_color[1],
+            clear_color[2],
+            clear_color[3],
+        );
         gl.clear(glow::COLOR_BUFFER_BIT);
     }
 }
diff --git a/examples/custom_window_frame/src/main.rs b/examples/custom_window_frame/src/main.rs
@@ -25,8 +25,8 @@ fn main() -> Result<(), eframe::Error> {
 struct MyApp {}
 
 impl eframe::App for MyApp {
-    fn clear_color(&self, _visuals: &egui::Visuals) -> egui::Rgba {
-        egui::Rgba::TRANSPARENT // Make sure we don't paint anything behind the rounded corners
+    fn clear_color(&self, _visuals: &egui::Visuals) -> [f32; 4] {
+        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) {

Original file line number	Diff line number	Diff line change
`@@ -175,8 +175,8 @@ impl eframe::App for WrapApp {`
`175`	`175`	`eframe::set_value(storage, eframe::APP_KEY, &self.state);`
`176`	`176`	`}`
`177`	`177`
`178`		`- fn clear_color(&self, visuals: &egui::Visuals) -> egui::Rgba {`
`179`		`- visuals.panel_fill.into()`
	`178`	`+ fn clear_color(&self, visuals: &egui::Visuals) -> [f32; 4] {`
	`179`	`+ visuals.panel_fill.to_normalized_gamma_f32()`
`180`	`180`	`}`
`181`	`181`
`182`	`182`	`fn update(&mut self, ctx: &egui::Context, frame: &mut eframe::Frame) {`
Original file line number	Diff line number	Diff line change
`@@ -25,8 +25,8 @@ fn main() -> Result<(), eframe::Error> {`
`25`	`25`	`struct MyApp {}`
`26`	`26`
`27`	`27`	`impl eframe::App for MyApp {`
`28`		`- fn clear_color(&self, _visuals: &egui::Visuals) -> egui::Rgba {`
`29`		`- egui::Rgba::TRANSPARENT // Make sure we don't paint anything behind the rounded corners`
	`28`	`+ fn clear_color(&self, _visuals: &egui::Visuals) -> [f32; 4] {`
	`29`	`+ egui::Rgba::TRANSPARENT.to_array() // Make sure we don't paint anything behind the rounded corners`
`30`	`30`	`}`
`31`	`31`
`32`	`32`	`fn update(&mut self, ctx: &egui::Context, frame: &mut eframe::Frame) {`