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

Ignore color information of completely transparent pixels when resizing images #159

Merged
merged 3 commits into from Oct 24, 2016

Conversation

iwsfg
Copy link
Contributor

@iwsfg iwsfg commented Aug 16, 2016

Fixes artifact reported in itgalaxy/favicons#123

Black faint on the edges of opaque areas appears because color information of completely transparent pixels is taken into account during calculation of average color, but these pixels could be of any color. Completely transparent black completely transparent red looks identicall, so it's somewhat a common practice to replace RGB values with 0 or 255 for series of completely transparent pixels. Many encoders do this, (probably) to allow for a better compression.

Two images below illustrate what transparent areas sometimes look like if we make every pixel opaque.

This PR changes both submodules responsible for image resizing in the way that only color data of visible pixels used in average color calculation. Also fixed incorrect channels value in Resize.prototype.resizeWidthInterpolatedRGBA method (iwsfg/jimp@65158a9) and reduced amount of dupplicated code in /resize.js

And here's a comparison table. 6 images, every transparent pixel of which was painted in black, were scaled down from ~1000x1000 to 200x200 px using every resampling method supported. Then results from before and after patch were glued together for every image. The difference is pretty noticeable

Resized to 200x200. Before/After (click to enlarge)
default ~ ~ ~ ~
bezier ~ ~ ~ ~
bicubic ~ ~ ~ ~
bilinear ~ ~ ~ ~
hermite ~ ~ ~ ~
nearest ~ ~ ~ ~

Here's list of images from the table above in their original size:

- Merge similar methods together, remove duplicated code
- Fix error in Resize.resizeWidthInterpolatedRGBA
@iwsfg
Copy link
Contributor Author

iwsfg commented Aug 16, 2016

It could be, that I broke upscaling with this.
I'll fix it if needed and reopen this PR once I'll be sure upscaling works as good as it was

@iwsfg iwsfg closed this Aug 16, 2016
@iwsfg iwsfg reopened this Aug 17, 2016
@iwsfg
Copy link
Contributor Author

iwsfg commented Aug 17, 2016

Fixed. Lost incrementation of few indexes in two lines while merging simillar methods of the Resize class together.

As for the "bug" mentioned in iwsfg/jimp@65158a9 — it turned out to be some kind of a magic number, controlling how far image should shift vertically/horizontally. Changes introduced by this commit were reverted in master.

@iwsfg
Copy link
Contributor Author

iwsfg commented Aug 17, 2016

I kept playing with that number (1 / 3) and noticed that the farther away original and target dimensions are from one another - the more noticeable image shifts to the top-left, especially if compared to output produced by Photoshop.

I don't know if one third is a mathematically justified value or original programmer picked it at random, but I decided to make this value scale based on originalSize/targetSize proportion and I've got the results I'm happy with.

Example: (it's a 60x60 image resized to 512x512, then zoomed out to 25% in PS)

These changes could be found in iwsfg/jimp@8c27ae5 if you want them, but they probably should not be used, since it's not backed by any actual math and the difference in output isn't too big to begin with.

@iwsfg iwsfg mentioned this pull request Sep 1, 2016
@oliver-moran oliver-moran merged commit 43ededf into jimp-dev:master Oct 24, 2016
@oliver-moran
Copy link
Collaborator

Merged. Thanks and sorry for the delay. Very busy IRL. Will do a publish shortly and include this.

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 this pull request may close these issues.

None yet

2 participants