<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[172326] trunk</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/172326">172326</a></dd>
<dt>Author</dt> <dd>zalan@apple.com</dd>
<dt>Date</dt> <dd>2014-08-07 20:51:44 -0700 (Thu, 07 Aug 2014)</dd>
</dl>

<h3>Log Message</h3>
<pre>Subpixel rendering: Border thickness and length flooring can result empty borders
due to losing precision during multiple float &lt;-&gt; LayoutUnit conversions.
https://bugs.webkit.org/show_bug.cgi?id=135686

Reviewed by Simon Fraser.

The combination of losing precision and flooring the border thickness/length to avoid
empty border rect drawing can lead to false positives of missing borders.
This patch moves empty border checking right before painting where we can safely use round
instead of floor.

Source/WebCore:

Tests: fast/borders/hidpi-border-width-flooring.html

* rendering/RenderObject.cpp:
(WebCore::drawBorderLineRect):
(WebCore::drawBorderLine):
(WebCore::RenderObject::drawLineForBoxSide):

LayoutTests:

* fast/borders/hidpi-border-width-flooring-expected.html: Added.
* fast/borders/hidpi-border-width-flooring.html: Added.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderObjectcpp">trunk/Source/WebCore/rendering/RenderObject.cpp</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastbordershidpiborderwidthflooringexpectedhtml">trunk/LayoutTests/fast/borders/hidpi-border-width-flooring-expected.html</a></li>
<li><a href="#trunkLayoutTestsfastbordershidpiborderwidthflooringhtml">trunk/LayoutTests/fast/borders/hidpi-border-width-flooring.html</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (172325 => 172326)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog        2014-08-08 02:40:07 UTC (rev 172325)
+++ trunk/LayoutTests/ChangeLog        2014-08-08 03:51:44 UTC (rev 172326)
</span><span class="lines">@@ -1,3 +1,19 @@
</span><ins>+2014-08-07  Zalan Bujtas  &lt;zalan@apple.com&gt;
+
+        Subpixel rendering: Border thickness and length flooring can result empty borders
+        due to losing precision during multiple float &lt;-&gt; LayoutUnit conversions.
+        https://bugs.webkit.org/show_bug.cgi?id=135686
+
+        Reviewed by Simon Fraser.
+
+        The combination of losing precision and flooring the border thickness/length to avoid
+        empty border rect drawing can lead to false positives of missing borders.
+        This patch moves empty border checking right before painting where we can safely use round
+        instead of floor.
+
+        * fast/borders/hidpi-border-width-flooring-expected.html: Added.
+        * fast/borders/hidpi-border-width-flooring.html: Added.
+
</ins><span class="cx"> 2014-08-07  Benjamin Poulain  &lt;bpoulain@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Get rid of SCRIPTED_SPEECH
</span></span></pre></div>
<a id="trunkLayoutTestsfastbordershidpiborderwidthflooringexpectedhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/borders/hidpi-border-width-flooring-expected.html (0 => 172326)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/borders/hidpi-border-width-flooring-expected.html                                (rev 0)
+++ trunk/LayoutTests/fast/borders/hidpi-border-width-flooring-expected.html        2014-08-08 03:51:44 UTC (rev 172326)
</span><span class="lines">@@ -0,0 +1,8 @@
</span><ins>+&lt;!DOCTYPE html&gt;
+&lt;html&gt;
+&lt;head&gt;
+&lt;title&gt;This tests that we don't paint border's smaller than 0.5px on retina displays. (border width is floored)&lt;/title&gt;
+&lt;/head&gt;
+&lt;body&gt;
+&lt;/body&gt;
+&lt;/html&gt;
</ins></span></pre></div>
<a id="trunkLayoutTestsfastbordershidpiborderwidthflooringhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/borders/hidpi-border-width-flooring.html (0 => 172326)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/borders/hidpi-border-width-flooring.html                                (rev 0)
+++ trunk/LayoutTests/fast/borders/hidpi-border-width-flooring.html        2014-08-08 03:51:44 UTC (rev 172326)
</span><span class="lines">@@ -0,0 +1,30 @@
</span><ins>+&lt;!DOCTYPE html&gt;
+&lt;html&gt;
+&lt;head&gt;
+&lt;title&gt;This tests that we don't paint border's smaller than 0.5px on retina displays. (border width is floored)&lt;/title&gt;
+&lt;style&gt;
+  div {
+    height: 100px;
+    width: 100px;
+    position: absolute;
+    left: 0px;
+    top: 0px;
+    border: solid 0px green;
+  }
+
+&lt;/style&gt;
+&lt;/head&gt;
+&lt;body&gt;
+&lt;p id=&quot;container&quot;&gt;&lt;/p&gt;
+&lt;script&gt;
+  var container = document.getElementById(&quot;container&quot;);
+  adjustment = 0;
+  for (i = 0; i &lt; 49; ++i) {
+    adjustment += 0.01;
+    var e = document.createElement(&quot;div&quot;);
+    e.style.borderWidth = adjustment + &quot;px&quot;; 
+    container.appendChild(e);
+  }
+&lt;/script&gt;
+&lt;/body&gt;
+&lt;/html&gt;
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (172325 => 172326)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog        2014-08-08 02:40:07 UTC (rev 172325)
+++ trunk/Source/WebCore/ChangeLog        2014-08-08 03:51:44 UTC (rev 172326)
</span><span class="lines">@@ -1,3 +1,23 @@
</span><ins>+2014-08-07  Zalan Bujtas  &lt;zalan@apple.com&gt;
+
+        Subpixel rendering: Border thickness and length flooring can result empty borders
+        due to losing precision during multiple float &lt;-&gt; LayoutUnit conversions.
+        https://bugs.webkit.org/show_bug.cgi?id=135686
+
+        Reviewed by Simon Fraser.
+
+        The combination of losing precision and flooring the border thickness/length to avoid
+        empty border rect drawing can lead to false positives of missing borders.
+        This patch moves empty border checking right before painting where we can safely use round
+        instead of floor.
+
+        Tests: fast/borders/hidpi-border-width-flooring.html
+
+        * rendering/RenderObject.cpp:
+        (WebCore::drawBorderLineRect):
+        (WebCore::drawBorderLine):
+        (WebCore::RenderObject::drawLineForBoxSide):
+
</ins><span class="cx"> 2014-08-07  Ryuan Choi  &lt;ryuan.choi@samsung.com&gt;
</span><span class="cx"> 
</span><span class="cx">         [EFL] Build break with mpegts since r167025
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderObjectcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderObject.cpp (172325 => 172326)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderObject.cpp        2014-08-08 02:40:07 UTC (rev 172325)
+++ trunk/Source/WebCore/rendering/RenderObject.cpp        2014-08-08 03:51:44 UTC (rev 172326)
</span><span class="lines">@@ -737,6 +737,23 @@
</span><span class="cx">     return toRenderBlock(o);
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+static void drawBorderLineRect(GraphicsContext&amp; graphicsContext, const LayoutRect&amp; rect, float deviceScaleFactor)
+{
+    FloatRect pixelSnappedRect = pixelSnappedForPainting(rect, deviceScaleFactor);
+    if (pixelSnappedRect.isEmpty())
+        return;
+    graphicsContext.drawRect(pixelSnappedRect);
+}
+
+static void drawBorderLine(GraphicsContext&amp; graphicsContext, const LayoutPoint&amp; point1, const LayoutPoint&amp; point2, float deviceScaleFactor)
+{
+    FloatPoint p1 = roundedForPainting(point1, deviceScaleFactor);
+    FloatPoint p2 = roundedForPainting(point2, deviceScaleFactor);
+    if (p1.x() == p2.x() || p1.y() == p2.y())
+        return;
+    graphicsContext.drawLine(point1, point2);
+}
+
</ins><span class="cx"> void RenderObject::drawLineForBoxSide(GraphicsContext* graphicsContext, float x1, float y1, float x2, float y2,
</span><span class="cx">     BoxSide side, Color color, EBorderStyle borderStyle, float adjacentWidth1, float adjacentWidth2, bool antialias) const
</span><span class="cx"> {
</span><span class="lines">@@ -750,10 +767,6 @@
</span><span class="cx">         thickness = x2 - x1;
</span><span class="cx">         length = y2 - y1;
</span><span class="cx">     }
</span><del>-    // FIXME: flooring is a temporary solution until the device pixel snapping is added here for all border types (including recursive calls such as groove-&gt;(inset/outset)).
-    thickness = floorToDevicePixel(thickness, deviceScaleFactor);
-    length = floorToDevicePixel(length, deviceScaleFactor);
-
</del><span class="cx">     if (borderStyle == DOUBLE &amp;&amp; (thickness * deviceScaleFactor) &lt; 3)
</span><span class="cx">         borderStyle = SOLID;
</span><span class="cx"> 
</span><span class="lines">@@ -784,11 +797,11 @@
</span><span class="cx">                 switch (side) {
</span><span class="cx">                     case BSBottom:
</span><span class="cx">                     case BSTop:
</span><del>-                        graphicsContext-&gt;drawLine(FloatPoint(x1, adjustedY), FloatPoint(x2, adjustedY));
</del><ins>+                        drawBorderLine(*graphicsContext, LayoutPoint(x1, adjustedY), LayoutPoint(x2, adjustedY), deviceScaleFactor);
</ins><span class="cx">                         break;
</span><span class="cx">                     case BSRight:
</span><span class="cx">                     case BSLeft:
</span><del>-                        graphicsContext-&gt;drawLine(FloatPoint(adjustedX, y1), FloatPoint(adjustedX, y2));
</del><ins>+                        drawBorderLine(*graphicsContext, LayoutPoint(adjustedX, y1), LayoutPoint(adjustedX, y2), deviceScaleFactor);
</ins><span class="cx">                         break;
</span><span class="cx">                 }
</span><span class="cx">                 graphicsContext-&gt;setShouldAntialias(wasAntialiased);
</span><span class="lines">@@ -811,13 +824,13 @@
</span><span class="cx">                 switch (side) {
</span><span class="cx">                     case BSTop:
</span><span class="cx">                     case BSBottom:
</span><del>-                        graphicsContext-&gt;drawRect(pixelSnappedForPainting(x1, y1, length, thirdOfThickness, deviceScaleFactor));
-                        graphicsContext-&gt;drawRect(pixelSnappedForPainting(x1, y2 - thirdOfThickness, length, thirdOfThickness, deviceScaleFactor));
</del><ins>+                        drawBorderLineRect(*graphicsContext, LayoutRect(x1, y1, length, thirdOfThickness), deviceScaleFactor);
+                        drawBorderLineRect(*graphicsContext, LayoutRect(x1, y2 - thirdOfThickness, length, thirdOfThickness), deviceScaleFactor);
</ins><span class="cx">                         break;
</span><span class="cx">                     case BSLeft:
</span><span class="cx">                     case BSRight:
</span><del>-                        graphicsContext-&gt;drawRect(pixelSnappedForPainting(x1, y1, thirdOfThickness, length, deviceScaleFactor));
-                        graphicsContext-&gt;drawRect(pixelSnappedForPainting(x2 - thirdOfThickness, y1, thirdOfThickness, length, deviceScaleFactor));
</del><ins>+                        drawBorderLineRect(*graphicsContext, LayoutRect(x1, y1, thirdOfThickness, length), deviceScaleFactor);
+                        drawBorderLineRect(*graphicsContext, LayoutRect(x2 - thirdOfThickness, y1, thirdOfThickness, length), deviceScaleFactor);
</ins><span class="cx">                         break;
</span><span class="cx">                 }
</span><span class="cx"> 
</span><span class="lines">@@ -954,16 +967,16 @@
</span><span class="cx">             FALLTHROUGH;
</span><span class="cx">         case SOLID: {
</span><span class="cx">             StrokeStyle oldStrokeStyle = graphicsContext-&gt;strokeStyle();
</span><del>-            graphicsContext-&gt;setStrokeStyle(NoStroke);
-            graphicsContext-&gt;setFillColor(color, style.colorSpace());
</del><span class="cx">             ASSERT(x2 &gt;= x1);
</span><span class="cx">             ASSERT(y2 &gt;= y1);
</span><span class="cx">             if (!adjacentWidth1 &amp;&amp; !adjacentWidth2) {
</span><span class="cx">                 // Turn off antialiasing to match the behavior of drawConvexPolygon();
</span><span class="cx">                 // this matters for rects in transformed contexts.
</span><ins>+                graphicsContext-&gt;setStrokeStyle(NoStroke);
+                graphicsContext-&gt;setFillColor(color, style.colorSpace());
</ins><span class="cx">                 bool wasAntialiased = graphicsContext-&gt;shouldAntialias();
</span><span class="cx">                 graphicsContext-&gt;setShouldAntialias(antialias);
</span><del>-                graphicsContext-&gt;drawRect(pixelSnappedForPainting(x1, y1, x2 - x1, y2 - y1, deviceScaleFactor));
</del><ins>+                drawBorderLineRect(*graphicsContext, LayoutRect(x1, y1, x2 - x1, y2 - y1), deviceScaleFactor);
</ins><span class="cx">                 graphicsContext-&gt;setShouldAntialias(wasAntialiased);
</span><span class="cx">                 graphicsContext-&gt;setStrokeStyle(oldStrokeStyle);
</span><span class="cx">                 return;
</span><span class="lines">@@ -974,6 +987,8 @@
</span><span class="cx">             y1 = roundToDevicePixel(y1, deviceScaleFactor);
</span><span class="cx">             x2 = roundToDevicePixel(x2, deviceScaleFactor);
</span><span class="cx">             y2 = roundToDevicePixel(y2, deviceScaleFactor);
</span><ins>+            if (x1 == x2 || y1 == y2)
+                return;
</ins><span class="cx">             FloatPoint quad[4];
</span><span class="cx">             switch (side) {
</span><span class="cx">                 case BSTop:
</span><span class="lines">@@ -1002,6 +1017,8 @@
</span><span class="cx">                     break;
</span><span class="cx">             }
</span><span class="cx"> 
</span><ins>+            graphicsContext-&gt;setStrokeStyle(NoStroke);
+            graphicsContext-&gt;setFillColor(color, style.colorSpace());
</ins><span class="cx">             graphicsContext-&gt;drawConvexPolygon(4, quad, antialias);
</span><span class="cx">             graphicsContext-&gt;setStrokeStyle(oldStrokeStyle);
</span><span class="cx">             break;
</span></span></pre>
</div>
</div>

</body>
</html>