Bug 60827 – [GTK] Implement GraphicsContextCairo::imageInterpolationQuality().

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Ryuan Choi]

imageInterpolationQuality() make image rendering faster in some special case.
But, GraphicsContextCairo doesn't implement it.

[Ryuan Choi]

Created attachment 93601 [details]
Patch

[Martin Robinson]

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?

[Ryuan Choi]

Created attachment 94172 [details]
Patch

[Ryuan Choi]

(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.

[Martin Robinson]

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. :)

[Ryuan Choi]

Created attachment 94583 [details]
Patch

[Ryuan Choi]

(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.

[Andreas Kling]

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.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 94583 [details]:

animations/suspend-resume-animation.html bug 48161 (authors: [e-mail] and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 94583 [details]
Patch

Clearing flags on attachment: 94583

Committed r87151: <http://trac.webkit.org/changeset/87151>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.