Bug 128339 - Should get rid of TileController's CoverageForSlowScrolling mode
Summary: Should get rid of TileController's CoverageForSlowScrolling mode
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-02-06 15:40 PST by Beth Dakin
Modified: 2014-02-07 18:08 PST (History)
10 users (show)

See Also:


Attachments
Patch (14.29 KB, patch)
2014-02-06 15:49 PST, Beth Dakin
buildbot: commit-queue-
Details | Formatted Diff | Diff
Patch with layout test results (17.38 KB, patch)
2014-02-06 16:04 PST, Beth Dakin
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 (730.50 KB, application/zip)
2014-02-06 16:37 PST, Build Bot
no flags Details
Patch (16.86 KB, patch)
2014-02-07 14:51 PST, Beth Dakin
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (464.17 KB, application/zip)
2014-02-07 16:01 PST, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion (487.05 KB, application/zip)
2014-02-07 16:24 PST, Build Bot
no flags Details
Patch (17.37 KB, patch)
2014-02-07 16:34 PST, Beth Dakin
no flags Details | Formatted Diff | Diff
Patch (17.23 KB, patch)
2014-02-07 17:54 PST, Beth Dakin
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Beth Dakin 2014-02-06 15:40:02 PST
We should get rid of TileController's CoverageForSlowScrolling. The mode creates large tiles so that we will paint less for these sites. At one time, that was an important performance improvement because a lot of sites were still in slow scrolling mode. Now that very few sites fall into that mode, we should get rid of it since it just adds complexity.
Comment 1 Beth Dakin 2014-02-06 15:49:16 PST
Created attachment 223396 [details]
Patch
Comment 2 Beth Dakin 2014-02-06 16:04:22 PST
Created attachment 223397 [details]
Patch with layout test results
Comment 3 Tim Horton 2014-02-06 16:08:32 PST
Comment on attachment 223397 [details]
Patch with layout test results

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

> LayoutTests/platform/mac-wk2/tiled-drawing/tile-coverage-slow-scrolling-expected.txt:-12
> -      (tile size 800 x 600)

is there any reason to keep this test around?
Comment 4 Beth Dakin 2014-02-06 16:09:22 PST
(In reply to comment #3)
> (From update of attachment 223397 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=223397&action=review
> 
> > LayoutTests/platform/mac-wk2/tiled-drawing/tile-coverage-slow-scrolling-expected.txt:-12
> > -      (tile size 800 x 600)
> 
> is there any reason to keep this test around?

Not unless we want a test to document this moment. :-)
Comment 5 Simon Fraser (smfr) 2014-02-06 16:10:32 PST
Comment on attachment 223396 [details]
Patch

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

> Source/WebCore/rendering/RenderObject.cpp:1291
> +    if (!shouldClipToLayer && repaintContainer == view)
> +        repaintRect = pixelSnappedIntRect(view->backgroundRect(view));

This confuses me. Do we ever have !shouldClipToLayer when |this| != view?
Comment 6 Beth Dakin 2014-02-06 16:18:05 PST
(In reply to comment #5)
> (From update of attachment 223396 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=223396&action=review
> 
> > Source/WebCore/rendering/RenderObject.cpp:1291
> > +    if (!shouldClipToLayer && repaintContainer == view)
> > +        repaintRect = pixelSnappedIntRect(view->backgroundRect(view));
> 
> This confuses me. Do we ever have !shouldClipToLayer when |this| != view?

Hmm, no I don't think so! I was just being extra careful, but I realize now that is not necessary.
Comment 7 Build Bot 2014-02-06 16:37:58 PST
Comment on attachment 223396 [details]
Patch

Attachment 223396 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6517240081940480

New failing tests:
platform/mac-wk2/tiled-drawing/tile-size-slow-zoomed.html
platform/mac-wk2/tiled-drawing/tile-coverage-slow-scrolling.html
platform/mac-wk2/tiled-drawing/fixed-background/fixed-non-propagated-body-background.html
Comment 8 Build Bot 2014-02-06 16:37:59 PST
Created attachment 223404 [details]
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-14  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 9 Beth Dakin 2014-02-07 14:51:16 PST
Created attachment 223501 [details]
Patch
Comment 10 Build Bot 2014-02-07 16:01:52 PST
Comment on attachment 223501 [details]
Patch

Attachment 223501 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4912089470599168

New failing tests:
compositing/repaint/fixed-background-scroll.html
Comment 11 Build Bot 2014-02-07 16:01:55 PST
Created attachment 223508 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 12 Build Bot 2014-02-07 16:24:22 PST
Comment on attachment 223501 [details]
Patch

Attachment 223501 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6079075974119424

New failing tests:
compositing/repaint/fixed-background-scroll.html
Comment 13 Build Bot 2014-02-07 16:24:26 PST
Created attachment 223512 [details]
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-02  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 14 Beth Dakin 2014-02-07 16:34:41 PST
Created attachment 223515 [details]
Patch

This will fix the latest test failure.
Comment 15 Beth Dakin 2014-02-07 17:54:28 PST
Created attachment 223530 [details]
Patch
Comment 16 Beth Dakin 2014-02-07 18:08:21 PST
http://trac.webkit.org/changeset/163675