fixup color image bugs

Author: gaker

celestial-images/src/formats/image/fits.rs

@@ -54,40 +54,55 @@ impl Image {
 
         let keywords = header.keywords().to_vec();
 
-        Ok(Self {
+        let mut img = Self {
             pixels,
             dimensions,
             keywords,
             xisf_properties: Vec::new(),
-        })
+        };
+        // FITS stores multi-channel images planar (all R, then all G,
+        // then all B). Callers expect interleaved layout for display.
+        img.planar_to_interleaved();
+        Ok(img)
     }
 
     pub(super) fn save_fits(&self, path: &Path) -> Result<()> {
         let mut writer = FitsWriter::create(path).map_err(ImageError::Fits)?;
 
-        match &self.pixels {
+        // FITS expects multi-channel data in planar layout (NAXIS3
+        // chunks each holding one full channel). In-memory we keep
+        // it interleaved, so flip for the write.
+        let source = if self.channels() == 3 {
+            let mut planar = self.clone();
+            planar.interleaved_to_planar();
+            std::borrow::Cow::Owned(planar)
+        } else {
+            std::borrow::Cow::Borrowed(self)
+        };
+
+        match &source.pixels {
             PixelData::U8(data) => writer
-                .write_primary_image(data, &self.dimensions, &self.keywords)
+                .write_primary_image(data, &source.dimensions, &source.keywords)
                 .map_err(ImageError::Fits)?,
             PixelData::U16(data) => {
                 let shifted: Vec<i16> =
                     data.iter().map(|&v| (v as i32 - 32768) as i16).collect();
-                let keywords = with_u16_scaling(&self.keywords);
+                let keywords = with_u16_scaling(&source.keywords);
                 writer
-                    .write_primary_image(&shifted, &self.dimensions, &keywords)
+                    .write_primary_image(&shifted, &source.dimensions, &keywords)
                     .map_err(ImageError::Fits)?
             }
             PixelData::I16(data) => writer
-                .write_primary_image(data, &self.dimensions, &self.keywords)
+                .write_primary_image(data, &source.dimensions, &source.keywords)
                 .map_err(ImageError::Fits)?,
             PixelData::I32(data) => writer
-                .write_primary_image(data, &self.dimensions, &self.keywords)
+                .write_primary_image(data, &source.dimensions, &source.keywords)
                 .map_err(ImageError::Fits)?,
             PixelData::F32(data) => writer
-                .write_primary_image(data, &self.dimensions, &self.keywords)
+                .write_primary_image(data, &source.dimensions, &source.keywords)
                 .map_err(ImageError::Fits)?,
             PixelData::F64(data) => writer
-                .write_primary_image(data, &self.dimensions, &self.keywords)
+                .write_primary_image(data, &source.dimensions, &source.keywords)
                 .map_err(ImageError::Fits)?,
         }
 
@@ -202,6 +217,27 @@ mod tests {
         assert_eq!(bscale.as_real(), Some(1.0));
     }
 
+    #[test]
+    fn roundtrip_rgb_f32_stays_interleaved_in_memory() {
+        // In-memory layout must be interleaved (RGB RGB RGB) both before
+        // save and after load — the on-disk FITS layout is planar but
+        // that conversion is handled transparently.
+        let tmp = tmp_fits();
+        let interleaved: Vec<f32> = vec![
+            1.0, 2.0, 3.0,    // pixel (0,0)
+            4.0, 5.0, 6.0,    // pixel (1,0)
+            7.0, 8.0, 9.0,    // pixel (0,1)
+            10.0, 11.0, 12.0, // pixel (1,1)
+        ];
+        let img = Image::new(PixelData::F32(interleaved.clone()), vec![2usize, 2, 3]);
+        img.save(tmp.path()).unwrap();
+
+        let restored = Image::open(tmp.path()).unwrap();
+        assert!(restored.is_rgb());
+        assert_eq!(restored.dimensions, vec![2usize, 2, 3]);
+        assert_eq!(restored.pixels.as_f32().unwrap(), &interleaved);
+    }
+
     #[test]
     fn keywords_roundtrip() {
         let tmp = tmp_fits();

celestial-images/src/formats/image/tiff.rs

@@ -3,6 +3,7 @@ use crate::core::{ImageError, Result};
 use crate::formats::pixel_data::PixelData;
 use std::path::Path;
 use tiff::encoder::{colortype, TiffEncoder};
+use tiff::ColorType;
 
 impl Image {
     pub fn open_tiff<P: AsRef<Path>>(path: P) -> Result<Self> {
@@ -16,12 +17,21 @@ impl Image {
             ImageError::FormatDetectionFailed(format!("TIFF dimensions error: {}", e))
         })?;
 
+        // Samples-per-pixel comes from the TIFF colortype, not from the
+        // decoded buffer (which is always flat). Without this, RGB TIFFs
+        // get reported as 1-channel and downstream code treats the
+        // interleaved R/G/B samples as 3× as many grayscale pixels.
+        let colortype = decoder.colortype().map_err(|e| {
+            ImageError::FormatDetectionFailed(format!("TIFF colortype error: {}", e))
+        })?;
+        let samples = samples_per_pixel(colortype);
+
         let image = decoder
             .read_image()
             .map_err(|e| ImageError::FormatDetectionFailed(format!("TIFF read error: {}", e)))?;
 
-        let (pixels, channels) = decode_tiff_image(image)?;
-        let dimensions = build_tiff_dimensions(width, height, channels);
+        let pixels = decode_tiff_image(image)?;
+        let dimensions = build_tiff_dimensions(width, height, samples);
 
         Ok(Self {
             pixels,
@@ -41,18 +51,30 @@ impl Image {
     }
 }
 
-fn decode_tiff_image(image: tiff::decoder::DecodingResult) -> Result<(PixelData, usize)> {
+fn samples_per_pixel(colortype: ColorType) -> usize {
+    match colortype {
+        ColorType::Gray(_) | ColorType::Palette(_) => 1,
+        ColorType::GrayA(_) => 2,
+        ColorType::RGB(_) | ColorType::YCbCr(_) | ColorType::Lab(_) => 3,
+        ColorType::RGBA(_) | ColorType::CMYK(_) => 4,
+        ColorType::CMYKA(_) => 5,
+        ColorType::Multiband { num_samples, .. } => num_samples as usize,
+        _ => 1,
+    }
+}
+
+fn decode_tiff_image(image: tiff::decoder::DecodingResult) -> Result<PixelData> {
     use tiff::decoder::DecodingResult;
 
     match image {
-        DecodingResult::U8(data) => Ok((PixelData::U8(data), 1)),
-        DecodingResult::U16(data) => Ok((PixelData::U16(data), 1)),
+        DecodingResult::U8(data) => Ok(PixelData::U8(data)),
+        DecodingResult::U16(data) => Ok(PixelData::U16(data)),
         DecodingResult::U32(data) => {
             let converted: Vec<i32> = data.iter().map(|&v| v as i32).collect();
-            Ok((PixelData::I32(converted), 1))
+            Ok(PixelData::I32(converted))
         }
-        DecodingResult::F32(data) => Ok((PixelData::F32(data), 1)),
-        DecodingResult::F64(data) => Ok((PixelData::F64(data), 1)),
+        DecodingResult::F32(data) => Ok(PixelData::F32(data)),
+        DecodingResult::F64(data) => Ok(PixelData::F64(data)),
         _ => Err(ImageError::UnsupportedFormat),
     }
 }
@@ -84,6 +106,9 @@ fn write_tiff_image(
         (PixelData::F32(data), 1) => {
             encoder.write_image::<colortype::Gray32Float>(width, height, data)
         }
+        (PixelData::F32(data), 3) => {
+            encoder.write_image::<colortype::RGB32Float>(width, height, data)
+        }
         _ => return Err(ImageError::UnsupportedFormat),
     }
     .map_err(|e| ImageError::FormatDetectionFailed(format!("TIFF write error: {}", e)))
@@ -147,6 +172,35 @@ mod tests {
         img.save(tmp.path()).unwrap();
     }
 
+    #[test]
+    fn roundtrip_rgb_u8_reports_three_channels() {
+        let tmp = tmp_tiff();
+        let img = Image::new(PixelData::U8((0u8..48).collect()), vec![4usize, 4, 3]);
+        img.save(tmp.path()).unwrap();
+
+        let restored = Image::open_tiff(tmp.path()).unwrap();
+        assert!(restored.is_rgb(), "RGB TIFF should round-trip with channels==3");
+        assert_eq!(restored.channels(), 3);
+        assert_eq!(restored.width(), 4);
+        assert_eq!(restored.height(), 4);
+        assert_eq!(restored.pixels.as_u8().unwrap().len(), 48);
+    }
+
+    #[test]
+    fn roundtrip_rgb_u16_reports_three_channels() {
+        let tmp = tmp_tiff();
+        let img = Image::new(
+            PixelData::U16((0u16..48).map(|i| i * 1000).collect()),
+            vec![4usize, 4, 3],
+        );
+        img.save(tmp.path()).unwrap();
+
+        let restored = Image::open_tiff(tmp.path()).unwrap();
+        assert!(restored.is_rgb());
+        assert_eq!(restored.channels(), 3);
+        assert_eq!(restored.pixels.as_u16().unwrap().len(), 48);
+    }
+
     #[test]
     fn save_rejects_unsupported_combos() {
         let tmp = tmp_tiff();

celestial-images/src/formats/image/xisf.rs

@@ -54,12 +54,17 @@ impl Image {
 
         let keywords = xisf.keywords().to_vec();
 
-        Ok(Self {
+        let mut img = Self {
             pixels,
             dimensions,
             keywords,
             xisf_properties: Vec::new(),
-        })
+        };
+        // XISF normal storage is read back in planar order
+        // (`deinterleave_normal_storage` does the deinterleave). Callers
+        // expect interleaved RGB for display, so reinterleave here.
+        img.planar_to_interleaved();
+        Ok(img)
     }
 
     pub(super) fn save_xisf(&self, path: &Path) -> Result<()> {
@@ -211,6 +216,9 @@ mod tests {
         let restored = Image::open(tmp.path()).unwrap();
         assert_eq!(restored.dimensions, vec![4usize, 4, 3]);
         assert_eq!(restored.channels(), 3);
+        // Pixel layout in memory must be interleaved (RGB RGB ...) so
+        // display code can pass the buffer straight to a texture upload.
+        assert_eq!(restored.pixels.as_u8().unwrap(), &original);
     }
 
     #[test]