381661 - Fix and reenable bilinear filtering for upscaled images for 1.9/Firefox 3 on Windows

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

This was disabled to fix bug 324698.  There are two ways to fix this:

1 - fix cairo so that it doesn't do stupid stuff for EXTEND_NONE in pixman

2 - fix cairo so that it understands EXTEND_PAD for image sources and use that

Once either of those is done, we can revert the patch in 324698.

Comment 1

[Gábor Stefanik]

Bilinear? Why not bicubic or similar? Or maybe a pref to change the algorithm between bilinear/bicubic/nearest-neighbor.

Comment 2

[David E. Weekly]

Staring until my eyes bled with trunk 20070614 I could not find or reproduce chunkiness in the image upscaling or find inferiority to IE7 / Photoshop upscaling. Perhaps this got quietly fixed in Cairo?

Comment 3

[Dão Gottwald [:dao]]

(In reply to comment #2)
> Staring until my eyes bled with trunk 20070614 I could not find or reproduce
> chunkiness in the image upscaling or find inferiority to IE7 / Photoshop
> upscaling.

IE7 uses nearest neighbor unless you apply -ms-interpolation-mode:bicubic;. Photoshop offers different algorithms as well.

> Perhaps this got quietly fixed in Cairo?

See comment 0. A Cairo update alone doesn't fix this -- attachment 265603 [details] [diff] [review] would have to be reverted, too.

Comment 4

[David E. Weekly]

Hm. I tried several different scaling filters in Photoshop and was unable to find one that was markedly visually superior to 20070614 in upscaling. Is there a test case that obviously exposes poor quality upscaling?

In other words, is there a visual demonstration of something that is broken that will be fixed when this bug is fixed? Cairo seems to be upscaling and downscaling excellently on XP.

Comment 5

[Dão Gottwald [:dao]]

Since a full zoom UI is wanted for Firefox 3, this should probably get a higher priority for Gecko 1.9.

Comment 6

[Bastien Nocera]

As a follow-up to:
http://svenfoo.geekheim.de/index.php/2007-08-15/old-bugs-must-die/

I checked the scaling of the original test image:
http://bugzilla.gnome.org/attachment.cgi?id=89819&action=view
And the scaled image looks worse than what EOG (GNOME's image viewer) can show for the same image, at the same scale.

Tested with Firefox 2.0.0.6

Comment 7

[Bastien Nocera]

Attachment: Screenshot.png

Same image in Epiphany (using Firefox, left) and Eye Of GNOME (right)

Comment 8

[Peter van der Woude [:Peter6]]

This isn't a Firefox 2 bug, so please stop spamming

Comment 9

[Bastien Nocera]

Damn, I didn't read the bug title properly, it's for upscaled images when I was talking about downscaled images. Filed as bug 392587

(In reply to comment #8)
> This isn't a Firefox 2 bug, so please stop spamming

2 comments hardly count for spam, maybe 3 actually do though...

Comment 10

[Alfred Kayser]

See http://lists.cairographics.org/archives/cairo/2007-September/011350.html
Carl Worth is planning to "Fix CAIRO_EXTEND_PAD for image surfaces".

Comment 11

[Justin Dolske [:Dolske]]

Vlad set this as blocking-1.9- right after filing back in May, but I'd like to renominate... Now that full page zoom has landed and is exposed via the UI in Firefox, the effect from this is much more visible. Apparently it's enabled on OS X (although bug 371316 implies it might need to be disabled?), but scaled images on unix tend to look terrible. Dunno what state Windows is in.

Comment 12

[Martijn Wargers (dead)]

This would cause performance regressions like bug 328388 (and maybe more), so I would prefer it not to be enabled.

Comment 13

[John Mellor (Jomel)]

Bilinear filtering isn't supposed to be as fast as nearest neighbor, however a minor speed drop seems a worthy compromise in order to make images much clearer. From what I can tell of bug 328388, the main regression was actually unrelated to the scaling anyway?

Without this full page zoom (in particular) is significantly crippled (which is a shame for accessibility reasons too)...


(According to bz in bug 98971 comment 39, images are rescaled every time they are painted. While he was against caching scaled versions of all images, it would seem an easy performance boost to cache scaled versions of images on the current page only...

Alternatively, this may be too complicated to be worthwhile, but might it at least be possible to switch between bilinear and nearest-neighbor depending on CPU load?

Or to only use it during full page zoom (when users will expect some speed hit).

Or to initially render using nearest-neighbor then in the background render using bilinear filtering and replace images as they are generated?)

Comment 14

[Boris Zbarsky [:bzbarsky]]

I'm not against caching upscaled images.  Last I checked stuart is.  And since he's the imagelib owner, that matters.

As for "minor speed drop"...  On Linux last I checked cairo's bilinear scaling is about 2-10 times slower than the nearest-neighbor scaling we used to have in non-cairo builds.  There are existing bugs filed on this (e.g. it makes reftest results incredibly painful to deal with on trunk).

Put another way, if scaling the image is going to completely hang the X server for several seconds (stops painting and ignores user input completely) it might be better to scale with a lower-quality algorithm in terms of user experience.  Last I checked, that's about where we are right now in performance terms.  And dbaron is seeing the same thing last I checked, so it's not a matter of "just an old RENDER".

Most of comment 13 assumes that we're the ones scaling the image.  On X, we aren't, last I checked.

Comment 15

[Justin Dolske [:Dolske]]

Should we spin off the "reenable bilinear filter" issue, and focus this one on making it work correctly? Enabling it might be a platform specific thing, maybe we'll end up enabling in on Windows but not Linux.

[Random note: I checked on Windows and the upscaling with pagezoom obviously isn't bilinear; it looks terrible.]

Comment 16

[Radek 'sysKin' Czyz]

This is strange, bilinear scaling shouldn't be any slower than nearest neighbour on any semi-modern CPU. Both operations are simply limited by available memory bandwidth (and requirements for that don't change between those two zoom methods).

I've been coding image-processing stuffs for quite some time - perhaps I'll find some time to take a look at this problem as well as performance problem. Not for another two weeks though :(

Comment 17

[Boris Zbarsky [:bzbarsky]]

> bilinear scaling shouldn't be any slower than nearest neighbour

Apparently the Render implementors didn't get the memo.  :(

Comment 18

[Stuart Parmenter]

if we're not going to globally re-enable this we should probably enable it in chrome (set higher quality in the imageboxframe code?

Comment 19

[Matt]

Attachment: Another IE7 vs. Firefox 3.0a9 image-smoothness camparison for comment #19

I uploaded another side-by-side comparison of a portion of a www.cnn.com page zoomed in at 133% in Firefox 3.0a9 and IE7 -- "image-zoom-smoothness.jpg" -- to point out a timing aspect. When the page loaded in IE, the image was as jagged as in Firefox -- but it got smoother after about 5 seconds. IE 7's image smoothing algorithm seems to happen asynchronously, at least on my mid-range laptop. (And sometimes it doesn't seem to happen at all -- perhaps an IE bug.) Maybe this delayed smoothing approach is something to consider for Firefox 3, if the smoothing algorithm takes more time than we'd like: First load, zoom, and render the images quickly at the somewhat jagged quality -- then smooth the images in the background, and redisplay each smoothed image as it finishes. And cache the smoothed versions so if the user comes back to the page they'll see the smoothed images without waiting. 

By the way, page zooming (and the smooth image algorithm) are particularly useful on some of the new hand-held devices coming out with very high pixel densities (e.g. 200 pixels-per-inch or more), since they require page zooming just to read Web pages. Basically EVERYONE has accessibility issues on these devices. So this page-zoom feature is going to be used a lot on those devices.

Comment 20

[Gábor Stefanik]

If I remember correctly, there has been a general consensus already to use some interpolation/filtering for upscaled *and downscaled* images, bilinear was even implemented for some time, just it was disabled to work around bug 324698. Let's not drop such an useful feature just because it's slower on a Pentium 90 when we already require Windows 2000. Any CPU which can run Windows 2000 or higher is fast enough to render bilinear-filtered images nearly instantly. Bicubic might be an issue I think, but bilinear definitely isn't. And without it, any non-integer zoom level will look bad. A multipass of nearest-neighbor, then bilinear, possibly with a final pass of bicubic would be a good idea, though.

Comment 21

[Gábor Stefanik]

Also, an extra suggestion: display the filtered image incrementally, as opposed to displaying only when the last pass is finished. Interlaced PNG is a bit harder: render the first pass as nearest-neighbor, then swap it with bilinear, render the subsequent passes using bilinear filtering, and when everything is loaded, swap the final image with a bicubic one (though the bicubic swapping should be pref-controlled). I support caching, too.

Comment 22

[Boris Zbarsky [:bzbarsky]]

> just because it's slower on a Pentium 90 when we already require Windows 2000

I assure you that dbaron is not using a Pentium 90.  Neither am I.  The scaling is dog-slow to the point that it interferes with daily work (debugging reftest failures, for example).

In the end, I don't care if we do reenable it, as long as we have the correctness bug fixed, and as long as I can turn it off...  But I'm not even sure what we're discussing, since the correctness bug remains to be fixed.

> Any CPU which can run Windows 2000 or higher is fast enough to render
> bilinear-filtered images nearly instantly.

You would think so, wouldn't you.  Apparently, that's not the case, however.  I've heard vlad talk about the scaling code in Render, and it mostly consists of swear words... ;)

Comment 23

[Norbert]

Trying Firefox 3.0 beta 1 I stumbled on the problem of bad quality of scaled bitmaps in zoomed pages. Although in general Firefox has better zoom-concept than IE, IE performs better in scaling images.

Is there a chance to get this fixed for Firefox 3.0 final release?

Comment 25

[Norbert]

Priority ist too low since the zoom feature is crippled!

Comment 26

[Topper]

Attachment: upscale image do not render properly

There are html and images inside this attachment.
Upscale image by drawImage do not render properly.

Firefox/2.0.0.11 
on Windows vista and Windows XP SP2

Comment 27

[Gábor Stefanik]

Topper, this is about Firefox 3. There is no way this will get fixed in any Firefox 2 release. Changing summary to make this clear.
Also, the wanted-1.9 radar is both deprecated (it's now a flag), and redundant (we are blocking1.9+).

Comment 28

[echoes]

why are people suggestion bilinear or bicubic when lanczos is the preferred method of upscaling because it provides the best quality?

does cairo not support it?

Comment 29

[Radek 'sysKin' Czyz]

Lanczos is by far the slowest, being anywhere between 4 and 8 taps. That's significantly more memory bandwidth needed for filtering.

More memory bandwidth means trashing CPU's cache more, and that's bad for an application that's supposed to remain responsive.

Bilinear is probably good enough, as it has pretty much the same cache requirements as nearest neighbour (it's creating one output line from two input lines of data, in case of zoom-in).

Comment 30

[Paul-Kenji Cahier]

simply a decent bilinear reenable would already make a gigantic difference in terms of quality.
With the new zoom feature going to be in the spotlight, a decent sampling method might be really nice.

Comment 31

[Stuart Parmenter]

vlad -- can we do this easily/should we do it?  I'd like to see it on...

Comment 32

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: let's do this

Now that bug 418353 has landed, along with the new pixman, we can do this.

Note that this only enables it for win32 -- mac doesn't need anything changed, and I fear that on linux setting EXTEND_PAD would cause massive performance issues since the X server doesn't yet support it.

Comment 33

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 304372 [details] [diff] [review]
let's do this

bah, patch needs to be changed -- the test should be != 1.0, not just > 1.0.

Comment 34

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: let's (really) do this

Ok, with right check.

Comment 35

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Checked in.

Comment 36

[Steve England [:stevee]]

This caused bug 419277

Comment 37

[Norbert]

The patch is not included in FF 3.0b4, right?

Comment 38

[Norbert]

Windows version of FF 3.0b4 zooms images nice, now.

But the linux build scales still ugly. :-(

Why has this bug been closed, since the patch works only on windows?

Comment 39

[Jo Hermans]

(In reply to comment #38)
> Windows version of FF 3.0b4 zooms images nice, now.
> 
> But the linux build scales still ugly. :-(
> 
> Why has this bug been closed, since the patch works only on windows?
> 

see comment 32

Comment 40

[doutor.zero]

(In reply to comment #38)
> Windows version of FF 3.0b4 zooms images nice, now.
> 
> But the linux build scales still ugly. :-(
> 
> Why has this bug been closed, since the patch works only on windows?
> 

My opinion too..

Why a bug can be considered fixed, and be closed, when the solution don't apply to one of the platforms?

And there is no other solution to make this work on Linux? Some hidden optional preference that the user can manually turn on, even knowing that this is will hurt Firefox's performance? 

Comment 41

[Norbert]

I like the idea with a switch in about:config.

If a Linux user has a fast Computer and accepts delay, he could turn filtering on.

Comment 42

[Peter van der Woude [:Peter6]]

Guys, stop spamming this bug.
It was filed/resolved for windows (not All)
Create a new bug for linux if you want to complain/comment.

Comment 43

[Norbert]

I have created Bug 422179 for Linux.

Comment 44

[[:Aleksej]]

(In reply to comment #42)
> It was filed/resolved for windows (not All)

According to the history, it was filed with All in the OS field.