RESOLVED FIXED 154985
Use larger tiles when possible to reduce per-tile painting overhead
https://bugs.webkit.org/show_bug.cgi?id=154985
Summary Use larger tiles when possible to reduce per-tile painting overhead
Simon Fraser (smfr)
Reported 2016-03-03 15:26:54 PST
Use larger tiles when possible to reduce per-tile painting overhead
Attachments
Patch (61.75 KB, patch)
2016-03-03 15:36 PST, Simon Fraser (smfr)
no flags
Archive of layout-test-results from ews100 for mac-yosemite (799.97 KB, application/zip)
2016-03-03 16:24 PST, Build Bot
no flags
Archive of layout-test-results from ews107 for mac-yosemite-wk2 (1.02 MB, application/zip)
2016-03-03 16:27 PST, Build Bot
no flags
Archive of layout-test-results from ews112 for mac-yosemite (922.84 KB, application/zip)
2016-03-03 16:35 PST, Build Bot
no flags
Patch (69.43 KB, patch)
2016-03-03 17:44 PST, Simon Fraser (smfr)
no flags
Patch (22.98 KB, patch)
2016-03-04 15:08 PST, Simon Fraser (smfr)
zalan: review+
Simon Fraser (smfr)
Comment 1 2016-03-03 15:36:40 PST
Tim Horton
Comment 2 2016-03-03 15:46:23 PST
Comment on attachment 272782 [details] Patch Overall, I think this is awesome. I am a bit worried about pages that jump repeatedly between scrollable and not scrollable (usually not intentionally); perhaps there could be some hysteresis? Or don't change until mayBegin phase? Or something?
Build Bot
Comment 3 2016-03-03 16:24:04 PST
Comment on attachment 272782 [details] Patch Attachment 272782 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/917964 New failing tests: compositing/tiling/tiled-layer-resize.html compositing/tiling/transform-origin-tiled.html pageoverlay/overlay-large-document-scrolled.html displaylists/replay-skip-clipped-rect.html compositing/tiling/rotated-tiled-preserve3d-clamped.html pageoverlay/overlay-large-document.html compositing/tiling/rotated-tiled-clamped.html
Build Bot
Comment 4 2016-03-03 16:24:07 PST
Created attachment 272790 [details] Archive of layout-test-results from ews100 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-yosemite Platform: Mac OS X 10.10.5
Build Bot
Comment 5 2016-03-03 16:27:37 PST
Comment on attachment 272782 [details] Patch Attachment 272782 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/917966 New failing tests: compositing/tiling/tiled-layer-resize.html compositing/tiling/transform-origin-tiled.html pageoverlay/overlay-large-document-scrolled.html displaylists/replay-skip-clipped-rect.html compositing/tiling/rotated-tiled-preserve3d-clamped.html imported/blink/fast/text/wide-preformatted.html pageoverlay/overlay-large-document.html compositing/tiling/rotated-tiled-clamped.html
Build Bot
Comment 6 2016-03-03 16:27:40 PST
Created attachment 272792 [details] Archive of layout-test-results from ews107 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Build Bot
Comment 7 2016-03-03 16:35:17 PST
Comment on attachment 272782 [details] Patch Attachment 272782 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/917968 New failing tests: compositing/tiling/tiled-layer-resize.html compositing/tiling/transform-origin-tiled.html pageoverlay/overlay-large-document-scrolled.html displaylists/replay-skip-clipped-rect.html compositing/tiling/rotated-tiled-preserve3d-clamped.html pageoverlay/overlay-large-document.html compositing/tiling/rotated-tiled-clamped.html
Build Bot
Comment 8 2016-03-03 16:35:20 PST
Created attachment 272797 [details] Archive of layout-test-results from ews112 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews112 Port: mac-yosemite Platform: Mac OS X 10.10.5
Simon Fraser (smfr)
Comment 9 2016-03-03 17:44:12 PST
Simon Fraser (smfr)
Comment 10 2016-03-03 18:27:07 PST
Simon Fraser (smfr)
Comment 11 2016-03-03 18:27:15 PST
Said Abou-Hallawa
Comment 12 2016-03-03 18:28:24 PST
Comment on attachment 272810 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=272810&action=review > Source/WebCore/platform/graphics/EdgeSet.h:32 > +class EdgeSet { The functionality of this class is a subset of BoxExtent in LengthBox.h. Why do we add a new class? You can change the name of BoxExtent if you want. Actually this is what I tried to do in https://bugs.webkit.org/show_bug.cgi?id=147495 but it's never got reviewed.
Simon Fraser (smfr)
Comment 13 2016-03-03 22:38:19 PST
(In reply to comment #12) > Comment on attachment 272810 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=272810&action=review > > > Source/WebCore/platform/graphics/EdgeSet.h:32 > > +class EdgeSet { > > The functionality of this class is a subset of BoxExtent in LengthBox.h. Why > do we add a new class? You can change the name of BoxExtent if you want. > Actually this is what I tried to do in > https://bugs.webkit.org/show_bug.cgi?id=147495 but it's never got reviewed. BoxExtent should be changed to use this template.
Antti Koivisto
Comment 14 2016-03-04 04:59:32 PST
Comment on attachment 272810 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=272810&action=review > Source/WebCore/platform/graphics/ca/TileGrid.cpp:340 > + m_tileSize = m_controller.tileSize(); > + if (m_tileSize != m_tileSizeAtLastRevalidate) { > + removeAllTiles(); > + m_tileSizeAtLastRevalidate = m_tileSize; > + } Is m_tileSizeAtLastRevalidate member actually needed? Doesn't this do the same? auto newTileSize = m_controller.tileSize(); if (newTileSize != m_tileSize) { removeAllTiles(); m_tileSize = newTileSize; }
Said Abou-Hallawa
Comment 15 2016-03-04 08:32:39 PST
Comment on attachment 272810 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=272810&action=review >>> Source/WebCore/platform/graphics/EdgeSet.h:32 >>> +class EdgeSet { >> >> The functionality of this class is a subset of BoxExtent in LengthBox.h. Why do we add a new class? You can change the name of BoxExtent if you want. Actually this is what I tried to do in https://bugs.webkit.org/show_bug.cgi?id=147495 but it's never got reviewed. > > BoxExtent should be changed to use this template. BoxExtent has the functionality to set/get a physical side (or edge) given the logical side name, the text direction and the writing mode. EdgeSet does not have such functionality. I think it does not matter which one stays and which one goes away. What it matters is we should not have two classes one of them is a subset of the other.
Darin Adler
Comment 16 2016-03-04 08:37:06 PST
Every since r197541 landed, 21 tiled drawing tests have been failing on bots like elcapitan-release-tests-wk2, and also locally on my computer and on EWS, with differences like this one: - (tile size 800 x 600) - (top left tile 0, 0 tiles grid 1 x 1) + (tile size 512 x 512) + (top left tile 0, 0 tiles grid 2 x 2) Should we roll this change out? Change expected results?
Darin Adler
Comment 17 2016-03-04 08:38:55 PST
Skip the tests until we resolve the problem?
Ryan Haddad
Comment 18 2016-03-04 09:59:37 PST
Simon Fraser (smfr)
Comment 19 2016-03-04 10:49:30 PST
Mac tests fixed via bug 155020.
Simon Fraser (smfr)
Comment 20 2016-03-04 13:44:44 PST
(In reply to comment #13) > (In reply to comment #12) > > Comment on attachment 272810 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=272810&action=review > > > > > Source/WebCore/platform/graphics/EdgeSet.h:32 > > > +class EdgeSet { > > > > The functionality of this class is a subset of BoxExtent in LengthBox.h. Why > > do we add a new class? You can change the name of BoxExtent if you want. > > Actually this is what I tried to do in > > https://bugs.webkit.org/show_bug.cgi?id=147495 but it's never got reviewed. > > BoxExtent should be changed to use this template. I recant. Bug 155040.
Simon Fraser (smfr)
Comment 21 2016-03-04 15:04:39 PST
Reopening. The landed patch was missing significant functionality.
Simon Fraser (smfr)
Comment 22 2016-03-04 15:08:14 PST
Simon Fraser (smfr)
Comment 23 2016-03-04 15:33:04 PST
Note You need to log in before you can comment on or make changes to this bug.