Bug 60827

Created attachment 93601 [details] Patch

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?

Created attachment 94172 [details] Patch

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

Created attachment 94583 [details] Patch

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

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 on attachment 94583 [details] Patch Clearing flags on attachment: 94583 Committed r87151: <http://trac.webkit.org/changeset/87151>

All reviewed patches have been landed. Closing bug.