Repository: image-rs/image (language: rust)

Issue:
`.hdr` images decoded as LDR.
In my renderer, I was using the following code to load radiance HDR image (`.hdr` extension), but noticed that all values were getting clamped to [0; 1] range:

```rs
pub fn load_dynamic_image(path: &str) -> Option<DynamicImage> {
    image::io::Reader::open(path).ok()?.decode().ok()
}
```

I was able to fix it by adding a special case:
```rs
pub fn load_dynamic_image(path: &str) -> Option<DynamicImage> {
    // Image crate does not by default decode .hdr images as HDR for some reason
    if path.ends_with(".hdr") {
        let hdr_decoder = image::codecs::hdr::HdrDecoder::new(std::io::BufReader::new(
            std::fs::File::open(&path).unwrap(),
        )).ok()?;
        let width = hdr_decoder.metadata().width;
        let height = hdr_decoder.metadata().height;
        let buffer = hdr_decoder.read_image_hdr().ok()?;
        return Some(DynamicImage::ImageRgb32F(image::ImageBuffer::from_vec(
            width,
            height,
            buffer.into_iter().flat_map(|c| vec![c[0], c[1], c[2]]).collect(),
        )?));
    }

    image::io::Reader::open(path).ok()?.decode().ok()
}
```

I noticed it seems to be caused by this line `HdrAdapter::read_image_data`:
```rs
fn read_image_data(&mut self, buf: &mut [u8]) -> ImageResult<()> {
        assert_eq!(u64::try_from(buf.len()), Ok(self.total_bytes()));
        match self.inner.take() {
            Some(decoder) => {
                let img: Vec<Rgb<u8>> = decoder.read_image_ldr()?; // <-- clamp to LDR
                ...
```
I guess the simple fix would just be to swap the call to `read_image_hdr` instead. I'm mainly opening this bug report to verify if this is intentional or indeed a bug. It seems like too obvious of a mistake to me.

## Expected
`.hdr` image is decoded as HDR values, nothing is clamped.

## Actual behaviour
`.hdr` image is decoded such that all pixels above 1.0 are clamped to 1.0.

## Reproduction steps
Run the code from the first snippet, passing a path to an `.hdr` image. Read the pixel values and observe all will be clamped. Attaching the image file I used to test.
https://dl.polyhaven.org/file/ph-assets/HDRIs/hdr/2k/clarens_midday_2k.hdr
HDR Decoder: float by default
I suggest we replace the default color type of the hdr format. It was rgb8, but instead we should consider making it f32, to avoid loosing information.

 I also thought about whether it was possible to implement `Pixel` for `Rgbe8`, which looks like the optimal solution, since it saves memory but retains the float precision and infinite range. However, there is no color type associated with that, and we cannot directly return a pixel buffer, so that seems impossible.

Also adds some tests for round-trip checks and (soon) pixel checks ns across image formats.

- [x] decode from file to f32 buffers
- [x] fix tests


Discussion / hints:

Anyone remember why this stalled? The changes look reasonable, modulo figuring out whether they include any API-breaking changes.
I believe the following happened:
1. I changed the code until I was pretty confident it might work
2. Add some tests for further confidence
3. Notice that an unconditional panic never is detected by the tests

   In detail: To make the tests fail, I added a simple unconditional panic, here:
   ```rust
   fn read_image_data_f32(&mut self, image_bytes: &mut [u8]) -> ImageResult<()> {
        panic!("dsfowsidjw"); // was never triggered when running any of the tests (existing and new tests)
   ```

Maybe you can check it out and try to investigate @fintelia? Maybe it already works but only the tests need some tweaking
Note: This PR only handles decoding, not encoding yet. 

The encoder itself is already full `f32`, and only needs to be called correctly somewhere else in the library. 

https://github.com/image-rs/image/blob/183e74ea7540da9ce11e613256f8f74640ea70c5/src/codecs/hdr/encoder.rs#L12-L20

It seems like it is never called at all yet? Probably because it does not implement the standard `ImageEncoder` trait yet. This probably needs to be added. Fortunately, this means the encoder will be a non-breaking change :)

It was planned to be called here 

https://github.com/image-rs/image/blob/9ed41d72920322079a2192545528629e57615f71/src/io/free_functions.rs#L182

But we should rather put it here I guess.
https://github.com/image-rs/image/blob/9ed41d72920322079a2192545528629e57615f71/src/io/free_functions.rs#L244-L246
Anyways, do we consider changing the returned `ColorType` to be a breaking change?

Some users might expect the returned type to always be `Rgb8`, even if the library never made such a guarantee. This would mean that with the new version, their code no longer works, failing at runtime.
The short answer: not really, but we don't 100% know.

Downstream could rely on matching `DynamicImage` to get results and panic on mismatching code paths. Code would break even though to the type system it isn't a SemVer major change. Note also, `DynamicImage::from_decoder(…).into_rgba8(self)` would continue to work fine.

Some changes to color types had been made previously but most from rgba to rgb and _not_ to the sample type. Potentially, there could be some code that requires opt-in to th changes in `image::io::Reader` (by specifying some 'support-level') but making a promise to keep color types stable is a significant maintenance burden.

I'd say it's okay, although we may seek to clarify this through the `Reader` or decoder at a future point. With jpeg (CMYK) and avif (Yuv) there are certainly potentially going to be even more drastic color type changes happening in the future.

Fix patch:
diff --git a/src/codecs/hdr/decoder.rs b/src/codecs/hdr/decoder.rs
index 7a64d48355..069372b0b7 100644
--- a/src/codecs/hdr/decoder.rs
+++ b/src/codecs/hdr/decoder.rs
@@ -1,19 +1,13 @@
-use crate::Primitive;
-use num_traits::identities::Zero;
-#[cfg(test)]
-use std::borrow::Cow;
-use std::io::{self, BufRead, Cursor, Read, Seek};
-use std::marker::PhantomData;
+use std::io::{self, Read};
+
 use std::num::{ParseFloatError, ParseIntError};
-use std::path::Path;
-use std::{error, fmt, mem};
+use std::{error, fmt};
 
 use crate::color::{ColorType, Rgb};
 use crate::error::{
-    DecodingError, ImageError, ImageFormatHint, ImageResult, ParameterError, ParameterErrorKind,
-    UnsupportedError, UnsupportedErrorKind,
+    DecodingError, ImageError, ImageFormatHint, ImageResult, UnsupportedError, UnsupportedErrorKind,
 };
-use crate::image::{self, ImageDecoder, ImageDecoderRect, ImageFormat};
+use crate::image::{ImageDecoder, ImageFormat};
 
 /// Errors that can occur during decoding and parsing of a HDR image
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -121,113 +115,6 @@ impl fmt::Display for LineType {
     }
 }
 
-/// Adapter to conform to `ImageDecoder` trait
-#[derive(Debug)]
-pub struct HdrAdapter<R: Read> {
-    inner: Option<HdrDecoder<R>>,
-    // data: Option<Vec<u8>>,
-    meta: HdrMetadata,
-}
-
-impl<R: Read> HdrAdapter<R> {
-    /// Creates adapter
-    pub fn new(r: R) -> ImageResult<HdrAdapter<R>> {
-        let decoder = HdrDecoder::new(r)?;
-        let meta = decoder.metadata();
-        Ok(HdrAdapter {
-            inner: Some(decoder),
-            meta,
-        })
-    }
-
-    /// Allows reading old Radiance HDR images
-    pub fn new_nonstrict(r: R) -> ImageResult<HdrAdapter<R>> {
-        let decoder = HdrDecoder::with_strictness(r, false)?;
-        let meta = decoder.metadata();
-        Ok(HdrAdapter {
-            inner: Some(decoder),
-            meta,
-        })
-    }
-
-    /// Read the actual data of the image, and store it in Self::data.
-    fn read_image_data(&mut self, buf: &mut [u8]) -> ImageResult<()> {
-        assert_eq!(u64::try_from(buf.len()), Ok(self.total_bytes()));
-        match self.inner.take() {
-            Some(decoder) => {
-                let img: Vec<Rgb<u8>> = decoder.read_image_ldr()?;
-                for (i, Rgb(data)) in img.into_iter().enumerate() {
-                    buf[(i * 3)..][..3].copy_from_slice(&data);
-                }
-
-                Ok(())
-            }
-            None => Err(ImageError::Parameter(ParameterError::from_kind(
-                ParameterErrorKind::NoMoreData,
-            ))),
-        }
-    }
-}
-
-/// Wrapper struct around a `Cursor<Vec<u8>>`
-pub struct HdrReader<R>(Cursor<Vec<u8>>, PhantomData<R>);
-impl<R> Read for HdrReader<R> {
-    fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
-        self.0.read(buf)
-    }
-    fn read_to_end(&mut self, buf: &mut Vec<u8>) -> io::Result<usize> {
-        if self.0.position() == 0 && buf.is_empty() {
-            mem::swap(buf, self.0.get_mut());
-            Ok(buf.len())
-        } else {
-            self.0.read_to_end(buf)
-        }
-    }
-}
-
-impl<R: Read> ImageDecoder for HdrAdapter<R> {
-    fn dimensions(&self) -> (u32, u32) {
-        (self.meta.width, self.meta.height)
-    }
-
-    fn color_type(&self) -> ColorType {
-        ColorType::Rgb8
-    }
-
-    fn read_image(mut self, buf: &mut [u8]) -> ImageResult<()> {
-        self.read_image_data(buf)
-    }
-
-    fn read_image_boxed(self: Box<Self>, buf: &mut [u8]) -> ImageResult<()> {
-        (*self).read_image(buf)
-    }
-}
-
-impl<R: BufRead + Seek> ImageDecoderRect for HdrAdapter<R> {
-    fn read_rect(
-        &mut self,
-        x: u32,
-        y: u32,
-        width: u32,
-        height: u32,
-        buf: &mut [u8],
-        row_pitch: usize,
-    ) -> ImageResult<()> {
-        image::load_rect(
-            x,
-            y,
-            width,
-            height,
-            buf,
-            row_pitch,
-            self,
-            self.total_bytes() as usize,
-            |_, _| unreachable!(),
-            |s, buf| s.read_image_data(buf),
-        )
-    }
-}
-
 /// Radiance HDR file signature
 pub const SIGNATURE: &[u8] = b"#?RADIANCE";
 const SIGNATURE_LENGTH: usize = 10;
@@ -244,22 +131,22 @@ pub struct HdrDecoder<R> {
 /// Refer to [wikipedia](https://en.wikipedia.org/wiki/RGBE_image_format)
 #[repr(C)]
 #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
-pub struct Rgbe8Pixel {
+pub(crate) struct Rgbe8Pixel {
     /// Color components
-    pub c: [u8; 3],
+    pub(crate) c: [u8; 3],
     /// Exponent
-    pub e: u8,
+    pub(crate) e: u8,
 }
 
 /// Creates `Rgbe8Pixel` from components
-pub fn rgbe8(r: u8, g: u8, b: u8, e: u8) -> Rgbe8Pixel {
+pub(crate) fn rgbe8(r: u8, g: u8, b: u8, e: u8) -> Rgbe8Pixel {
     Rgbe8Pixel { c: [r, g, b], e }
 }
 
 impl Rgbe8Pixel {
     /// Converts `Rgbe8Pixel` into `Rgb<f32>` linearly
     #[inline]
-    pub fn to_hdr(self) -> Rgb<f32> {
+    pub(crate) fn to_hdr(self) -> Rgb<f32> {
         if self.e == 0 {
             Rgb([0.0, 0.0, 0.0])
         } else {
@@ -272,63 +159,21 @@ impl Rgbe8Pixel {
             ])
         }
     }
-
-    /// Converts `Rgbe8Pixel` into `Rgb<T>` with scale=1 and gamma=2.2
-    ///
-    /// color_ldr = (color_hdr*scale)<sup>gamma</sup>
-    ///
-    /// # Panic
-    ///
-    /// Panics when `T::max_value()` cannot be represented as f32.
-    #[inline]
-    pub fn to_ldr<T: Primitive + Zero>(self) -> Rgb<T> {
-        self.to_ldr_scale_gamma(1.0, 2.2)
-    }
-
-    /// Converts `Rgbe8Pixel` into `Rgb<T>` using provided scale and gamma
-    ///
-    /// color_ldr = (color_hdr*scale)<sup>gamma</sup>
-    ///
-    /// # Panic
-    ///
-    /// Panics when `T::max_value()` cannot be represented as f32.
-    /// Panics when scale or gamma is NaN
-    #[inline]
-    pub fn to_ldr_scale_gamma<T: Primitive + Zero>(self, scale: f32, gamma: f32) -> Rgb<T> {
-        let Rgb(data) = self.to_hdr();
-        let (r, g, b) = (data[0], data[1], data[2]);
-        #[inline]
-        fn sg<T: Primitive + Zero>(v: f32, scale: f32, gamma: f32) -> T {
-            let t_max = T::max_value();
-            // Disassembly shows that t_max_f32 is compiled into constant
-            let t_max_f32: f32 = num_traits::NumCast::from(t_max)
-                .expect("to_ldr_scale_gamma: maximum value of type is not representable as f32");
-            let fv = f32::powf(v * scale, gamma) * t_max_f32 + 0.5;
-            if fv < 0.0 {
-                T::zero()
-            } else if fv > t_max_f32 {
-                t_max
-            } else {
-                num_traits::NumCast::from(fv)
-                    .expect("to_ldr_scale_gamma: cannot convert f32 to target type. NaN?")
-            }
-        }
-        Rgb([
-            sg(r, scale, gamma),
-            sg(g, scale, gamma),
-            sg(b, scale, gamma),
-        ])
-    }
 }
 
 impl<R: Read> HdrDecoder<R> {
     /// Reads Radiance HDR image header from stream ```r```
     /// if the header is valid, creates HdrDecoder
     /// strict mode is enabled
-    pub fn new(reader: R) -> ImageResult<HdrDecoder<R>> {
+    pub fn new(reader: R) -> ImageResult<Self> {
         HdrDecoder::with_strictness(reader, true)
     }
 
+    /// Allows reading old Radiance HDR images
+    pub fn new_nonstrict(reader: R) -> ImageResult<Self> {
+        Self::with_strictness(reader, false)
+    }
+
     /// Reads Radiance HDR image header from stream `reader`,
     /// if the header is valid, creates `HdrDecoder`.
     ///
@@ -421,23 +266,8 @@ impl<R: Read> HdrDecoder<R> {
         self.meta.clone()
     }
 
-    /// Consumes decoder and returns a vector of RGBE8 pixels
-    pub fn read_image_native(mut self) -> ImageResult<Vec<Rgbe8Pixel>> {
-        // Don't read anything if image is empty
-        if self.width == 0 || self.height == 0 {
-            return Ok(vec![]);
-        }
-        // expression self.width > 0 && self.height > 0 is true from now to the end of this method
-        let pixel_count = self.width as usize * self.height as usize;
-        let mut ret = vec![Default::default(); pixel_count];
-        for chunk in ret.chunks_mut(self.width as usize) {
-            read_scanline(&mut self.r, chunk)?;
-        }
-        Ok(ret)
-    }
-
     /// Consumes decoder and returns a vector of transformed pixels
-    pub fn read_image_transform<T: Send, F: Send + Sync + Fn(Rgbe8Pixel) -> T>(
+    fn read_image_transform<T: Send, F: Send + Sync + Fn(Rgbe8Pixel) -> T>(
         mut self,
         f: F,
         output_slice: &mut [T],
@@ -465,118 +295,35 @@ impl<R: Read> HdrDecoder<R> {
         }
         Ok(())
     }
+}
 
-    /// Consumes decoder and returns a vector of `Rgb<u8>` pixels.
-    /// scale = 1, gamma = 2.2
-    pub fn read_image_ldr(self) -> ImageResult<Vec<Rgb<u8>>> {
-        let mut ret = vec![Rgb([0, 0, 0]); self.width as usize * self.height as usize];
-        self.read_image_transform(|pix| pix.to_ldr(), &mut ret[..])?;
-        Ok(ret)
+impl<R: Read> ImageDecoder for HdrDecoder<R> {
+    fn dimensions(&self) -> (u32, u32) {
+        (self.meta.width, self.meta.height)
     }
 
-    /// Consumes decoder and returns a vector of `Rgb<f32>` pixels.
-    ///
-    pub fn read_image_hdr(self) -> ImageResult<Vec<Rgb<f32>>> {
-        let mut ret = vec![Rgb([0.0, 0.0, 0.0]); self.width as usize * self.height as usize];
-        self.read_image_transform(|pix| pix.to_hdr(), &mut ret[..])?;
-        Ok(ret)
+    fn color_type(&self) -> ColorType {
+        ColorType::Rgb32F
     }
-}
-
-impl<R: Read> IntoIterator for HdrDecoder<R> {
-    type Item = ImageResult<Rgbe8Pixel>;
-    type IntoIter = HdrImageDecoderIterator<R>;
 
-    fn into_iter(self) -> Self::IntoIter {
-        HdrImageDecoderIterator {
-            r: self.r,
-            scanline_cnt: self.height as usize,
-            buf: vec![Default::default(); self.width as usize],
-            col: 0,
-            scanline: 0,
-            trouble: true, // make first call to `next()` read scanline
-            error_encountered: false,
-        }
-    }
-}
+    fn read_image(self, buf: &mut [u8]) -> ImageResult<()> {
+        assert_eq!(u64::try_from(buf.len()), Ok(self.total_bytes()));
 
-/// Scanline buffered pixel by pixel iterator
-pub struct HdrImageDecoderIterator<R: Read> {
-    r: R,
-    scanline_cnt: usize,
-    buf: Vec<Rgbe8Pixel>, // scanline buffer
-    col: usize,           // current position in scanline
-    scanline: usize,      // current scanline
-    trouble: bool,        // optimization, true indicates that we need to check something
-    error_encountered: bool,
-}
+        let mut img = vec![Rgb([0.0, 0.0, 0.0]); self.width as usize * self.height as usize];
+        self.read_image_transform(|pix| pix.to_hdr(), &mut img[..])?;
 
-impl<R: Read> HdrImageDecoderIterator<R> {
-    // Advances counter to the next pixel
-    #[inline]
-    fn advance(&mut self) {
-        self.col += 1;
-        if self.col == self.buf.len() {
-            self.col = 0;
-            self.scanline += 1;
-            self.trouble = true;
+        for (i, Rgb(data)) in img.into_iter().enumerate() {
+            buf[(i * 12)..][..12].copy_from_slice(bytemuck::cast_slice(&data));
         }
-    }
-}
-
-impl<R: Read> Iterator for HdrImageDecoderIterator<R> {
-    type Item = ImageResult<Rgbe8Pixel>;
 
-    fn next(&mut self) -> Option<Self::Item> {
-        if !self.trouble {
-            let ret = self.buf[self.col];
-            self.advance();
-            Some(Ok(ret))
-        } else {
-            // some condition is pending
-            if self.buf.is_empty() || self.scanline == self.scanline_cnt {
-                // No more pixels
-                return None;
-            } // no else
-            if self.error_encountered {
-                self.advance();
-                // Error was encountered. Keep producing errors.
-                // ImageError can't implement Clone, so just dump some error
-                return Some(Err(ImageError::Parameter(ParameterError::from_kind(
-                    ParameterErrorKind::FailedAlready,
-                ))));
-            } // no else
-            if self.col == 0 {
-                // fill scanline buffer
-                match read_scanline(&mut self.r, &mut self.buf[..]) {
-                    Ok(_) => {
-                        // no action required
-                    }
-                    Err(err) => {
-                        self.advance();
-                        self.error_encountered = true;
-                        self.trouble = true;
-                        return Some(Err(err));
-                    }
-                }
-            } // no else
-            self.trouble = false;
-            let ret = self.buf[0];
-            self.advance();
-            Some(Ok(ret))
-        }
+        Ok(())
     }
 
-    fn size_hint(&self) -> (usize, Option<usize>) {
-        let total_cnt = self.buf.len() * self.scanline_cnt;
-        let cur_cnt = self.buf.len() * self.scanline + self.col;
-        let remaining = total_cnt - cur_cnt;
-        (remaining, Some(remaining))
+    fn read_image_boxed(self: Box<Self>, buf: &mut [u8]) -> ImageResult<()> {
+        (*self).read_image(buf)
     }
 }
 
-impl<R: Read> ExactSizeIterator for HdrImageDecoderIterator<R> {}
-
 // Precondition: buf.len() > 0
 fn read_scanline<R: Read>(r: &mut R, buf: &mut [Rgbe8Pixel]) -> ImageResult<()> {
     assert!(!buf.is_empty());
@@ -936,30 +683,6 @@ fn split_at_first<'a>(s: &'a str, separator: &str) -> Option<(&'a str, &'a str)>
     }
 }
 
-#[test]
-fn split_at_first_test() {
-    assert_eq!(split_at_first(&Cow::Owned("".into()), "="), None);
-    assert_eq!(split_at_first(&Cow::Owned("=".into()), "="), None);
-    assert_eq!(split_at_first(&Cow::Owned("= ".into()), "="), None);
-    assert_eq!(
-        split_at_first(&Cow::Owned(" = ".into()), "="),
-        Some((" ", " "))
-    );
-    assert_eq!(
-        split_at_first(&Cow::Owned("EXPOSURE= ".into()), "="),
-        Some(("EXPOSURE", " "))
-    );
-    assert_eq!(
-        split_at_first(&Cow::Owned("EXPOSURE= =".into()), "="),
-        Some(("EXPOSURE", " ="))
-    );
-    assert_eq!(
-        split_at_first(&Cow::Owned("EXPOSURE== =".into()), "=="),
-        Some(("EXPOSURE", " ="))
-    );
-    assert_eq!(split_at_first(&Cow::Owned("EXPOSURE".into()), ""), None);
-}
-
 // Reads input until b"\n" or EOF
 // Returns vector of read bytes NOT including end of line characters
 //   or return None to indicate end of file
@@ -978,50 +701,54 @@ fn read_line_u8<R: Read>(r: &mut R) -> ::std::io::Result<Option<Vec<u8>>> {
     }
 }
 
-#[test]
-fn read_line_u8_test() {
-    let buf: Vec<_> = (&b"One\nTwo\nThree\nFour\n\n\n"[..]).into();
-    let input = &mut ::std::io::Cursor::new(buf);
-    assert_eq!(&read_line_u8(input).unwrap().unwrap()[..], &b"One"[..]);
-    assert_eq!(&read_line_u8(input).unwrap().unwrap()[..], &b"Two"[..]);
-    assert_eq!(&read_line_u8(input).unwrap().unwrap()[..], &b"Three"[..]);
-    assert_eq!(&read_line_u8(input).unwrap().unwrap()[..], &b"Four"[..]);
-    assert_eq!(&read_line_u8(input).unwrap().unwrap()[..], &b""[..]);
-    assert_eq!(&read_line_u8(input).unwrap().unwrap()[..], &b""[..]);
-    assert_eq!(read_line_u8(input).unwrap(), None);
-}
+#[cfg(test)]
+mod tests {
+    use std::{borrow::Cow, io::Cursor};
 
-/// Helper function for reading raw 3-channel f32 images
-pub fn read_raw_file<P: AsRef<Path>>(path: P) -> ::std::io::Result<Vec<Rgb<f32>>> {
-    use byteorder::{LittleEndian as LE, ReadBytesExt};
-    use std::fs::File;
-    use std::io::BufReader;
+    use super::*;
 
-    let mut r = BufReader::new(File::open(path)?);
-    let w = r.read_u32::<LE>()? as usize;
-    let h = r.read_u32::<LE>()? as usize;
-    let c = r.read_u32::<LE>()? as usize;
-    assert_eq!(c, 3);
-    let cnt = w * h;
-    let mut ret = Vec::with_capacity(cnt);
-    for _ in 0..cnt {
-        let cr = r.read_f32::<LE>()?;
-        let cg = r.read_f32::<LE>()?;
-        let cb = r.read_f32::<LE>()?;
-        ret.push(Rgb([cr, cg, cb]));
+    #[test]
+    fn split_at_first_test() {
+        assert_eq!(split_at_first(&Cow::Owned("".into()), "="), None);
+        assert_eq!(split_at_first(&Cow::Owned("=".into()), "="), None);
+        assert_eq!(split_at_first(&Cow::Owned("= ".into()), "="), None);
+        assert_eq!(
+            split_at_first(&Cow::Owned(" = ".into()), "="),
+            Some((" ", " "))
+        );
+        assert_eq!(
+            split_at_first(&Cow::Owned("EXPOSURE= ".into()), "="),
+            Some(("EXPOSURE", " "))
+        );
+        assert_eq!(
+            split_at_first(&Cow::Owned("EXPOSURE= =".into()), "="),
+            Some(("EXPOSURE", " ="))
+        );
+        assert_eq!(
+            split_at_first(&Cow::Owned("EXPOSURE== =".into()), "=="),
+            Some(("EXPOSURE", " ="))
+        );
+        assert_eq!(split_at_first(&Cow::Owned("EXPOSURE".into()), ""), None);
     }
-    Ok(ret)
-}
 
-#[cfg(test)]
-mod test {
-    use super::*;
+    #[test]
+    fn read_line_u8_test() {
+        let buf: Vec<_> = (&b"One\nTwo\nThree\nFour\n\n\n"[..]).into();
+        let input = &mut ::std::io::Cursor::new(buf);
+        assert_eq!(&read_line_u8(input).unwrap().unwrap()[..], &b"One"[..]);
+        assert_eq!(&read_line_u8(input).unwrap().unwrap()[..], &b"Two"[..]);
+        assert_eq!(&read_line_u8(input).unwrap().unwrap()[..], &b"Three"[..]);
+        assert_eq!(&read_line_u8(input).unwrap().unwrap()[..], &b"Four"[..]);
+        assert_eq!(&read_line_u8(input).unwrap().unwrap()[..], &b""[..]);
+        assert_eq!(&read_line_u8(input).unwrap().unwrap()[..], &b""[..]);
+        assert_eq!(read_line_u8(input).unwrap(), None);
+    }
 
     #[test]
     fn dimension_overflow() {
         let data = b"#?RADIANCE\nFORMAT=32-bit_rle_rgbe\n\n -Y 4294967295 +X 4294967295";
 
-        assert!(HdrAdapter::new(Cursor::new(data)).is_err());
-        assert!(HdrAdapter::new_nonstrict(Cursor::new(data)).is_err());
+        assert!(HdrDecoder::new(Cursor::new(data)).is_err());
+        assert!(HdrDecoder::new_nonstrict(Cursor::new(data)).is_err());
     }
 }
diff --git a/src/codecs/hdr/encoder.rs b/src/codecs/hdr/encoder.rs
index aa97186cf3..86b4fcf189 100644
--- a/src/codecs/hdr/encoder.rs
+++ b/src/codecs/hdr/encoder.rs
@@ -227,7 +227,7 @@ fn write_rgbe8<W: Write>(w: &mut W, v: Rgbe8Pixel) -> Result<()> {
 }
 
 /// Converts ```Rgb<f32>``` into ```Rgbe8Pixel```
-pub fn to_rgbe8(pix: Rgb<f32>) -> Rgbe8Pixel {
+pub(crate) fn to_rgbe8(pix: Rgb<f32>) -> Rgbe8Pixel {
     let pix = pix.0;
     let mx = f32::max(pix[0], f32::max(pix[1], pix[2]));
     if mx <= 0.0 {
diff --git a/src/codecs/openexr.rs b/src/codecs/openexr.rs
index 86ca2a9298..13177c4a49 100644
--- a/src/codecs/openexr.rs
+++ b/src/codecs/openexr.rs
@@ -336,12 +336,13 @@ fn to_image_err(exr_error: Error) -> ImageError {
 mod test {
     use super::*;
 
+    use std::fs::File;
     use std::io::{BufReader, Cursor};
     use std::path::{Path, PathBuf};
 
     use crate::buffer_::{Rgb32FImage, Rgba32FImage};
     use crate::error::{LimitError, LimitErrorKind};
-    use crate::{ImageBuffer, Rgb, Rgba};
+    use crate::{DynamicImage, ImageBuffer, Rgb, Rgba};
 
     const BASE_PATH: &[&str] = &[".", "tests", "images", "exr"];
 
@@ -417,26 +418,25 @@ mod test {
 
         #[cfg(feature = "hdr")]
         {
+            use crate::codecs::hdr::HdrDecoder;
+
             let folder = BASE_PATH.iter().collect::<PathBuf>();
             let reference_path = folder.clone().join("overexposed gradient.hdr");
             let exr_path = folder
                 .clone()
                 .join("overexposed gradient - data window equals display window.exr");
 
-            let hdr: Vec<Rgb<f32>> = crate::codecs::hdr::HdrDecoder::new(std::io::BufReader::new(
-                std::fs::File::open(reference_path).unwrap(),
-            ))
-            .unwrap()
-            .read_image_hdr()
-            .unwrap();
+            let hdr_decoder =
+                HdrDecoder::new(BufReader::new(File::open(reference_path).unwrap())).unwrap();
+            let hdr: Rgb32FImage = match DynamicImage::from_decoder(hdr_decoder).unwrap() {
+                DynamicImage::ImageRgb32F(image) => image,
+                _ => panic!("expected rgb32f image"),
+            };
 
             let exr_pixels: Rgb32FImage = read_as_rgb_image_from_file(exr_path).unwrap();
-            assert_eq!(
-                exr_pixels.dimensions().0 * exr_pixels.dimensions().1,
-                hdr.len() as u32
-            );
+            assert_eq!(exr_pixels.dimensions(), hdr.dimensions());
 
-            for (expected, found) in hdr.iter().zip(exr_pixels.pixels()) {
+            for (expected, found) in hdr.pixels().zip(exr_pixels.pixels()) {
                 for (expected, found) in expected.0.iter().zip(found.0.iter()) {
                     // the large tolerance seems to be caused by
                     // the RGBE u8x4 pixel quantization of the hdr image format
diff --git a/src/io/reader.rs b/src/io/reader.rs
index 75565de73f..700de037a3 100644
--- a/src/io/reader.rs
+++ b/src/io/reader.rs
@@ -168,7 +168,7 @@ impl<'a, R: 'a + Read + Seek> Reader<R> {
             #[cfg(feature = "ico")]
             ImageFormat::Ico => Box::new(ico::IcoDecoder::new(reader)?),
             #[cfg(feature = "hdr")]
-            ImageFormat::Hdr => Box::new(hdr::HdrAdapter::new(reader)?),
+            ImageFormat::Hdr => Box::new(hdr::HdrDecoder::new(reader)?),
             #[cfg(feature = "exr")]
             ImageFormat::OpenExr => Box::new(openexr::OpenExrDecoder::new(reader)?),
             #[cfg(feature = "pnm")]


Test patch:
diff --git a/tests/reference/hdr/images/image1.hdr.bd02cf8f.png b/tests/reference/hdr/images/image1.hdr.bd02cf8f.png
deleted file mode 100644
index a4682a0d5b..0000000000
Binary files a/tests/reference/hdr/images/image1.hdr.bd02cf8f.png and /dev/null differ
diff --git a/tests/reference/hdr/images/rgbr4x4.hdr.72ffeca1.png b/tests/reference/hdr/images/rgbr4x4.hdr.72ffeca1.png
deleted file mode 100644
index 0172c3a2f0..0000000000
Binary files a/tests/reference/hdr/images/rgbr4x4.hdr.72ffeca1.png and /dev/null differ
diff --git a/tests/reference_images.rs b/tests/reference_images.rs
index 2f10a5f8ee..9defa31201 100644
--- a/tests/reference_images.rs
+++ b/tests/reference_images.rs
@@ -45,6 +45,10 @@ fn render_images() {
     process_images(IMAGE_DIR, None, |base, path, decoder| {
         println!("render_images {}", path.display());
         let img = match image::open(&path) {
+            Ok(DynamicImage::ImageRgb32F(_)) | Ok(DynamicImage::ImageRgba32F(_)) => {
+                println!("Skipping {} - HDR codec is not enabled", path.display());
+                return;
+            }
             Ok(img) => img,
             // Do not fail on unsupported error
             // This might happen because the testsuite contains unsupported images
@@ -311,6 +315,28 @@ fn check_references() {
 #[cfg(feature = "hdr")]
 #[test]
 fn check_hdr_references() {
+    use byteorder::{LittleEndian as LE, ReadBytesExt};
+    use std::fs::File;
+    use std::io::BufReader;
+    use std::path::Path;
+
+    /// Helper function for reading raw 3-channel f32 images
+    fn read_raw_file<P: AsRef<Path>>(path: P) -> ::std::io::Result<Vec<f32>> {
+        let mut r = BufReader::new(File::open(path)?);
+        let w = r.read_u32::<LE>()? as usize;
+        let h = r.read_u32::<LE>()? as usize;
+        let c = r.read_u32::<LE>()? as usize;
+        assert_eq!(c, 3);
+        let cnt = w * h;
+        let mut ret = Vec::with_capacity(cnt);
+        for _ in 0..cnt {
+            ret.push(r.read_f32::<LE>()?);
+            ret.push(r.read_f32::<LE>()?);
+            ret.push(r.read_f32::<LE>()?);
+        }
+        Ok(ret)
+    }
+
     let mut ref_path: PathBuf = BASE_PATH.iter().collect();
     ref_path.push(REFERENCE_DIR);
     ref_path.push("hdr");
@@ -343,8 +369,12 @@ fn check_hdr_references() {
         let decoder =
             image::codecs::hdr::HdrDecoder::new(io::BufReader::new(fs::File::open(&path).unwrap()))
                 .unwrap();
-        let decoded = decoder.read_image_hdr().unwrap();
-        let reference = image::codecs::hdr::read_raw_file(&ref_path).unwrap();
+        let decoded = match DynamicImage::from_decoder(decoder).unwrap() {
+            DynamicImage::ImageRgb32F(img) => img.into_vec(),
+            _ => unreachable!(),
+        };
+
+        let reference = read_raw_file(&ref_path).unwrap();
         assert_eq!(decoded, reference);
     }
 }