Fix miter clipping of borders with large radius opposite a wide edge

third_party/WebKit/Source/core/paint/BoxBorderPainter.cpp

Index: third_party/WebKit/Source/core/paint/BoxBorderPainter.cpp
diff --git a/third_party/WebKit/Source/core/paint/BoxBorderPainter.cpp b/third_party/WebKit/Source/core/paint/BoxBorderPainter.cpp
index 31355c04a9bc26a4a9e38a89a2294df5336349e3..d5f1d5388e1b9836d07564ca5c4bd455e74157af 100644
--- a/third_party/WebKit/Source/core/paint/BoxBorderPainter.cpp
+++ b/third_party/WebKit/Source/core/paint/BoxBorderPainter.cpp
@@ -1124,7 +1124,9 @@ void BoxBorderPainter::clipBorderSidePolygon(GraphicsContext& graphicsContext,
                                              MiterType secondMiter) const {
   DCHECK(firstMiter != NoMiter || secondMiter != NoMiter);
 
-  FloatPoint quad[4];
+  FloatPoint edgeQuad[4];  // The boundary of the edge for fill
+  FloatPoint boundQuad1;  // Point 1 of the rectilinear bounding box of EdgeQuad
+  FloatPoint boundQuad2;  // Point 2 of the rectilinear bounding box of EdgeQuad
 
   const LayoutRect outerRect(m_outer.rect());
   const LayoutRect innerRect(m_inner.rect());
@@ -1133,170 +1135,242 @@ void BoxBorderPainter::clipBorderSidePolygon(GraphicsContext& graphicsContext,
   // may draw, including areas inside the innerBorder.
   //
   //         0----------------3
-  //       0  \              /  0
+  //       3  \              /  0
   //       |\  1----------- 2  /|
-  //       | 1                1 |
+  //       | 2                1 |
   //       | |                | |
   //       | |                | |
-  //       | 2                2 |
-  //       |/  1------------2  \|
-  //       3  /              \  3
-  //         0----------------3
-  //
+  //       | 1                2 |
+  //       |/  2------------1  \|
+  //       0  /              \  3
+  //         3----------------0
+
+  // Offset size and direction to expand clipping quad
+  const static float kExtensionLength = 1e-1f;
+  FloatSize extensionOffset;
   switch (side) {
     case BSTop:
-      quad[0] = FloatPoint(outerRect.minXMinYCorner());
-      quad[1] = FloatPoint(innerRect.minXMinYCorner());
-      quad[2] = FloatPoint(innerRect.maxXMinYCorner());
-      quad[3] = FloatPoint(outerRect.maxXMinYCorner());
+      edgeQuad[0] = FloatPoint(outerRect.minXMinYCorner());
+      edgeQuad[1] = FloatPoint(innerRect.minXMinYCorner());
+      edgeQuad[2] = FloatPoint(innerRect.maxXMinYCorner());
+      edgeQuad[3] = FloatPoint(outerRect.maxXMinYCorner());
+
+      DCHECK(edgeQuad[0].y() == edgeQuad[3].y());
+      DCHECK(edgeQuad[1].y() == edgeQuad[2].y());
+
+      boundQuad1 = FloatPoint(edgeQuad[0].x(), edgeQuad[1].y());
+      boundQuad2 = FloatPoint(edgeQuad[3].x(), edgeQuad[2].y());
+
+      extensionOffset.setWidth(-kExtensionLength);
+      extensionOffset.setHeight(0);
 
       if (!m_inner.getRadii().topLeft().isZero()) {
         findIntersection(
-            quad[0], quad[1],
-            FloatPoint(quad[1].x() + m_inner.getRadii().topLeft().width(),
-                       quad[1].y()),
-            FloatPoint(quad[1].x(),
-                       quad[1].y() + m_inner.getRadii().topLeft().height()),
-            quad[1]);
+            edgeQuad[0], edgeQuad[1],
+            FloatPoint(edgeQuad[1].x() + m_inner.getRadii().topLeft().width(),
+                       edgeQuad[1].y()),
+            FloatPoint(edgeQuad[1].x(),
+                       edgeQuad[1].y() + m_inner.getRadii().topLeft().height()),
+            edgeQuad[1]);
+        DCHECK(boundQuad1.y() <= edgeQuad[1].y());
+        boundQuad1.setY(edgeQuad[1].y());
+        boundQuad2.setY(edgeQuad[1].y());
       }
 
       if (!m_inner.getRadii().topRight().isZero()) {
         findIntersection(
-            quad[3], quad[2],
-            FloatPoint(quad[2].x() - m_inner.getRadii().topRight().width(),
-                       quad[2].y()),
-            FloatPoint(quad[2].x(),
-                       quad[2].y() + m_inner.getRadii().topRight().height()),
-            quad[2]);
+            edgeQuad[3], edgeQuad[2],
+            FloatPoint(edgeQuad[2].x() - m_inner.getRadii().topRight().width(),
+                       edgeQuad[2].y()),
+            FloatPoint(
+                edgeQuad[2].x(),
+                edgeQuad[2].y() + m_inner.getRadii().topRight().height()),
+            edgeQuad[2]);
+        if (boundQuad1.y() < edgeQuad[2].y()) {
+          boundQuad1.setY(edgeQuad[2].y());
+          boundQuad2.setY(edgeQuad[2].y());
+        }
       }
       break;
 
     case BSLeft:
-      quad[0] = FloatPoint(outerRect.minXMinYCorner());
-      quad[1] = FloatPoint(innerRect.minXMinYCorner());
-      quad[2] = FloatPoint(innerRect.minXMaxYCorner());
-      quad[3] = FloatPoint(outerRect.minXMaxYCorner());
+      // Swap the order of adjacent edges to allow common code
+      std::swap(firstMiter, secondMiter);
+      edgeQuad[0] = FloatPoint(outerRect.minXMaxYCorner());
+      edgeQuad[1] = FloatPoint(innerRect.minXMaxYCorner());
+      edgeQuad[2] = FloatPoint(innerRect.minXMinYCorner());
+      edgeQuad[3] = FloatPoint(outerRect.minXMinYCorner());
+
+      DCHECK(edgeQuad[0].x() == edgeQuad[3].x());
+      DCHECK(edgeQuad[1].x() == edgeQuad[2].x());
+
+      boundQuad1 = FloatPoint(edgeQuad[1].x(), edgeQuad[0].y());
+      boundQuad2 = FloatPoint(edgeQuad[2].x(), edgeQuad[3].y());
+
+      extensionOffset.setWidth(0);
+      extensionOffset.setHeight(kExtensionLength);
 
       if (!m_inner.getRadii().topLeft().isZero()) {
         findIntersection(
-            quad[0], quad[1],
-            FloatPoint(quad[1].x() + m_inner.getRadii().topLeft().width(),
-                       quad[1].y()),
-            FloatPoint(quad[1].x(),
-                       quad[1].y() + m_inner.getRadii().topLeft().height()),
-            quad[1]);
+            edgeQuad[3], edgeQuad[2],
+            FloatPoint(edgeQuad[2].x() + m_inner.getRadii().topLeft().width(),
+                       edgeQuad[2].y()),
+            FloatPoint(edgeQuad[2].x(),
+                       edgeQuad[2].y() + m_inner.getRadii().topLeft().height()),
+            edgeQuad[2]);
+        DCHECK(boundQuad2.x() <= edgeQuad[2].x());
+        boundQuad1.setX(edgeQuad[2].x());
+        boundQuad2.setX(edgeQuad[2].x());
       }
 
       if (!m_inner.getRadii().bottomLeft().isZero()) {
         findIntersection(
-            quad[3], quad[2],
-            FloatPoint(quad[2].x() + m_inner.getRadii().bottomLeft().width(),
-                       quad[2].y()),
-            FloatPoint(quad[2].x(),
-                       quad[2].y() - m_inner.getRadii().bottomLeft().height()),
-            quad[2]);
+            edgeQuad[0], edgeQuad[1],
+            FloatPoint(
+                edgeQuad[1].x() + m_inner.getRadii().bottomLeft().width(),
+                edgeQuad[1].y()),
+            FloatPoint(
+                edgeQuad[1].x(),
+                edgeQuad[1].y() - m_inner.getRadii().bottomLeft().height()),
+            edgeQuad[1]);
+        if (boundQuad1.x() < edgeQuad[1].x()) {
+          boundQuad1.setX(edgeQuad[1].x());
+          boundQuad2.setX(edgeQuad[1].x());
+        }
       }
       break;
 
     case BSBottom:
-      quad[0] = FloatPoint(outerRect.minXMaxYCorner());
-      quad[1] = FloatPoint(innerRect.minXMaxYCorner());
-      quad[2] = FloatPoint(innerRect.maxXMaxYCorner());
-      quad[3] = FloatPoint(outerRect.maxXMaxYCorner());
+      // Swap the order of adjacent edges to allow common code
+      std::swap(firstMiter, secondMiter);
+      edgeQuad[0] = FloatPoint(outerRect.maxXMaxYCorner());
+      edgeQuad[1] = FloatPoint(innerRect.maxXMaxYCorner());
+      edgeQuad[2] = FloatPoint(innerRect.minXMaxYCorner());
+      edgeQuad[3] = FloatPoint(outerRect.minXMaxYCorner());
+
+      DCHECK(edgeQuad[0].y() == edgeQuad[3].y());
+      DCHECK(edgeQuad[1].y() == edgeQuad[2].y());
+
+      boundQuad1 = FloatPoint(edgeQuad[0].x(), edgeQuad[1].y());
+      boundQuad2 = FloatPoint(edgeQuad[3].x(), edgeQuad[2].y());
+
+      extensionOffset.setWidth(kExtensionLength);
+      extensionOffset.setHeight(0);
 
       if (!m_inner.getRadii().bottomLeft().isZero()) {
         findIntersection(
-            quad[0], quad[1],
-            FloatPoint(quad[1].x() + m_inner.getRadii().bottomLeft().width(),
-                       quad[1].y()),
-            FloatPoint(quad[1].x(),
-                       quad[1].y() - m_inner.getRadii().bottomLeft().height()),
-            quad[1]);
+            edgeQuad[3], edgeQuad[2],
+            FloatPoint(
+                edgeQuad[2].x() + m_inner.getRadii().bottomLeft().width(),
+                edgeQuad[2].y()),
+            FloatPoint(
+                edgeQuad[2].x(),
+                edgeQuad[2].y() - m_inner.getRadii().bottomLeft().height()),
+            edgeQuad[2]);
+        DCHECK(boundQuad2.y() >= edgeQuad[2].y());
+        boundQuad1.setY(edgeQuad[2].y());
+        boundQuad2.setY(edgeQuad[2].y());
       }
 
       if (!m_inner.getRadii().bottomRight().isZero()) {
         findIntersection(
-            quad[3], quad[2],
-            FloatPoint(quad[2].x() - m_inner.getRadii().bottomRight().width(),
-                       quad[2].y()),
-            FloatPoint(quad[2].x(),
-                       quad[2].y() - m_inner.getRadii().bottomRight().height()),
-            quad[2]);
+            edgeQuad[0], edgeQuad[1],
+            FloatPoint(
+                edgeQuad[1].x() - m_inner.getRadii().bottomRight().width(),
+                edgeQuad[1].y()),
+            FloatPoint(
+                edgeQuad[1].x(),
+                edgeQuad[1].y() - m_inner.getRadii().bottomRight().height()),
+            edgeQuad[1]);
+        if (boundQuad1.y() > edgeQuad[1].y()) {
+          boundQuad1.setY(edgeQuad[1].y());
+          boundQuad2.setY(edgeQuad[1].y());
+        }
       }
       break;
 
     case BSRight:
-      quad[0] = FloatPoint(outerRect.maxXMinYCorner());
-      quad[1] = FloatPoint(innerRect.maxXMinYCorner());
-      quad[2] = FloatPoint(innerRect.maxXMaxYCorner());
-      quad[3] = FloatPoint(outerRect.maxXMaxYCorner());
+      edgeQuad[0] = FloatPoint(outerRect.maxXMinYCorner());
+      edgeQuad[1] = FloatPoint(innerRect.maxXMinYCorner());
+      edgeQuad[2] = FloatPoint(innerRect.maxXMaxYCorner());
+      edgeQuad[3] = FloatPoint(outerRect.maxXMaxYCorner());
+
+      DCHECK(edgeQuad[0].x() == edgeQuad[3].x());
+      DCHECK(edgeQuad[1].x() == edgeQuad[2].x());
+
+      boundQuad1 = FloatPoint(edgeQuad[1].x(), edgeQuad[0].y());
+      boundQuad2 = FloatPoint(edgeQuad[2].x(), edgeQuad[3].y());
+
+      extensionOffset.setWidth(0);
+      extensionOffset.setHeight(-kExtensionLength);
 
       if (!m_inner.getRadii().topRight().isZero()) {
         findIntersection(
-            quad[0], quad[1],
-            FloatPoint(quad[1].x() - m_inner.getRadii().topRight().width(),
-                       quad[1].y()),
-            FloatPoint(quad[1].x(),
-                       quad[1].y() + m_inner.getRadii().topRight().height()),
-            quad[1]);
+            edgeQuad[0], edgeQuad[1],
+            FloatPoint(edgeQuad[1].x() - m_inner.getRadii().topRight().width(),
+                       edgeQuad[1].y()),
+            FloatPoint(
+                edgeQuad[1].x(),
+                edgeQuad[1].y() + m_inner.getRadii().topRight().height()),
+            edgeQuad[1]);
+        DCHECK(boundQuad1.x() >= edgeQuad[1].x());
+        boundQuad1.setX(edgeQuad[1].x());
+        boundQuad2.setX(edgeQuad[1].x());
       }
 
       if (!m_inner.getRadii().bottomRight().isZero()) {
         findIntersection(
-            quad[3], quad[2],
-            FloatPoint(quad[2].x() - m_inner.getRadii().bottomRight().width(),
-                       quad[2].y()),
-            FloatPoint(quad[2].x(),
-                       quad[2].y() - m_inner.getRadii().bottomRight().height()),
-            quad[2]);
+            edgeQuad[3], edgeQuad[2],
+            FloatPoint(
+                edgeQuad[2].x() - m_inner.getRadii().bottomRight().width(),
+                edgeQuad[2].y()),
+            FloatPoint(
+                edgeQuad[2].x(),
+                edgeQuad[2].y() - m_inner.getRadii().bottomRight().height()),
+            edgeQuad[2]);
+        if (boundQuad1.x() > edgeQuad[2].x()) {
+          boundQuad1.setX(edgeQuad[2].x());
+          boundQuad2.setX(edgeQuad[2].x());
+        }
       }
       break;
   }
 
   if (firstMiter == secondMiter) {
-    clipQuad(graphicsContext, quad, firstMiter == SoftMiter);
+    clipQuad(graphicsContext, edgeQuad, firstMiter == SoftMiter);
     return;
   }
 
-  // If antialiasing settings for the first edge and second edge is different,
-  // they have to be addressed separately. We do this by breaking the quad into
-  // two parallelograms, made by moving quad[1] and quad[2].
-  float ax = quad[1].x() - quad[0].x();
-  float ay = quad[1].y() - quad[0].y();
-  float bx = quad[2].x() - quad[1].x();
-  float by = quad[2].y() - quad[1].y();
-  float cx = quad[3].x() - quad[2].x();
-  float cy = quad[3].y() - quad[2].y();
-
-  const static float kEpsilon = 1e-2f;
-  float r1, r2;
-  if (fabsf(bx) < kEpsilon && fabsf(by) < kEpsilon) {
-    // The quad was actually a triangle.
-    r1 = r2 = 1.0f;
-  } else {
-    // Extend parallelogram a bit to hide calculation error
-    const static float kExtendFill = 1e-2f;
+  // If antialiasing settings for the first edge and second edge are different,
+  // they have to be addressed separately. We do this by applying 2 clips, one
+  // for each miter, with the appropriate anti-aliasing setting. Each clip uses
+  // 3 sides of the quad rectilinear bounding box and a 4th side aligned with
+  // the miter edge. We extend the clip in the miter direction to ensure overlap
+  // as each edge is drawn.
+  if (firstMiter != NoMiter) {
+    FloatPoint clippingQuad[4];
 
-    r1 = (-ax * by + ay * bx) / (cx * by - cy * bx) + kExtendFill;
-    r2 = (-cx * by + cy * bx) / (ax * by - ay * bx) + kExtendFill;
-  }
+    clippingQuad[0] = edgeQuad[0];
+    findIntersection(edgeQuad[0], edgeQuad[1], boundQuad1, boundQuad2,
+                     clippingQuad[1]);
+    clippingQuad[1] += extensionOffset;

[f(malita), 2017/02/17 21:43:45]

I don't fully follow the extension logic.  For e.g. BSTop/left miter, it seems
we would want to shift the left miter edge outward - that implies nudging both
clippingQuad[0] & clippingQuad[1], but instead we're nudging 1 & 2?

[Stephen Chennney, 2017/02/17 21:56:25]

You are right. That explains my issues with the amount we need to nudge.

+    clippingQuad[2] = boundQuad2 + extensionOffset;
+    clippingQuad[3] = edgeQuad[3];
 
-  if (firstMiter != NoMiter) {
-    FloatPoint firstQuad[4];
-    firstQuad[0] = quad[0];
-    firstQuad[1] = quad[1];
-    firstQuad[2] = FloatPoint(quad[3].x() + r2 * ax, quad[3].y() + r2 * ay);
-    firstQuad[3] = quad[3];
-    clipQuad(graphicsContext, firstQuad, firstMiter == SoftMiter);
+    clipQuad(graphicsContext, clippingQuad, firstMiter == SoftMiter);
   }
 
   if (secondMiter != NoMiter) {
-    FloatPoint secondQuad[4];
-    secondQuad[0] = quad[0];
-    secondQuad[1] = FloatPoint(quad[0].x() - r1 * cx, quad[0].y() - r1 * cy);
-    secondQuad[2] = quad[2];
-    secondQuad[3] = quad[3];
-    clipQuad(graphicsContext, secondQuad, secondMiter == SoftMiter);
+    FloatPoint clippingQuad[4];
+
+    clippingQuad[0] = edgeQuad[0];
+    clippingQuad[1] = boundQuad1 - extensionOffset;
+    findIntersection(edgeQuad[2], edgeQuad[3], boundQuad1, boundQuad2,
+                     clippingQuad[2]);
+    clippingQuad[2] -= extensionOffset;
+    clippingQuad[3] = edgeQuad[3];
+
+    clipQuad(graphicsContext, clippingQuad, secondMiter == SoftMiter);
   }
 }