Summary: | -webkit-box-shadow causes awful scroll/resize/redraw performance | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Will Will <will> | ||||||||||||||||||
Component: | CSS | Assignee: | Simon Fraser (smfr) <simon.fraser> | ||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||
Severity: | Normal | CC: | 7raivis, abarth, alex, ariya.hidayat, buildbot, ddkilzer, dglazkov, divya, elanehart, eric, gianni, helder, jerem.selier, jhoneycutt, joshbryant, Justin, krit, mathias, mdelaney7, mike.wille, mitz, mrobinson, paulirish, psolanki, richardconnamacher, rik, simon.fraser, tbrearley, webkit, webkit.org, webkit.review.bot, webmaster | ||||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||
Hardware: | Mac | ||||||||||||||||||||
OS: | OS X 10.5 | ||||||||||||||||||||
Bug Depends on: | 14606, 51420 | ||||||||||||||||||||
Bug Blocks: | |||||||||||||||||||||
Attachments: |
|
Description
Will Will
2008-11-06 09:03:06 PST
Created attachment 24950 [details] Testcase Here is a testcase adapted from the one found in bugzilla : https://bugzilla.mozilla.org/show_bug.cgi?id=458031 This is probably a dupe of bug 14606. I also noticed this was *awful* tonight, and made a test case, which I've attached. Created attachment 27719 [details]
Another example of awful performance
Please retest on a recently WebKit nightly build. Testing with r42583, the test case is still pretty darned slow for window resizing. Better than 3.2.1, but still obviously slower than resizing a regular window. Created attachment 29636 [details]
Testcase #3
Testcase #3 has an example that shows the scrolling performance severely degraded Safari 3, 4, and the latest webkit nightly. I believe the two triggers are a fixed position div over a div with rounded corners, drop shadow, and a background image.
That last comment was a little off. I was a bit tired. I forgot to mention that the earlier mentioned fix by comment #4, showed little difference between r415something and r42662. There was a barely perceptible improvement. I've recently been having the same issues. However, i was using -webkit-box-shadow: 0px 0px 200px #000; when i reduced it to: -webkit-box-shadow: 0px 0px 125px #000; I found that a lot of the issues went. Similar to "Will Will". I've tested on the latest WebKit nightly (r48454), and i have the same issues there also. Created attachment 57207 [details]
Test Case (Box shadow over a fixed background image).
Test case showing very poor performance scrolling a long page where the main content div has a box shadow and the background is a fixed image.
I am pretty sure there is an older bug about this in the database but I cannot find it at the moment. I ran into this bug today while working on a site design. My design has a fixed background image and the main content in a DIV with a CSS box shadow behind it. Firefox handles it well, but WebKit stutters awfully while scrolling. (The above test case I posted was distilled from that design.) Comment on attachment 57207 [details]
Test Case (Box shadow over a fixed background image).
Includes both WebKit and Mozilla styles so performance can be compared.
I found that Safari performs far better when I recreate the box shadow using translucent PNGs. This is for the same page design: scrollable main box against a fixed background image. So compositing speed can't be the main problem, since it has to re-composite my translucent PNGs just as it's doing with the shadow. Is WebKit recalculating the shadow itself every time the user scrolls? If so then this bug could be fixed by calculating the shadow once and reusing it whenever the page is scrolled or resized. The shadow itself should only be recalculated when the dimensions of its box changes. > The shadow itself should only be recalculated when the dimensions of its box changes
What's slow is painting the shadow (it involves a gaussian blur). To speed this up, we'd have to cache a bitmap for the shadow.
*** Bug 27505 has been marked as a duplicate of this bug. *** Created attachment 66800 [details]
Test of box-shadows with animation
I noticed this bug when I was working with floating windows and scriptaculous. The shadows cause poor animation performance when fading in and out the floating windows.
*** Bug 39601 has been marked as a duplicate of this bug. *** The solution we used with ContextShadow and tiling helped to solve this issue for us, the integration is in bug 46475. *** Bug 41031 has been marked as a duplicate of this bug. *** Created attachment 77399 [details]
Patch
Comment on attachment 77399 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=77399&action=review > LayoutTests/ChangeLog:37 > + * platform/mac/fast/transforms/shadows-expected.checksum: > + * platform/mac/fast/transforms/shadows-expected.png: The shadows in the third and fourth column in the new results look wrong. They appear to be clipped. (In reply to comment #23) > (From update of attachment 77399 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=77399&action=review > > > LayoutTests/ChangeLog:37 > > + * platform/mac/fast/transforms/shadows-expected.checksum: > > + * platform/mac/fast/transforms/shadows-expected.png: > > The shadows in the third and fourth column in the new results look wrong. They appear to be clipped. I'll take a look. Created attachment 77421 [details]
Patch
I think it is better to make the tilling code platform independent, than adapting the same code for every port. So we may get rid of most of the #if PLATFORM's and minimize the separation of the code. Just one notice. Simon, if you use ContextShadow for -webkit-svg-shadow as well in the future, you'll also change the behavior. CG supports group shadow, while ContextShadow doesn't: <g style="-webkit-svg-shadow_..."> <rect x="10" y="10" width="100" height="100"/> <rect x="30" y="30" width="100" height="100"/> <rect x="50" y="50" width="100" height="100"/> </g> for CG you'll get one common shadow for all three rects, for ContextShadow every rect gets it's own shadow. (In reply to comment #26) > I think it is better to make the tilling code platform independent, than adapting the same code for every port. So we may get rid of most of the #if PLATFORM's and minimize the separation of the code. That's covered by bug 51312. > Just one notice. Simon, if you use ContextShadow for -webkit-svg-shadow as well in the future, you'll also change the behavior. CG supports group shadow, while ContextShadow doesn't: > > <g style="-webkit-svg-shadow_..."> > <rect x="10" y="10" width="100" height="100"/> > <rect x="30" y="30" width="100" height="100"/> > <rect x="50" y="50" width="100" height="100"/> > </g> > > for CG you'll get one common shadow for all three rects, for ContextShadow every rect gets it's own shadow. Right, but that should be possible to fix by choosing when to start and end the shadow layer. Comment on attachment 77421 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=77421&action=review Looks great in general, but please share the code of drawRectShadow() as much as possible with other platforms. r- because of the duplication. > WebCore/platform/graphics/cg/ContextShadowCG.cpp:141 > + // FIXME: can't get at the ImageBuffer bits directly, so we have to copy. We have a bug report for this (not sure about the bug id right now), can you add the link to this bug here please? > WebCore/platform/graphics/cg/ContextShadowCG.cpp:145 > + IntSize(layerData->width(), layerData->height()), ImageData has size() now, please use this. > WebCore/platform/graphics/cg/ContextShadowCG.cpp:369 > + > + // Draw the bottom left corner. > + tileRect = FloatRect(0, shadowTemplateSize.height() - radiusTwice - bottomLeftRadius.height(), > + radiusTwice + bottomLeftRadius.width(), radiusTwice + bottomLeftRadius.height()); > + destRect = tileRect; > + destRect.move(shadowRect.x(), shadowRect.y() + shadowedRect.height() - shadowTemplateSize.height() + radiusTwice); > + graphicsContext->drawImageBuffer(m_layerImage, ColorSpaceDeviceRGB, destRect, tileRect); Most of 60 lines above look similar of identical to the code in ContextShadowCairo. We don't need to share the complete function, but at least this code should be shared. If you want you can drop this function into a follow up patch. I'm very interested in ContexShadow for CG, since it is ideal for a sample implementation of feDropShadow. Something I'm working on ;-) Created attachment 80611 [details]
Patch
Attachment 80611 [details] did not build on win: Build output: http://queues.webkit.org/results/7584892 Attachment 80611 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7681074 http://trac.webkit.org/changeset/77101 might have broken GTK Linux 32-bit Release The following tests are not passing: fast/js/toInt32UInt32.html |