Fixup FX_RECT and FX_SMALL_RECT classes.

core/include/fxcrt/fx_coordinates.h

Index: core/include/fxcrt/fx_coordinates.h
diff --git a/core/include/fxcrt/fx_coordinates.h b/core/include/fxcrt/fx_coordinates.h
index c6bf2300146148e6543955ffcc4c4ef36cb8a4e4..6df26680e76146d25404b1bef3c42c3df109579a 100644
--- a/core/include/fxcrt/fx_coordinates.h
+++ b/core/include/fxcrt/fx_coordinates.h
@@ -120,6 +120,210 @@ class CFX_VTemplate : public CFX_PSTemplate<BaseType> {
 typedef CFX_VTemplate<int32_t> CFX_Vector;
 typedef CFX_VTemplate<FX_FLOAT> CFX_VectorF;
 
+// Rectangles.
+// TODO(tsepez): Consolidate all these different rectangle classes.
+
+// LTRB rectangles (y-axis runs downwards).
+struct FX_SMALL_RECT {
+  FX_SMALL_RECT() : FX_SMALL_RECT(kInvalid, kInvalid, kInvalid, kInvalid) {}
+
+  FX_SMALL_RECT(int16_t l, int16_t t, int16_t r, int16_t b)
+      : Left(l), Top(t), Right(r), Bottom(b) {}
+
+  static const int16_t kInvalid = -1;
+
+  int16_t Left;

[dsinclair, 2016/02/27 03:11:44]

Why does this one use upper case (Left) and the one below use lower case (left)?

[Tom Sepez, 2016/02/29 18:42:08]

Because they're inconsistent with public members.  Usually they leave off the m_
prefix for public struct members.  It feels about 50/50 as to whether they then
use the hungarian notation (wLeft), and then lacking that, they're incosnsitet
about caps.  These are used in a few spots, I'll make them |left| to match the
below.

+  int16_t Top;
+  int16_t Right;
+  int16_t Bottom;
+};
+
+struct FX_RECT {
+  FX_RECT() : left(0), top(0), right(0), bottom(0) {}
+
+  FX_RECT(int l, int t, int r, int b) : left(l), top(t), right(r), bottom(b) {}
+
+  explicit FX_RECT(const FX_SMALL_RECT& small)
+      : FX_RECT(small.Left, small.Top, small.Right, small.Bottom) {}
+
+  int Width() const { return right - left; }
+  int Height() const { return bottom - top; }
+  bool IsEmpty() const { return right <= left || bottom <= top; }
+
+  void Normalize();
+
+  void Intersect(const FX_RECT& src);
+  void Intersect(int left1, int top1, int right1, int bottom1) {

[dsinclair, 2016/02/27 03:11:44]

nit: Why the 1's in the names?

[Tom Sepez, 2016/02/29 18:42:07]

They conflict with instance variables in the class.  I'll make them |l| |t| |r|
|b| as above.

+    Intersect(FX_RECT(left1, top1, right1, bottom1));
+  }
+
+  void Union(const FX_RECT& other_rect);
+  void Union(int left1, int top1, int right1, int bottom1) {
+    Union(FX_RECT(left1, top1, right1, bottom1));
+  }
+
+  void Offset(int dx, int dy) {
+    left += dx;
+    right += dx;
+    top += dy;
+    bottom += dy;
+  }
+
+  bool operator==(const FX_RECT& src) const {
+    return left == src.left && right == src.right && top == src.top &&
+           bottom == src.bottom;
+  }
+
+  FX_BOOL Contains(const FX_RECT& other_rect) const {
+    return other_rect.left >= left && other_rect.right <= right &&
+           other_rect.top >= top && other_rect.bottom <= bottom;
+  }
+
+  FX_BOOL Contains(int x, int y) const {
+    return x >= left && x < right && y >= top && y < bottom;

[dsinclair, 2016/02/27 03:11:44]

Why >= for left and top but < for right and bottom?

[Tom Sepez, 2016/02/29 18:42:08]

Beats me.  I'd expect a lot of 1px diffs if we changed it.

+  }
+
+  FX_SMALL_RECT ToSmallRect() const {
+    return FX_SMALL_RECT(
+        static_cast<uint16_t>(left), static_cast<uint16_t>(top),
+        static_cast<uint16_t>(right), static_cast<uint16_t>(bottom));
+  }
+
+  int left;
+  int top;
+  int right;
+  int bottom;
+};
+
+// LBRT rectangles (y-axis runs upwards).
+class CFX_FloatPoint {
+ public:
+  CFX_FloatPoint(FX_FLOAT xx, FX_FLOAT yy) : x(xx), y(yy) {}
+
+  FX_FLOAT x;
+  FX_FLOAT y;
+};
+
+class CFX_FloatRect {
+ public:
+  CFX_FloatRect() { left = right = bottom = top = 0; }

[dsinclair, 2016/02/27 03:11:44]

CFX_FloatRect() : left(0), right(0), bottom(0), top(0) {} ?

[Tom Sepez, 2016/02/29 18:42:07]

Done.

+
+  CFX_FloatRect(FX_FLOAT left1,
+                FX_FLOAT bottom1,
+                FX_FLOAT right1,
+                FX_FLOAT top1) {
+    left = left1;
+    bottom = bottom1;
+    right = right1;
+    top = top1;
+  }
+
+  CFX_FloatRect(const FX_FLOAT* pArray) {

[dsinclair, 2016/02/27 03:11:44]

nit: explicit

[Tom Sepez, 2016/02/29 18:42:08]

Yep. Good catch. Done.

+    left = pArray[0];
+    bottom = pArray[1];
+    right = pArray[2];
+    top = pArray[3];
+  }
+
+  CFX_FloatRect(const FX_RECT& rect);

[dsinclair, 2016/02/27 03:11:44]

nit: explicit

[Tom Sepez, 2016/02/29 18:42:08]

Done.

+
+  FX_BOOL IsEmpty() const { return left >= right || bottom >= top; }
+
+  void Normalize();
+
+  void Reset() { left = right = bottom = top = 0; }
+
+  FX_BOOL Contains(const CFX_FloatRect& other_rect) const;
+
+  FX_BOOL Contains(FX_FLOAT x, FX_FLOAT y) const;
+
+  void Transform(const CFX_Matrix* pMatrix);
+
+  void Intersect(const CFX_FloatRect& other_rect);
+
+  void Union(const CFX_FloatRect& other_rect);
+
+  FX_RECT GetInnerRect() const;
+

[dsinclair, 2016/02/27 03:11:44]

nit: Can a bunch of these spaces be removed?

[Tom Sepez, 2016/02/29 18:42:08]

Done.

+  FX_RECT GetOutterRect() const;
+
+  FX_RECT GetClosestRect() const;
+
+  int Substract4(CFX_FloatRect& substract_rect, CFX_FloatRect* pRects);
+
+  void InitRect(FX_FLOAT x, FX_FLOAT y) {

[dsinclair, 2016/02/27 03:11:44]

This seems like a very weird init method? We initialize to 0 width/height and
empty?

[Tom Sepez, 2016/02/29 18:42:08]

yes. looking at usage, its always followed by a bunch of UpdateRect() calls to
make it non-empty.  Something for later.

+    left = right = x;
+    bottom = top = y;
+  }
+
+  void UpdateRect(FX_FLOAT x, FX_FLOAT y);
+
+  FX_FLOAT Width() const { return right - left; }
+
+  FX_FLOAT Height() const { return top - bottom; }
+
+  void Inflate(FX_FLOAT x, FX_FLOAT y) {

[dsinclair, 2016/02/27 03:11:44]

Maybe add a comment that this inflates by 2x and 2y?

[Tom Sepez, 2016/02/29 18:42:08]

Maybe.

+    Normalize();
+    left -= x;
+    right += x;
+    bottom -= y;
+    top += y;
+  }
+
+  void Inflate(FX_FLOAT other_left,
+               FX_FLOAT other_bottom,
+               FX_FLOAT other_right,
+               FX_FLOAT other_top) {
+    Normalize();
+    left -= other_left;
+    bottom -= other_bottom;
+    right += other_right;
+    top += other_top;
+  }
+
+  void Inflate(const CFX_FloatRect& rt) {
+    Inflate(rt.left, rt.bottom, rt.right, rt.top);
+  }
+
+  void Deflate(FX_FLOAT x, FX_FLOAT y) {

[dsinclair, 2016/02/27 03:11:44]

ditto: deflates by 2x, 2y

+    Normalize();
+    left += x;
+    right -= x;
+    bottom += y;
+    top -= y;
+  }
+
+  void Deflate(FX_FLOAT other_left,
+               FX_FLOAT other_bottom,
+               FX_FLOAT other_right,
+               FX_FLOAT other_top) {
+    Normalize();
+    left += other_left;
+    bottom += other_bottom;
+    right -= other_right;
+    top -= other_top;
+  }
+
+  void Deflate(const CFX_FloatRect& rt) {
+    Deflate(rt.left, rt.bottom, rt.right, rt.top);
+  }
+
+  void Translate(FX_FLOAT e, FX_FLOAT f) {
+    left += e;
+    right += e;
+    top += f;
+    bottom += f;
+  }
+
+  static CFX_FloatRect GetBBox(const CFX_PointF* pPoints, int nPoints);
+
+  FX_FLOAT left;
+  FX_FLOAT right;
+  FX_FLOAT bottom;
+  FX_FLOAT top;
+};
+
+// LTWH rectangles (y-axis runs downwards).

[dsinclair, 2016/02/27 03:11:44]

What does LTWH stand for?

[Tom Sepez, 2016/02/29 18:42:07]

Left, Top, Width, Height -- as in we store a point plus a displacement.  My
feeling is that this causes you to do a lot of needless addition along the way.
But there it is.

 template <class baseType>
 class CFX_RTemplate {
  public:
@@ -350,197 +554,6 @@ typedef CFX_RTemplate<FX_FLOAT>* FX_LPRECTF;
 typedef CFX_RTemplate<int32_t> const* FX_LPCRECT;
 typedef CFX_RTemplate<FX_FLOAT> const* FX_LPCRECTF;
 typedef CFX_ArrayTemplate<CFX_RectF> CFX_RectFArray;
-struct FX_RECT {
-  int left;
-
-  int top;
-
-  int right;
-
-  int bottom;
-
-  FX_RECT() : left(0), top(0), right(0), bottom(0) {}
-
-  FX_RECT(int left1, int top1, int right1, int bottom1) {
-    left = left1;
-    top = top1;
-    right = right1;
-    bottom = bottom1;
-  }
-
-  int Width() const { return right - left; }
-
-  int Height() const { return bottom - top; }
-
-  FX_BOOL IsEmpty() const { return right <= left || bottom <= top; }
-
-  void Normalize();
-
-  void Intersect(const FX_RECT& src);
-
-  void Intersect(int left1, int top1, int right1, int bottom1) {
-    Intersect(FX_RECT(left1, top1, right1, bottom1));
-  }
-
-  void Union(const FX_RECT& other_rect);
-
-  bool operator==(const FX_RECT& src) const {
-    return left == src.left && right == src.right && top == src.top &&
-           bottom == src.bottom;
-  }
-
-  void Offset(int dx, int dy) {
-    left += dx;
-    right += dx;
-    top += dy;
-    bottom += dy;
-  }
-
-  FX_BOOL Contains(const FX_RECT& other_rect) const {
-    return other_rect.left >= left && other_rect.right <= right &&
-           other_rect.top >= top && other_rect.bottom <= bottom;
-  }
-
-  FX_BOOL Contains(int x, int y) const {
-    return x >= left && x < right && y >= top && y < bottom;
-  }
-};
-struct FX_SMALL_RECT {
-  int16_t Left;
-
-  int16_t Top;
-
-  int16_t Right;
-
-  int16_t Bottom;
-};
-
-class CFX_FloatPoint {
- public:
-  CFX_FloatPoint(FX_FLOAT xx, FX_FLOAT yy) : x(xx), y(yy) {}
-
-  FX_FLOAT x;
-  FX_FLOAT y;
-};
-
-class CFX_FloatRect {
- public:
-  CFX_FloatRect() { left = right = bottom = top = 0; }
-
-  CFX_FloatRect(FX_FLOAT left1,
-                FX_FLOAT bottom1,
-                FX_FLOAT right1,
-                FX_FLOAT top1) {
-    left = left1;
-    bottom = bottom1;
-    right = right1;
-    top = top1;
-  }
-
-  CFX_FloatRect(const FX_FLOAT* pArray) {
-    left = pArray[0];
-    bottom = pArray[1];
-    right = pArray[2];
-    top = pArray[3];
-  }
-
-  CFX_FloatRect(const FX_RECT& rect);
-
-  FX_BOOL IsEmpty() const { return left >= right || bottom >= top; }
-
-  void Normalize();
-
-  void Reset() { left = right = bottom = top = 0; }
-
-  FX_BOOL Contains(const CFX_FloatRect& other_rect) const;
-
-  FX_BOOL Contains(FX_FLOAT x, FX_FLOAT y) const;
-
-  void Transform(const CFX_Matrix* pMatrix);
-
-  void Intersect(const CFX_FloatRect& other_rect);
-
-  void Union(const CFX_FloatRect& other_rect);
-
-  FX_RECT GetInnerRect() const;
-
-  FX_RECT GetOutterRect() const;
-
-  FX_RECT GetClosestRect() const;
-
-  int Substract4(CFX_FloatRect& substract_rect, CFX_FloatRect* pRects);
-
-  void InitRect(FX_FLOAT x, FX_FLOAT y) {
-    left = right = x;
-    bottom = top = y;
-  }
-
-  void UpdateRect(FX_FLOAT x, FX_FLOAT y);
-
-  FX_FLOAT Width() const { return right - left; }
-
-  FX_FLOAT Height() const { return top - bottom; }
-
-  void Inflate(FX_FLOAT x, FX_FLOAT y) {
-    Normalize();
-    left -= x;
-    right += x;
-    bottom -= y;
-    top += y;
-  }
-
-  void Inflate(FX_FLOAT other_left,
-               FX_FLOAT other_bottom,
-               FX_FLOAT other_right,
-               FX_FLOAT other_top) {
-    Normalize();
-    left -= other_left;
-    bottom -= other_bottom;
-    right += other_right;
-    top += other_top;
-  }
-
-  void Inflate(const CFX_FloatRect& rt) {
-    Inflate(rt.left, rt.bottom, rt.right, rt.top);
-  }
-
-  void Deflate(FX_FLOAT x, FX_FLOAT y) {
-    Normalize();
-    left += x;
-    right -= x;
-    bottom += y;
-    top -= y;
-  }
-
-  void Deflate(FX_FLOAT other_left,
-               FX_FLOAT other_bottom,
-               FX_FLOAT other_right,
-               FX_FLOAT other_top) {
-    Normalize();
-    left += other_left;
-    bottom += other_bottom;
-    right -= other_right;
-    top -= other_top;
-  }
-
-  void Deflate(const CFX_FloatRect& rt) {
-    Deflate(rt.left, rt.bottom, rt.right, rt.top);
-  }
-
-  void Translate(FX_FLOAT e, FX_FLOAT f) {
-    left += e;
-    right += e;
-    top += f;
-    bottom += f;
-  }
-
-  static CFX_FloatRect GetBBox(const CFX_PointF* pPoints, int nPoints);
-
-  FX_FLOAT left;
-  FX_FLOAT right;
-  FX_FLOAT bottom;
-  FX_FLOAT top;
-};
 
 class CFX_Matrix {
  public: