Bug 147826

Summary: [Cairo] Improve image quality when using newer versions of cairo/pixman
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: mrobinson, ossy, zan
Priority: P2 Keywords: Cairo
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
mrobinson: review+
Before (bilinear)
none
After (good) none

Description Carlos Garcia Campos 2015-08-10 06:05:03 PDT
Since cairo 1.14 the image filters changed a bit:

 - CAIRO_FILTER_GOOD uses a box filter when downscaling if the scale factor is less than 0.75, otherwise it uses a filter equivalent to CAIRO_FILTER_BILINEAR.
 - CAIRO_FILTER_BEST uses always a Catmull-Rom filter.

We are currently using CAIRO_FILTER_BILINEAR for medium, high and default interpolation levels. We could use CAIRO_FILTER_GOOD for medium and default, and CAIRO_FILTER_BEST for high. This will not have any effect in previous versions of cairo because before 1.14 CAIRO_FILTER_GOOD, CAIRO_FILTER_BILINEAR and CAIRO_FILTER_BEST had the same implementation in pixman.
Comment 1 Carlos Garcia Campos 2015-08-10 06:08:11 PDT
Created attachment 258615 [details]
Patch

I took screenshots of MiniBrowser downscaling this image http://bighugelabs.com/flickr/onblack.php?id=4305303148&size=large before and after the patch. I'll submit them so you can see the differences.
Comment 2 Carlos Garcia Campos 2015-08-10 06:08:55 PDT
Created attachment 258616 [details]
Before (bilinear)
Comment 3 Carlos Garcia Campos 2015-08-10 06:09:30 PDT
Created attachment 258617 [details]
After (good)
Comment 4 Martin Robinson 2015-08-10 19:07:05 PDT
This seems like a reasonable change, though I have some concerns about the performance implications. It'd probably best to match the Mac port as closely as possible here.
Comment 5 Carlos Garcia Campos 2015-08-10 23:12:51 PDT
(In reply to comment #4)
> This seems like a reasonable change, though I have some concerns about the
> performance implications. It'd probably best to match the Mac port as
> closely as possible here.

I don't think there's much difference in terms of performance, GOOD only uses the box filter for downscaling, otherwise it does bilinear, so in most of the cases it will be the same, and the performance of the box filter is not that different to bilinear either. The difference will be when using BEST, because the Catmull-Rom filter will be definitely slower than bilinear, but it's only used for the High quality interpolation that is supposed to be used when the performance is not that important. WebCore already uses no interpolation (FAST which is NEAREST) when loading or scrolling, and after a timeout of staying in the same position, a better interpolation is used. I don't know exactly the details, though.

And regarding matching Mac, well, it's very difficult to match the performance of two different rendering libraries, in my opinion. But reading the documentation of CGInterpolationQuality this patch matches the definitions of the Medium and High values, I would say:

kCGInterpolationMedium: A medium level of interpolation quality. This setting is slower than the low setting but faster than the high setting.

kCGInterpolationHigh: A high level of interpolation quality. This setting may slow down image rendering.
Comment 6 Carlos Garcia Campos 2015-08-12 23:07:52 PDT
Committed r188379: <http://trac.webkit.org/changeset/188379>
Comment 7 Zan Dobersek 2015-08-24 00:11:03 PDT
This caused a regression in wk4wl on Raspberry Pi, where layer flushes can take up to 3 seconds because of the time spent updating the tiled backing store. With a couple of flushes stringed together, a whole page can often freeze up for a longer period.
Comment 8 Carlos Garcia Campos 2015-08-24 06:17:49 PDT
Comment on attachment 258615 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=258615&action=review

> Source/WebCore/platform/graphics/cairo/PlatformContextCairo.cpp:203
> +        cairo_pattern_set_filter(pattern.get(), CAIRO_FILTER_GOOD);
> +    case InterpolationHigh:

Here is the problem, I'm afraid . . . there's a break missing there, which means we are now using CAIRO_FILTER_BEST as default :-P