Improve color picker cache (#886)

* colorpicker: try to maintain hue even when saturation goes to zero
* More consistent arguments to color types
* implement `Hash` for `Rgba`.

CHANGELOG.md

@@ -22,6 +22,7 @@ NOTE: [`epaint`](epaint/CHANGELOG.md), [`eframe`](eframe/CHANGELOG.md), [`egui_w
 
 ### Fixed 🐛
 * Fix `ComboBox` and other popups getting clipped to parent window ([#885](https://github.com/emilk/egui/pull/885)).
+* The color picker is now better att keeping the same hue even when saturation goes to zero ([#886](https://github.com/emilk/egui/pull/886)).
 
 ### Removed 🔥
 * Removed `egui::math` (use `egui::emath` instead).

egui/src/ui.rs

@@ -1284,22 +1284,14 @@ impl Ui {
     /// If the user clicks the button, a full color picker is shown.
     /// The given color is in `sRGB` space.
     pub fn color_edit_button_srgb(&mut self, srgb: &mut [u8; 3]) -> Response {
-        let mut hsva = Hsva::from_srgb(*srgb);
-        let response =
-            color_picker::color_edit_button_hsva(self, &mut hsva, color_picker::Alpha::Opaque);
-        *srgb = hsva.to_srgb();
-        response
+        color_picker::color_edit_button_srgb(self, srgb)
     }
 
     /// Shows a button with the given color.
     /// If the user clicks the button, a full color picker is shown.
     /// The given color is in linear RGB space.
     pub fn color_edit_button_rgb(&mut self, rgb: &mut [f32; 3]) -> Response {
-        let mut hsva = Hsva::from_rgb(*rgb);
-        let response =
-            color_picker::color_edit_button_hsva(self, &mut hsva, color_picker::Alpha::Opaque);
-        *rgb = hsva.to_rgb();
-        response
+        color_picker::color_edit_button_rgb(self, rgb)
     }
 
     /// Shows a button with the given color.
@@ -1317,36 +1309,46 @@ impl Ui {
     /// The given color is in `sRGBA` space without premultiplied alpha.
     /// If unsure, what "premultiplied alpha" is, then this is probably the function you want to use.
     pub fn color_edit_button_srgba_unmultiplied(&mut self, srgba: &mut [u8; 4]) -> Response {
-        let mut hsva = Hsva::from_srgba_unmultiplied(*srgba);
+        let mut rgba = Rgba::from_srgba_unmultiplied(srgba[0], srgba[1], srgba[2], srgba[3]);
         let response =
-            color_picker::color_edit_button_hsva(self, &mut hsva, color_picker::Alpha::OnlyBlend);
-        *srgba = hsva.to_srgba_unmultiplied();
+            color_picker::color_edit_button_rgba(self, &mut rgba, color_picker::Alpha::OnlyBlend);
+        *srgba = rgba.to_srgba_unmultiplied();
         response
     }
 
     /// Shows a button with the given color.
     /// If the user clicks the button, a full color picker is shown.
     /// The given color is in linear RGBA space with premultiplied alpha
-    pub fn color_edit_button_rgba_premultiplied(&mut self, rgba: &mut [f32; 4]) -> Response {
-        let mut hsva = Hsva::from_rgba_premultiplied(*rgba);
-        let response = color_picker::color_edit_button_hsva(
+    pub fn color_edit_button_rgba_premultiplied(&mut self, rgba_premul: &mut [f32; 4]) -> Response {
+        let mut rgba = Rgba::from_rgba_premultiplied(
+            rgba_premul[0],
+            rgba_premul[1],
+            rgba_premul[2],
+            rgba_premul[3],
+        );
+        let response = color_picker::color_edit_button_rgba(
             self,
-            &mut hsva,
+            &mut rgba,
             color_picker::Alpha::BlendOrAdditive,
         );
-        *rgba = hsva.to_rgba_premultiplied();
+        *rgba_premul = rgba.to_array();
         response
     }
 
     /// Shows a button with the given color.
     /// If the user clicks the button, a full color picker is shown.
     /// The given color is in linear RGBA space without premultiplied alpha.
     /// If unsure, what "premultiplied alpha" is, then this is probably the function you want to use.
-    pub fn color_edit_button_rgba_unmultiplied(&mut self, rgba: &mut [f32; 4]) -> Response {
-        let mut hsva = Hsva::from_rgba_unmultiplied(*rgba);
+    pub fn color_edit_button_rgba_unmultiplied(&mut self, rgba_unmul: &mut [f32; 4]) -> Response {
+        let mut rgba = Rgba::from_rgba_unmultiplied(
+            rgba_unmul[0],
+            rgba_unmul[1],
+            rgba_unmul[2],
+            rgba_unmul[3],
+        );
         let response =
-            color_picker::color_edit_button_hsva(self, &mut hsva, color_picker::Alpha::OnlyBlend);
-        *rgba = hsva.to_rgba_unmultiplied();
+            color_picker::color_edit_button_rgba(self, &mut rgba, color_picker::Alpha::OnlyBlend);
+        *rgba_unmul = rgba.to_rgba_unmultiplied();
         response
     }
 }

egui/src/widgets/color_picker.rs

@@ -324,18 +324,10 @@ fn color_picker_hsva_2d(ui: &mut Ui, hsva: &mut Hsva, alpha: Alpha) -> bool {
 
 /// Returns `true` on change.
 pub fn color_picker_color32(ui: &mut Ui, srgba: &mut Color32, alpha: Alpha) -> bool {
-    // To ensure we keep hue slider when `srgba` is gray we store the
-    // full `Hsva` in a cache:
-
-    let mut hsva = use_color_cache(ui.ctx(), |cc| cc.get(srgba).cloned())
-        .unwrap_or_else(|| Hsva::from(*srgba));
-
+    let mut hsva = color_cache_get(ui.ctx(), *srgba);
     let response = color_picker_hsva_2d(ui, &mut hsva, alpha);
-
     *srgba = Color32::from(hsva);
-
-    use_color_cache(ui.ctx(), |cc| cc.set(*srgba, hsva));
-
+    color_cache_set(ui.ctx(), *srgba, hsva);
     response
 }
 
@@ -378,21 +370,59 @@ pub fn color_edit_button_hsva(ui: &mut Ui, hsva: &mut Hsva, alpha: Alpha) -> Res
 /// Shows a button with the given color.
 /// If the user clicks the button, a full color picker is shown.
 pub fn color_edit_button_srgba(ui: &mut Ui, srgba: &mut Color32, alpha: Alpha) -> Response {
-    // To ensure we keep hue slider when `srgba` is gray we store the
-    // full `Hsva` in a cache:
+    let mut hsva = color_cache_get(ui.ctx(), *srgba);
+    let response = color_edit_button_hsva(ui, &mut hsva, alpha);
+    *srgba = Color32::from(hsva);
+    color_cache_set(ui.ctx(), *srgba, hsva);
+    response
+}
 
-    let mut hsva = use_color_cache(ui.ctx(), |cc| cc.get(srgba).cloned())
-        .unwrap_or_else(|| Hsva::from(*srgba));
+/// Shows a button with the given color.
+/// If the user clicks the button, a full color picker is shown.
+/// The given color is in `sRGB` space.
+pub fn color_edit_button_srgb(ui: &mut Ui, srgb: &mut [u8; 3]) -> Response {
+    let mut srgba = Color32::from_rgb(srgb[0], srgb[1], srgb[2]);
+    let response = color_edit_button_srgba(ui, &mut srgba, Alpha::Opaque);
+    srgb[0] = srgba[0];
+    srgb[1] = srgba[1];
+    srgb[2] = srgba[2];
+    response
+}
 
+/// Shows a button with the given color.
+/// If the user clicks the button, a full color picker is shown.
+pub fn color_edit_button_rgba(ui: &mut Ui, rgba: &mut Rgba, alpha: Alpha) -> Response {
+    let mut hsva = color_cache_get(ui.ctx(), *rgba);
     let response = color_edit_button_hsva(ui, &mut hsva, alpha);
+    *rgba = Rgba::from(hsva);
+    color_cache_set(ui.ctx(), *rgba, hsva);
+    response
+}
 
-    *srgba = Color32::from(hsva);
+/// Shows a button with the given color.
+/// If the user clicks the button, a full color picker is shown.
+pub fn color_edit_button_rgb(ui: &mut Ui, rgb: &mut [f32; 3]) -> Response {
+    let mut rgba = Rgba::from_rgb(rgb[0], rgb[1], rgb[2]);
+    let response = color_edit_button_rgba(ui, &mut rgba, Alpha::Opaque);
+    rgb[0] = rgba[0];
+    rgb[1] = rgba[1];
+    rgb[2] = rgba[2];
+    response
+}
 
-    use_color_cache(ui.ctx(), |cc| cc.set(*srgba, hsva));
+// To ensure we keep hue slider when `srgba` is gray we store the full `Hsva` in a cache:
+fn color_cache_get(ctx: &Context, rgba: impl Into<Rgba>) -> Hsva {
+    let rgba = rgba.into();
+    use_color_cache(ctx, |cc| cc.get(&rgba).cloned()).unwrap_or_else(|| Hsva::from(rgba))
+}
 
-    response
+// To ensure we keep hue slider when `srgba` is gray we store the full `Hsva` in a cache:
+fn color_cache_set(ctx: &Context, rgba: impl Into<Rgba>, hsva: Hsva) {
+    let rgba = rgba.into();
+    use_color_cache(ctx, |cc| cc.set(rgba, hsva));
 }
 
-fn use_color_cache<R>(ctx: &Context, f: impl FnOnce(&mut FixedCache<Color32, Hsva>) -> R) -> R {
+// To ensure we keep hue slider when `srgba` is gray we store the full `Hsva` in a cache:
+fn use_color_cache<R>(ctx: &Context, f: impl FnOnce(&mut FixedCache<Rgba, Hsva>) -> R) -> R {
     f(ctx.memory().data.get_temp_mut_or_default(Id::null()))
 }

egui_demo_lib/src/apps/color_test.rs

@@ -156,7 +156,7 @@ impl ColorTest {
             ui,
             tex_allocator,
             RED,
-            (TRANSPARENT, Color32::from_rgba_premultiplied(0, 0, 255, 0)),
+            (TRANSPARENT, Color32::from_rgb_additive(0, 0, 255)),
         );
 
         ui.separator();

epaint/CHANGELOG.md

@@ -4,6 +4,7 @@ All notable changes to the epaint crate will be documented in this file.
 
 
 ## Unreleased
+* `Rgba` now implements `Hash` ([#886](https://github.com/emilk/egui/pull/886)).
 
 
 ## 0.15.0 - 2021-10-24

epaint/src/color.rs

@@ -88,7 +88,7 @@ impl Color32 {
     /// From `sRGBA` WITHOUT premultiplied alpha.
     pub fn from_rgba_unmultiplied(r: u8, g: u8, b: u8, a: u8) -> Self {
         if a == 255 {
-            Self::from_rgba_premultiplied(r, g, b, 255) // common-case optimization
+            Self::from_rgb(r, g, b) // common-case optimization
         } else if a == 0 {
             Self::TRANSPARENT // common-case optimization
         } else {
@@ -173,6 +173,10 @@ impl Color32 {
         (self.r(), self.g(), self.b(), self.a())
     }
 
+    pub fn to_srgba_unmultiplied(&self) -> [u8; 4] {
+        Rgba::from(*self).to_srgba_unmultiplied()
+    }
+
     /// Multiply with 0.5 to make color half as opaque.
     pub fn linear_multiply(self, factor: f32) -> Color32 {
         crate::epaint_assert!(0.0 <= factor && factor <= 1.0);
@@ -207,6 +211,17 @@ impl std::ops::IndexMut<usize> for Rgba {
     }
 }
 
+#[allow(clippy::derive_hash_xor_eq)]
+impl std::hash::Hash for Rgba {
+    #[inline]
+    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+        crate::f32_hash(state, self.0[0]);
+        crate::f32_hash(state, self.0[1]);
+        crate::f32_hash(state, self.0[2]);
+        crate::f32_hash(state, self.0[3]);
+    }
+}
+
 impl Rgba {
     pub const TRANSPARENT: Rgba = Rgba::from_rgba_premultiplied(0.0, 0.0, 0.0, 0.0);
     pub const BLACK: Rgba = Rgba::from_rgb(0.0, 0.0, 0.0);
@@ -220,6 +235,29 @@ impl Rgba {
         Self([r, g, b, a])
     }
 
+    #[inline(always)]
+    pub fn from_rgba_unmultiplied(r: f32, g: f32, b: f32, a: f32) -> Self {
+        Self([r * a, g * a, b * a, a])
+    }
+
+    #[inline(always)]
+    pub fn from_srgba_premultiplied(r: u8, g: u8, b: u8, a: u8) -> Self {
+        let r = linear_f32_from_gamma_u8(r);
+        let g = linear_f32_from_gamma_u8(g);
+        let b = linear_f32_from_gamma_u8(b);
+        let a = linear_f32_from_linear_u8(a);
+        Self::from_rgba_premultiplied(r, g, b, a)
+    }
+
+    #[inline(always)]
+    pub fn from_srgba_unmultiplied(r: u8, g: u8, b: u8, a: u8) -> Self {
+        let r = linear_f32_from_gamma_u8(r);
+        let g = linear_f32_from_gamma_u8(g);
+        let b = linear_f32_from_gamma_u8(b);
+        let a = linear_f32_from_linear_u8(a);
+        Self::from_rgba_premultiplied(r * a, g * a, b * a, a)
+    }
+
     #[inline(always)]
     pub const fn from_rgb(r: f32, g: f32, b: f32) -> Self {
         Self([r, g, b, 1.0])
@@ -298,14 +336,13 @@ impl Rgba {
     pub fn to_opaque(&self) -> Self {
         if self.a() == 0.0 {
             // Additive or fully transparent black.
-            Self::from_rgba_premultiplied(self.r(), self.g(), self.b(), 1.0)
+            Self::from_rgb(self.r(), self.g(), self.b())
         } else {
             // un-multiply alpha:
-            Self::from_rgba_premultiplied(
+            Self::from_rgb(
                 self.r() / self.a(),
                 self.g() / self.a(),
                 self.b() / self.a(),
-                1.0,
             )
         }
     }
@@ -321,6 +358,28 @@ impl Rgba {
     pub fn to_tuple(&self) -> (f32, f32, f32, f32) {
         (self.r(), self.g(), self.b(), self.a())
     }
+
+    /// unmultiply the alpha
+    pub fn to_rgba_unmultiplied(&self) -> [f32; 4] {
+        let a = self.a();
+        if a == 0.0 {
+            // Additive, let's assume we are black
+            self.0
+        } else {
+            [self.r() / a, self.g() / a, self.b() / a, a]
+        }
+    }
+
+    /// unmultiply the alpha
+    pub fn to_srgba_unmultiplied(&self) -> [u8; 4] {
+        let [r, g, b, a] = self.to_rgba_unmultiplied();
+        [
+            gamma_u8_from_linear_f32(r),
+            gamma_u8_from_linear_f32(g),
+            gamma_u8_from_linear_f32(b),
+            linear_u8_from_linear_f32(a.abs()),
+        ]
+    }
 }
 
 impl std::ops::Add for Rgba {
@@ -497,26 +556,26 @@ impl Hsva {
 
     /// From `sRGBA` with premultiplied alpha
     pub fn from_srgba_premultiplied(srgba: [u8; 4]) -> Self {
-        Self::from_rgba_premultiplied([
+        Self::from_rgba_premultiplied(
             linear_f32_from_gamma_u8(srgba[0]),
             linear_f32_from_gamma_u8(srgba[1]),
             linear_f32_from_gamma_u8(srgba[2]),
             linear_f32_from_linear_u8(srgba[3]),
-        ])
+        )
     }
 
     /// From `sRGBA` without premultiplied alpha
     pub fn from_srgba_unmultiplied(srgba: [u8; 4]) -> Self {
-        Self::from_rgba_unmultiplied([
+        Self::from_rgba_unmultiplied(
             linear_f32_from_gamma_u8(srgba[0]),
             linear_f32_from_gamma_u8(srgba[1]),
             linear_f32_from_gamma_u8(srgba[2]),
             linear_f32_from_linear_u8(srgba[3]),
-        ])
+        )
     }
 
     /// From linear RGBA with premultiplied alpha
-    pub fn from_rgba_premultiplied([r, g, b, a]: [f32; 4]) -> Self {
+    pub fn from_rgba_premultiplied(r: f32, g: f32, b: f32, a: f32) -> Self {
         #![allow(clippy::many_single_char_names)]
         if a == 0.0 {
             if r == 0.0 && b == 0.0 && a == 0.0 {
@@ -531,7 +590,7 @@ impl Hsva {
     }
 
     /// From linear RGBA without premultiplied alpha
-    pub fn from_rgba_unmultiplied([r, g, b, a]: [f32; 4]) -> Self {
+    pub fn from_rgba_unmultiplied(r: f32, g: f32, b: f32, a: f32) -> Self {
         #![allow(clippy::many_single_char_names)]
         let (h, s, v) = hsv_from_rgb([r, g, b]);
         Hsva { h, s, v, a }
@@ -624,7 +683,7 @@ impl From<Hsva> for Rgba {
 }
 impl From<Rgba> for Hsva {
     fn from(rgba: Rgba) -> Hsva {
-        Self::from_rgba_premultiplied(rgba.0)
+        Self::from_rgba_premultiplied(rgba.0[0], rgba.0[1], rgba.0[2], rgba.0[3])
     }
 }