I found a bug when comparing images with a very high resolution, with a pixel count above 16777216 to be precise. This happens because 16777216 is a maximum safe integer that can be stored in f32,
When comparing the large image to themselves, the deviation array contains only elements 1.0, and sum::<f32> essentially increments an accumulator by 1.0 in a loop, which clips at 16777216, and if the pixel count was higher, the later division by element count yields score lower than 1.0.
This PR properly casts each element to f64 before calculating the sum with f64 accumulator, which solves this problem (since the maximum safe integer in f64 is about 9**15)
Note
I wasn't sure how to cover it with tests, since
I don't fully understand how it fits cucumber
In debug build this test takes 27 seconds to run on a AMD 3900x CPU (0.43s in release build)
#[test]
fn check_image_with_pixel_count_overflowing_f32() {
let image = image::RgbaImage::from_vec(1, 16777300, vec![0; 16777300 * 4]).unwrap();
// Fails right now on main branch with score 0.9999949932349067
assert_eq!(
image_compare::rgba_hybrid_compare(&image, &image)
.expect("Could not compare")
.score,
1.0
);
}
Hi,
I found a bug when comparing images with a very high resolution, with a pixel count above
16777216
to be precise. This happens because16777216
is a maximum safe integer that can be stored in f32,You can check it, as the following test passes:
When comparing the large image to themselves, the deviation array contains only elements
1.0
, andsum::<f32>
essentially increments an accumulator by1.0
in a loop, which clips at16777216
, and if the pixel count was higher, the later division by element count yields score lower than1.0
.This PR properly casts each element to f64 before calculating the sum with f64 accumulator, which solves this problem (since the maximum safe integer in f64 is about
9**15
)Note
I wasn't sure how to cover it with tests, since