Bug 1763741 - Make gfx types take the right amount of space in MSVC. r=jrmuizel,glandium

This is an attribute that could be useful elsewhere so seems worth putting in mfbt. Differential Revision: https://phabricator.services.mozilla.com/D143249
2022-04-08 13:27:28 +00:00 · 2022-04-08 13:27:28 +00:00 · a9a85f3eca
--- a/gfx/2d/Coord.h
+++ b/gfx/2d/Coord.h
@ -86,7 +86,7 @@ struct CoordOperatorsHelper<true, coord, primitive> {
 };

 template <class units, class Rep>
-struct IntCoordTyped
+struct MOZ_EMPTY_BASES IntCoordTyped
    : public BaseCoord<Rep, IntCoordTyped<units, Rep>>,
      public units,
      public CoordOperatorsHelper<true, IntCoordTyped<units, Rep>, float>,
@ -96,30 +96,46 @@ struct IntCoordTyped

  using Super = BaseCoord<Rep, IntCoordTyped<units, Rep>>;

-  constexpr IntCoordTyped() : Super() {}
-  constexpr MOZ_IMPLICIT IntCoordTyped(Rep aValue) : Super(aValue) {}
+  constexpr IntCoordTyped() : Super() {
+    static_assert(sizeof(IntCoordTyped) == sizeof(Rep),
+                  "Would be unfortunate otherwise!");
+  }
+  constexpr MOZ_IMPLICIT IntCoordTyped(Rep aValue) : Super(aValue) {
+    static_assert(sizeof(IntCoordTyped) == sizeof(Rep),
+                  "Would be unfortunate otherwise!");
+  }
 };

 template <class units, class F>
-struct CoordTyped : public BaseCoord<F, CoordTyped<units, F>>,
-                    public units,
-                    public CoordOperatorsHelper<!std::is_same_v<F, int32_t>,
-                                                CoordTyped<units, F>, int32_t>,
-                    public CoordOperatorsHelper<!std::is_same_v<F, uint32_t>,
-                                                CoordTyped<units, F>, uint32_t>,
-                    public CoordOperatorsHelper<!std::is_same_v<F, double>,
-                                                CoordTyped<units, F>, double>,
-                    public CoordOperatorsHelper<!std::is_same_v<F, float>,
-                                                CoordTyped<units, F>, float> {
+struct MOZ_EMPTY_BASES CoordTyped
+    : public BaseCoord<F, CoordTyped<units, F>>,
+      public units,
+      public CoordOperatorsHelper<!std::is_same_v<F, int32_t>,
+                                  CoordTyped<units, F>, int32_t>,
+      public CoordOperatorsHelper<!std::is_same_v<F, uint32_t>,
+                                  CoordTyped<units, F>, uint32_t>,
+      public CoordOperatorsHelper<!std::is_same_v<F, double>,
+                                  CoordTyped<units, F>, double>,
+      public CoordOperatorsHelper<!std::is_same_v<F, float>,
+                                  CoordTyped<units, F>, float> {
  static_assert(IsPixel<units>::value,
                "'units' must be a coordinate system tag");

  using Super = BaseCoord<F, CoordTyped<units, F>>;

-  constexpr CoordTyped() : Super() {}
-  constexpr MOZ_IMPLICIT CoordTyped(F aValue) : Super(aValue) {}
+  constexpr CoordTyped() : Super() {
+    static_assert(sizeof(CoordTyped) == sizeof(F),
+                  "Would be unfortunate otherwise!");
+  }
+  constexpr MOZ_IMPLICIT CoordTyped(F aValue) : Super(aValue) {
+    static_assert(sizeof(CoordTyped) == sizeof(F),
+                  "Would be unfortunate otherwise!");
+  }
  explicit constexpr CoordTyped(const IntCoordTyped<units>& aCoord)
-      : Super(F(aCoord.value)) {}
+      : Super(F(aCoord.value)) {
+    static_assert(sizeof(CoordTyped) == sizeof(F),
+                  "Would be unfortunate otherwise!");
+  }

  void Round() { this->value = floor(this->value + 0.5); }
  void Truncate() { this->value = int32_t(this->value); }
--- a/gfx/2d/Point.h
+++ b/gfx/2d/Point.h
@ -70,7 +70,7 @@ template <class units, class>
 struct SizeTyped;

 template <class units>
-struct IntPointTyped
+struct MOZ_EMPTY_BASES IntPointTyped
    : public BasePoint<int32_t, IntPointTyped<units>, IntCoordTyped<units> >,
      public units {
  static_assert(IsPixel<units>::value,
@ -80,7 +80,10 @@ struct IntPointTyped
  typedef IntCoordTyped<units> Coord;
  typedef BasePoint<int32_t, IntPointTyped<units>, IntCoordTyped<units> > Super;

-  constexpr IntPointTyped() : Super() {}
+  constexpr IntPointTyped() : Super() {
+    static_assert(sizeof(IntPointTyped) == sizeof(int32_t) * 2,
+                  "Would be unfortunate otherwise!");
+  }
  constexpr IntPointTyped(ToInt aX, ToInt aY)
      : Super(Coord(aX.value), Coord(aY.value)) {}

@ -121,7 +124,7 @@ struct IntPointTyped
 typedef IntPointTyped<UnknownUnits> IntPoint;

 template <class units, class F = Float>
-struct PointTyped
+struct MOZ_EMPTY_BASES PointTyped
    : public BasePoint<F, PointTyped<units, F>, CoordTyped<units, F> >,
      public units {
  static_assert(IsPixel<units>::value,
@ -130,7 +133,10 @@ struct PointTyped
  typedef CoordTyped<units, F> Coord;
  typedef BasePoint<F, PointTyped<units, F>, CoordTyped<units, F> > Super;

-  constexpr PointTyped() : Super() {}
+  constexpr PointTyped() : Super() {
+    static_assert(sizeof(PointTyped) == sizeof(F) * 2,
+                  "Would be unfortunate otherwise!");
+  }
  constexpr PointTyped(F aX, F aY) : Super(Coord(aX), Coord(aY)) {}
  // The mixed-type constructors (Float, Coord) and (Coord, Float) are needed to
  // avoid ambiguities because Coord is implicitly convertible to Float.
@ -177,7 +183,10 @@ struct Point3DTyped : public BasePoint3D<F, Point3DTyped<units, F> > {

  typedef BasePoint3D<F, Point3DTyped<units, F> > Super;

-  Point3DTyped() : Super() {}
+  Point3DTyped() : Super() {
+    static_assert(sizeof(Point3DTyped) == sizeof(F) * 3,
+                  "Would be unfortunate otherwise!");
+  }
  Point3DTyped(F aX, F aY, F aZ) : Super(aX, aY, aZ) {}

  // XXX When all of the code is ported, the following functions to convert to
@ -226,7 +235,10 @@ struct Point4DTyped : public BasePoint4D<F, Point4DTyped<units, F> > {

  typedef BasePoint4D<F, Point4DTyped<units, F> > Super;

-  Point4DTyped() : Super() {}
+  Point4DTyped() : Super() {
+    static_assert(sizeof(Point4DTyped) == sizeof(F) * 4,
+                  "Would be unfortunate otherwise!");
+  }
  Point4DTyped(F aX, F aY, F aZ, F aW) : Super(aX, aY, aZ, aW) {}

  explicit Point4DTyped(const Point3DTyped<units, F>& aPoint)
@ -257,15 +269,19 @@ typedef Point4DTyped<UnknownUnits> Point4D;
 typedef Point4DTyped<UnknownUnits, double> PointDouble4D;

 template <class units>
-struct IntSizeTyped : public BaseSize<int32_t, IntSizeTyped<units> >,
-                      public units {
+struct MOZ_EMPTY_BASES IntSizeTyped
+    : public BaseSize<int32_t, IntSizeTyped<units> >,
+      public units {
  static_assert(IsPixel<units>::value,
                "'units' must be a coordinate system tag");

  typedef IntParam<int32_t> ToInt;
  typedef BaseSize<int32_t, IntSizeTyped<units> > Super;

-  constexpr IntSizeTyped() : Super() {}
+  constexpr IntSizeTyped() : Super() {
+    static_assert(sizeof(IntSizeTyped) == sizeof(int32_t) * 2,
+                  "Would be unfortunate otherwise!");
+  }
  constexpr IntSizeTyped(ToInt aWidth, ToInt aHeight)
      : Super(aWidth.value, aHeight.value) {}

@ -307,13 +323,17 @@ typedef IntSizeTyped<UnknownUnits> IntSize;
 typedef Maybe<IntSize> MaybeIntSize;

 template <class units, class F = Float>
-struct SizeTyped : public BaseSize<F, SizeTyped<units, F> >, public units {
+struct MOZ_EMPTY_BASES SizeTyped : public BaseSize<F, SizeTyped<units, F> >,
+                                   public units {
  static_assert(IsPixel<units>::value,
                "'units' must be a coordinate system tag");

  typedef BaseSize<F, SizeTyped<units, F> > Super;

-  constexpr SizeTyped() : Super() {}
+  constexpr SizeTyped() : Super() {
+    static_assert(sizeof(SizeTyped) == sizeof(F) * 2,
+                  "Would be unfortunate otherwise!");
+  }
  constexpr SizeTyped(F aWidth, F aHeight) : Super(aWidth, aHeight) {}
  explicit SizeTyped(const IntSizeTyped<units>& size)
      : Super(F(size.width), F(size.height)) {}
--- a/gfx/2d/Rect.h
+++ b/gfx/2d/Rect.h
@ -27,14 +27,18 @@ template <class units, class F>
 struct RectTyped;

 template <class units>
-struct IntMarginTyped : public BaseMargin<int32_t, IntMarginTyped<units> >,
-                        public units {
+struct MOZ_EMPTY_BASES IntMarginTyped
+    : public BaseMargin<int32_t, IntMarginTyped<units> >,
+      public units {
  static_assert(IsPixel<units>::value,
                "'units' must be a coordinate system tag");

  typedef BaseMargin<int32_t, IntMarginTyped<units> > Super;

-  IntMarginTyped() : Super() {}
+  IntMarginTyped() : Super() {
+    static_assert(sizeof(IntMarginTyped) == sizeof(int32_t) * 4,
+                  "Would be unfortunate otherwise!");
+  }
  IntMarginTyped(int32_t aTop, int32_t aRight, int32_t aBottom, int32_t aLeft)
      : Super(aTop, aRight, aBottom, aLeft) {}

@ -92,7 +96,7 @@ IntMarginTyped<units> RoundedToInt(const MarginTyped<units>& aMargin) {
 }

 template <class units>
-struct IntRectTyped
+struct MOZ_EMPTY_BASES IntRectTyped
    : public BaseRect<int32_t, IntRectTyped<units>, IntPointTyped<units>,
                      IntSizeTyped<units>, IntMarginTyped<units> >,
      public units {
@ -105,7 +109,10 @@ struct IntRectTyped
  typedef IntRectTyped<units> Self;
  typedef IntParam<int32_t> ToInt;

-  IntRectTyped() : Super() {}
+  IntRectTyped() : Super() {
+    static_assert(sizeof(IntRectTyped) == sizeof(int32_t) * 4,
+                  "Would be unfortunate otherwise!");
+  }
  IntRectTyped(const IntPointTyped<units>& aPos,
               const IntSizeTyped<units>& aSize)
      : Super(aPos, aSize) {}
@ -242,9 +249,10 @@ struct IntRectTyped
 typedef IntRectTyped<UnknownUnits> IntRect;

 template <class units, class F = Float>
-struct RectTyped : public BaseRect<F, RectTyped<units, F>, PointTyped<units, F>,
-                                   SizeTyped<units, F>, MarginTyped<units, F> >,
-                   public units {
+struct MOZ_EMPTY_BASES RectTyped
+    : public BaseRect<F, RectTyped<units, F>, PointTyped<units, F>,
+                      SizeTyped<units, F>, MarginTyped<units, F> >,
+      public units {
  static_assert(IsPixel<units>::value,
                "'units' must be a coordinate system tag");

@ -252,7 +260,10 @@ struct RectTyped : public BaseRect<F, RectTyped<units, F>, PointTyped<units, F>,
                   SizeTyped<units, F>, MarginTyped<units, F> >
      Super;

-  RectTyped() : Super() {}
+  RectTyped() : Super() {
+    static_assert(sizeof(RectTyped) == sizeof(F) * 4,
+                  "Would be unfortunate otherwise!");
+  }
  RectTyped(const PointTyped<units, F>& aPos, const SizeTyped<units, F>& aSize)
      : Super(aPos, aSize) {}
  RectTyped(F _x, F _y, F _width, F _height) : Super(_x, _y, _width, _height) {}
--- a/mfbt/Attributes.h
+++ b/mfbt/Attributes.h
@ -910,4 +910,17 @@
 #  define MOZ_XPCOM_ABI
 #endif

+/**
+ * MSVC / clang-cl don't optimize empty bases correctly unless we explicitly
+ * tell it to, see:
+ *
+ * https://stackoverflow.com/questions/12701469/why-is-the-empty-base-class-optimization-ebo-is-not-working-in-msvc
+ * https://devblogs.microsoft.com/cppblog/optimizing-the-layout-of-empty-base-classes-in-vs2015-update-2-3/
+ */
+#if defined(_MSC_VER)
+#  define MOZ_EMPTY_BASES __declspec(empty_bases)
+#else
+#  define MOZ_EMPTY_BASES
+#endif
+
 #endif /* mozilla_Attributes_h */