Paint dotted borders using circular dots

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

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/paint/BoxBorderPainter.h"
 
 #include "core/paint/BoxPainter.h"
 #include "core/paint/ObjectPainter.h"
 #include "core/paint/PaintInfo.h"
 #include "core/style/BorderEdge.h"
@@ skipping 958 matching lines @@
 
   if (borderStyle == BorderStyleDouble && thickness < 3)
     borderStyle = BorderStyleSolid;
 
   switch (borderStyle) {
     case BorderStyleNone:
     case BorderStyleHidden:
       return;
     case BorderStyleDotted:
     case BorderStyleDashed: {
-      drawDashedDottedBoxSideFromPath(graphicsContext, borderPath, thickness,
-                                      drawThickness, color, borderStyle);
+      drawDashedDottedBoxSideFromPath(graphicsContext, borderRect, borderPath,
+                                      thickness, drawThickness, color,
+                                      borderStyle);
       return;
     }
     case BorderStyleDouble: {
       drawDoubleBoxSideFromPath(graphicsContext, borderRect, borderPath,
                                 thickness, drawThickness, side, color);
       return;
     }
     case BorderStyleRidge:
     case BorderStyleGroove: {
       drawRidgeGrooveBoxSideFromPath(graphicsContext, borderRect, borderPath,
@@ skipping 13 matching lines @@
       break;
   }
 
   graphicsContext.setStrokeStyle(NoStroke);
   graphicsContext.setFillColor(color);
   graphicsContext.drawRect(pixelSnappedIntRect(borderRect));
 }
 
 void BoxBorderPainter::drawDashedDottedBoxSideFromPath(
     GraphicsContext& graphicsContext,
+    const LayoutRect& borderRect,
     const Path& borderPath,
     float thickness,
     float drawThickness,
     Color color,
     EBorderStyle borderStyle) const {
   graphicsContext.setStrokeColor(color);
 
+  if (borderStyle == BorderStyleDotted && thickness > 3) {

[f(malita), 2017/03/03 15:34:08]

Why is dotted predicated on thickness > 3?

These are not device pixels, and can be transformed arbitrarily, no?

[Stephen Chennney, 2017/03/03 22:19:22]

Done.

+    drawWideDottedBoxSideFromPath(graphicsContext, borderRect, borderPath,
+                                  thickness);
+    return;
+  }
+
   // The stroke is doubled here because the provided path is the
   // outside edge of the border so half the stroke is clipped off.
   // The extra multiplier is so that the clipping mask can antialias
   // the edges to prevent jaggies.
   graphicsContext.setStrokeThickness(drawThickness * 2 * 1.1f);
   graphicsContext.setStrokeStyle(
       borderStyle == BorderStyleDashed ? DashedStroke : DottedStroke);
 
   // If the number of dashes that fit in the path is odd and non-integral
   // then we will have an awkwardly-sized dash at the end of the path. To
   // try to avoid that here, we simply make the whitespace dashes ever so
   // slightly bigger.
   // TODO(schenney): This code for setting up the dash effect is trying to
   // do the same thing as StrokeData::setupPaintDashPathEffect and should be
   // refactored to re-use that code. It would require
   // GraphicsContext::strokePath to take a length parameter.
-
   float dashLength =
       thickness * ((borderStyle == BorderStyleDashed) ? 3.0f : 1.0f);
   float gapLength = dashLength;
   float numberOfDashes = borderPath.length() / dashLength;
   // Don't try to show dashes if we have less than 2 dashes + 2 gaps.
   // FIXME: should do this test per side.
   if (numberOfDashes >= 4) {
     bool evenNumberOfFullDashes = !((int)numberOfDashes % 2);
     bool integralNumberOfDashes = !(numberOfDashes - (int)numberOfDashes);
     if (!evenNumberOfFullDashes && !integralNumberOfDashes) {
       float numberOfGaps = numberOfDashes / 2;
       gapLength += (dashLength / numberOfGaps);
     }
 
     DashArray lineDash;
     lineDash.push_back(dashLength);
     lineDash.push_back(gapLength);
     graphicsContext.setLineDash(lineDash, dashLength);
   }
 
   // FIXME: stroking the border path causes issues with tight corners:
   // https://bugs.webkit.org/show_bug.cgi?id=58711
   // Also, to get the best appearance we should stroke a path between the
   // two borders.
   graphicsContext.strokePath(borderPath);
 }
 
+void BoxBorderPainter::drawWideDottedBoxSideFromPath(
+    GraphicsContext& graphicsContext,
+    const LayoutRect& borderRect,
+    const Path& borderPath,

[f(malita), 2017/03/03 15:34:08]

It's kind of annoying that we don't use the provided path (as hinted by the
method name), but instead recompute it from borderRect.

I understand why, but it indicates we're computing the path too early upstack,
and in this case we're throwing it away.  Maybe add a TODO for us to look at
refactoring these methods?

[Stephen Chennney, 2017/03/03 22:19:22]

Done.

+    float thickness) const {
+  // Convert the path to be down the middle of the dots.
+  const LayoutRectOutsets centerOffsets(
+      -roundf(m_edges[BSTop].usedWidth() * 0.5),

[f(malita), 2017/03/03 15:34:08]

Is rounding needed here?  I'm always wary of snapping in non-device coordinates,
and for dots in particular pixel-alignment doesn't seem important.

Except maybe to match the geometry of different style border sides - but then
again, we're computing a "synthetic" mid-border path, so I'm not sure it
matters.

[Stephen Chennney, 2017/03/03 22:19:22]

Not needed, probably. Have changed it and we'll see what happens to test
results.

+      -roundf(m_edges[BSRight].usedWidth() * 0.5),
+      -roundf(m_edges[BSBottom].usedWidth() * 0.5),
+      -roundf(m_edges[BSLeft].usedWidth() * 0.5));
+  FloatRoundedRect innerClip = m_style.getRoundedInnerBorderFor(

[f(malita), 2017/03/03 15:34:08]

Naming nit: not an inner clip.  Maybe centeredBorder or similar?

Or better yet, just inline in addRounderRect() below and skip the name.

[Stephen Chennney, 2017/03/03 22:19:22]

Inlined.

+      borderRect, centerOffsets, m_includeLogicalLeftEdge,
+      m_includeLogicalRightEdge);
+  Path centerlinePath;
+  centerlinePath.addRoundedRect(innerClip);
+
+  graphicsContext.setStrokeThickness(thickness);
+  graphicsContext.setStrokeStyle(DottedStroke);
+
+  // TODO(schenney): This code for setting up the dash effect is largely
+  // duplicated from StrokeData::setupPaintDashPathEffect and both this code
+  // and the method above should be refactored to re-use that code. It would
+  // require GraphicsContext::strokePath to take a length parameter.
+  graphicsContext.setLineCap(RoundCap);
+
+  // Adjust the width to get equal dot spacing as much as possible.
+  float perDotLength = thickness * 2;
+  static float epsilon = 1.0e-2f;
+  float pathLength = centerlinePath.length();
+
+  if (pathLength < perDotLength + thickness) {
+    // Exactly 2 dots with whatever space we can get
+    DashArray lineDash;
+    lineDash.push_back(0);
+    lineDash.push_back(pathLength - thickness - epsilon);
+    graphicsContext.setLineDash(lineDash, 0);
+  } else {
+    // Determine what number of dots gives the minimum deviation from
+    // idealGap between dots. Set the gap to that width.
+    float minNumDots = floorf((pathLength + thickness) / perDotLength);
+    float maxNumDots = minNumDots + 1;
+    float minGap = (pathLength - minNumDots * thickness) / (minNumDots - 1);
+    float maxGap = (pathLength - maxNumDots * thickness) / (maxNumDots - 1);
+    if (fabs(minGap - thickness) < fabs(maxGap - thickness)) {
+      DashArray lineDash;
+      lineDash.push_back(0);
+      lineDash.push_back(minGap + thickness - epsilon);
+      graphicsContext.setLineDash(lineDash, 0);
+    } else {
+      DashArray lineDash;
+      lineDash.push_back(0);
+      lineDash.push_back(maxGap + thickness - epsilon);
+      graphicsContext.setLineDash(lineDash, 0);
+    }

[f(malita), 2017/03/03 15:34:08]

nit: these two branches are mostly the same; maybe consolidate as

  auto gap = fabs(minGap - thickness) < fabs(maxGap - thickness)
      ? minGap : maxGap;
  ...

[Stephen Chennney, 2017/03/03 22:19:22]

Done.

+  }
+
+  // TODO(schenney): stroking the border path causes issues with tight corners:
+  // https://bugs.webkit.org/show_bug.cgi?id=58711
+  graphicsContext.strokePath(centerlinePath);
+}
+
 void BoxBorderPainter::drawDoubleBoxSideFromPath(
     GraphicsContext& graphicsContext,
     const LayoutRect& borderRect,
     const Path& borderPath,
     float thickness,
     float drawThickness,
     BoxSide side,
     Color color) const {
   // Draw inner border line
   {
@@ skipping 333 matching lines @@
     findIntersection(edgeQuad[2], edgeQuad[3], boundQuad1, boundQuad2,
                      clippingQuad[2]);
     clippingQuad[2] -= extensionOffset;
     clippingQuad[3] = edgeQuad[3] - extensionOffset;
 
     clipQuad(graphicsContext, clippingQuad, secondMiter == SoftMiter);
   }
 }
 
 }  // namespace blink