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

Filter regression in PNG encoding #305

Closed
mbrubeck opened this issue Feb 26, 2015 · 3 comments · Fixed by #306
Closed

Filter regression in PNG encoding #305

mbrubeck opened this issue Feb 26, 2015 · 3 comments · Fixed by #306

Comments

@mbrubeck
Copy link
Contributor

#298 created a regression that causes some images to be encoded incorrectly as PNG. This following test case demonstrates the problem. The PPM file is correct, but every other row of the PNG file is incorrect.

let white = image::Pixel::from_channels(255,255,255,255);
let black = image::Pixel::from_channels(0,0,0,255);
let img = image::ImageRgba8(image::ImageBuffer::from_fn(100, 100, move |x, y| {
    if (x + y) % 2 == 0 { white } else { black }
}));
img.save(&mut File::create(&Path::new("test.png")).unwrap(), image::PNG);
img.save(&mut File::create(&Path::new("test.ppm")).unwrap(), image::PPM);

Reverting #298 fixes the bug.

Looking at that patch, I think that this line and this line are incorrect, since current[i - bpp] is not the same as orig[i - bpp] in the previous code; it is a value that may have been changed by the preceding for loop.

@mbrubeck
Copy link
Contributor Author

Another problem is that this code uses -1 as an unsigned integer literal, so range_step_inclusive tries to step by usize::MAX and stops immediately due to overflow.

@mbrubeck
Copy link
Contributor Author

The issue in the previous comment could have been prevented by rust-lang/rust#5477.

mbrubeck added a commit to mbrubeck/image that referenced this issue Feb 26, 2015
Fixes image-rs#305.  This partially reverts image-rs#298, but keeps the correct parts.
@nwin
Copy link
Contributor

nwin commented Feb 26, 2015

Thanks. Good findings!

mbrubeck added a commit to mbrubeck/image that referenced this issue Feb 26, 2015
Fixes image-rs#305.  This partially reverts image-rs#298, but keeps the correct parts.
fintelia pushed a commit that referenced this issue Dec 20, 2024
…on-animation-images

Fix 301 regression encode non animation images
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants