Fix bounds computation in GrAAHairlineRenderer

R=robertphillips@google.com, jvanverth@google.com

Author: bsalomon@google.com

Review URL: https://chromiumcodereview.appspot.com/23684008

git-svn-id: http://skia.googlecode.com/svn/trunk@11054 2bbb7eff-a529-9590-31e7-b0007b416f81

3 files changed, 70 insertions, 16 deletions

diff --git a/gm/hairlines.cpp b/gm/hairlines.cpp
index e73cc6ddf0..7be4423483 100644
--- a/gm/hairlines.cpp
+++ b/gm/hairlines.cpp
@@ -80,6 +80,30 @@ protected:
             unevenClosedQuad->quadTo(SkIntToScalar(100), SkIntToScalar(100),
                                      SkIntToScalar(75), SkIntToScalar(75));
         }
+
+        // Two problem cases for gpu hairline renderer found by shapeops testing. These used
+        // to assert that the computed bounding box didn't contain all the vertices.
+        {
+            SkPath* problem1 = &fPaths.push_back();
+            problem1->moveTo(SkIntToScalar(4), SkIntToScalar(6));
+            problem1->cubicTo(SkIntToScalar(5), SkIntToScalar(6),
+                              SkIntToScalar(5), SkIntToScalar(4),
+                              SkIntToScalar(4), SkIntToScalar(0));
+            problem1->close();
+        }
+
+        {
+            SkPath* problem2 = &fPaths.push_back();
+            problem2->moveTo(SkIntToScalar(5), SkIntToScalar(1));
+            problem2->lineTo(SkFloatToScalar(4.32787323f), SkFloatToScalar(1.67212653f));
+            problem2->cubicTo(SkFloatToScalar(2.75223875f), SkFloatToScalar(3.24776125f),
+                              SkFloatToScalar(3.00581908f), SkFloatToScalar(4.51236057f),
+                              SkFloatToScalar(3.7580452f), SkFloatToScalar(4.37367964f));
+            problem2->cubicTo(SkFloatToScalar(4.66472578f), SkFloatToScalar(3.888381f),
+                              SkFloatToScalar(5.f), SkFloatToScalar(2.875f),
+                              SkFloatToScalar(5.f), SkFloatToScalar(1.f));
+            problem2->close();
+        }
     }
 
     virtual void onDraw(SkCanvas* canvas) SK_OVERRIDE {
diff --git a/include/core/SkRect.h b/include/core/SkRect.h
index d0eaac40e4..d8919ae5d6 100644
--- a/include/core/SkRect.h
+++ b/include/core/SkRect.h
@@ -691,6 +691,21 @@ struct SK_API SkRect {
         fBottom = SkMaxScalar(y, fBottom);
     }
 
+    /** Bulk version of growToInclude */
+    void growToInclude(const SkPoint pts[], int count) {
+        this->growToInclude(pts, sizeof(SkPoint), count);
+    }
+
+    /** Bulk version of growToInclude with stride. */
+    void growToInclude(const SkPoint pts[], size_t stride, int count) {
+        SkASSERT(count >= 0);
+        SkASSERT(stride >= sizeof(SkPoint));
+        const SkPoint* end = (const SkPoint*)((intptr_t)pts + count * stride);
+        for (; pts < end; pts = (const SkPoint*)((intptr_t)pts + stride)) {
+            this->growToInclude(pts->fX, pts->fY);
+        }
+    }
+
     /**
      *  Returns true if (p.fX,p.fY) is inside the rectangle, and the rectangle
      *  is not empty.
diff --git a/src/gpu/GrAAHairLinePathRenderer.cpp b/src/gpu/GrAAHairLinePathRenderer.cpp
index 60992fd806..5078467fff 100644
--- a/src/gpu/GrAAHairLinePathRenderer.cpp
+++ b/src/gpu/GrAAHairLinePathRenderer.cpp
@@ -589,10 +589,8 @@ void bloat_quad(const SkPoint qpts[3], const SkMatrix* toDevice,
     c1.fPos = c;
     c1.fPos -= cbN;
 
-    // This point may not be within 1 pixel of a control point. We update the bounding box to
-    // include it.
     intersect_lines(a0.fPos, abN, c0.fPos, cbN, &b0.fPos);
-    devBounds->growToInclude(b0.fPos.fX, b0.fPos.fY);
+    devBounds->growToInclude(&verts[0].fPos, sizeof(BezierVertex), kVertsPerQuad);
 
     if (toSrc) {
         toSrc->mapPointsWithStride(&verts[0].fPos, sizeof(BezierVertex), kVertsPerQuad);
@@ -723,8 +721,6 @@ bool GrAAHairLinePathRenderer::createLineGeom(
 
     const SkMatrix& viewM = drawState->getViewMatrix();
 
-    *devBounds = path.getBounds();
-    viewM.mapRect(devBounds);
     devBounds->outset(SK_Scalar1, SK_Scalar1);
 
     int vertCnt = kVertsPerLineSeg * lineCnt;
@@ -746,10 +742,14 @@ bool GrAAHairLinePathRenderer::createLineGeom(
             toSrc = &ivm;
         }
     }
-
+    devBounds->set(lines.begin(), lines.count());
     for (int i = 0; i < lineCnt; ++i) {
         add_line(&lines[2*i], rtHeight, toSrc, drawState->getCoverage(), &verts);
     }
+    // All the verts computed by add_line are within unit distance of the end points. Add a little
+    // extra to account for vector normalization precision.
+    static const SkScalar kOutset = SK_Scalar1 + SK_Scalar1 / 20;
+    devBounds->outset(kOutset, kOutset);
 
     return true;
 }
@@ -769,13 +769,6 @@ bool GrAAHairLinePathRenderer::createBezierGeom(
 
     const SkMatrix& viewM = drawState->getViewMatrix();
 
-    // All the vertices that we compute are within 1 of path control points with the exception of
-    // one of the bounding vertices for each quad. The add_quads() function will update the bounds
-    // for each quad added.
-    *devBounds = path.getBounds();
-    viewM.mapRect(devBounds);
-    devBounds->outset(SK_Scalar1, SK_Scalar1);
-
     int vertCnt = kVertsPerQuad * quadCnt + kVertsPerQuad * conicCnt;
 
     target->drawState()->setVertexAttribs<gHairlineBezierAttribs>(SK_ARRAY_COUNT(gHairlineBezierAttribs));
@@ -798,6 +791,21 @@ bool GrAAHairLinePathRenderer::createBezierGeom(
         }
     }
 
+    // Seed the dev bounds with some pts known to be inside. Each quad and conic grows the bounding
+    // box to include its vertices.
+    SkPoint seedPts[2];
+    if (quadCnt) {
+        seedPts[0] = quads[0];
+        seedPts[1] = quads[2];
+    } else if (conicCnt) {
+        seedPts[0] = conics[0];
+        seedPts[1] = conics[2];
+    }
+    if (NULL != toDevice) {
+        toDevice->mapPoints(seedPts, 2);
+    }
+    devBounds->set(seedPts[0], seedPts[1]);
+
     int unsubdivQuadCnt = quads.count() / 3;
     for (int i = 0; i < unsubdivQuadCnt; ++i) {
         SkASSERT(qSubdivs[i] >= 0);
@@ -830,7 +838,14 @@ template <class VertexType>
 bool check_bounds(GrDrawState* drawState, const SkRect& devBounds, void* vertices, int vCount)
 {
     SkRect tolDevBounds = devBounds;
-    tolDevBounds.outset(SK_Scalar1 / 10000, SK_Scalar1 / 10000);
+    // The bounds ought to be tight, but in perspective the below code runs the verts
+    // through the view matrix to get back to dev coords, which can introduce imprecision.
+    if (drawState->getViewMatrix().hasPerspective()) {
+        tolDevBounds.outset(SK_Scalar1 / 1000, SK_Scalar1 / 1000);
+    } else {
+        // Non-persp matrices cause this path renderer to draw in device space.
+        SkASSERT(drawState->getViewMatrix().isIdentity());
+    }
     SkRect actualBounds;
 
     VertexType* verts = reinterpret_cast<VertexType*>(vertices);
@@ -880,7 +895,7 @@ bool GrAAHairLinePathRenderer::onDrawPath(const SkPath& path,
     conicCnt = conics.count() / 3;
 
     // do lines first
-    {
+    if (lineCnt) {
         GrDrawTarget::AutoReleaseGeometry arg;
         SkRect devBounds;
 
@@ -926,7 +941,7 @@ bool GrAAHairLinePathRenderer::onDrawPath(const SkPath& path,
     }
 
     // then quadratics/conics
-    {
+    if (quadCnt || conicCnt) {
         GrDrawTarget::AutoReleaseGeometry arg;
         SkRect devBounds;