Bug 103896 – [skia] Improve performance of GraphicsContext::createCompatibleBuffer by using SkDevice:createCompatibleDevice

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Justin Novosad]

[skia] Improve performance of GraphicsContext::createCompatibleBuffer by using SkDevice:createCompatibleDevice

[Justin Novosad]

Created attachment 177261 [details]
Patch

[Early Warning System Bot]

Comment on attachment 177261 [details]
Patch

Attachment 177261 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15097576

[Early Warning System Bot]

Comment on attachment 177261 [details]
Patch

Attachment 177261 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15086967

[Build Bot]

Comment on attachment 177261 [details]
Patch

Attachment 177261 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15119388

[EFL EWS Bot]

Comment on attachment 177261 [details]
Patch

Attachment 177261 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15106505

[Build Bot]

Comment on attachment 177261 [details]
Patch

Attachment 177261 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15104550

[WebKit Review Bot]

Comment on attachment 177261 [details]
Patch

Attachment 177261 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15100560

New failing tests:
fast/backgrounds/gradient-background-leakage.html

[Stephen White]

Comment on attachment 177261 [details]
Patch

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

> Source/WebCore/platform/graphics/GraphicsContext.h:437
> +        PassOwnPtr<ImageBuffer> createCompatibleBuffer(const IntSize&, bool hasAlpha = true) const;

Prefer a flags enum to a bool here, to make the callsites self-documenting.  See WebKit coding style.

> Source/WebCore/platform/graphics/ImageBuffer.cpp:79
> +ImageBuffer::ImageBuffer(const IntSize&, float resolutionScale, ColorSpace colorSpace, const GraphicsContext* compatibleContext, bool hasAlpha, bool& success)
> +    : ImageBuffer(IntSize&, resolutionScale, colorSpace, compatibleContext->isAccelerated() ? Accelerated : Unaccelerated, NonDeferred, success)
> +{ }

Hmm, yeah, that's not going to work.

You could use an init() function, but I wonder if it would be possible to simply add the GraphicsContext* to the existing constructor.  If you pass NULL, it does the regular construction, otherwise it does the compatible construction.  You'd also have to put back the accelerated flag.

> Source/WebCore/platform/graphics/ImageBuffer.h:96
> +        // Creates a buffer compativble with 'context'. Will return a null pointer on allocation failure.

Typo:  compativble.

[kov's GTK+ EWS bot]

Comment on attachment 177261 [details]
Patch

Attachment 177261 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/15104559

[Justin Novosad]

> Prefer a flags enum to a bool here, to make the callsites self-documenting.  See WebKit coding style.
> 

Style guide, rule 10: "Prefer enums to bools on function parameters if callers are likely to be passing constants".

[Justin Novosad]

Created attachment 177297 [details]
Patch

[Early Warning System Bot]

Comment on attachment 177297 [details]
Patch

Attachment 177297 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15121359

[Early Warning System Bot]

Comment on attachment 177297 [details]
Patch

Attachment 177297 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15101620

[Stephen White]

(In reply to comment #10)
> > Prefer a flags enum to a bool here, to make the callsites self-documenting.  See WebKit coding style.
> > 
> 
> Style guide, rule 10: "Prefer enums to bools on function parameters if callers are likely to be passing constants".

Good point.  Justin pointed out to me offline that the callsites are all passing foo->hasAlpha(), which is self-documenting.

[Justin Novosad]

Created attachment 177305 [details]
Patch

[kov's GTK+ EWS bot]

Comment on attachment 177305 [details]
Patch

Attachment 177305 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/15126232

[Early Warning System Bot]

Comment on attachment 177305 [details]
Patch

Attachment 177305 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15117498

[Build Bot]

Comment on attachment 177305 [details]
Patch

Attachment 177305 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15098671

[Early Warning System Bot]

Comment on attachment 177305 [details]
Patch

Attachment 177305 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15105626

[Justin Novosad]

Created attachment 177311 [details]
Patch

[Stephen White]

Comment on attachment 177311 [details]
Patch

OK.  r=me

[WebKit Review Bot]

Comment on attachment 177311 [details]
Patch

Clearing flags on attachment: 177311

Committed r136755: <http://trac.webkit.org/changeset/136755>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.