WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
139543
Regression(
r176212
): Carousel on mbusa.com is choppy
https://bugs.webkit.org/show_bug.cgi?id=139543
Summary
Regression(r176212): Carousel on mbusa.com is choppy
Chris Dumez
Reported
2014-12-11 10:08:12 PST
Carousel on mbusa.com is choppy due to a DOM Timer being throttled when it shouldn't. radar: <
rdar://problem/19209406
>.
Attachments
Patch
(15.95 KB, patch)
2014-12-11 12:17 PST
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(11.21 KB, patch)
2014-12-11 13:25 PST
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(14.57 KB, patch)
2014-12-12 10:14 PST
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(18.10 KB, patch)
2014-12-13 20:11 PST
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Chris Dumez
Comment 1
2014-12-11 12:17:19 PST
Created
attachment 243137
[details]
Patch
Chris Dumez
Comment 2
2014-12-11 13:25:56 PST
Created
attachment 243143
[details]
Patch
Chris Dumez
Comment 3
2014-12-11 13:51:10 PST
Comment on
attachment 243143
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=243143&action=review
> Source/WebCore/rendering/RenderElement.cpp:1338 > + accountForSelfPaintingDescendantsOverflow(child, repaintRect);
We can probably optimize further by not traversing the descendants if hasOverflowClip() return true?
Chris Dumez
Comment 4
2014-12-12 10:14:17 PST
Created
attachment 243203
[details]
Patch
Chris Dumez
Comment 5
2014-12-12 10:15:59 PST
Comment on
attachment 243203
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=243203&action=review
> Source/WebCore/rendering/RenderElement.cpp:1341 > + if (renderer.hasOverflowClip())
We now stop traversing descendants if the renderer clips its overflow to avoid unnecessary work. I also added a layout test to make sure we still throttle if the overflow is hidden.
Chris Dumez
Comment 6
2014-12-12 10:55:53 PST
Simon, what do you think?
Simon Fraser (smfr)
Comment 7
2014-12-12 11:01:39 PST
Comment on
attachment 243203
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=243203&action=review
> Source/WebCore/rendering/RenderElement.cpp:1348 > + for (auto& child : childrenOfType<RenderLayerModelObject>(renderer)) { > + if (child.hasSelfPaintingLayer()) > + repaintRect.unite(child.absoluteClippedOverflowRect()); > + accountForSelfPaintingDescendantsOverflow(child, repaintRect); > + }
This seems very costly for deep trees with no overflow. You're also doing a tree walk where you do a mapping back to the root (under child.absoluteClippedOverflowRect()) for each node, so this is potentially O(N^2).
Chris Dumez
Comment 8
2014-12-12 12:12:09 PST
(In reply to
comment #7
)
> Comment on
attachment 243203
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=243203&action=review
> > > Source/WebCore/rendering/RenderElement.cpp:1348 > > + for (auto& child : childrenOfType<RenderLayerModelObject>(renderer)) { > > + if (child.hasSelfPaintingLayer()) > > + repaintRect.unite(child.absoluteClippedOverflowRect()); > > + accountForSelfPaintingDescendantsOverflow(child, repaintRect); > > + } > > This seems very costly for deep trees with no overflow. You're also doing a > tree walk where you do a mapping back to the root (under > child.absoluteClippedOverflowRect()) for each node, so this is potentially > O(N^2).
As per our offline discussion. I will re-upload a patch that is very conservative and does not attempt to compute the overflow rect if: - The element does not have its own layer - Any descendant has its own layer Both checks are very cheap and it will still cover the cases I am interested in. I will also prepare another patch to only throttle timers that change a few CSS properties we know won't cause other elements to relayout (and will cause the element to have its own layer), namely left/opacity.
Chris Dumez
Comment 9
2014-12-13 20:11:13 PST
Created
attachment 243262
[details]
Patch
Chris Dumez
Comment 10
2014-12-16 14:32:15 PST
Simon, is the latest iteration acceptable?
Chris Dumez
Comment 11
2015-01-06 08:12:38 PST
ping review?
WebKit Commit Bot
Comment 12
2015-01-06 09:57:53 PST
Comment on
attachment 243262
[details]
Patch Clearing flags on attachment: 243262 Committed
r177964
: <
http://trac.webkit.org/changeset/177964
>
WebKit Commit Bot
Comment 13
2015-01-06 09:57:58 PST
All reviewed patches have been landed. Closing bug.
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