image-rs / imageproc

Image processing operations
MIT License
735 stars 145 forks source link

BresenhamLineIter crash #545

Closed erwanvivien closed 5 months ago

erwanvivien commented 10 months ago

I found this code to be panicking (because of out-of-bound access)

let (from, to) = ((240.49998, 0.5), (12.726593, 164.87411));
let image = GrayImage::from_pixel(480, 480, Luma::black());
for _ in BresenhamLinePixelIter::new(&image, from, to) {
    //
}

Crashes with

thread 'main' panicked at 'Image index (240, 4294967295) out of bounds (480, 480)'
C:\Users\XXX\.cargo\registry\src\index.crates.io-6f17d22bba15001f\imageproc-0.23.0\src\drawing\line.rs:123:33

Has anyone an idea?

erwanvivien commented 10 months ago

Added a debug in src/drawing/line.rs:66:

[imageproc\src\drawing\line.rs:67] ret = (
    239,
    0,
)
[imageproc\src\drawing\line.rs:67] ret = (
    240,
    -1,
)

And then src/drawing/line.rs:123 casts it to u32

So changing the positions to ⬇️ fixes it

let (from, to) = (
    (240.49998f32.floor(), 0.5f32.floor()),
    (12.726593f32.floor(), 164.87411f32.floor()
);

I think Brehendam should not take f32 if it cannot handle it properly (we cast to i32 directly) Other code like draw_antialiased_line_segment already takes (i32, i32)

erwanvivien commented 5 months ago

Thanks to both of you :)