Ensure roundtrips for simple color type

src/buffer/canvas.rs

@@ -52,7 +52,10 @@ impl Canvas {
     /// allocate. Also panics if the allocator fails.
     pub fn new(color: ColorType, w: u32, h: u32) -> Self {
         let texel = color_to_texel(color);
-        let layout = InnerLayout::with_texel(&texel, w, h).expect("layout error");
+        let color = color_to_color(color);
+
+        let mut layout = InnerLayout::with_texel(&texel, w, h).expect("layout error");
+        layout.set_color(color).expect("layout error");
 
         Canvas {
             inner: Inner::new(layout),
@@ -192,6 +195,8 @@ impl Canvas {
     ///
     /// The subpixel of the result type is constrained by a sealed, internal trait that is
     /// implemented for all primitive numeric types.
+    ///
+    /// This is essentially an optimized method compared to using an intermediate `DynamicImage`.
     pub fn to_buffer<P: PixelWithColorType>(&self) -> ImageBuffer<P, Vec<P::Subpixel>>
     where
         [P::Subpixel]: EncodableLayout,
@@ -204,7 +209,8 @@ impl Canvas {
         // convert into the output buffer instead?
         let canvas = if false {
             /* self.inner.layout().texel() == texel */
-            // FIXME: can select `self` if it's correct color type and layout.
+            // FIXME: can select `self` if it's exactly the correct color type and layout. For now
+            // we just note the possibility down in the type system.
             &self.inner
         } else {
             let texel = color_to_texel(P::COLOR_TYPE);
@@ -273,45 +279,61 @@ impl Canvas {
     }
 
     fn color_type(layout: &InnerLayout) -> Option<ColorType> {
-        if let &Color::SRGB = layout.color()? {
-        } else {
-            return None;
-        };
-
         let texel = layout.texel();
+        let color = layout.color()?;
 
-        if let Block::Pixel = texel.block {
-        } else {
+        if !matches!(texel.block, Block::Pixel) {
             return None;
         };
 
-        if let SampleBits::UInt8x3 = texel.bits {
-            if let SampleParts::Rgb = texel.parts {
-                return Some(ColorType::Rgb8);
-            }
-        } else if let SampleBits::UInt8x4 = texel.bits {
-            if let SampleParts::RgbA = texel.parts {
-                return Some(ColorType::Rgba8);
-            }
-        } else if let SampleBits::UInt16x3 = texel.bits {
-            if let SampleParts::Rgb = texel.parts {
-                return Some(ColorType::Rgb16);
+        if let &Color::SRGB = color {
+            // Recognized color type, no color management required.
+            if let SampleBits::UInt8x3 = texel.bits {
+                if let SampleParts::Rgb = texel.parts {
+                    return Some(ColorType::Rgb8);
+                }
+            } else if let SampleBits::UInt8x4 = texel.bits {
+                if let SampleParts::RgbA = texel.parts {
+                    return Some(ColorType::Rgba8);
+                }
+            } else if let SampleBits::UInt16x3 = texel.bits {
+                if let SampleParts::Rgb = texel.parts {
+                    return Some(ColorType::Rgb16);
+                }
+            } else if let SampleBits::UInt16x4 = texel.bits {
+                if let SampleParts::RgbA = texel.parts {
+                    return Some(ColorType::Rgba16);
+                }
+            } else if let SampleBits::Float32x3 = texel.bits {
+                if let SampleParts::Rgb = texel.parts {
+                    return Some(ColorType::Rgb32F);
+                }
+            } else if let SampleBits::Float32x4 = texel.bits {
+                if let SampleParts::RgbA = texel.parts {
+                    return Some(ColorType::Rgba32F);
+                }
             }
-        } else if let SampleBits::UInt16x4 = texel.bits {
-            if let SampleParts::RgbA = texel.parts {
-                return Some(ColorType::Rgba16);
-            }
-        } else if let SampleBits::Float32x3 = texel.bits {
-            if let SampleParts::Rgb = texel.parts {
-                return Some(ColorType::Rgb32F);
-            }
-        } else if let SampleBits::Float32x4 = texel.bits {
-            if let SampleParts::RgbA = texel.parts {
-                return Some(ColorType::Rgba32F);
+        } else if let &Color::BT709 = color {
+            if let SampleBits::UInt8 = texel.bits {
+                if let SampleParts::Luma = texel.parts {
+                    return Some(ColorType::L8);
+                }
+            } else if let SampleBits::UInt8x2 = texel.bits {
+                if let SampleParts::LumaA = texel.parts {
+                    return Some(ColorType::La8);
+                }
+            } else if let SampleBits::UInt16 = texel.bits {
+                if let SampleParts::Luma = texel.parts {
+                    return Some(ColorType::L16);
+                }
+            } else if let SampleBits::UInt16x2 = texel.bits {
+                if let SampleParts::LumaA = texel.parts {
+                    return Some(ColorType::La16);
+                }
             }
         }
 
-        return None;
+        None
     }
 
     fn has_alpha(layout: &InnerLayout) -> bool {
@@ -323,7 +345,11 @@ impl CanvasLayout {
     /// Construct a row-major matrix for a given color type.
     pub fn new(color: ColorType, w: u32, h: u32) -> Option<Self> {
         let texel = color_to_texel(color);
-        let layout = InnerLayout::with_texel(&texel, w, h).ok()?;
+        let color = color_to_color(color);
+
+        let mut layout = InnerLayout::with_texel(&texel, w, h).ok()?;
+        layout.set_color(color).ok()?;
+
         Some(CanvasLayout { inner: layout })
     }
 
@@ -429,6 +455,15 @@ where
     }
 }
 
+impl From<&'_ DynamicImage> for Canvas {
+    fn from(image: &'_ DynamicImage) -> Self {
+        image.to_canvas()
+    }
+}
+
+/// Convert a dynamic image to a canvas.
+///
+/// This *may* reuse the underlying buffer if it is suitably aligned already.
 impl From<DynamicImage> for Canvas {
     fn from(image: DynamicImage) -> Self {
         image.to_canvas()

tests/canvas.rs

@@ -1,4 +1,4 @@
-use image::buffer::Canvas;
+use image::{buffer::Canvas, ColorType, DynamicImage};
 #[cfg(feature = "png")]
 use std::{fs, io};
 
@@ -26,3 +26,73 @@ fn read_canvas() {
     assert_eq!(canvas.width(), 32);
     assert_eq!(canvas.height(), 32);
 }
+
+#[test]
+fn conversion_to_buffer() {
+    let canvas = Canvas::new(ColorType::Rgb8, 32, 32);
+
+    assert!(canvas.as_flat_samples_u8().is_some());
+    assert!(canvas.as_flat_samples_u16().is_none());
+    assert!(canvas.as_flat_samples_f32().is_none());
+
+    assert!(matches!(canvas.to_dynamic(), DynamicImage::ImageRgb8(_)));
+
+    let buffer_rgb8 = canvas.to_buffer::<image::Rgb<u8>>();
+    let buffer_rgb16 = canvas.to_buffer::<image::Rgb<u16>>();
+    let compare_rgb16 = DynamicImage::ImageRgb8(buffer_rgb8).into_rgb16();
+    assert_eq!(buffer_rgb16, compare_rgb16);
+
+    let canvas_rgb16 = Canvas::from(&buffer_rgb16);
+    assert!(canvas_rgb16.as_flat_samples_u16().is_some());
+}
+
+#[test]
+fn test_dynamic_image() {
+    for ct in [
+        ColorType::L8,
+        ColorType::La8,
+        ColorType::Rgb8,
+        ColorType::Rgba8,
+        ColorType::L16,
+        ColorType::La16,
+        ColorType::Rgb16,
+        ColorType::Rgba16,
+        ColorType::Rgb32F,
+        ColorType::Rgba32F,
+    ] {
+        let dynamic = dynamic_image_with_color(ct, 32, 32);
+
+        let canvas = Canvas::new(ct, 32, 32);
+        assert_eq!(
+            core::mem::discriminant(&canvas.to_dynamic()),
+            core::mem::discriminant(&dynamic),
+            "{:?}",
+            ct
+        );
+
+        let canvas = Canvas::from(&dynamic);
+        assert_eq!(
+            core::mem::discriminant(&canvas.to_dynamic()),
+            core::mem::discriminant(&dynamic),
+            "{:?}",
+            ct
+        );
+    }
+}
+
+/// Creates a dynamic image based off a color type for the buffer.
+pub(crate) fn dynamic_image_with_color(color: ColorType, w: u32, h: u32) -> DynamicImage {
+    (match color {
+        ColorType::L8 => DynamicImage::new_luma8,
+        ColorType::La8 => DynamicImage::new_luma_a8,
+        ColorType::Rgb8 => DynamicImage::new_rgb8,
+        ColorType::Rgba8 => DynamicImage::new_rgba8,
+        ColorType::L16 => DynamicImage::new_luma16,
+        ColorType::La16 => DynamicImage::new_luma_a16,
+        ColorType::Rgb16 => DynamicImage::new_rgb16,
+        ColorType::Rgba16 => DynamicImage::new_rgba16,
+        ColorType::Rgb32F => DynamicImage::new_rgb32f,
+        ColorType::Rgba32F => DynamicImage::new_rgba32f,
+        _ => unreachable!(),
+    })(w, h)
+}