Bug 1498571 - Cleanup FontPropertyTypes. r=jwatt
☠☠ backed out by 616ced661907 ☠ ☠
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 12 Oct 2018 14:20:23 +0000
changeset 499332 e9b7f6a58cc27e19f3012ffac4f81e5067b689bf
parent 499331 f3d26ec88c8d6e045d4eb8defbf09810a4c440a9
child 499333 c4726a1aafe604602172cd6cf9aff330029667ee
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwatt
bugs1498571
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1498571 - Cleanup FontPropertyTypes. r=jwatt Now that they can be constexpr, do that. Also, make default constructors initialize to the minimum value, now that we no longer need to use the default due to nsCSSValue. Differential Revision: https://phabricator.services.mozilla.com/D7757
gfx/src/FontPropertyTypes.h
--- a/gfx/src/FontPropertyTypes.h
+++ b/gfx/src/FontPropertyTypes.h
@@ -48,25 +48,22 @@ namespace mozilla {
  * get created, particularly when styles are animated or set to arbitrary
  * values (e.g. by sliders in the UI), which should reduce pressure on
  * graphics resources and improve cache hit rates.
  */
 template<class InternalType, unsigned FractionBits, int Min, int Max>
 class FontPropertyValue
 {
 public:
-  // Ugh. We need a default constructor to allow this type to be used in the
-  // union in nsCSSValue. Furthermore we need the default and copy
-  // constructors to be "trivial" (i.e. the compiler implemented defaults that
-  // do no initialization).
-  // Annoyingly we can't make the default implementations constexpr (at least
-  // in clang). That would be nice to do in order to allow the methods of
-  // subclasses that always return the same value (e.g., FontWeight::Thin())
-  // to also be constexpr. :/
-  FontPropertyValue() = default;
+  // Initialize to the minimum value by default.
+  constexpr FontPropertyValue()
+    : FontPropertyValue(Min)
+  {
+  }
+
   explicit FontPropertyValue(const FontPropertyValue& aOther) = default;
   FontPropertyValue& operator=(const FontPropertyValue& aOther) = default;
 
   bool operator==(const FontPropertyValue& aOther) const
   {
     return mValue == aOther.mValue;
   }
   bool operator!=(const FontPropertyValue& aOther) const
@@ -105,31 +102,31 @@ public:
   }
 
   static constexpr const float kMin = float(Min);
   static constexpr const float kMax = float(Max);
 
 protected:
   // Construct from a floating-point or integer value, checking that it is
   // within the allowed range and converting to fixed-point representation.
-  explicit FontPropertyValue(float aValue)
+  explicit constexpr FontPropertyValue(float aValue)
     : mValue(std::round(aValue * kScale))
   {
     MOZ_ASSERT(aValue >= kMin && aValue <= kMax);
   }
-  explicit FontPropertyValue(int aValue)
+  explicit constexpr FontPropertyValue(int aValue)
     : mValue(aValue << kFractionBits)
   {
     MOZ_ASSERT(aValue >= Min && aValue <= Max);
   }
 
   // Construct directly from a fixed-point value of type T, with no check;
   // note that there may be special "flag" values that are outside the normal
   // min/max range (e.g. for font-style:italic, distinct from oblique angle).
-  explicit FontPropertyValue(InternalType aValue)
+  explicit constexpr FontPropertyValue(InternalType aValue)
     : mValue(aValue)
   {
   }
 
   // This is protected as it may not be the most appropriate accessor for a
   // given instance to expose. It's up to each individual property to provide
   // public accessors that forward to this as required.
   float ToFloat() const { return mValue * kInverseScale; }
@@ -151,61 +148,59 @@ protected:
  * 'normal', 'bold' aliased to 400, 700 respectively; relative keywords
  * 'lighter', 'bolder' (not currently handled here).
  *
  * We use an unsigned 10.6 fixed-point value (range 0.0 - 1023.984375)
  */
 class FontWeight final : public FontPropertyValue<uint16_t,6,1,1000>
 {
 public:
-  // See comment in FontPropertyValue regarding requirement for a trivial
-  // default constructor.
-  FontWeight() = default;
+  constexpr FontWeight() = default;
 
-  explicit FontWeight(float aValue)
+  explicit constexpr FontWeight(float aValue)
     : FontPropertyValue(aValue)
   {
   }
 
   /**
    * CSS font weights can have fractional values, but this constructor exists
    * for convenience when writing constants such as FontWeight(700) in code.
    */
-  explicit FontWeight(int aValue)
+  explicit constexpr FontWeight(int aValue)
     : FontPropertyValue(aValue)
   {
   }
 
-  static FontWeight Normal()
+  static constexpr FontWeight Normal()
   {
     return FontWeight(kNormal);
   }
 
-  static FontWeight Thin()
+  static constexpr FontWeight Thin()
   {
     return FontWeight(kThin);
   }
 
-  static FontWeight Bold()
+  static constexpr FontWeight Bold()
   {
     return FontWeight(kBold);
   }
 
   bool IsNormal() const { return mValue == kNormal; }
   bool IsBold() const { return mValue >= kBoldThreshold; }
 
   float ToFloat() const { return FontPropertyValue::ToFloat(); }
   int ToIntRounded() const { return FontPropertyValue::ToIntRounded(); }
 
   typedef uint16_t InternalType;
 
 private:
   friend class WeightRange;
 
-  explicit FontWeight(InternalType aValue)
+  explicit constexpr FontWeight(InternalType aValue)
     : FontPropertyValue(aValue)
   {
   }
 
   static const InternalType kNormal        = 400u << kFractionBits;
   static const InternalType kBold          = 700u << kFractionBits;
   static const InternalType kBoldThreshold = 600u << kFractionBits;
   static const InternalType kThin          = 100u << kFractionBits;
@@ -224,81 +219,79 @@ private:
  * 0.0 - 1023.984375).
  *
  * We arbitrarily limit here to 1000%. (If that becomes a problem, we
  * could reduce the number of fractional bits and increase the limit.)
  */
 class FontStretch final : public FontPropertyValue<uint16_t,6,0,1000>
 {
 public:
-  // See comment in FontPropertyValue regarding requirement for a trivial
-  // default constructor.
-  FontStretch() = default;
+  constexpr FontStretch() = default;
 
-  explicit FontStretch(float aPercent)
+  explicit constexpr FontStretch(float aPercent)
     : FontPropertyValue(aPercent)
   {
   }
 
-  static FontStretch Normal()
+  static constexpr FontStretch Normal()
   {
     return FontStretch(kNormal);
   }
-  static FontStretch UltraCondensed()
+  static constexpr FontStretch UltraCondensed()
   {
     return FontStretch(kUltraCondensed);
   }
-  static FontStretch ExtraCondensed()
+  static constexpr FontStretch ExtraCondensed()
   {
     return FontStretch(kExtraCondensed);
   }
-  static FontStretch Condensed()
+  static constexpr FontStretch Condensed()
   {
     return FontStretch(kCondensed);
   }
-  static FontStretch SemiCondensed()
+  static constexpr FontStretch SemiCondensed()
   {
     return FontStretch(kSemiCondensed);
   }
-  static FontStretch SemiExpanded()
+  static constexpr FontStretch SemiExpanded()
   {
     return FontStretch(kSemiExpanded);
   }
-  static FontStretch Expanded()
+  static constexpr FontStretch Expanded()
   {
     return FontStretch(kExpanded);
   }
-  static FontStretch ExtraExpanded()
+  static constexpr FontStretch ExtraExpanded()
   {
     return FontStretch(kExtraExpanded);
   }
-  static FontStretch UltraExpanded()
+  static constexpr FontStretch UltraExpanded()
   {
     return FontStretch(kUltraExpanded);
   }
 
   // The style system represents percentages in the 0.0..1.0 range, and
   // FontStretch does it in the 0.0..100.0 range.
   //
   // TODO(emilio): We should consider changing this class to deal with the same
   // range as the style system.
-  static FontStretch FromStyle(float aStylePercentage)
+  static constexpr FontStretch FromStyle(float aStylePercentage)
   {
     return FontStretch(std::min(aStylePercentage * 100.0f, float(kMax)));
   }
 
   bool IsNormal() const { return mValue == kNormal; }
   float Percentage() const { return ToFloat(); }
 
   typedef uint16_t InternalType;
 
 private:
   friend class StretchRange;
 
-  explicit FontStretch(InternalType aValue)
+  explicit constexpr FontStretch(InternalType aValue)
     : FontPropertyValue(aValue)
   {
   }
 
   static const InternalType kUltraCondensed =  50u << kFractionBits;
   static const InternalType kExtraCondensed = (62u << kFractionBits) + kPointFive;
   static const InternalType kCondensed      =  75u << kFractionBits;
   static const InternalType kSemiCondensed  = (87u << kFractionBits) + kPointFive;
@@ -319,54 +312,52 @@ private:
  * - Other values represent 'oblique <angle>'
  * - Note that 'oblique 0deg' is distinct from 'normal' (should it be?)
  */
 class FontSlantStyle final : public FontPropertyValue<int16_t,8,-90,90>
 {
 public:
   const static constexpr float kDefaultAngle = 14.0;
 
-  // See comment in FontPropertyValue regarding requirement for a trivial
-  // default constructor.
-  FontSlantStyle() = default;
+  constexpr FontSlantStyle() = default;
 
-  static FontSlantStyle Normal()
+  static constexpr FontSlantStyle Normal()
   {
     return FontSlantStyle(kNormal);
   }
 
-  static FontSlantStyle Italic()
+  static constexpr FontSlantStyle Italic()
   {
     return FontSlantStyle(kItalic);
   }
 
-  static FontSlantStyle Oblique(float aAngle = kDefaultAngle)
+  static constexpr FontSlantStyle Oblique(float aAngle = kDefaultAngle)
   {
     return FontSlantStyle(aAngle);
   }
 
   // Create from a string as generated by ToString. This is for internal use
   // when serializing/deserializing entries for the startupcache, and is not
   // intended to parse arbitrary (untrusted) strings.
   static FontSlantStyle FromString(const char* aString)
   {
     if (strcmp(aString, "normal") == 0) {
       return Normal();
-    } else if (strcmp(aString, "italic") == 0) {
+    }
+    if (strcmp(aString, "italic") == 0) {
       return Italic();
-    } else {
-      if (mozilla::IsAsciiDigit(aString[0]) && strstr(aString, "deg")) {
-        float angle = strtof(aString, nullptr);
-        return Oblique(angle);
-      }
-      // Not recognized as an oblique angle; maybe it's from a startup-cache
-      // created by an older version. The style field there used a simple 0/1
-      // for normal/italic respectively.
-      return aString[0] == '0' ? Normal() : Italic();
+    }
+    if (mozilla::IsAsciiDigit(aString[0]) && strstr(aString, "deg")) {
+      float angle = strtof(aString, nullptr);
+      return Oblique(angle);
     }
+    // Not recognized as an oblique angle; maybe it's from a startup-cache
+    // created by an older version. The style field there used a simple 0/1
+    // for normal/italic respectively.
+    return aString[0] == '0' ? Normal() : Italic();
   }
 
   bool IsNormal() const { return mValue == kNormal; }
   bool IsItalic() const { return mValue == kItalic; }
   bool IsOblique() const { return mValue != kItalic && mValue != kNormal; }
 
   float ObliqueAngle() const
   {
@@ -393,22 +384,22 @@ public:
     }
   }
 
   typedef int16_t InternalType;
 
 private:
   friend class SlantStyleRange;
 
-  explicit FontSlantStyle(InternalType aConstant)
+  explicit constexpr FontSlantStyle(InternalType aConstant)
     : FontPropertyValue(aConstant)
   {
   }
 
-  explicit FontSlantStyle(float aAngle)
+  explicit constexpr FontSlantStyle(float aAngle)
     : FontPropertyValue(aAngle)
   {
   }
 
   static const InternalType kNormal = INT16_MIN;
   static const InternalType kItalic = INT16_MAX;
 };