image icon indicating copy to clipboard operation
image copied to clipboard

Add `par_enumerate_rows(_mut)` to `ImageBuffer` and friends

Open JSorngard opened this issue 5 months ago • 6 comments

Thanks for all the work that goes into this crate!
I would like to be able to iterate over the rows of an image (ideally mutably).

I need all pixels to one side of the y-axis to be finished rendering before I can render the others. A parallel iterator that returns references to the rows would be very helpful here as that would give me control of which parts of the row I render first.

This is more generally applicable to anyone that wants to speed up the normal enumerate_rows(_mut) by using more cores.

Draft

Right now I'm using an ImageBuffer and emulate this with

    buffer.as_mut()
        .par_chunks_exact_mut(color_type.bytes_per_pixel() as usize * x_resolution)
        .map(|row| row.iter().enumerate())
        .enumerate()
        ...

I realize that this has the additional property that the pixels of the rows iterated in order, which you may or may not wish to guarantee. Even without that I still think it might be worth it to add parallel versions of the normal enumerate_rows(_mut) just for the optional speed increase.

JSorngard avatar Jan 16 '24 21:01 JSorngard

If this were implemented with the types in this crate I would map each row to an EnumeratePixels(Mut) instead of a normal slice iterator.

JSorngard avatar Jan 16 '24 21:01 JSorngard

So I guess the weird thing here is that you want to iterate over the rows of the image in parallel, and then for each row iterate over the pixels sequentially. We already have support if you want to do both sequentially (enumerate_rows_mut().map(|row| ...)) or both in parallel (par_enumerate_pixels_mut).

I guess I don't see an issue with adding a par_enumerate_rows{_mut} that would let you enumerate the pixels sequentially. Though it makes me wonder if all the rows iteration methods should really have Item = &[P] / Item = &mut [P] rather than in producing iterators themselves, so that the user had the option of iterating or just doing random accesses

fintelia avatar Jan 17 '24 05:01 fintelia

I think I personally would prefer that API as it would allow, as you say, the user to call either iter or par_iter on the slices, which gives more options for how to iterate over the data.

I think changing the sequential versions of the row iterators would be a breaking change though right?

JSorngard avatar Jan 17 '24 06:01 JSorngard

It would even let you call chunks or windows or other fancy methods on it. But yeah, it would be a breaking change for the sequential row iterations and sightly awkward to have inconsistent behavior if we didn't match for the parallel iterations

fintelia avatar Jan 20 '24 18:01 fintelia

I think it is a good idea to have them be consistent, since then you can just add a par_ in front and magically have parallelism. But it also sounds like it would be a very good idea to change the row iterators to return slices in the next major version.
These two things together mean that either the parallel versions should be implemented to be consistent now and then change both them and the serial ones in the next major version, or don't implement them until the next major version where the serial ones are changed.

JSorngard avatar Jan 20 '24 21:01 JSorngard