Add base types for path ops

src/pathops/SkPathOpsBounds.h

Index: src/pathops/SkPathOpsBounds.h
===================================================================
--- src/pathops/SkPathOpsBounds.h	(revision 0)
+++ src/pathops/SkPathOpsBounds.h	(revision 0)
@@ -0,0 +1,65 @@
+/*
+ * Copyright 2012 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+#ifndef SkPathOpBounds_DEFINED
+#define SkPathOpBounds_DEFINED
+
+#include "SkPathOpsRect.h"
+#include "SkRect.h"
+
+// Bounds, unlike Rect, does not consider a line to be empty.
+struct SkPathOpsBounds : public SkRect {
+    static bool Intersects(const SkPathOpsBounds& a, const SkPathOpsBounds& b) {
+        return a.fLeft <= b.fRight && b.fLeft <= a.fRight &&
+                a.fTop <= b.fBottom && b.fTop <= a.fBottom;
+    }
+
+    void add(SkScalar left, SkScalar top, SkScalar right, SkScalar bottom) {

[whunt, 2013/03/22 18:16:06]

add is a very overloaded word.  I'd recommend "Union" in this case as you are
unioning a new point into a point set.

[caryclark, 2013/03/22 19:38:51]

Point taken. However, union is unavailable -- Skia coding standards requires
that members are lowercase. 

[danakj, 2013/03/22 19:52:18]

We've used "unite" in the past instead

+        if (left < fLeft) {

[whunt, 2013/03/22 18:16:06]

Use std::min and std::max. In 64 bit mode and any platform that assumes SSE
you'll get the scalar SSE min/max operations witch are *way* faster than
branches.
https://uma.googleplex.com/histograms/?dayCount=28&endDate=03-21-2013&version...

[caryclark, 2013/03/22 19:38:51]

Certainly worth investigating. Skia uses the  'if (a op b) a = b' pattern in
some  places and 'a = max(b, c)'  in others, so it's worth making consistent.
I'd be interested to see for which compilers and platforms this is appropriate.

In any case, it's worth noting that Skia does not use std:: as a rule, but that
SkTMin and SkTMax are part of this CL.

+            fLeft = left;
+        }
+        if (top < fTop) {
+            fTop = top;
+        }
+        if (right > fRight) {
+            fRight = right;
+        }
+        if (bottom > fBottom) {
+            fBottom = bottom;
+        }
+    }
+
+    void add(const SkPathOpsBounds& toAdd) {

[reed1, 2013/03/22 15:58:04]

this, and add(point), could likely be added to SkRect, since they look generic.
Perhaps a comment to that effect for now?

[caryclark, 2013/03/22 19:38:51]

Done.

+        add(toAdd.fLeft, toAdd.fTop, toAdd.fRight, toAdd.fBottom);
+    }
+
+    void add(const SkPoint& pt) {
+        if (pt.fX < fLeft) fLeft = pt.fX;
+        if (pt.fY < fTop) fTop = pt.fY;
+        if (pt.fX > fRight) fRight = pt.fX;
+        if (pt.fY > fBottom) fBottom = pt.fY;
+    }
+
+    bool isEmpty() {

[reed1, 2013/03/22 15:58:04]

const?

Can we find a different name, since this one directly matches/hides one on
SkRect with diff semantics?

[danakj, 2013/03/22 19:11:42]

it seems interesting that a 0x0 area is empty but a 0x10 area is not? that
certainly is not how other geometry types in our code base work, is that going
to cause a problem?

[whunt, 2013/03/22 19:37:28]

0x0 should not be empty, it should contain a point (e.g. the result of an
setPoint() call)

[caryclark, 2013/03/22 19:38:51]

Done. isReallyEmpty() ? (BTW, this function turns out to be unused for now.)

[caryclark, 2013/03/22 20:05:00]

In this use case, that is the correct behavior. Note that all code here solely
exists to perform path ops, and is private to that purpose.

[caryclark, 2013/03/22 20:05:00]

Not for this application.

+        return fLeft > fRight || fTop > fBottom

[whunt, 2013/03/22 18:16:06]

the implementation should be:
!(fLeft < fRight && fTop < fBottom)
this will catch all cases you have here

[whunt, 2013/03/22 19:37:28]

revised to to account for zero area, non-empty bounds
!(fLeft <= fRight && fTop <= fBottom)

[caryclark, 2013/03/22 19:38:51]

Done.

[caryclark, 2013/03/22 20:05:00]

Noted.

+                || (fLeft == fRight && fTop == fBottom)

[whunt, 2013/03/22 19:37:28]

I think this check (for single point) is wrong, see Dana's comment.

[caryclark, 2013/03/22 20:05:00]

For this use case, it is not wrong.

+                || sk_double_isnan(fLeft) || sk_double_isnan(fRight)

[reed1, 2013/03/22 15:58:04]

fLeft is SkScalar, not double. Should we use SkScalarIsNaN?

[caryclark, 2013/03/22 19:38:51]

Done.

+                || sk_double_isnan(fTop) || sk_double_isnan(fBottom);
+    }
+
+    void setCubicBounds(const SkPoint a[4]);
+    void setLineBounds(const SkPoint a[2]);
+    void setQuadBounds(const SkPoint a[3]);
+
+    void setPoint(const SkPoint& pt) {

[reed1, 2013/03/22 15:58:04]

Crazy suggestion: setPointBounds (like the above methods)?

[caryclark, 2013/03/22 19:38:51]

Done.

+        fLeft = fRight = pt.fX;
+        fTop = fBottom = pt.fY;
+    }
+};
+
+extern void (SkPathOpsBounds::*SetCurveBounds[])(const SkPoint[]);
+
+#endif