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

Improve quality of images #488

Closed
umputun opened this issue Dec 9, 2019 · 35 comments · Fixed by #582
Closed

Improve quality of images #488

umputun opened this issue Dec 9, 2019 · 35 comments · Fixed by #582

Comments

@umputun
Copy link
Owner

umputun commented Dec 9, 2019

Looks like resized images with text in are very hard to read, super blurry. It is unclear what the reason - could be non-proportional ratio, could be the resizing method. Github, for instance, makes images more readable.

This is an example of the blurry one: https://remark42.radio-t.com/api/v1/picture/github_ef0f706a79cc24b17bbbb374cd234a691d034128/bnmk3aqhajfjbc6s020g.png

@akosourov
Copy link
Contributor

Ok, I will take a look and try to understand how it could be improved

@mullakhmetov
Copy link
Contributor

@akosourov are you working on this in any way? I am quite interested and want to solve it

@akosourov
Copy link
Contributor

@mullakhmetov Yeah, sure. I tried to solve it two weeks ago but stopped and planned to continue on weekends. Ok, I don't mind :)

@mullakhmetov
Copy link
Contributor

Current resize implementation uses draw.BiLinear interpolation. I've compare it with draw.CatmullRom (described as slow but high quality) and Lanczos3 (wiki).

Quality comparison

Text image

source image/draw : BiLinear image/draw : CatmullRom resize :: Lanczos3

Cat image

source image/draw : BiLinear image/draw : CatmullRom resize :: Lanczos3

Green image

source image/draw : BiLinear image/draw : CatmullRom resize :: Lanczos3

I see the difference between BiLinear and CatmullRom, and don't see any advantages of Lanczos3 over CatmullRom.

Size comparison

Text image

Source image size: 673KB
Source image dimensions: 1125x2436
Dest image dimensions: 138x300

Algorithm dest size
BiLinear 21KB
CatmullRom 23KB
Lanczos3 28KB

Cat image

Source image size: 817KB
Source image dimensions: 756x1008
Dest image dimensions: 143x190

Algorithm dest size
BiLinear 41KB
CatmullRom 44KB
Lanczos3 45KB

Green image

Source image size: 1.6MB
Source image dimensions: 756x1008
Dest image dimensions: 143x190

Algorithm dest size
BiLinear 59KB
CatmullRom 63KB
Lanczos3 64KB

Benchmarks

About the CatmullRom slowness. CatmullRom vs BiLinear bench results on my early 2015 macbook pro:

Text image

BenchmarkCatmullRom-4           1000000000               0.151 ns/op
BenchmarkBiLinear-4             1000000000               0.0980 ns/op

Cat image

BenchmarkCatmullRom-4           1000000000               0.0657 ns/op
BenchmarkBiLinear-4             1000000000               0.0482 ns/op

Green image

BenchmarkCatmullRom-4           1000000000               0.0781 ns/op
BenchmarkBiLinear-4             1000000000               0.0638 ns/op

Conclusion

So, due to clear quality advantages of CatmullRom over BiLinear and minor (in the web world) performance degradation I suggest to switch to CatmullRom.

@paskal @umputun what do you think?

@paskal
Copy link
Sponsor Collaborator

paskal commented Jan 2, 2020

CatmullRom is better than BiLinear, Lanczos3 looks a little better than both, but text image quality after resize is still awful. Can we play around with the quality settings to get a more decent image?

@umputun
Copy link
Owner Author

umputun commented Jan 2, 2020

I still think this is not really due to scaling algo, but something about the ratio

@mullakhmetov
Copy link
Contributor

I can't get any more decent scaling result even in Photoshop. I tried to crop source image to 1120x1120 and resize it to 140x140 (to prevent round off dimensions, which could cause text blurriness).

Do you have some references? Maybe it is impossible to do better for such a small destination image size?

@umputun
Copy link
Owner Author

umputun commented Jan 2, 2020

4o0ie-202001-02153723-lh4tr

this is how it looks on github. As you may see - perfectly readable

@umputun
Copy link
Owner Author

umputun commented Jan 2, 2020

btw, if the problem related to the small final size (800/300 default) we can increase it in cli/env (ImageGroup.ResizeWidth and ResizeHeight)

@mullakhmetov
Copy link
Contributor

mullakhmetov commented Jan 2, 2020

As you may see - perfectly readable

Agree. But in your example image was scaled down with ~2.7 factor (from 1746x1790 to 663x647) while I use ~8 factor in comparison above.

Here's CatmullRom-scaled to the same as github's size:

github_img_catmull

Pretty close to the reference and certainly readable.

btw, if the problem related to the small final size

BiLinear algo is always much worse than CatmullRom, Lanczos3 is always a little better than the CatmullRom (in quality terms). But you are right, this is becoming a problem only with high scale factor.

(800/300 default)

Hmm. Image in original post downscaled to 143x300. Why so?

@paskal
Copy link
Sponsor Collaborator

paskal commented Jan 2, 2020

To clarify, GitHub image above is 1746x1790, it's scaled in render but is not altered in quality at all.

@mullakhmetov
Copy link
Contributor

but is not altered in quality at all

Of course it did :) The scale factor is just too low to see quality degradation. Here's original image github-like scaled at browser-side to 400 width:

@umputun
Copy link
Owner Author

umputun commented Jan 2, 2020

Hmm. Image in original post downscaled to 143x300. Why so?

https://github.com/umputun/remark/blob/master/backend/app/store/image/image.go#L174

@paskal
Copy link
Sponsor Collaborator

paskal commented Jan 2, 2020

but is not altered in quality at all

Of course it did :) The scale factor is just too low to see quality degradation. Here's original image github-like scaled at browser-side to 400 width:

I mean GitHub presents the original picture and it's up to the browser to scale it, and we are playing a completely different game here. If we do resize, let's not look at GitHub example and rather check what is desired quality vs size of a picture we are willing to store.

@umputun
Copy link
Owner Author

umputun commented Jan 2, 2020

I'm not sure why browser scaling is a "completely different game". If it can downscale the image to the proper size and keep it readable, what prevents us to do a similar thing on the backend side?

@umputun
Copy link
Owner Author

umputun commented Jan 2, 2020

just to clarify - I don't really care if we have to keep a larger image in order to make it readable. If the only way is to downscale it to some 1200/1200 - so be it

@mullakhmetov
Copy link
Contributor

I still don't get why the OP image dimensions so small. They both are lower than the corresponding default limits. It is posible only if source image dimensions were under limits and resize make no sense. Or ImageGroup settings were different in the days when OP image was created.

@umputun
Copy link
Owner Author

umputun commented Jan 2, 2020

as far as I recall dimensions adjusted on both X and Y. 300 is the max height so, it was downscaled from whateverW-whateverY to (max 800 W)- (max 300 Y)

@mullakhmetov
Copy link
Contributor

Ohh, I am sorry, I misread default limits (at least twice) and thought they are 800x600.
Ok, 300px height limit for the portrait oriented screenshots is too small. 800x800 would be more reasonable.

@umputun
Copy link
Owner Author

umputun commented Jan 3, 2020

setting 800x800 doesn't solve the problem by itself. The image is still blurry and hard to read. This is an example:

bo7a9guq9r87k7p617ag

@mullakhmetov
Copy link
Contributor

  1. The interpolation algorithm definitely needs to be changed. Lanczos3 provides the best quality, but it's presented only in external dependencies: imaging for instance. If the new dependency for such thing is not acceptable, let's just switch to CatmullRom, otherwise let's chooseimaging.
  2. Setting new limits doesn't affect anything above because the image in addition to server side scaling (with 800x800 limits) was scaled on browser side: https://github.com/umputun/remark/blob/master/frontend/app/components/raw-content/raw-content.scss#L28
    I don't know how to properly customize CSS and pass env vars there. Also I'm not sure raw-content.scss is the only place where IMAGE_RESIZE_* env vars should be handled.

@Ivajkin
Copy link
Contributor

Ivajkin commented Jan 4, 2020

The problem is not scaling algorithms themselves but scaling to serve for retina displays.

@Ivajkin
Copy link
Contributor

Ivajkin commented Jan 4, 2020

Please review and approve PR #526 (comprehensively tested).
@umputun

  • Fixed "Improve quality of images Improve quality of images #488" (sharp vibrant awesome images)
  • Added retina display support
  • Support for 3x devices including 11 Pro and MacPro
  • Resources usage is good as was for scaled images (change is subtle after scaling), example for big image is 180 KB and 298 KB.
  • All checks have passed successfully ✅

Example

Screen Shot 2020-01-04 at 11 03 45 PM

@paskal
Copy link
Sponsor Collaborator

paskal commented Jan 6, 2020

Can we resolve this one after merging #526?

@umputun
Copy link
Owner Author

umputun commented Jan 6, 2020

not sure. Maybe switching to CatmullRom will give us something good. Will be nice to see side-by-side with high res resize

@mullakhmetov
Copy link
Contributor

What do you mean by "high res resize"? Something like downscale 4000x3000 image with 2400x900 limits?

@mullakhmetov
Copy link
Contributor

Source and ApproxBiLinear, BiLinear, CatmullRom and Lanczos3 for the reference from top to bottom side-by-side

I'm not sure why browser scaling is a "completely different game". If it can downscale the image to the proper size and keep it readable, what prevents us to do a similar thing on the backend side?

Browser knows device pixel ratio and use it to get physical linear resolution from logical height and width CSS properties. In other words <img src=http://example.com/img_900x900.png width="300" height="300"> may be scaled to different physical resolution on different displays.

@paskal
Copy link
Sponsor Collaborator

paskal commented Jan 7, 2020

Source and ApproxBiLinear, BiLinear, CatmullRom and Lanczos3 for the reference from top to bottom side-by-side

№1 is worst (too sharp), №3 and №4 are the same and better than №2 (too blurry) is you'd ask me.

@umputun
Copy link
Owner Author

umputun commented Jan 19, 2020

What do you mean by "high res resize"? Something like downscale 4000x3000 image with 2400x900 limits?

Correct

@umputun umputun added this to the v1.6 milestone Jan 22, 2020
@paskal
Copy link
Sponsor Collaborator

paskal commented Jan 24, 2020

@mullakhmetov, thank you very much! Would you be so kind as to repeat your research and show four algorithms side-by-side? If you don't mind, better as separate pictures so it would be possible to cycle through them.

@mullakhmetov
Copy link
Contributor

It's not easy to find suitable (by license and for comparison) ~12mpx picture, as it turned out.

Original
ApproxBiLinear
BiLinear
CatmullRom
Lanczos3

@paskal
Copy link
Sponsor Collaborator

paskal commented Jan 25, 2020

  • ApproxBiLinear still too sharp.
  • BiLinear still too blurry in comparison with others.
  • CatmullRom and Lanczos3 almost the same, and the size is almost the same. My vote is with sticking with one of them which is easier \ faster.

@umputun?

@mullakhmetov
Copy link
Contributor

keep in mind there's no Lanczos3 implementation in stdlib

@paskal
Copy link
Sponsor Collaborator

paskal commented Jan 26, 2020

Let’s switch to CatmullRom then, could you please prepare a PR for that?

@mullakhmetov
Copy link
Contributor

@paskal sure, see #582

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants