Bug 48738 – GraphicsContext: Don't do full save/restore when painting with low quality scaling

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Andreas Kling]

When "useLowQualityScale" is true, GraphicsContext will currently save() and restore() in drawImage(), drawTiledImage() and drawImageBuffer().
This is unnecessary, we can just remember the current imageInterpolationQuality() and reset it after drawing.

[Andreas Kling]

Created attachment 72477 [details]
Proposed patch

[WebKit Review Bot]

Attachment 72477 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/platform/graphics/GraphicsContext.cpp:406:  More than one command on the same line  [whitespace/newline] [4]
WebCore/platform/graphics/GraphicsContext.cpp:418:  More than one command on the same line  [whitespace/newline] [4]
WebCore/platform/graphics/GraphicsContext.cpp:431:  More than one command on the same line  [whitespace/newline] [4]
WebCore/platform/graphics/GraphicsContext.cpp:483:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 4 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Attachment 72477 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/4877050

[Andreas Kling]

Comment on attachment 72477 [details]
Proposed patch

This'll need some cleaning.

[Andreas Kling]

Created attachment 72500 [details]
Proposed patch v2

Add missing GraphicsContext::imageInterpolationQuality() implementations to Skia, Haiku and WinCE.
Also avoid the call to imageInterpolationQuality() if !useLowQualityScale.

[Nikolas Zimmermann]

Comment on attachment 72500 [details]
Proposed patch v2

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

> WebCore/platform/graphics/GraphicsContext.cpp:415
> +    InterpolationQuality previousInterpolationQuality = InterpolationDefault;
>      if (useLowQualityScale) {
> -        save();
> +        previousInterpolationQuality = imageInterpolationQuality();
>          setImageInterpolationQuality(InterpolationNone);
>      }
> +
>      image->draw(this, FloatRect(dest.location(), FloatSize(tw, th)), FloatRect(src.location(), FloatSize(tsw, tsh)), styleColorSpace, op);
> +
>      if (useLowQualityScale)
> -        restore();
> +        setImageInterpolationQuality(previousInterpolationQuality);

Hm, I think we could do better, and avoid the temporary variable, if !useLowQualityScale is used:
if (useLowQualityScale) {
    InterpolationQuality previousInterpolationQuality = imageInterpolationQuality();
    setImageInterpolationQuality(InterpolationLow); // NOTE: The InterpolationNone looks wrong to me.
    image->draw(this, FloatRect(dest.location(), FloatSize(tw, th)), FloatRect(src.location(), FloatSize(tsw, tsh)), styleColorSpace, op);
    setImageInterpolationQuality(previousInterpolationQuality);
} else
    image->draw(this, FloatRect(dest.location(), FloatSize(tw, th)), FloatRect(src.location(), FloatSize(tsw, tsh)), styleColorSpace, op);

etc.
What do you think?

[Andreas Kling]

Created attachment 72938 [details]
Proposed patch v3

Updated wrt Niko's suggestion.

[Nikolas Zimmermann]

Comment on attachment 72938 [details]
Proposed patch v3

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

> WebCore/platform/graphics/GraphicsContext.cpp:408
>          setImageInterpolationQuality(InterpolationNone);

Please add a FIXME here, that None is wrong.

[Andreas Kling]

Committed r71337: <http://trac.webkit.org/changeset/71337>