Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Panic when call blur with sigma == 0.0 #983

Closed
Aloxaf opened this issue Jul 6, 2019 · 4 comments · Fixed by #1362
Closed

Panic when call blur with sigma == 0.0 #983

Aloxaf opened this issue Jul 6, 2019 · 4 comments · Fixed by #1362

Comments

@Aloxaf
Copy link

Aloxaf commented Jul 6, 2019

This happens in when call image::imageops::blur with sigma == 0.0, however imageproc::filter::gaussian_blur_f32 works well (edit: seee image-rs/imageproc#331 )

Expected

Nothing happen

Actual behaviour

It panicked

Reproduction steps

use image::RgbaImage;
use image::imageops::blur;
use imageproc::filter::gaussian_blur_f32;

fn main() {
    let image = RgbaImage::new(50, 50);
    dbg!();
    let _ = gaussian_blur_f32(&image, 0.0);
    dbg!();
    let _ = blur(&image, 0.0);
}

Backtrace

thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', src/libcore/option.rs:347:21
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.29/src/backtrace/libunwind.rs:88
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.29/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:47
   3: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:36
   4: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:198
   5: std::panicking::default_hook
             at src/libstd/panicking.rs:212
   6: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:475
   7: std::panicking::continue_panic_fmt
             at src/libstd/panicking.rs:382
   8: rust_begin_unwind
             at src/libstd/panicking.rs:309
   9: core::panicking::panic_fmt
             at src/libcore/panicking.rs:85
  10: core::panicking::panic
             at src/libcore/panicking.rs:49
  11: core::option::Option<T>::unwrap
             at /rustc/b25ee644971a168287ee166edbd11642dbcfeab8/src/libcore/macros.rs:12
  12: image::imageops::sample::vertical_sample
             at /home/aloxaf/.cargo/registry/src/github.com-1ecc6299db9ec823/image-0.21.2/./src/imageops/sample.rs:284
  13: image::imageops::sample::blur
             at /home/aloxaf/.cargo/registry/src/github.com-1ecc6299db9ec823/image-0.21.2/./src/imageops/sample.rs:710
  14: tmp2::main
             at src/main.rs:10
  15: std::rt::lang_start::{{closure}}
             at /rustc/b25ee644971a168287ee166edbd11642dbcfeab8/src/libstd/rt.rs:64
  16: std::rt::lang_start_internal::{{closure}}
             at src/libstd/rt.rs:49
  17: std::panicking::try::do_call
             at src/libstd/panicking.rs:294
  18: __rust_maybe_catch_panic
             at src/libpanic_unwind/lib.rs:82
  19: std::panicking::try
             at src/libstd/panicking.rs:273
  20: std::panic::catch_unwind
             at src/libstd/panic.rs:388
  21: std::rt::lang_start_internal
             at src/libstd/rt.rs:48
  22: std::rt::lang_start
             at /rustc/b25ee644971a168287ee166edbd11642dbcfeab8/src/libstd/rt.rs:64
  23: main
  24: __libc_start_main
  25: _start
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
@Aloxaf
Copy link
Author

Aloxaf commented Jul 6, 2019

I thinks this should be changed to something like let sigma = if sigma < 0.0 { /* return the original image */ } else { sigma };

@HeroicKatora
Copy link
Member

HeroicKatora commented Jul 6, 2019

This may be caused by having an empty kernel support, hence the sum being 0.0 resulting in division by 0 in floating points cause the NumCast::from argument to be NaN which can not be cast to an integral type and instead yields a None value that is then unwrapped.

The proposed fix, with a slight change, should be good enough then:

let sigma = if sigma <= 0.0 { /* return the original image */ } else { sigma };

@Aloxaf
Copy link
Author

Aloxaf commented Jul 6, 2019

@HeroicKatora Thank you. I forgot to change the operator.

However, image-rs/imageproc#331 was fixed by explicitly rejecting sigama <= 0.0. Is there a need to maintain consistency between these two functions? (But this seems to lead a breaking change on this crate

@HeroicKatora
Copy link
Member

Right. That's something to consider for the future. It also relates to #977 in that removing it from this crate would certainly make it consistent ;) But it would be very prompt to do it because of a bug, the issue should involve better planning.

@HeroicKatora HeroicKatora added this to Bugs in Version 0.23 Feb 9, 2020
HeroicKatora added a commit that referenced this issue Nov 16, 2020
@HeroicKatora HeroicKatora moved this from Bugs to Done in Version 0.23 Feb 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Version 0.23
  
Done
Development

Successfully merging a pull request may close this issue.

2 participants