Bug 60827 - [GTK] Implement GraphicsContextCairo::imageInterpolationQuality().
Summary: [GTK] Implement GraphicsContextCairo::imageInterpolationQuality().
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-13 20:33 PDT by Ryuan Choi
Modified: 2011-05-24 08:13 PDT (History)
4 users (show)

See Also:


Attachments
Patch (3.58 KB, patch)
2011-05-15 21:51 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (5.10 KB, patch)
2011-05-19 21:57 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (4.95 KB, patch)
2011-05-24 01:26 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryuan Choi 2011-05-13 20:33:21 PDT
imageInterpolationQuality() make image rendering faster in some special case.
But, GraphicsContextCairo doesn't implement it.
Comment 1 Ryuan Choi 2011-05-15 21:51:23 PDT
Created attachment 93601 [details]
Patch
Comment 2 Martin Robinson 2011-05-16 12:27:03 PDT
Comment on attachment 93601 [details]
Patch

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

Nice! I have a few comments below. In general, I think this patch needs a bit more context either in the ChangeLog or in the bug report. Are you just filling out a stub here?

> Source/WebCore/ChangeLog:8
> +        Implement GraphicsContextCairo::imageInterpolationQuality.

Might make sense to describe how this affects drawing performance or give more information about what prompted this change.

> Source/WebCore/ChangeLog:16
> +        * platform/graphics/cairo/GraphicsContextCairo.cpp:
> +        (WebCore::GraphicsContext::setImageInterpolationQuality):
> +        (WebCore::GraphicsContext::imageInterpolationQuality):
> +        * platform/graphics/cairo/GraphicsContextPlatformPrivateCairo.h:
> +        * platform/graphics/cairo/PlatformContextCairo.cpp:
> +        (WebCore::PlatformContextCairo::drawSurfaceToContext):
> +

Please fill these out.

> Source/WebCore/platform/graphics/cairo/GraphicsContextPlatformPrivateCairo.h:105
> +    InterpolationQuality imageInterpolationQuality;

It makes more sense to have this as a member of PlatformContextCairo, don't you think?
Comment 3 Ryuan Choi 2011-05-19 21:57:52 PDT
Created attachment 94172 [details]
Patch
Comment 4 Ryuan Choi 2011-05-19 21:59:42 PDT
(In reply to comment #2)
> (From update of attachment 93601 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=93601&action=review
> 
> Nice! I have a few comments below. In general, I think this patch needs a bit more context either in the ChangeLog or in the bug report. Are you just filling out a stub here?
> 
> > Source/WebCore/ChangeLog:8
> > +        Implement GraphicsContextCairo::imageInterpolationQuality.
> 
> Might make sense to describe how this affects drawing performance or give more information about what prompted this change.
> 
> > Source/WebCore/ChangeLog:16
> > +        * platform/graphics/cairo/GraphicsContextCairo.cpp:
> > +        (WebCore::GraphicsContext::setImageInterpolationQuality):
> > +        (WebCore::GraphicsContext::imageInterpolationQuality):
> > +        * platform/graphics/cairo/GraphicsContextPlatformPrivateCairo.h:
> > +        * platform/graphics/cairo/PlatformContextCairo.cpp:
> > +        (WebCore::PlatformContextCairo::drawSurfaceToContext):
> > +
> 
> Please fill these out.
> 
> > Source/WebCore/platform/graphics/cairo/GraphicsContextPlatformPrivateCairo.h:105
> > +    InterpolationQuality imageInterpolationQuality;
> 
> It makes more sense to have this as a member of PlatformContextCairo, don't you think?

Thank you for your review.
I prepared patch like you mentioned.
Comment 5 Martin Robinson 2011-05-23 11:38:04 PDT
Comment on attachment 94172 [details]
Patch

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

Looks good, I think the ChangeLog just lacks a bit of context.

> Source/WebCore/ChangeLog:12
> +        Implement getter/setter of imageInterpolationQuality and logic to change
> +        interpolation algorithm when drawing image.
> +        WebCore control image quality using ImageQualityController so GraphicsContext
> +        can draw image faster using proper interpolation.
> +        Mac and Qt already implemented it.

I guess I was wondering if there are particular cases where things will speed up. When does WebCore change the interpolation quality of an image? Please either even out these line or split them into separate paragraphs. :)
Comment 6 Ryuan Choi 2011-05-24 01:26:34 PDT
Created attachment 94583 [details]
Patch
Comment 7 Ryuan Choi 2011-05-24 01:33:53 PDT
(In reply to comment #5)
> (From update of attachment 94172 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=94172&action=review
> 
> Looks good, I think the ChangeLog just lacks a bit of context.
> 
> > Source/WebCore/ChangeLog:12
> > +        Implement getter/setter of imageInterpolationQuality and logic to change
> > +        interpolation algorithm when drawing image.
> > +        WebCore control image quality using ImageQualityController so GraphicsContext
> > +        can draw image faster using proper interpolation.
> > +        Mac and Qt already implemented it.
> 
> I guess I was wondering if there are particular cases where things will speed up. When does WebCore change the interpolation quality of an image? Please either even out these line or split them into separate paragraphs. :)

I removed poor explanation.

And I didn't explain all of conditions but RenderBoxModelObject check it
When panning or scaling maps.google.com, it will work.

Thank you.
Comment 8 Andreas Kling 2011-05-24 06:02:40 PDT
Comment on attachment 94583 [details]
Patch

Nice, r=me.

A little about this setting, for anyone who is interested:

The most common use case for GC's imageInterpolationQuality is indeed the ImageQualityController which is a mechanism that ensures that we don't waste time painting an image with high-quality scaling when it's likely to be repainted very soon again.

For example, when the page is being scrolled, an image may be repainted over and over (with a primitive drawing model) and WebCore then tells the GC to draw it with low quality scaling until it sits in the same place for 0.5s (cLowQualityTimeThreshold in RenderBoxModelObject.cpp.) It's also used when animating images via JavaScript, e.g by dynamically altering the width/height attributes.
Comment 9 WebKit Commit Bot 2011-05-24 08:11:11 PDT
The commit-queue encountered the following flaky tests while processing attachment 94583 [details]:

animations/suspend-resume-animation.html bug 48161 (authors: cmarrin@apple.com and simon.fraser@apple.com)
The commit-queue is continuing to process your patch.
Comment 10 WebKit Commit Bot 2011-05-24 08:13:28 PDT
Comment on attachment 94583 [details]
Patch

Clearing flags on attachment: 94583

Committed r87151: <http://trac.webkit.org/changeset/87151>
Comment 11 WebKit Commit Bot 2011-05-24 08:13:33 PDT
All reviewed patches have been landed.  Closing bug.