Use larger tiles when possible to reduce per-tile painting overhead
Created attachment 272782 [details] Patch
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?
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
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
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
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
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
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
Created attachment 272810 [details] Patch
https://trac.webkit.org/r197541
rdar://problem/23635219
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.
(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.
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; }
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.
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?
Skip the tests until we resolve the problem?
This change is also causing ios-simulator testing to exit early due to crashes: <https://build.webkit.org/results/Apple%20iOS%209%20Simulator%20Release%20WK2%20(Tests)/r197541%20(3629)/fast/frames/lots-of-iframes-crash-log.txt>
Mac tests fixed via bug 155020.
(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.
Reopening. The landed patch was missing significant functionality.
Created attachment 273050 [details] Patch
https://trac.webkit.org/r197594