Skip to content

Unnecessary memset()+memcpy() in HDR decoding #1882

@Shnatsel

Description

@Shnatsel

As of image 0.24.5, the HDR decoding needlessly copies the entire output in memory, acquiring a Vec and then calling copy_from_slice to create another Vec:

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,
))),
}
}

The following function can directly return the Vec it gets from the underlying decoder:

    /// Read the actual data of the image, and store it in Self::data.
    fn get_image_data(&mut self, buf: &mut [u8]) -> ImageResult<Vec<u8>> {
        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()?;
                Ok(bytemuck::allocation::cast_vec(img))
            }
            None => Err(ImageError::Parameter(ParameterError::from_kind(
                ParameterErrorKind::NoMoreData,
            ))),
        }
    }

..or it could if image's Rgb struct implemented bytemuck::Pod. Right now it doesn't, which causes the above to fail to compile. I understand this would require an unsafe impl since a #[derive] won't work on a generic struct.

Once the Pod implementation is sorted, the HDR decoding can be specialized to return the underlying Vec where possible similar to how it's done for JPEG in #1879

Metadata

Metadata

Assignees

No one assigned

    Labels

    kind: slowNot wrong, but still unusabletopic: formatsTowards better encoding format coverage

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions