From 7133d5133193b105094b8cc663daf86ea58718f6 Mon Sep 17 00:00:00 2001
From: Talin <viridia@gmail.com>
Date: Fri, 22 Mar 2024 12:53:10 -0700
Subject: [PATCH] Interpolating hues should use rem_euclid. (#12641)

Also, added additional tests for the hue interpolation.

Fixes #12632
Fixes #12631
---
 crates/bevy_color/src/color_ops.rs | 29 ++++++++++++++++++++++++++++-
 crates/bevy_color/src/hsla.rs      | 10 +---------
 crates/bevy_color/src/hsva.rs      | 10 +---------
 crates/bevy_color/src/hwba.rs      | 10 +---------
 4 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/crates/bevy_color/src/color_ops.rs b/crates/bevy_color/src/color_ops.rs
index 460fc985d1..1f39f3254c 100644
--- a/crates/bevy_color/src/color_ops.rs
+++ b/crates/bevy_color/src/color_ops.rs
@@ -97,10 +97,18 @@ pub trait ClampColor: Sized {
     fn is_within_bounds(&self) -> bool;
 }
 
+/// Utility function for interpolating hue values. This ensures that the interpolation
+/// takes the shortest path around the color wheel, and that the result is always between
+/// 0 and 360.
+pub(crate) fn lerp_hue(a: f32, b: f32, t: f32) -> f32 {
+    let diff = (b - a + 180.0).rem_euclid(360.) - 180.;
+    (a + diff * t).rem_euclid(360.0)
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::Hsla;
+    use crate::{testing::assert_approx_eq, Hsla};
 
     #[test]
     fn test_rotate_hue() {
@@ -113,4 +121,23 @@ mod tests {
         assert_eq!(hsla.rotate_hue(360.0), hsla);
         assert_eq!(hsla.rotate_hue(-360.0), hsla);
     }
+
+    #[test]
+    fn test_hue_wrap() {
+        assert_approx_eq!(lerp_hue(10., 20., 0.25), 12.5, 0.001);
+        assert_approx_eq!(lerp_hue(10., 20., 0.5), 15., 0.001);
+        assert_approx_eq!(lerp_hue(10., 20., 0.75), 17.5, 0.001);
+
+        assert_approx_eq!(lerp_hue(20., 10., 0.25), 17.5, 0.001);
+        assert_approx_eq!(lerp_hue(20., 10., 0.5), 15., 0.001);
+        assert_approx_eq!(lerp_hue(20., 10., 0.75), 12.5, 0.001);
+
+        assert_approx_eq!(lerp_hue(10., 350., 0.25), 5., 0.001);
+        assert_approx_eq!(lerp_hue(10., 350., 0.5), 0., 0.001);
+        assert_approx_eq!(lerp_hue(10., 350., 0.75), 355., 0.001);
+
+        assert_approx_eq!(lerp_hue(350., 10., 0.25), 355., 0.001);
+        assert_approx_eq!(lerp_hue(350., 10., 0.5), 0., 0.001);
+        assert_approx_eq!(lerp_hue(350., 10., 0.75), 5., 0.001);
+    }
 }
diff --git a/crates/bevy_color/src/hsla.rs b/crates/bevy_color/src/hsla.rs
index 37bc74fab5..c9db008940 100644
--- a/crates/bevy_color/src/hsla.rs
+++ b/crates/bevy_color/src/hsla.rs
@@ -105,16 +105,8 @@ impl Mix for Hsla {
     #[inline]
     fn mix(&self, other: &Self, factor: f32) -> Self {
         let n_factor = 1.0 - factor;
-        // TODO: Refactor this into EuclideanModulo::lerp_modulo
-        let shortest_angle = ((((other.hue - self.hue) % 360.) + 540.) % 360.) - 180.;
-        let mut hue = self.hue + shortest_angle * factor;
-        if hue < 0. {
-            hue += 360.;
-        } else if hue >= 360. {
-            hue -= 360.;
-        }
         Self {
-            hue,
+            hue: crate::color_ops::lerp_hue(self.hue, other.hue, factor),
             saturation: self.saturation * n_factor + other.saturation * factor,
             lightness: self.lightness * n_factor + other.lightness * factor,
             alpha: self.alpha * n_factor + other.alpha * factor,
diff --git a/crates/bevy_color/src/hsva.rs b/crates/bevy_color/src/hsva.rs
index d536cc1294..4423d047ac 100644
--- a/crates/bevy_color/src/hsva.rs
+++ b/crates/bevy_color/src/hsva.rs
@@ -73,16 +73,8 @@ impl Mix for Hsva {
     #[inline]
     fn mix(&self, other: &Self, factor: f32) -> Self {
         let n_factor = 1.0 - factor;
-        // TODO: Refactor this into EuclideanModulo::lerp_modulo
-        let shortest_angle = ((((other.hue - self.hue) % 360.) + 540.) % 360.) - 180.;
-        let mut hue = self.hue + shortest_angle * factor;
-        if hue < 0. {
-            hue += 360.;
-        } else if hue >= 360. {
-            hue -= 360.;
-        }
         Self {
-            hue,
+            hue: crate::color_ops::lerp_hue(self.hue, other.hue, factor),
             saturation: self.saturation * n_factor + other.saturation * factor,
             value: self.value * n_factor + other.value * factor,
             alpha: self.alpha * n_factor + other.alpha * factor,
diff --git a/crates/bevy_color/src/hwba.rs b/crates/bevy_color/src/hwba.rs
index dc8a96c259..0122139780 100644
--- a/crates/bevy_color/src/hwba.rs
+++ b/crates/bevy_color/src/hwba.rs
@@ -77,16 +77,8 @@ impl Mix for Hwba {
     #[inline]
     fn mix(&self, other: &Self, factor: f32) -> Self {
         let n_factor = 1.0 - factor;
-        // TODO: Refactor this into EuclideanModulo::lerp_modulo
-        let shortest_angle = ((((other.hue - self.hue) % 360.) + 540.) % 360.) - 180.;
-        let mut hue = self.hue + shortest_angle * factor;
-        if hue < 0. {
-            hue += 360.;
-        } else if hue >= 360. {
-            hue -= 360.;
-        }
         Self {
-            hue,
+            hue: crate::color_ops::lerp_hue(self.hue, other.hue, factor),
             whiteness: self.whiteness * n_factor + other.whiteness * factor,
             blackness: self.blackness * n_factor + other.blackness * factor,
             alpha: self.alpha * n_factor + other.alpha * factor,