Make Take iterator for gradient inclusive of both end colors, add tests

README.md

@@ -99,7 +99,7 @@ This results in the following three colors:
 
 There is also a linear gradient type which makes it easy to interpolate between a series of colors. This gradient can be used in any color space and it can be used to make color sequence iterators.
 
-The following example shows two gradients between the same two endpoints, but one is in RGB and the other in is HSV space.
+The following example shows three gradients between the same two endpoints, but the top is in RGB space while the middle and bottom are in HSV space. The bottom gradient is an example of using the color sequence iterator.
 
 ```Rust
 extern crate palette;
@@ -116,7 +116,7 @@ let grad2 = Gradient::new(vec![
 ]);
 ```
 
-The RGB gradient goes through gray, while the HSV gradients changes only the hue:
+The RGB gradient goes through gray, while the HSV gradients only change hue:
 
 ![Gradient Comparison](gfx/readme_gradients.png)
 

palette/examples/readme_examples.rs

@@ -93,7 +93,7 @@ fn display_gradients<A: Mix<Scalar = f32> + Clone, B: Mix<Scalar = f32> + Clone>
     LinSrgb: From<A>,
     LinSrgb: From<B>,
 {
-    let mut image = RgbImage::new(256, 64);
+    let mut image = RgbImage::new(256, 96);
     {
         let mut sub_image = image.sub_image(0, 0, 256, 32);
         let (width, height) = sub_image.dimensions();
@@ -130,6 +130,26 @@ fn display_gradients<A: Mix<Scalar = f32> + Clone, B: Mix<Scalar = f32> + Clone>
         }
     }
 
+    {
+        let mut sub_image = image.sub_image(0, 64, 256, 32);
+        let swatch_size = 32;
+        let mut v1 = Vec::new();
+        for color in grad2.take(8) {
+            let pix: [u8; 3] = Srgb::from_linear(LinSrgb::from(color))
+                .into_format()
+                .into_raw();
+            v1.push(pix);
+        }
+        for (s, color) in v1.into_iter().enumerate() {
+            for x in (s * swatch_size)..((s + 1) * swatch_size) {
+                for y in 0..swatch_size {
+                    let pixel = sub_image.get_pixel_mut(x as u32, y as u32);
+                    *pixel = image::Rgb(color);
+                }
+            }
+        }
+    }
+
     match image.save(filename) {
         Ok(()) => println!("see '{}' for the result", filename),
         Err(e) => println!("failed to write '{}': {}", filename, e),

palette/src/gradient.rs

@@ -92,7 +92,34 @@ impl<C: Mix + Clone> Gradient<C> {
         min_color.mix(max_color, factor)
     }
 
-    ///Take `n` evenly spaced colors from the gradient, as an iterator.
+    ///Take `n` evenly spaced colors from the gradient, as an iterator. The
+    ///iterator includes both ends of the gradient, for `n > 1`, or just
+    ///the lower end of the gradient for `n = 0`.
+    ///
+    ///For example, `take(5)` will include point 0.0 of the gradient, three
+    ///intermediate colors, and point 1.0 spaced apart at 1/4 the distance
+    ///between colors 0.0 and 1.0 on the gradient.
+    /// ```
+    /// #[macro_use] extern crate approx;
+    /// use palette::{Gradient, LinSrgb};
+    ///
+    /// let gradient = Gradient::new(vec![
+    ///     LinSrgb::new(1.0, 1.0, 0.0),
+    ///     LinSrgb::new(0.0, 0.0, 1.0),
+    /// ]);
+    ///
+    /// let taken_colors: Vec<_> = gradient.take(5).collect();
+    /// let colors = vec![
+    ///     LinSrgb::new(1.0, 1.0, 0.0),
+    ///     LinSrgb::new(0.75, 0.75, 0.25),
+    ///     LinSrgb::new(0.5, 0.5, 0.5),
+    ///     LinSrgb::new(0.25, 0.25, 0.75),
+    ///     LinSrgb::new(0.0, 0.0, 1.0),
+    /// ];
+    /// for (c1, c2) in taken_colors.iter().zip(colors.iter()) {
+    ///     assert_relative_eq!(c1, c2);
+    /// }
+    /// ```
     pub fn take(&self, n: usize) -> Take<C> {
         let (min, max) = self.domain();
 
@@ -142,9 +169,14 @@ impl<'a, C: Mix + Clone> Iterator for Take<'a, C> {
 
     fn next(&mut self) -> Option<C> {
         if self.from_head + self.from_end < self.len {
-            let i = self.from + (self.diff / cast(self.len)) * cast(self.from_head);
-            self.from_head += 1;
-            Some(self.gradient.get(i))
+            if self.len == 1 {
+                self.from_head += 1;
+                Some(self.gradient.get(self.from))
+            } else {
+                let i = self.from + (self.diff / cast(self.len - 1)) * cast(self.from_head);
+                self.from_head += 1;
+                Some(self.gradient.get(i))
+            }
         } else {
             None
         }
@@ -160,9 +192,14 @@ impl<'a, C: Mix + Clone> ExactSizeIterator for Take<'a, C> {}
 impl<'a, C: Mix + Clone> DoubleEndedIterator for Take<'a, C> {
     fn next_back(&mut self) -> Option<Self::Item> {
         if self.from_head + self.from_end < self.len {
-            let i = self.from + (self.diff / cast(self.len)) * cast(self.len - self.from_end - 1);
-            self.from_end += 1;
-            Some(self.gradient.get(i))
+            if self.len == 1 {
+                self.from_end += 1;
+                Some(self.gradient.get(self.from))
+            } else {
+                let i = self.from + (self.diff / cast(self.len - 1)) * cast(self.len - self.from_end - 1);
+                self.from_end += 1;
+                Some(self.gradient.get(i))
+            }
         } else {
             None
         }
@@ -439,7 +476,7 @@ mod test {
         ]);
         let g2 = g1.slice(..0.5);
 
-        let v1: Vec<_> = g1.take(10).take(5).collect();
+        let v1: Vec<_> = g1.take(9).take(5).collect();
         let v2: Vec<_> = g2.take(5).collect();
         for (t1, t2) in v1.iter().zip(v2.iter()) {
             assert_relative_eq!(t1, t2);
@@ -456,5 +493,29 @@ mod test {
         let v1: Vec<_> = g.take(10).collect::<Vec<_>>().iter().rev().cloned().collect();
         let v2: Vec<_> = g.take(10).rev().collect();
         assert_eq!(v1, v2);
+
+        //make sure `take(1).rev()` doesn't produce NaN results
+        let v1: Vec<_> = g.take(1).collect::<Vec<_>>().iter().rev().cloned().collect();
+        let v2: Vec<_> = g.take(1).rev().collect();
+        assert_eq!(v1, v2);
+    }
+
+    #[test]
+    fn inclusive_take() {
+        let g = Gradient::new(vec![
+            LinSrgb::new(1.0, 1.0, 0.0),
+            LinSrgb::new(0.0, 0.0, 1.0),
+        ]);
+
+        //take(0) returns None
+        let v1: Vec<_> = g.take(0).collect();
+        assert_eq!(v1.len(), 0);
+        //`Take` produces minimum gradient boundary for n=1
+        let v1: Vec<_> = g.take(1).collect();
+        assert_eq!(v1[0], LinSrgb::new(1.0, 1.0, 0.0));
+        //`Take` includes the maximum gradient color
+        let v1: Vec<_> = g.take(5).collect();
+        assert_eq!(v1[0], LinSrgb::new(1.0, 1.0, 0.0));
+        assert_eq!(v1[4], LinSrgb::new(0.0, 0.0, 1.0));
     }
 }