Bug 98574 - Some GraphicsLayer cleanup to separate the concepts of using a tile cache, and being the main tile cache layer
Summary: Some GraphicsLayer cleanup to separate the concepts of using a tile cache, an...
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: Simon Fraser (smfr)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-05 16:24 PDT by Simon Fraser (smfr)
Modified: 2012-10-08 13:11 PDT (History)
6 users (show)

See Also:


Attachments
Patch (17.93 KB, patch)
2012-10-05 16:38 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (20.86 KB, patch)
2012-10-05 17:55 PDT, Simon Fraser (smfr)
dino: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2012-10-05 16:24:44 PDT
Some GraphicsLayer cleanup to separate the concepts of using a tile cache, and being the main tile cache layer
Comment 1 Simon Fraser (smfr) 2012-10-05 16:38:13 PDT
Created attachment 167414 [details]
Patch
Comment 2 Build Bot 2012-10-05 17:27:19 PDT
Comment on attachment 167414 [details]
Patch

Attachment 167414 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14178648
Comment 3 Simon Fraser (smfr) 2012-10-05 17:55:18 PDT
Created attachment 167429 [details]
Patch
Comment 4 Build Bot 2012-10-05 18:41:37 PDT
Comment on attachment 167429 [details]
Patch

Attachment 167429 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14179685
Comment 5 Dean Jackson 2012-10-05 19:29:43 PDT
7>..\FullscreenVideoController.cpp(250) : error C2259: 'FullscreenVideoController::LayerClient' : cannot instantiate abstract class
7>        due to following members:
7>        'bool WebCore::PlatformCALayerClient::platformCALayerShowRepaintCounter(WebCore::PlatformCALayer *) const' : is abstract
7>        c:\cygwin\home\buildbot\webkit\webkitbuild\debug\include\webcore\PlatformCALayerClient.h(55) : see declaration of 'WebCore::PlatformCALayerClient::platformCALayerShowRepaintCounter'
Comment 6 Simon Fraser (smfr) 2012-10-05 20:12:57 PDT
Boo.
Comment 7 Dean Jackson 2012-10-08 10:47:26 PDT
Comment on attachment 167429 [details]
Patch

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

> Source/WebCore/platform/graphics/GraphicsLayer.cpp:341
> +void GraphicsLayer::debugBorderInfo(Color& color, float& width) const

In a recent review from Darin, he said "We reserve the word 'get' for functions that have no return value and use out arguments instead". When I was seeing this used in action below, it wasn't clear the function was doing anything since you don't get return values. Maybe it should be getDebugBorderInfo?

> Source/WebCore/platform/graphics/GraphicsLayer.cpp:373
> +    Color borderColor;
> +    float width = 0;
> +    debugBorderInfo(borderColor, width);
> +    setDebugBorder(borderColor, width);

Here's the place I was talking about above.

> Source/WebCore/platform/graphics/ca/win/PlatformCALayerWinInternal.cpp:99
> -    if (owner()->owner()->platformCALayerShowRepaintCounter()) {
> +    if (owner()->owner()->platformCALayerShowRepaintCounter(owner())) {

wow.
Comment 8 Simon Fraser (smfr) 2012-10-08 13:11:45 PDT
http://trac.webkit.org/changeset/130676