Bug 103896 - [skia] Improve performance of GraphicsContext::createCompatibleBuffer by using SkDevice:createCompatibleDevice
Summary: [skia] Improve performance of GraphicsContext::createCompatibleBuffer by usin...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Justin Novosad
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-03 08:17 PST by Justin Novosad
Modified: 2012-12-05 13:41 PST (History)
8 users (show)

See Also:


Attachments
Patch (12.51 KB, patch)
2012-12-03 08:39 PST, Justin Novosad
no flags Details | Formatted Diff | Diff
Patch (12.44 KB, patch)
2012-12-03 11:48 PST, Justin Novosad
no flags Details | Formatted Diff | Diff
Patch (13.57 KB, patch)
2012-12-03 12:12 PST, Justin Novosad
no flags Details | Formatted Diff | Diff
Patch (13.58 KB, patch)
2012-12-03 12:41 PST, Justin Novosad
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Justin Novosad 2012-12-03 08:17:15 PST
[skia] Improve performance of GraphicsContext::createCompatibleBuffer by using SkDevice:createCompatibleDevice
Comment 1 Justin Novosad 2012-12-03 08:39:51 PST
Created attachment 177261 [details]
Patch
Comment 2 Early Warning System Bot 2012-12-03 08:45:57 PST
Comment on attachment 177261 [details]
Patch

Attachment 177261 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15097576
Comment 3 Early Warning System Bot 2012-12-03 08:47:30 PST
Comment on attachment 177261 [details]
Patch

Attachment 177261 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15086967
Comment 4 Build Bot 2012-12-03 08:49:03 PST
Comment on attachment 177261 [details]
Patch

Attachment 177261 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15119388
Comment 5 EFL EWS Bot 2012-12-03 08:49:55 PST
Comment on attachment 177261 [details]
Patch

Attachment 177261 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15106505
Comment 6 Build Bot 2012-12-03 08:56:53 PST
Comment on attachment 177261 [details]
Patch

Attachment 177261 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15104550
Comment 7 WebKit Review Bot 2012-12-03 09:23:17 PST
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
Comment 8 Stephen White 2012-12-03 09:23:57 PST
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.
Comment 9 kov's GTK+ EWS bot 2012-12-03 09:25:29 PST
Comment on attachment 177261 [details]
Patch

Attachment 177261 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/15104559
Comment 10 Justin Novosad 2012-12-03 11:47:19 PST
> 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".
Comment 11 Justin Novosad 2012-12-03 11:48:05 PST
Created attachment 177297 [details]
Patch
Comment 12 Early Warning System Bot 2012-12-03 11:54:10 PST
Comment on attachment 177297 [details]
Patch

Attachment 177297 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15121359
Comment 13 Early Warning System Bot 2012-12-03 11:54:15 PST
Comment on attachment 177297 [details]
Patch

Attachment 177297 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15101620
Comment 14 Stephen White 2012-12-03 11:57:52 PST
(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.
Comment 15 Justin Novosad 2012-12-03 12:12:34 PST
Created attachment 177305 [details]
Patch
Comment 16 kov's GTK+ EWS bot 2012-12-03 12:22:51 PST
Comment on attachment 177305 [details]
Patch

Attachment 177305 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/15126232
Comment 17 Early Warning System Bot 2012-12-03 12:25:14 PST
Comment on attachment 177305 [details]
Patch

Attachment 177305 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15117498
Comment 18 Build Bot 2012-12-03 12:30:41 PST
Comment on attachment 177305 [details]
Patch

Attachment 177305 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15098671
Comment 19 Early Warning System Bot 2012-12-03 12:31:07 PST
Comment on attachment 177305 [details]
Patch

Attachment 177305 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15105626
Comment 20 Justin Novosad 2012-12-03 12:41:52 PST
Created attachment 177311 [details]
Patch
Comment 21 Stephen White 2012-12-04 13:35:17 PST
Comment on attachment 177311 [details]
Patch

OK.  r=me
Comment 22 WebKit Review Bot 2012-12-05 13:41:21 PST
Comment on attachment 177311 [details]
Patch

Clearing flags on attachment: 177311

Committed r136755: <http://trac.webkit.org/changeset/136755>
Comment 23 WebKit Review Bot 2012-12-05 13:41:25 PST
All reviewed patches have been landed.  Closing bug.