Support subpixel layout for borders.

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

 /*
  * Copyright (C) 2000 Lars Knoll (knoll@kde.org)
  *           (C) 2000 Antti Koivisto (koivisto@kde.org)
  *           (C) 2000 Dirk Mueller (mueller@kde.org)
  * Copyright (C) 2003, 2005, 2006, 2007, 2008 Apple Inc. All rights reserved.
  * Copyright (C) 2006 Graham Dennis (graham.dennis@gmail.com)
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
@@ skipping 14 matching lines @@
 #ifndef BorderValue_h
 #define BorderValue_h
 
 #include "core/css/StyleColor.h"
 #include "core/style/ComputedStyleConstants.h"
 #include "platform/graphics/Color.h"
 #include "wtf/Allocator.h"
 
 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;
 
  public:
   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); }
 
   bool isTransparent() const {
     return !m_colorIsCurrentColor && !m_color.alpha();
   }
 
   bool operator==(const BorderValue& o) const {
     return m_width == o.m_width && m_style == o.m_style &&
            m_color == o.m_color &&
@@ skipping 15 matching lines @@
   void setColor(const StyleColor& color) {
     m_color = color.resolve(Color());
     m_colorIsCurrentColor = color.isCurrentColor();
   }
 
   StyleColor color() const {
     return m_colorIsCurrentColor ? StyleColor::currentColor()
                                  : 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.
   unsigned m_isAuto : 1;  // OutlineIsAuto
 };
 
 }  // namespace blink
 
 #endif  // BorderValue_h