I would like to be able to have a simple linear iterator over image positions.
My specific use case for this functionality is, ... it would reduce to loops into one(from O(n^2) into O(n)).
This is more generally applicable to ... for example in image::copy_from, since "pixels_mut" is deprecated.
// before
for i in 0..other.width() {
for k in 0..other.height() {
let p = other.get_pixel(i, k);
self.put_pixel(i + x, k + y, p);
}
}
// after, something like this
for (i, k) in other.position_iter() {
let p = other.get_pixel(i, k);
self.put_pixel(i + x, k + y, p);
}
PS.
Please let me know if this is already in work, and if not, could I jump in?
There isn't really a change from O(n虏) to O(n) here, as the "before" code is O(w*h) and the second is O(n) where n = w*h. It may reduce code complexity a little, but if you still want to know both i and k inside the iteration, it hardly matters.
There is other code to iterate over the _pixels_ of an image, though, which is both less complex and more efficient when implementing "local" filters (where each pixel in the output is a function of only that same pixel in the input, such as color-space transformations.
@kaj such as?
The pixels method on ImageBuffer and on GenericImagView (and their mutable counterparts) returns Pixels (or PixelsMut) iterators.
Also, when looking this up, I found ImageBuffer::enumerate_pixels_mut, which also includes the coordiates.
@kaj There is no parallel on GenericImage though, no way to apply a function to all mutable pixels鈥攚ith or without their position. I think that is the main concern here, not the concrete ImageBuffer. The reasoning against the _iterator_ is that this is in general unsound to create from the generic interface alone. However there are some alternative interfaces that are possible regardless:
``rust
// All SemVer compatible additions as we can default implement them
trait GenericImage {
// As proposed.
fn positions(&self) -> impl Iterator<(u32, u32)> { /* doable by bounds */ }
// Also doable safely by iterating the positions as in the loop above.
// This _can_ be overriden inImageBuffer` to be somewhat more performant by doing less bounds check.
fn apply(&mut self, f: impl Fn(&mut Self::Pixel)) { /* internal loop / }
// Similar but with coordinates.
fn apply_with_coordinates(&mut self, f: impl Fn(&mut Self::Pixel, u32, u32)) { / internal loop */ }
}
@HeroicKatora if possible, can I contribute and implement iterator?
If I ever tell someone __not__ to post contributions and pull requests we're in deep trouble :smile:
So yes by all means do, somewhat generalizing mutable pixel iteration is something I'm longing for as well.
I'm certain there will be some amount of iteration and bikeshedding an the PR but it could improve performance for generic code by quite a lot when applied to an ImageBuffer.
@HeroicKatora doing position iterator seems pretty redundat due to existing pixel iter, so I'll close this ticket
Most helpful comment
There isn't really a change from O(n虏) to O(n) here, as the "before" code is
O(w*h)and the second isO(n)wheren = w*h. It may reduce code complexity a little, but if you still want to know both i and k inside the iteration, it hardly matters.There is other code to iterate over the _pixels_ of an image, though, which is both less complex and more efficient when implementing "local" filters (where each pixel in the output is a function of only that same pixel in the input, such as color-space transformations.