- Revision
- 141160
- Author
- ju...@google.com
- Date
- 2013-01-29 13:32:07 -0800 (Tue, 29 Jan 2013)
Log Message
REGRESSION (r135628-135632): Double box shadow failure to render
https://bugs.webkit.org/show_bug.cgi?id=107833
Reviewed by Simon Fraser.
Source/WebCore:
Regression caused by http://trac.webkit.org/changeset/135629
The regression was due to faulty occlusion logic that was assuming
that drawing the background color of a render box background layer
could be skipped when the same layer also has an opaque image attached.
In the case where the background color is drawn for the purpose of
rendering a box shadow, the shadow is typically not
completely occluded by the background image because of the shadow
blur and/or offset. This patch fixes the problem by not culling a
background draw if it is used to draw a box shadow.
Test: fast/backgrounds/gradient-background-shadow.html
* rendering/RenderBoxModelObject.cpp:
(WebCore::RenderBoxModelObject::paintFillLayerExtended):
Changing occlusion culling test to never cull background color
draw if it is used to draw a box shadow. This is because box shadows
can draw outside the border fill region.
LayoutTests:
New ref test verifies that box shadow is drawn when
background is an opaque image. Test uses an blue gradient
as background image. Reference uses blue background color.
* fast/backgrounds/gradient-background-shadow-expected.html: Added.
* fast/backgrounds/gradient-background-shadow.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (141159 => 141160)
--- trunk/LayoutTests/ChangeLog 2013-01-29 21:22:27 UTC (rev 141159)
+++ trunk/LayoutTests/ChangeLog 2013-01-29 21:32:07 UTC (rev 141160)
@@ -1,3 +1,17 @@
+2013-01-29 Justin Novosad <ju...@google.com>
+
+ REGRESSION (r135628-135632): Double box shadow failure to render
+ https://bugs.webkit.org/show_bug.cgi?id=107833
+
+ Reviewed by Simon Fraser.
+
+ New ref test verifies that box shadow is drawn when
+ background is an opaque image. Test uses an blue gradient
+ as background image. Reference uses blue background color.
+
+ * fast/backgrounds/gradient-background-shadow-expected.html: Added.
+ * fast/backgrounds/gradient-background-shadow.html: Added.
+
2013-01-29 Pravin D <pravind....@gmail.com>
Current error reporting method used by check-layout.js should not affect subsequent sub-tests using checking data-offset-y.
Added: trunk/LayoutTests/fast/backgrounds/gradient-background-shadow-expected.html (0 => 141160)
--- trunk/LayoutTests/fast/backgrounds/gradient-background-shadow-expected.html (rev 0)
+++ trunk/LayoutTests/fast/backgrounds/gradient-background-shadow-expected.html 2013-01-29 21:32:07 UTC (rev 141160)
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style type="text/css">
+ .box {
+ height: 50px;
+ width: 50px;
+ background-color: blue;
+ -webkit-box-shadow: 20px 20px 5px green;
+ }
+ </style>
+</head>
+<body>
+ Test passes when a blue box is drawn with a green shadow.
+ <div class="box"></div>
+</body>
+</html>
Added: trunk/LayoutTests/fast/backgrounds/gradient-background-shadow.html (0 => 141160)
--- trunk/LayoutTests/fast/backgrounds/gradient-background-shadow.html (rev 0)
+++ trunk/LayoutTests/fast/backgrounds/gradient-background-shadow.html 2013-01-29 21:32:07 UTC (rev 141160)
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style type="text/css">
+ .box {
+ height: 50px;
+ width: 50px;
+ background-image: -webkit-linear-gradient(blue, blue);
+ background-color: red;
+ -webkit-box-shadow: 20px 20px 5px green;
+ }
+ </style>
+</head>
+<body>
+ Test passes when a blue box is drawn with a green shadow.
+ <div class="box"></div>
+</body>
+</html>
Modified: trunk/LayoutTests/platform/chromium/TestExpectations (141159 => 141160)
--- trunk/LayoutTests/platform/chromium/TestExpectations 2013-01-29 21:22:27 UTC (rev 141159)
+++ trunk/LayoutTests/platform/chromium/TestExpectations 2013-01-29 21:32:07 UTC (rev 141160)
@@ -4284,6 +4284,7 @@
# Ref tests that now fail with Skia changes
crbug.com/169602 fast/backgrounds/background-opaque-clipped-gradients.html [ ImageOnlyFailure ]
crbug.com/169602 fast/backgrounds/background-opaque-images-over-color.html [ ImageOnlyFailure ]
+crbug.com/169602 fast/backgrounds/gradient-background-shadow.html [ ImageOnlyFailure ]
webkit.org/b/106858 [ Mac Debug ] scrollingcoordinator/non-fast-scrollable-region-transformed-iframe.html [ ImageOnlyFailure ]
webkit.org/b/106858 [ Linux Win Debug ] scrollingcoordinator/non-fast-scrollable-region-transformed-iframe.html [ ImageOnlyFailure Pass ]
Modified: trunk/Source/WebCore/ChangeLog (141159 => 141160)
--- trunk/Source/WebCore/ChangeLog 2013-01-29 21:22:27 UTC (rev 141159)
+++ trunk/Source/WebCore/ChangeLog 2013-01-29 21:32:07 UTC (rev 141160)
@@ -1,3 +1,28 @@
+2013-01-29 Justin Novosad <ju...@google.com>
+
+ REGRESSION (r135628-135632): Double box shadow failure to render
+ https://bugs.webkit.org/show_bug.cgi?id=107833
+
+ Reviewed by Simon Fraser.
+
+ Regression caused by http://trac.webkit.org/changeset/135629
+ The regression was due to faulty occlusion logic that was assuming
+ that drawing the background color of a render box background layer
+ could be skipped when the same layer also has an opaque image attached.
+ In the case where the background color is drawn for the purpose of
+ rendering a box shadow, the shadow is typically not
+ completely occluded by the background image because of the shadow
+ blur and/or offset. This patch fixes the problem by not culling a
+ background draw if it is used to draw a box shadow.
+
+ Test: fast/backgrounds/gradient-background-shadow.html
+
+ * rendering/RenderBoxModelObject.cpp:
+ (WebCore::RenderBoxModelObject::paintFillLayerExtended):
+ Changing occlusion culling test to never cull background color
+ draw if it is used to draw a box shadow. This is because box shadows
+ can draw outside the border fill region.
+
2013-01-29 Ian Vollick <voll...@chromium.org>
Add RenderLayer::enclosingStackingContainer
Modified: trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp (141159 => 141160)
--- trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp 2013-01-29 21:22:27 UTC (rev 141159)
+++ trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp 2013-01-29 21:32:07 UTC (rev 141160)
@@ -919,35 +919,37 @@
// Paint the color first underneath all images, culled if background image occludes it.
// FIXME: In the bgLayer->hasFiniteBounds() case, we could improve the culling test
// by verifying whether the background image covers the entire layout rect.
- if (!bgLayer->next() && !(shouldPaintBackgroundImage && bgLayer->hasOpaqueImage(this) && bgLayer->hasRepeatXY())) {
+ if (!bgLayer->next()) {
IntRect backgroundRect(pixelSnappedIntRect(scrolledPaintRect));
bool boxShadowShouldBeAppliedToBackground = this->boxShadowShouldBeAppliedToBackground(bleedAvoidance, box);
- if (!boxShadowShouldBeAppliedToBackground)
- backgroundRect.intersect(paintInfo.rect);
+ if (boxShadowShouldBeAppliedToBackground || !shouldPaintBackgroundImage || !bgLayer->hasOpaqueImage(this) || !bgLayer->hasRepeatXY()) {
+ if (!boxShadowShouldBeAppliedToBackground)
+ backgroundRect.intersect(paintInfo.rect);
- // If we have an alpha and we are painting the root element, go ahead and blend with the base background color.
- Color baseColor;
- bool shouldClearBackground = false;
- if (isOpaqueRoot) {
- baseColor = view()->frameView()->baseBackgroundColor();
- if (!baseColor.alpha())
- shouldClearBackground = true;
- }
+ // If we have an alpha and we are painting the root element, go ahead and blend with the base background color.
+ Color baseColor;
+ bool shouldClearBackground = false;
+ if (isOpaqueRoot) {
+ baseColor = view()->frameView()->baseBackgroundColor();
+ if (!baseColor.alpha())
+ shouldClearBackground = true;
+ }
- GraphicsContextStateSaver shadowStateSaver(*context, boxShadowShouldBeAppliedToBackground);
- if (boxShadowShouldBeAppliedToBackground)
- applyBoxShadowForBackground(context, style());
+ GraphicsContextStateSaver shadowStateSaver(*context, boxShadowShouldBeAppliedToBackground);
+ if (boxShadowShouldBeAppliedToBackground)
+ applyBoxShadowForBackground(context, style());
- if (baseColor.alpha()) {
- if (bgColor.alpha())
- baseColor = baseColor.blend(bgColor);
+ if (baseColor.alpha()) {
+ if (bgColor.alpha())
+ baseColor = baseColor.blend(bgColor);
- context->fillRect(backgroundRect, baseColor, style()->colorSpace(), CompositeCopy);
- } else if (bgColor.alpha()) {
- CompositeOperator operation = shouldClearBackground ? CompositeCopy : context->compositeOperation();
- context->fillRect(backgroundRect, bgColor, style()->colorSpace(), operation);
- } else if (shouldClearBackground)
- context->clearRect(backgroundRect);
+ context->fillRect(backgroundRect, baseColor, style()->colorSpace(), CompositeCopy);
+ } else if (bgColor.alpha()) {
+ CompositeOperator operation = shouldClearBackground ? CompositeCopy : context->compositeOperation();
+ context->fillRect(backgroundRect, bgColor, style()->colorSpace(), operation);
+ } else if (shouldClearBackground)
+ context->clearRect(backgroundRect);
+ }
}
// no progressive loading of the background image
