Panic documentation for `ImageEncoder::write_image` is outdated
The condition for when ImageEncoder::write_image is expected to panic is currently documented as follows:
/// # Panics
///
/// Panics if `width * height * color_type.bytes_per_pixel() != buf.len()`.
fn write_image(
self,
buf: &[u8],
width: u32,
height: u32,
color_type: ExtendedColorType,
) -> ImageResult<()>;
This doesn't make sense, because ExtendedColorType doesn't have a bytes_per_pixel() method, only bits_per_pixel. This is only natural as ExtendedColorType supports formats like L1.
Currently, it is unclear how ImageEncoder::write_image behaves when color_type.bits_per_pixel() is not divisible by 8.
(Also, this doc comment has been copied a lot. Search for "Panics if `width * height" and you'll find 7 copies in 6 files. There might be more, I didn't search for long.)
As for how this came to be: 9a8591ac35e89fb6043f31574e3955c3fb798e5f added the documentation and #2142 changed the signature of write_image without updating it.
Somewhat related, it would also be nice if the data layout of ExtendedColorType::L1 was explained. I assume that it stores 8 pixels in one byte, but it's not clear to me (1) how it handles images with width % 8 != 0 and (2) which bit corresponds to which pixel (i.e. is the pixel at x=0,y=0 the LSB or MSB in the first byte of buf?). Same for a lot of the other variants.
Thanks for raising this issue! The real requirement on write_image is that the provided byte slice length is precisely what's expected given the color type.
We probably need to write out a description somewhere explaining the expected memory layout for pixel data. The general overview is:
- 8-bit pixel data is arranged sequentially in row-major format without any padding.
- 16-bit and 32-bit data is expected to be in native endian
- sub 8-bit pixel data is packed with multiple samples in a single byte. Rows of pixels are padded to byte boundaries. IIRC both PNG and TIFF follow the same convention on the ordering of bits within a byte and I think we do the same. But I'd have to double check.
- Places that accept byte slices do not require any alignment.