Support subpixel layout for borders.

third_party/WebKit/Source/core/style/BorderValue.h

Index: third_party/WebKit/Source/core/style/BorderValue.h
diff --git a/third_party/WebKit/Source/core/style/BorderValue.h b/third_party/WebKit/Source/core/style/BorderValue.h
index 9c97bfcda155c9b0726d7f34fd3fa0df352cf24d..7260183a0572b3b42283310620ff357fab6507e8 100644
--- a/third_party/WebKit/Source/core/style/BorderValue.h
+++ b/third_party/WebKit/Source/core/style/BorderValue.h
@@ -32,6 +32,10 @@
 
 namespace blink {
 
+static const int kBorderWidthFractionalBits = 6;

[pdr., 2017/01/27 20:28:44]

Can you add a comment above here describing the fixed point representation in a
little more detail, for folks encountering this for this first time? Maybe note
that it is just like LayoutUnit.h.

+static const int kBorderWidthDenominator = 1 << kBorderWidthFractionalBits;
+static const int kMaxForBorderWidth = ((1 << 26) - 1) / kBorderWidthDenominator;
+

[pdr., 2017/01/27 20:28:44]

Could you add a short unittest for the new fixed point behavior? Basically just
copy LayoutUnitTest.cpp and assert the basic width get/set behavior.

 class BorderValue {
   DISALLOW_NEW();
   friend class ComputedStyle;
@@ -40,9 +44,10 @@ class BorderValue {
   BorderValue()
       : m_color(0),
         m_colorIsCurrentColor(true),
-        m_width(3),
         m_style(BorderStyleNone),
-        m_isAuto(OutlineIsAutoOff) {}
+        m_isAuto(OutlineIsAutoOff) {
+    setWidth(3);
+  }
 
   bool nonZero() const { return width() && (m_style != BorderStyleNone); }
 
@@ -78,16 +83,27 @@ class BorderValue {
                                  : StyleColor(m_color);
   }
 
-  int width() const { return m_width; }
+  float width() const {
+    return static_cast<float>(m_width) / kBorderWidthDenominator;
+  }
+  void setWidth(float width) { m_width = widthToFixedPoint(width); }
+  bool widthEquals(float width) const {

[pdr., 2017/01/27 20:28:44]

Is this used over checking width() to avoid the perf hit of a divide? If so, can
you add a short comment mentioning that?

[Karl Øygard, 2017/01/28 16:46:27]

We have to do this, since width now loses precision.  Comparing a float
value with the value from width() may give incorrect results, since
the difference may now be less than what we can represent.

I'll try to add a comment to that effect.

+    return widthToFixedPoint(width) == m_width;
+  }
 
   EBorderStyle style() const { return static_cast<EBorderStyle>(m_style); }
   void setStyle(EBorderStyle style) { m_style = style; }
 
  protected:
+  static unsigned widthToFixedPoint(float width) {
+    return static_cast<unsigned>(std::min<float>(width, kMaxForBorderWidth) *

[pdr., 2017/01/27 20:28:44]

Is this safe when width is negative? Could you add a DCHECK that width >= 0?

[Karl Øygard, 2017/02/10 12:53:18]

Done.

+                                 kBorderWidthDenominator);
+  }
+
   Color m_color;
   unsigned m_colorIsCurrentColor : 1;
 
-  unsigned m_width : 26;
+  unsigned m_width : 26;  // Fixed point width

[pdr., 2017/01/27 20:28:44]

WDYT of using a typedef to make this a little more clear?
I'm thinking something like this at the top of the file:
using FixedPointBorderWidth = unsigned;

Then:
static FixedPointBorderWidth widthToFixedPoint(float width) { ...
and
FixedPointBorderWidth m_width : 26;

[Karl Øygard, 2017/02/10 12:53:18]

I agree and tried, but the check-webkit-style presubmit disagrees;
bitfields have to be declared unsigned.

   unsigned m_style : 4;  // EBorderStyle
 
   // This is only used by OutlineValue but moved here to keep the bits packed.