Box shadow drawing takes an unnecessarily slow code path in some single-shadow, opaque-background cases
<rdar://problem/10870238>
Created attachment 127213 [details] Let the background cast the shadow directly in some cases where this is possible
Comment on attachment 127213 [details] Let the background cast the shadow directly in some cases where this is possible It's slightly slower, but you could simplify the patch considerably if you just moved: if (!boxShadowShouldBeAppliedToBackground(determineBackgroundBleedAvoidance(paintInfo.context))) paintBoxShadow(paintInfo, rect, style(), Normal); inside RenderBoxModelObject::paintBoxShadow. Then you wouldn't have to patch a bunch of call sites, and you wouldn't even have to touch InlineFlowBox. I'll leave it up to you, though, since your way is ever-so-slightly faster at the cost of complicating every call site.
I can't do this because InlineFlowBox doesn't use determineBackgroundBleedAvoidance(). Thanks for the review!
Fixed in <http://trac.webkit.org/r107836>.
On the chromium bots, this changed 3 pixel results in unexpected ways. In fast/box-shadow/spread.html, the black shapes got smaller. http://svn.webkit.org/repository/webkit/trunk/LayoutTests/platform/chromium-mac-snowleopard/fast/box-shadow/spread-expected.png http://build.chromium.org/f/chromium/layout_test_results/Webkit_Mac10_6/results/layout-test-results/fast/box-shadow/spread-actual.png http://build.chromium.org/f/chromium/layout_test_results/Webkit_Mac10_6/results/layout-test-results/fast/box-shadow/spread-diff.png In fast/writing-mode/box-shadow-horizontal-bt.html and fast/writing-mode/box-shadow-vertical-lr.html, the shadow disappeared from the first line of text. http://svn.webkit.org/repository/webkit/trunk/LayoutTests/platform/chromium-mac-snowleopard/fast/writing-mode/box-shadow-horizontal-bt-expected.png http://build.chromium.org/f/chromium/layout_test_results/Webkit_Mac10_6/results/layout-test-results/fast/writing-mode/box-shadow-horizontal-bt-actual.png http://build.chromium.org/f/chromium/layout_test_results/Webkit_Mac10_6/results/layout-test-results/fast/writing-mode/box-shadow-horizontal-bt-diff.png Was there no change for these tests on Apple Mac?
Thanks. These differences do occur in OS X WebKit, but I failed to notice them.
Filed bug 78759.
Comment on attachment 127213 [details] Let the background cast the shadow directly in some cases where this is possible View in context: https://bugs.webkit.org/attachment.cgi?id=127213&action=review > Source/WebCore/rendering/RenderBoxModelObject.cpp:2611 > + for (const FillLayer* next = lastBackgroundLayer->next(); next; ) > + lastBackgroundLayer = next; This loop is bad. We never iterate after the first iteration. Did you mean to have next = lastBackgroundLayer->next() as the third part of the for statement?
(In reply to comment #9) > (From update of attachment 127213 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=127213&action=review > > > Source/WebCore/rendering/RenderBoxModelObject.cpp:2611 > > + for (const FillLayer* next = lastBackgroundLayer->next(); next; ) > > + lastBackgroundLayer = next; > > This loop is bad. We never iterate after the first iteration. Did you mean to have next = lastBackgroundLayer->next() as the third part of the for statement? Yes. Thanks for noticing it!