Merge #196

196: Add uniformity tests for random colors and fix mistakes r=Ogeon a=Ogeon

Adds tests that makes sure randomly generated colors are uniform within their "main" color space. I realized a Chi-squared goodness-of-fit test would do the trick, so I put this together over the past couple of days. Also found a few mistakes. I'm considering replacing the RNG with a fake RNG to avoid requiring as many samples, but that can wait.

The only space I didn't find a good test method for is `Lch`, which covers a circle. My attempts showed a slight bias towards the center, but I can't explain it. I ended up not looking further into it for now, since the test method I used (sampling only a central square) rejects too many samples anyway.

Co-authored-by: Erik Hedvall <[email protected]>

Author: bors[bot]

palette/Cargo.toml

@@ -38,7 +38,7 @@ version = "0.8"
 optional = true
 
 [dependencies.rand]
-version = "0.7"
+version = "0.8"
 default-features = false
 optional = true
 
@@ -67,6 +67,11 @@ version = "0.23"
 default-features = false
 features = ["png"]
 
+[dev-dependencies.rand_mt]
+version = "4"
+default-features = false
+features = ["rand-traits"]
+
 [build-dependencies.phf_codegen]
 version = "0.8"
 optional = true

palette/src/alpha/alpha.rs

@@ -678,4 +678,16 @@ mod test {
 
         assert_eq!(deserialized, Rgba::<Srgb>::new(0.3, 0.8, 0.1, 0.5));
     }
+
+    #[cfg(feature = "random")]
+    test_uniform_distribution! {
+        Rgba<Srgb, f32> {
+            red: (0.0, 1.0),
+            green: (0.0, 1.0),
+            blue: (0.0, 1.0),
+            alpha: (0.0, 1.0)
+        },
+        min: Rgba::new(0.0f32, 0.0, 0.0, 0.0),
+        max: Rgba::new(1.0, 1.0, 1.0, 1.0)
+    }
 }

palette/src/hsl.rs

@@ -682,7 +682,7 @@ where
 pub struct UniformHsl<S, T>
 where
     T: FloatComponent + SampleUniform,
-    S: RgbStandard + SampleUniform,
+    S: RgbStandard,
 {
     hue: crate::hues::UniformRgbHue<T>,
     u1: Uniform<T>,
@@ -694,7 +694,7 @@ where
 impl<S, T> SampleUniform for Hsl<S, T>
 where
     T: FloatComponent + SampleUniform,
-    S: RgbStandard + SampleUniform,
+    S: RgbStandard,
 {
     type Sampler = UniformHsl<S, T>;
 }
@@ -703,7 +703,7 @@ where
 impl<S, T> UniformSampler for UniformHsl<S, T>
 where
     T: FloatComponent + SampleUniform,
-    S: RgbStandard + SampleUniform,
+    S: RgbStandard,
 {
     type X = Hsl<S, T>;
 
@@ -860,4 +860,49 @@ mod test {
 
         assert_eq!(deserialized, Hsl::new(0.3, 0.8, 0.1));
     }
+
+    #[cfg(feature = "random")]
+    test_uniform_distribution! {
+        Hsl<crate::encoding::Srgb, f32> as crate::rgb::Rgb {
+            red: (0.0, 1.0),
+            green: (0.0, 1.0),
+            blue: (0.0, 1.0)
+        },
+        min: Hsl::new(0.0f32, 0.0, 0.0),
+        max: Hsl::new(360.0, 1.0, 1.0)
+    }
+
+    /// Sanity check to make sure the test doesn't start accepting known
+    /// non-uniform distributions.
+    #[cfg(feature = "random")]
+    #[test]
+    #[should_panic(expected = "is not uniform enough")]
+    fn uniform_distribution_fail() {
+        use rand::Rng;
+
+        const BINS: usize = crate::random_sampling::test_utils::BINS;
+        const SAMPLES: usize = crate::random_sampling::test_utils::SAMPLES;
+
+        let mut red = [0; BINS];
+        let mut green = [0; BINS];
+        let mut blue = [0; BINS];
+
+        let mut rng = rand_mt::Mt::new(1234); // We want the same seed on every run to avoid random fails
+
+        for _ in 0..SAMPLES {
+            let color = Hsl::<crate::encoding::Srgb, f32>::new(
+                rng.gen::<f32>() * 360.0,
+                rng.gen(),
+                rng.gen(),
+            );
+            let color: crate::rgb::Rgb = crate::IntoColor::into_color(color);
+            red[((color.red * BINS as f32) as usize).min(9)] += 1;
+            green[((color.green * BINS as f32) as usize).min(9)] += 1;
+            blue[((color.blue * BINS as f32) as usize).min(9)] += 1;
+        }
+
+        assert_uniform_distribution!(red);
+        assert_uniform_distribution!(green);
+        assert_uniform_distribution!(blue);
+    }
 }

palette/src/hsv.rs

@@ -689,7 +689,7 @@ where
 pub struct UniformHsv<S, T>
 where
     T: FloatComponent + SampleUniform,
-    S: RgbStandard + SampleUniform,
+    S: RgbStandard,
 {
     hue: crate::hues::UniformRgbHue<T>,
     u1: Uniform<T>,
@@ -701,7 +701,7 @@ where
 impl<S, T> SampleUniform for Hsv<S, T>
 where
     T: FloatComponent + SampleUniform,
-    S: RgbStandard + SampleUniform,
+    S: RgbStandard,
 {
     type Sampler = UniformHsv<S, T>;
 }
@@ -710,7 +710,7 @@ where
 impl<S, T> UniformSampler for UniformHsv<S, T>
 where
     T: FloatComponent + SampleUniform,
-    S: RgbStandard + SampleUniform,
+    S: RgbStandard,
 {
     type X = Hsv<S, T>;
 
@@ -872,4 +872,15 @@ mod test {
 
         assert_eq!(deserialized, Hsv::new(0.3, 0.8, 0.1));
     }
+
+    #[cfg(feature = "random")]
+    test_uniform_distribution! {
+        Hsv<crate::encoding::Srgb, f32> as crate::rgb::Rgb {
+            red: (0.0, 1.0),
+            green: (0.0, 1.0),
+            blue: (0.0, 1.0)
+        },
+        min: Hsv::new(0.0f32, 0.0, 0.0),
+        max: Hsv::new(360.0, 1.0, 1.0)
+    }
 }

palette/src/hues.rs

@@ -317,13 +317,18 @@ where
         B2: SampleBorrow<Self::X> + Sized,
     {
         let low = *low_b.borrow();
+        let normalized_low = LabHue::to_positive_degrees(low);
         let high = *high_b.borrow();
+        let normalized_high = LabHue::to_positive_degrees(high);
+
+        let normalized_high = if normalized_low >= normalized_high && low.0 < high.0 {
+            normalized_high + from_f64(360.0)
+        } else {
+            normalized_high
+        };
 
         UniformLabHue {
-            hue: Uniform::new(
-                LabHue::to_positive_degrees(low),
-                LabHue::to_positive_degrees(high),
-            ),
+            hue: Uniform::new(normalized_low, normalized_high),
         }
     }
 
@@ -333,13 +338,18 @@ where
         B2: SampleBorrow<Self::X> + Sized,
     {
         let low = *low_b.borrow();
+        let normalized_low = LabHue::to_positive_degrees(low);
         let high = *high_b.borrow();
+        let normalized_high = LabHue::to_positive_degrees(high);
+
+        let normalized_high = if normalized_low >= normalized_high && low.0 < high.0 {
+            normalized_high + from_f64(360.0)
+        } else {
+            normalized_high
+        };
 
         UniformLabHue {
-            hue: Uniform::new_inclusive(
-                LabHue::to_positive_degrees(low),
-                LabHue::to_positive_degrees(high),
-            ),
+            hue: Uniform::new_inclusive(normalized_low, normalized_high),
         }
     }
 
@@ -377,13 +387,18 @@ where
         B2: SampleBorrow<Self::X> + Sized,
     {
         let low = *low_b.borrow();
+        let normalized_low = RgbHue::to_positive_degrees(low);
         let high = *high_b.borrow();
+        let normalized_high = RgbHue::to_positive_degrees(high);
+
+        let normalized_high = if normalized_low >= normalized_high && low.0 < high.0 {
+            normalized_high + from_f64(360.0)
+        } else {
+            normalized_high
+        };
 
         UniformRgbHue {
-            hue: Uniform::new(
-                RgbHue::to_positive_degrees(low),
-                RgbHue::to_positive_degrees(high),
-            ),
+            hue: Uniform::new(normalized_low, normalized_high),
         }
     }
 
@@ -393,13 +408,18 @@ where
         B2: SampleBorrow<Self::X> + Sized,
     {
         let low = *low_b.borrow();
+        let normalized_low = RgbHue::to_positive_degrees(low);
         let high = *high_b.borrow();
+        let normalized_high = RgbHue::to_positive_degrees(high);
+
+        let normalized_high = if normalized_low >= normalized_high && low.0 < high.0 {
+            normalized_high + from_f64(360.0)
+        } else {
+            normalized_high
+        };
 
         UniformRgbHue {
-            hue: Uniform::new_inclusive(
-                RgbHue::to_positive_degrees(low),
-                RgbHue::to_positive_degrees(high),
-            ),
+            hue: Uniform::new_inclusive(normalized_low, normalized_high),
         }
     }
 

palette/src/hwb.rs

@@ -641,7 +641,7 @@ where
 pub struct UniformHwb<S, T>
 where
     T: FloatComponent + SampleUniform,
-    S: RgbStandard + SampleUniform,
+    S: RgbStandard,
 {
     sampler: crate::hsv::UniformHsv<S, T>,
     space: PhantomData<S>,
@@ -651,7 +651,7 @@ where
 impl<S, T> SampleUniform for Hwb<S, T>
 where
     T: FloatComponent + SampleUniform,
-    S: RgbStandard + SampleUniform,
+    S: RgbStandard,
 {
     type Sampler = UniformHwb<S, T>;
 }
@@ -660,7 +660,7 @@ where
 impl<S, T> UniformSampler for UniformHwb<S, T>
 where
     T: FloatComponent + SampleUniform,
-    S: RgbStandard + SampleUniform,
+    S: RgbStandard,
 {
     type X = Hwb<S, T>;
 
@@ -669,12 +669,20 @@ where
         B1: SampleBorrow<Self::X> + Sized,
         B2: SampleBorrow<Self::X> + Sized,
     {
-        let low = *low_b.borrow();
-        let high = *high_b.borrow();
-        let sampler = crate::hsv::UniformHsv::<S, _>::new(
-            Hsv::from_color_unclamped(low),
-            Hsv::from_color_unclamped(high),
+        let low_input = Hsv::from_color_unclamped(*low_b.borrow());
+        let high_input = Hsv::from_color_unclamped(*high_b.borrow());
+
+        let low = Hsv::with_wp(
+            low_input.hue,
+            low_input.saturation.min(high_input.saturation),
+            low_input.value.min(high_input.value),
+        );
+        let high = Hsv::with_wp(
+            high_input.hue,
+            low_input.saturation.max(high_input.saturation),
+            low_input.value.max(high_input.value),
         );
+        let sampler = crate::hsv::UniformHsv::<S, _>::new(low, high);
 
         UniformHwb {
             sampler,
@@ -687,13 +695,22 @@ where
         B1: SampleBorrow<Self::X> + Sized,
         B2: SampleBorrow<Self::X> + Sized,
     {
-        let low = *low_b.borrow();
-        let high = *high_b.borrow();
-        let sampler = crate::hsv::UniformHsv::<S, _>::new_inclusive(
-            Hsv::from_color_unclamped(low),
-            Hsv::from_color_unclamped(high),
+        let low_input = Hsv::from_color_unclamped(*low_b.borrow());
+        let high_input = Hsv::from_color_unclamped(*high_b.borrow());
+
+        let low = Hsv::with_wp(
+            low_input.hue,
+            low_input.saturation.min(high_input.saturation),
+            low_input.value.min(high_input.value),
+        );
+        let high = Hsv::with_wp(
+            high_input.hue,
+            low_input.saturation.max(high_input.saturation),
+            low_input.value.max(high_input.value),
         );
 
+        let sampler = crate::hsv::UniformHsv::<S, _>::new_inclusive(low, high);
+
         UniformHwb {
             sampler,
             space: PhantomData,
@@ -808,4 +825,15 @@ mod test {
 
         assert_eq!(deserialized, Hwb::new(0.3, 0.8, 0.1));
     }
+
+    #[cfg(feature = "random")]
+    test_uniform_distribution! {
+        Hwb<crate::encoding::Srgb, f32> as crate::rgb::Rgb {
+            red: (0.0, 1.0),
+            green: (0.0, 1.0),
+            blue: (0.0, 1.0)
+        },
+        min: Hwb::new(0.0f32, 0.0, 0.0),
+        max: Hwb::new(360.0, 1.0, 1.0)
+    }
 }

palette/src/lab.rs

@@ -713,7 +713,7 @@ where
 pub struct UniformLab<Wp, T>
 where
     T: FloatComponent + SampleUniform,
-    Wp: WhitePoint + SampleUniform,
+    Wp: WhitePoint,
 {
     l: Uniform<T>,
     a: Uniform<T>,
@@ -725,7 +725,7 @@ where
 impl<Wp, T> SampleUniform for Lab<Wp, T>
 where
     T: FloatComponent + SampleUniform,
-    Wp: WhitePoint + SampleUniform,
+    Wp: WhitePoint,
 {
     type Sampler = UniformLab<Wp, T>;
 }
@@ -734,7 +734,7 @@ where
 impl<Wp, T> UniformSampler for UniformLab<Wp, T>
 where
     T: FloatComponent + SampleUniform,
-    Wp: WhitePoint + SampleUniform,
+    Wp: WhitePoint,
 {
     type X = Lab<Wp, T>;
 
@@ -849,4 +849,15 @@ mod test {
 
         assert_eq!(deserialized, Lab::new(0.3, 0.8, 0.1));
     }
+
+    #[cfg(feature = "random")]
+    test_uniform_distribution! {
+        Lab<D65, f32> {
+            l: (0.0, 100.0),
+            a: (-128.0, 127.0),
+            b: (-128.0, 127.0)
+        },
+        min: Lab::new(0.0f32, -128.0, -128.0),
+        max: Lab::new(100.0, 127.0, 127.0)
+    }
 }