WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
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
Details
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
Details
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
Details
Patch
(69.43 KB, patch)
2016-03-03 17:44 PST
,
Simon Fraser (smfr)
no flags
Details
Formatted Diff
Diff
Patch
(22.98 KB, patch)
2016-03-04 15:08 PST
,
Simon Fraser (smfr)
zalan
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1
2016-03-03 15:36:40 PST
Created
attachment 272782
[details]
Patch
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
Created
attachment 272810
[details]
Patch
Simon Fraser (smfr)
Comment 10
2016-03-03 18:27:07 PST
https://trac.webkit.org/r197541
Simon Fraser (smfr)
Comment 11
2016-03-03 18:27:15 PST
rdar://problem/23635219
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
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
>
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
Created
attachment 273050
[details]
Patch
Simon Fraser (smfr)
Comment 23
2016-03-04 15:33:04 PST
https://trac.webkit.org/r197594
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug