Implement a rows() iterator on SubImage and GenericImageView to unlock better performance

[Shnatsel]

Right now the ImageBuffer type has a rows() function to iterate over the rows of the image, but GenericImageView SubImage does not.

Working with rows rather than individual pixels is often much faster, because it lets you leverage SIMD to process multiple pixels at once. For example, #2295 would be trivial to fix given this API.

[fintelia]

What would be the return type of this function? ImageBuffer::rows() gets good performance because it is able to return a wrapper on top of ChunksExact that works really well with SIMD. But GenericImageView currently doesn't make any guarantee pixel layout, so it would presumably have to return something different

[Shnatsel]

Ah, you are correct! With GenericImageView we have no guarantee that pixels in a row are laid out in memory contiguously.

We can however guarantee this about the SubImage type. I've updated the description to make this about SubImage.

[fintelia]

The SubImage type is a wrapper around SubImageInner which is a wrapper around an GenericImageView (the struct itself doesn't have the trait bound, but the methods/impl blocks do have it).

I've gone in circles about this part of the API. First I think that it would be nice to constrain the layout of GenericImage. Then I realize that you can just use ImageBuffer directly if you want that, so I think about deprecating GenericImage. But that would break a lot of downstream code, so might as well just leave it around?

[Shnatsel]

I don't think GenericImageView being that abstract makes a whole lot of sense. Per-pixels accesses will always be slow, so building an API solely around them feels rather pointless.

I think we need a type or a trait for a rectangular sub-image of an existing image, so that we could implement rows() on it. This is what's needed for operations like "make a copy of this region" (crop), "draw something in this region", etc. Naturally it can also be the entire image. We could change GenericImageView to be that, or add an entirely new type. Personally I'd rather change GenericImageView.

[ripytide]

How often is image pixel data non-contiguous, and is it worth catering to the potential non-contiguous image use-cases at the cost of heavier genericness? Additionally, do crop and region drawing functions even need a SubImage type at all rather than simply taking a Rect type?

Perhaps we should simply remove/deprecate all the generic traits since they have dubious usefulness and could be misleading to users.

[kornelski]

From perf perspective having a match on an enum per row is not too bad, as long as the innermost loop can be iterating over a slice (loop per pixel type).

[Shnatsel]

Another bottleneck was pointed out in a trivial 180 rotation function that's currently implemented like this:

image/src/imageops/affine.rs

Lines 90 to 95 in 25fbd3f

	for y in 0..h0 {
	for x in 0..w0 {
	let p = image.get_pixel(x, y);
	destination.put_pixel(w0 - x - 1, h0 - y - 1, p);
	}
	}

We would like to write it like this instead to reduce the amount of bounds checks:

    for (src_row, dest_row) in image.rows().zip(destination.rows_mut().rev()) {
        for (src_pixel, dst_pixel) in (src_row, dest_row.rev()) {
            *dst_pixel = src_pixel;
        }
    }

But that is not possible now because there is no rows() on GenericImageView.

[Shnatsel]

Also there is a need for exposing rows as slices rather than a simple iterator. It is not necessary for a 180 degree rotation function, but for more complex image processing that's going to access each pixel lots of times, like blur, it's absolutely crucial.