Closed
Bug 381661
Opened 18 years ago
Closed 17 years ago
Fix and reenable bilinear filtering for upscaled images for 1.9/Firefox 3 on Windows
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9beta4
People
(Reporter: vlad, Assigned: vlad)
References
(Depends on 1 open bug)
Details
Attachments
(3 files, 2 obsolete files)
84.54 KB,
image/jpeg
|
Details | |
1.27 KB,
patch
|
vlad
:
review-
|
Details | Diff | Splinter Review |
1.31 KB,
patch
|
pavlov
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Updated•18 years ago
|
Flags: blocking1.9-
Whiteboard: [wanted-1.9]
Comment 1•18 years ago
|
||
Bilinear? Why not bicubic or similar? Or maybe a pref to change the algorithm between bilinear/bicubic/nearest-neighbor.
Comment 2•18 years ago
|
||
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•18 years ago
|
||
(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•18 years ago
|
||
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•17 years ago
|
||
Since a full zoom UI is wanted for Firefox 3, this should probably get a higher priority for Gecko 1.9.
Blocks: 389628
Comment 6•17 years ago
|
||
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•17 years ago
|
||
Same image in Epiphany (using Firefox, left) and Eye Of GNOME (right)
Comment 8•17 years ago
|
||
This isn't a Firefox 2 bug, so please stop spamming
Comment 9•17 years ago
|
||
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...
Updated•17 years ago
|
Attachment #277081 -
Attachment is obsolete: true
Comment 10•17 years ago
|
||
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•17 years ago
|
||
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.
Flags: blocking1.9- → blocking1.9?
Comment 12•17 years ago
|
||
This would cause performance regressions like bug 328388 (and maybe more), so I would prefer it not to be enabled.
Comment 13•17 years ago
|
||
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•17 years ago
|
||
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•17 years ago
|
||
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•17 years ago
|
||
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•17 years ago
|
||
> bilinear scaling shouldn't be any slower than nearest neighbour
Apparently the Render implementors didn't get the memo. :(
Comment 18•17 years ago
|
||
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•17 years ago
|
||
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.
Updated•17 years ago
|
Flags: blocking1.9? → blocking1.9+
Priority: -- → P4
Comment 20•17 years ago
|
||
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•17 years ago
|
||
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•17 years ago
|
||
> 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•17 years ago
|
||
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•17 years ago
|
||
Priority ist too low since the zoom feature is crippled!
Assignee | ||
Updated•17 years ago
|
Assignee: nobody → vladimir
Comment 26•17 years ago
|
||
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•17 years ago
|
||
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+).
Summary: fix and reenable bilinear filtering for upscaled images → [TRUNK]fix and reenable bilinear filtering for upscaled images for 1.9/Firefox 3
Whiteboard: [wanted-1.9]
Updated•17 years ago
|
Attachment #296007 -
Attachment is obsolete: true
Comment 28•17 years ago
|
||
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•17 years ago
|
||
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•17 years ago
|
||
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•17 years ago
|
||
vlad -- can we do this easily/should we do it? I'd like to see it on...
Priority: P4 → --
Assignee | ||
Comment 32•17 years ago
|
||
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.
Attachment #304372 -
Flags: review?(pavlov)
Assignee | ||
Comment 33•17 years ago
|
||
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.
Attachment #304372 -
Flags: review?(pavlov) → review-
Assignee | ||
Comment 34•17 years ago
|
||
Ok, with right check.
Attachment #304373 -
Flags: review?(pavlov)
Updated•17 years ago
|
Attachment #304373 -
Flags: review?(pavlov) → review+
Assignee | ||
Comment 35•17 years ago
|
||
Checked in.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Updated•17 years ago
|
OS: All → Windows XP
Comment 36•17 years ago
|
||
This caused bug 419277
Comment 37•17 years ago
|
||
The patch is not included in FF 3.0b4, right?
Updated•17 years ago
|
Target Milestone: --- → mozilla1.9beta4
Comment 38•17 years ago
|
||
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•17 years ago
|
||
(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•17 years ago
|
||
(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•17 years ago
|
||
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•17 years ago
|
||
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•17 years ago
|
||
I have created Bug 422179 for Linux.
Updated•17 years ago
|
Summary: [TRUNK]fix and reenable bilinear filtering for upscaled images for 1.9/Firefox 3 → Fix and reenable bilinear filtering for upscaled images for 1.9/Firefox 3 on Windows
Comment 44•17 years ago
|
||
(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.
You need to log in
before you can comment on or make changes to this bug.
Description
•