Subpixel rendering: Make webkit-box-shadow painting subpixel aware.
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 23 May 2014 14:27:19 +0000 (14:27 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 23 May 2014 14:27:19 +0000 (14:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=133201
<rdar://problem/16072830>

Reviewed by Simon Fraser.

This patch enables webkit-box-shadow to be painted on a subpixel position. However, we
still truncate -webkit-box-shadow property values. Tracked here: http://webkit.org/b/133202

Source/WebCore:
Test: fast/box-shadow/hidpi-webkit-box-shadow-subpixel-position.html

* platform/graphics/FloatRoundedRect.cpp:
(WebCore::FloatRoundedRect::inflateWithRadii): same as in from RoundedRect.
(WebCore::FloatRoundedRect::adjustRadii): same as in from RoundedRect.
* platform/graphics/FloatRoundedRect.h:
* rendering/RenderBoxModelObject.cpp:
(WebCore::RenderBoxModelObject::paintBoxShadow):

LayoutTests:
Currently not ref-testable.

* fast/box-shadow/hidpi-webkit-box-shadow-subpixel-position.html: Added.
* platform/mac/fast/box-shadow/hidpi-webkit-box-shadow-subpixel-position-expected.png: Added.
* platform/mac/fast/box-shadow/hidpi-webkit-box-shadow-subpixel-position-expected.txt: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@169257 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/box-shadow/hidpi-webkit-box-shadow-subpixel-position.html [new file with mode: 0644]
LayoutTests/platform/mac/fast/box-shadow/hidpi-webkit-box-shadow-subpixel-position-expected.png [new file with mode: 0644]
LayoutTests/platform/mac/fast/box-shadow/hidpi-webkit-box-shadow-subpixel-position-expected.txt [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/FloatRoundedRect.cpp
Source/WebCore/platform/graphics/FloatRoundedRect.h
Source/WebCore/rendering/RenderBoxModelObject.cpp

index 209b7f3..c89902e 100644 (file)
@@ -1,3 +1,20 @@
+2014-05-23  Zalan Bujtas  <zalan@apple.com>
+
+        Subpixel rendering: Make webkit-box-shadow painting subpixel aware.
+        https://bugs.webkit.org/show_bug.cgi?id=133201
+        <rdar://problem/16072830>
+
+        Reviewed by Simon Fraser.
+
+        This patch enables webkit-box-shadow to be painted on a subpixel position. However, we
+        still truncate -webkit-box-shadow property values. Tracked here: http://webkit.org/b/133202
+
+        Currently not ref-testable.
+
+        * fast/box-shadow/hidpi-webkit-box-shadow-subpixel-position.html: Added.
+        * platform/mac/fast/box-shadow/hidpi-webkit-box-shadow-subpixel-position-expected.png: Added.
+        * platform/mac/fast/box-shadow/hidpi-webkit-box-shadow-subpixel-position-expected.txt: Added.
+
 2014-05-22  Simon Fraser  <simon.fraser@apple.com>
 
         Make viewport units work in CSS gradients
diff --git a/LayoutTests/fast/box-shadow/hidpi-webkit-box-shadow-subpixel-position.html b/LayoutTests/fast/box-shadow/hidpi-webkit-box-shadow-subpixel-position.html
new file mode 100644 (file)
index 0000000..e012b62
--- /dev/null
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>This tests whether -webkit-box-shadow on subpixel position is rendered correctly.</title>
+<style>
+  div {
+    display: inline-block;
+    width: 340px;
+    height: 40px;
+  }
+</style>
+</head>
+<body>
+spacingspacing<div style="-webkit-box-shadow: inset 0 0 1px rgba(0, 0, 0, 1.0);">inset webkit-box-shadow</div></br>
+spacingspacing<div style="-webkit-box-shadow: 0 0 1px rgba(0, 0, 0, 1.0);">normal webkit-box-shadow</div>
+</body>
+</html>
diff --git a/LayoutTests/platform/mac/fast/box-shadow/hidpi-webkit-box-shadow-subpixel-position-expected.png b/LayoutTests/platform/mac/fast/box-shadow/hidpi-webkit-box-shadow-subpixel-position-expected.png
new file mode 100644 (file)
index 0000000..410ad17
Binary files /dev/null and b/LayoutTests/platform/mac/fast/box-shadow/hidpi-webkit-box-shadow-subpixel-position-expected.png differ
diff --git a/LayoutTests/platform/mac/fast/box-shadow/hidpi-webkit-box-shadow-subpixel-position-expected.txt b/LayoutTests/platform/mac/fast/box-shadow/hidpi-webkit-box-shadow-subpixel-position-expected.txt
new file mode 100644 (file)
index 0000000..f58105b
--- /dev/null
@@ -0,0 +1,17 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x96
+  RenderBlock {HTML} at (0,0) size 800x96
+    RenderBody {BODY} at (8,8) size 784x80
+      RenderText {#text} at (0,0) size 96x18
+        text run at (0,0) width 96: "spacingspacing"
+      RenderBlock {DIV} at (96,0) size 340x40
+        RenderText {#text} at (0,0) size 159x18
+          text run at (0,0) width 159: "inset webkit-box-shadow"
+      RenderBR {BR} at (436,0) size 0x18
+      RenderText {#text} at (0,40) size 96x18
+        text run at (0,40) width 96: "spacingspacing"
+      RenderBlock {DIV} at (96,40) size 340x40
+        RenderText {#text} at (0,0) size 174x18
+          text run at (0,0) width 174: "normal webkit-box-shadow"
+      RenderText {#text} at (0,0) size 0x0
index a4a56d6..28a79b9 100644 (file)
@@ -1,3 +1,23 @@
+2014-05-23  Zalan Bujtas  <zalan@apple.com>
+
+        Subpixel rendering: Make webkit-box-shadow painting subpixel aware.
+        https://bugs.webkit.org/show_bug.cgi?id=133201
+        <rdar://problem/16072830>
+
+        Reviewed by Simon Fraser.
+
+        This patch enables webkit-box-shadow to be painted on a subpixel position. However, we
+        still truncate -webkit-box-shadow property values. Tracked here: http://webkit.org/b/133202
+
+        Test: fast/box-shadow/hidpi-webkit-box-shadow-subpixel-position.html
+
+        * platform/graphics/FloatRoundedRect.cpp:
+        (WebCore::FloatRoundedRect::inflateWithRadii): same as in from RoundedRect.
+        (WebCore::FloatRoundedRect::adjustRadii): same as in from RoundedRect.
+        * platform/graphics/FloatRoundedRect.h:
+        * rendering/RenderBoxModelObject.cpp:
+        (WebCore::RenderBoxModelObject::paintBoxShadow):
+
 2014-05-22  peavo@outlook.com  <peavo@outlook.com>
 
         [Curl] Crash when exceeding maximum cache limit.
index 18d5e45..a3c9d28 100644 (file)
@@ -151,4 +151,34 @@ bool FloatRoundedRect::isRenderable() const
         && m_radii.topRight().height() + m_radii.bottomRight().height() <= m_rect.height();
 }
 
+void FloatRoundedRect::inflateWithRadii(float size)
+{
+    FloatRect old = m_rect;
+
+    m_rect.inflate(size);
+    // Considering the inflation factor of shorter size to scale the radii seems appropriate here
+    float factor;
+    if (m_rect.width() < m_rect.height())
+        factor = old.width() ? m_rect.width() / old.width() : 0;
+    else
+        factor = old.height() ? m_rect.height() / old.height() : 0;
+
+    m_radii.scale(factor);
+}
+
+void FloatRoundedRect::adjustRadii()
+{
+    float maxRadiusWidth = std::max(m_radii.topLeft().width() + m_radii.topRight().width(), m_radii.bottomLeft().width() + m_radii.bottomRight().width());
+    float maxRadiusHeight = std::max(m_radii.topLeft().height() + m_radii.bottomLeft().height(), m_radii.topRight().height() + m_radii.bottomRight().height());
+
+    if (maxRadiusWidth <= 0 || maxRadiusHeight <= 0) {
+        m_radii.scale(0.0f);
+        return;
+    }
+    float widthRatio = m_rect.width() / maxRadiusWidth;
+    float heightRatio = m_rect.height() / maxRadiusHeight;
+    m_radii.scale(widthRatio < heightRatio ? widthRatio : heightRatio);
+}
+
+
 } // namespace WebCore
index 60a6a5d..7aeb8ae 100644 (file)
@@ -98,6 +98,8 @@ public:
     void inflate(float size) { m_rect.inflate(size);  }
     void expandRadii(float size) { m_radii.expand(size); }
     void shrinkRadii(float size) { m_radii.shrink(size); }
+    void inflateWithRadii(float size);
+    void adjustRadii();
 
     FloatRect topLeftCorner() const
     {
index 6f45754..03fe9a1 100644 (file)
@@ -2369,18 +2369,20 @@ void RenderBoxModelObject::paintBoxShadow(const PaintInfo& info, const LayoutRec
     if (context->paintingDisabled() || !style.boxShadow())
         return;
 
-    RoundedRect border = (shadowStyle == Inset)
-        ? style.getRoundedInnerBorderFor(paintRect, includeLogicalLeftEdge, includeLogicalRightEdge)
+    RoundedRect border = (shadowStyle == Inset) ? style.getRoundedInnerBorderFor(paintRect, includeLogicalLeftEdge, includeLogicalRightEdge)
         : style.getRoundedBorderFor(paintRect, &view(), includeLogicalLeftEdge, includeLogicalRightEdge);
 
     bool hasBorderRadius = style.hasBorderRadius();
     bool isHorizontal = style.isHorizontalWritingMode();
+    float deviceScaleFactor = document().deviceScaleFactor();
     
     bool hasOpaqueBackground = style.visitedDependentColor(CSSPropertyBackgroundColor).isValid() && style.visitedDependentColor(CSSPropertyBackgroundColor).alpha() == 255;
     for (const ShadowData* shadow = style.boxShadow(); shadow; shadow = shadow->next()) {
         if (shadow->style() != shadowStyle)
             continue;
 
+        // FIXME: Add subpixel support for the shadow values. Soon after the shadow offset becomes fractional,
+        // all the early snappings here need to be pushed to the actual painting operations.
         IntSize shadowOffset(shadow->x(), shadow->y());
         int shadowRadius = shadow->radius();
         int shadowPaintingExtent = shadow->paintingExtent();
@@ -2397,16 +2399,16 @@ void RenderBoxModelObject::paintBoxShadow(const PaintInfo& info, const LayoutRec
             if (fillRect.isEmpty())
                 continue;
 
-            IntRect shadowRect(border.rect());
-            shadowRect.inflate(shadowPaintingExtent + shadowSpread);
-            shadowRect.move(shadowOffset);
+            FloatRect pixelSnappedShadowRect = pixelSnappedForPainting(border.rect(), deviceScaleFactor);
+            pixelSnappedShadowRect.inflate(shadowPaintingExtent + shadowSpread);
+            pixelSnappedShadowRect.move(shadowOffset);
 
             GraphicsContextStateSaver stateSaver(*context);
-            context->clip(shadowRect);
+            context->clip(pixelSnappedShadowRect);
 
             // Move the fill just outside the clip, adding 1 pixel separation so that the fill does not
             // bleed in (due to antialiasing) if the context is transformed.
-            IntSize extraOffset(paintRect.pixelSnappedWidth() + std::max(0, shadowOffset.width()) + shadowPaintingExtent + 2 * shadowSpread + 1, 0);
+            IntSize extraOffset(roundToInt(paintRect.width()) + std::max(0, shadowOffset.width()) + shadowPaintingExtent + 2 * shadowSpread + 1, 0);
             shadowOffset -= extraOffset;
             fillRect.move(extraOffset);
 
@@ -2415,32 +2417,30 @@ void RenderBoxModelObject::paintBoxShadow(const PaintInfo& info, const LayoutRec
             else
                 context->setShadow(shadowOffset, shadowRadius, shadowColor, style.colorSpace());
 
+            FloatRoundedRect rectToClipOut = border.pixelSnappedRoundedRectForPainting(deviceScaleFactor);
+            FloatRoundedRect pixelSnappedFillRect = fillRect.pixelSnappedRoundedRectForPainting(deviceScaleFactor);
             if (hasBorderRadius) {
-                RoundedRect rectToClipOut = border;
-
                 // If the box is opaque, it is unnecessary to clip it out. However, doing so saves time
                 // when painting the shadow. On the other hand, it introduces subpixel gaps along the
                 // corners. Those are avoided by insetting the clipping path by one pixel.
-                if (hasOpaqueBackground) {
-                    rectToClipOut.inflateWithRadii(-1);
-                }
+                if (hasOpaqueBackground)
+                    rectToClipOut.inflateWithRadii(LayoutUnit::fromPixel(-1));
 
                 if (!rectToClipOut.isEmpty())
-                    context->clipOutRoundedRect(FloatRoundedRect(rectToClipOut));
+                    context->clipOutRoundedRect(rectToClipOut);
 
-                RoundedRect influenceRect(shadowRect, border.radii());
+                RoundedRect influenceRect(LayoutRect(pixelSnappedShadowRect), border.radii());
                 influenceRect.expandRadii(2 * shadowPaintingExtent + shadowSpread);
+
                 if (allCornersClippedOut(influenceRect, info.rect))
-                    context->fillRect(fillRect.rect(), Color::black, style.colorSpace());
+                    context->fillRect(pixelSnappedFillRect.rect(), Color::black, style.colorSpace());
                 else {
-                    fillRect.expandRadii(shadowSpread);
-                    if (!fillRect.isRenderable())
-                        fillRect.adjustRadii();
-                    context->fillRoundedRect(FloatRoundedRect(fillRect), Color::black, style.colorSpace());
+                    pixelSnappedFillRect.expandRadii(shadowSpread);
+                    if (!pixelSnappedFillRect.isRenderable())
+                        pixelSnappedFillRect.adjustRadii();
+                    context->fillRoundedRect(pixelSnappedFillRect, Color::black, style.colorSpace());
                 }
             } else {
-                LayoutRect rectToClipOut = border.rect();
-
                 // If the box is opaque, it is unnecessary to clip it out. However, doing so saves time
                 // when painting the shadow. On the other hand, it introduces subpixel gaps along the
                 // edges if they are not pixel-aligned. Those are avoided by insetting the clipping path
@@ -2450,57 +2450,58 @@ void RenderBoxModelObject::paintBoxShadow(const PaintInfo& info, const LayoutRec
                     // FIXME: It's not clear if this check is right. What about integral scale factors?
                     AffineTransform transform = context->getCTM();
                     if (transform.a() != 1 || (transform.d() != 1 && transform.d() != -1) || transform.b() || transform.c())
-                        rectToClipOut.inflate(-1);
+                        rectToClipOut.inflate(LayoutUnit::fromPixel(-1).toFloat());
                 }
 
                 if (!rectToClipOut.isEmpty())
-                    context->clipOut(rectToClipOut);
-                context->fillRect(fillRect.rect(), Color::black, style.colorSpace());
+                    context->clipOut(rectToClipOut.rect());
+                context->fillRect(pixelSnappedFillRect.rect(), Color::black, style.colorSpace());
             }
         } else {
             // Inset shadow.
-            IntRect holeRect(border.rect());
-            holeRect.inflate(-shadowSpread);
+            FloatRoundedRect pixelSnappedBorderRect = border.pixelSnappedRoundedRectForPainting(deviceScaleFactor);
+            FloatRect pixelSnappedHoleRect = pixelSnappedBorderRect.rect();
+            pixelSnappedHoleRect.inflate(-shadowSpread);
 
-            if (holeRect.isEmpty()) {
+            if (pixelSnappedHoleRect.isEmpty()) {
                 if (hasBorderRadius)
-                    context->fillRoundedRect(FloatRoundedRect(border), shadowColor, style.colorSpace());
+                    context->fillRoundedRect(pixelSnappedBorderRect, shadowColor, style.colorSpace());
                 else
-                    context->fillRect(border.rect(), shadowColor, style.colorSpace());
+                    context->fillRect(pixelSnappedBorderRect.rect(), shadowColor, style.colorSpace());
                 continue;
             }
 
             if (!includeLogicalLeftEdge) {
                 if (isHorizontal) {
-                    holeRect.move(-std::max(shadowOffset.width(), 0) - shadowPaintingExtent, 0);
-                    holeRect.setWidth(holeRect.width() + std::max(shadowOffset.width(), 0) + shadowPaintingExtent);
+                    pixelSnappedHoleRect.move(-std::max(shadowOffset.width(), 0) - shadowPaintingExtent, 0);
+                    pixelSnappedHoleRect.setWidth(pixelSnappedHoleRect.width() + std::max(shadowOffset.width(), 0) + shadowPaintingExtent);
                 } else {
-                    holeRect.move(0, -std::max(shadowOffset.height(), 0) - shadowPaintingExtent);
-                    holeRect.setHeight(holeRect.height() + std::max(shadowOffset.height(), 0) + shadowPaintingExtent);
+                    pixelSnappedHoleRect.move(0, -std::max(shadowOffset.height(), 0) - shadowPaintingExtent);
+                    pixelSnappedHoleRect.setHeight(pixelSnappedHoleRect.height() + std::max(shadowOffset.height(), 0) + shadowPaintingExtent);
                 }
             }
             if (!includeLogicalRightEdge) {
                 if (isHorizontal)
-                    holeRect.setWidth(holeRect.width() - std::min(shadowOffset.width(), 0) + shadowPaintingExtent);
+                    pixelSnappedHoleRect.setWidth(pixelSnappedHoleRect.width() - std::min(shadowOffset.width(), 0) + shadowPaintingExtent);
                 else
-                    holeRect.setHeight(holeRect.height() - std::min(shadowOffset.height(), 0) + shadowPaintingExtent);
+                    pixelSnappedHoleRect.setHeight(pixelSnappedHoleRect.height() - std::min(shadowOffset.height(), 0) + shadowPaintingExtent);
             }
 
             Color fillColor(shadowColor.red(), shadowColor.green(), shadowColor.blue(), 255);
 
-            LayoutRect outerRect = areaCastingShadowInHole(border.rect(), shadowPaintingExtent, shadowSpread, shadowOffset);
-            RoundedRect roundedHole(holeRect, border.radii());
+            FloatRect pixelSnappedOuterRect = pixelSnappedForPainting(areaCastingShadowInHole(LayoutRect(pixelSnappedBorderRect.rect()), shadowPaintingExtent, shadowSpread, shadowOffset), deviceScaleFactor);
+            FloatRoundedRect pixelSnappedRoundedHole = FloatRoundedRect(pixelSnappedHoleRect, pixelSnappedBorderRect.radii());
 
             GraphicsContextStateSaver stateSaver(*context);
             if (hasBorderRadius) {
                 Path path;
-                path.addRoundedRect(border);
+                path.addRoundedRect(pixelSnappedBorderRect);
                 context->clip(path);
-                roundedHole.shrinkRadii(shadowSpread);
+                pixelSnappedRoundedHole.shrinkRadii(shadowSpread);
             } else
-                context->clip(border.rect());
+                context->clip(pixelSnappedBorderRect.rect());
 
-            IntSize extraOffset(2 * paintRect.pixelSnappedWidth() + std::max(0, shadowOffset.width()) + shadowPaintingExtent - 2 * shadowSpread + 1, 0);
+            IntSize extraOffset(2 * roundToInt(paintRect.width()) + std::max(0, shadowOffset.width()) + shadowPaintingExtent - 2 * shadowSpread + 1, 0);
             context->translate(extraOffset.width(), extraOffset.height());
             shadowOffset -= extraOffset;
 
@@ -2509,7 +2510,7 @@ void RenderBoxModelObject::paintBoxShadow(const PaintInfo& info, const LayoutRec
             else
                 context->setShadow(shadowOffset, shadowRadius, shadowColor, style.colorSpace());
 
-            context->fillRectWithRoundedHole(outerRect, FloatRoundedRect(roundedHole), fillColor, style.colorSpace());
+            context->fillRectWithRoundedHole(pixelSnappedOuterRect, pixelSnappedRoundedHole, fillColor, style.colorSpace());
         }
     }
 }