fix: premultiply alpha before resizing (fix #14351) #14353
Conversation
ChaseKnowlden
force-pushed
the
icon-pipeline-fix
branch
from
8c344e6 to
cc6b3b1
Compare
Package Changes Through cc6b3b1No changes. Add a change file through the GitHub UI by following this link. Read about change files or the docs at github.com/jbolda/covector |
There was a problem hiding this comment.
Thanks! Also do you mind adding a change file?
https://github.com/tauri-apps/tauri/blob/dev/.changes/README.md
| Self::DynamicImage(i) => { | ||
| // Step 1: Premultiply alpha | ||
| let mut buf = i.to_rgba8(); | ||
| for pixel in buf.pixels_mut() { | ||
| let a = pixel[3] as u32; | ||
| pixel[0] = ((pixel[0] as u32 * a + 127) / 255) as u8; | ||
| pixel[1] = ((pixel[1] as u32 * a + 127) / 255) as u8; | ||
| pixel[2] = ((pixel[2] as u32 * a + 127) / 255) as u8; | ||
| } | ||
| let premultiplied = DynamicImage::ImageRgba8(buf); | ||
|
|
||
| // Step 2: Resize | ||
| let mut resized = premultiplied.resize_exact(size, size, FilterType::Lanczos3).to_rgba8(); | ||
|
|
||
| // Step 3: Unpremultiply alpha and zero RGB for fully transparent | ||
| for pixel in resized.pixels_mut() { | ||
| let a = pixel[3] as u32; | ||
| if a == 0 { | ||
| pixel[0] = 0; | ||
| pixel[1] = 0; | ||
| pixel[2] = 0; | ||
| } else { | ||
| pixel[0] = ((pixel[0] as u32 * 255 + a / 2) / a).min(255) as u8; | ||
| pixel[1] = ((pixel[1] as u32 * 255 + a / 2) / a).min(255) as u8; | ||
| pixel[2] = ((pixel[2] as u32 * 255 + a / 2) / a).min(255) as u8; | ||
| } | ||
| } | ||
| Ok(DynamicImage::ImageRgba8(resized)) | ||
| } |
There was a problem hiding this comment.
| Self::DynamicImage(i) => { | |
| // Step 1: Premultiply alpha | |
| let mut buf = i.to_rgba8(); | |
| for pixel in buf.pixels_mut() { | |
| let a = pixel[3] as u32; | |
| pixel[0] = ((pixel[0] as u32 * a + 127) / 255) as u8; | |
| pixel[1] = ((pixel[1] as u32 * a + 127) / 255) as u8; | |
| pixel[2] = ((pixel[2] as u32 * a + 127) / 255) as u8; | |
| } | |
| let premultiplied = DynamicImage::ImageRgba8(buf); | |
| // Step 2: Resize | |
| let mut resized = premultiplied.resize_exact(size, size, FilterType::Lanczos3).to_rgba8(); | |
| // Step 3: Unpremultiply alpha and zero RGB for fully transparent | |
| for pixel in resized.pixels_mut() { | |
| let a = pixel[3] as u32; | |
| if a == 0 { | |
| pixel[0] = 0; | |
| pixel[1] = 0; | |
| pixel[2] = 0; | |
| } else { | |
| pixel[0] = ((pixel[0] as u32 * 255 + a / 2) / a).min(255) as u8; | |
| pixel[1] = ((pixel[1] as u32 * 255 + a / 2) / a).min(255) as u8; | |
| pixel[2] = ((pixel[2] as u32 * 255 + a / 2) / a).min(255) as u8; | |
| } | |
| } | |
| Ok(DynamicImage::ImageRgba8(resized)) | |
| } | |
| Self::DynamicImage(image) => { | |
| // Premultiply alpha | |
| let premultiplied_image = DynamicImage::ImageRgba8(ImageBuffer::from_par_fn( | |
| image.width(), | |
| image.height(), | |
| |x, y| { | |
| let mut pixel = image.get_pixel(x, y); | |
| let alpha = pixel.0[3] as f32 / u8::MAX as f32 | |
| pixel.apply_without_alpha(|channel_value| (channel_value as f32 * alpha) as u8); | |
| pixel | |
| }, | |
| )); | |
| let mut resized = premultiplied_image.resize_exact(size, size, FilterType::Lanczos3); | |
| // Unmultiply alpha | |
| resized | |
| .as_mut_rgba8() | |
| .unwrap() | |
| .par_pixels_mut() | |
| .for_each(|pixel| { | |
| let alpha = pixel.0[3] as f32 / u8::MAX as f32 | |
| pixel.apply_without_alpha(|channel_value| (channel_value as f32 / alpha) as u8); | |
| }); | |
| Ok(resized) | |
| } |
We should be able to rayon here to speed up the process
(the code needs adding rayon to Cargo.toml and import use rayon::iter::ParallelIterator;)
Also, a comment with a link (for example image-rs/image#1655 or the issue #14351) and some reasoning behind this would be nice
There was a problem hiding this comment.
isn't rayon a bit overkill or did we have that in the deps already?
There was a problem hiding this comment.
It's pulled in by image crate already, we are just importing rayon here so we can actually use the rayon iter image crate returned us
| expression: resolved | ||
| --- | ||
| Resolved { | ||
| has_app_acl: false, |
|
Hey! Just checking in, do you still have time to work on this? |
|
@ChaseKnowlden Hi, do you mind if I take over this if you don't have enough time to work on this anymore? |
4 participants
Fix #14351