Trait for bitmaps with arbitrary impl

[kornelski]

Currently the struct is tied to a particular layout/implementation and doesn't support:

• upside-down bitmaps (negative stride)
• byte-based stride that isn't multiple of full pixels (e.g. 2-byte gaps in RGBA bitmaps)

So perhaps this should be a trait first and foremost.

[federicomenaquintero]

I was hoping to iterate on GdkPixbufs easily with imgref, but they do not guarantee that the stride is a multiple of the pixel size (by default rows are aligned to 32 bits even for packed RGB pixels; stride there can in fact be arbitrary).

I can hack around this by making my own iterator and the rgb crate, to cast &[u8] to &[RGB8].

What kind of trait were you envisioning? I may be able to work on this.

[kornelski]

Currently ImgRef<T> == Img<[T]>, but a buffer with a stride that isn't a multiple of the pixel size in Rust wouldn't be correctly defined as [PixelType] (since access in the middle would be "unaligned" from slice's perspective).

I see two ways around it:

1. Ignore Rust's semantics of [T], and treat it as a byte buffer that T pixels can be read from. From implementation perspective that may be doable with enough unsafe code. Instead of buffer = &buffer[stride..] it could use pointer arithmetic (buffer as *const u8).add(stride_in_bytes) as *const PixelType. For unknown types it would still have to enforce stride to be multiple of size_of to be safe, but there could be an unsafe trait that opts in pixel types to be "unaligned".

2. OR, make a breaking change. Be explicit about the container type being [u8] and use some other trait that describes pixel type, size and can make instances of it from byte slices. Something like ImgRef<T> where T: PixelFormat.

Maybe you could experiment with your own implementation. Lessons I've learned with this one:

• support for width, stride or height being 0 is useful for users, but it was super annoying to retrofit it to my original implementation that assumed they're always non-0 (some methods still don't allow them to be 0).

• I initially used it for a JPEG library where image buffers have to be a multiple of 8 (stride being multiple of 8 pixels, and height too), but images are of any size. I've added various *_padded methods to work with the full buffer (full blocks), but never found them useful beyond original narrow case. And they don't make sense on images cut out via sub_image.

• It would be useful to have support for splitting images into tiles, but supporting splitting into non-overlapping mutable tiles (2d analog of split_at_mut) requires being very strict about controlling access to the underlying buffer. My *_padded methods and public .buf are not compatible with that.

---

In this issue I floated idea of making it a trait, like:

trait ImgLike {
  fn width(&self) -> usize;
  fn rows(&self) -> Self::RowsIter;
  type RowsIter;
}

because I was annoyed that Img<Vec<RGB>> and Img<[RGB]> aren't interchangeable. Due to Rust's limitations, I can't implement Deref or AsRef to convert between them as nicely as Vec<T> to [T].

But in practice having a concrete .as_ref() method to convert isn't too bad.

[federicomenaquintero]

OK, maybe I was thinking about this incorrectly. My first thought was, "OK, I have a buffer allocated by GdkPixbuf with whatever weird stride; imgref can help me turn it into something slice-like so I can operate on it safely".

A few weird things about GdkPixbuf:

• Due to gdk_pixbuf_new_from_data(char *data, size_t stride, width, height), the start of the buffer is not guaranteed to be aligned to anything. In practice it will most likely be aligned to whatever malloc() does.
• Strides are not guaranteed to be multiples of the pixel size.
• Because of weirdness in gdk_pixbuf_get_byte_length(), currently it is hard to know if the last row has a complete rowstride, or if the buffer's allocation really is clipped to the last pixel. (I may fix this soon for pixbufs with buffers created by the library; can't fix it for the general gdk_pixbuf_new_from_data). The way of wrapping a GdkPixbuf in Rust is to ask a pixbuf for its data pointer, and its byte length - but that may not be the complete buffer size.

I've solved the thing I wanted to solve in librsvg by just using the rgb crate and as_pixels() / as_pixels_mut() (while hoping for the best and ignoring alignment requirements of RGB8/RGBA8). So I don't really need to use imgref in librsvg. I'll definitely keep using rgb :)

(I guess this question is, do you intend imgref to be a "wrap me an &[u8] in a 2D slice", or am I just wanting to use it incorrectly.)

I do want to move librsvg away from depending on GdkPixbuf for image loading, and cairo ImageSurface for image operations, so imgref may come in handy with its owned images. We'll see.

[kornelski]

The third point is inherent to the problem, and imgref already has to deal with it. If you take a large bitmap where stride=width, and then make a sub image that is a view of its bottom right corner, you will have an image with stride>width, but the last row won't have any space for the full stride.

Fortunately RGB/RGBA is byte-aligned, so its fine to cast any buffer to it. It's only byte-based stride that makes it tricky to express the whole image as a slice.

This crate is supposed to be a Rusty type-safe Pixbuf. You can wrap vecs and slices into 2D images if they use buf[stride * y + x] to address pixels.