Revert of Modify GrBWFillRectBatch to use GrQuad (patchset #3 id:40001 of https://codereview.chromium.org/1311793002/ )

Reason for revert:
on some bots this patch creates bad diffs

Original issue's description:
> Modify GrBWFillRectBatch to use GrQuad
>
> BUG=skia:
>
> Committed: https://skia.googlesource.com/skia/+/c611b5afbef6df6c02e949fdf092aab2bb7e0e2e

TBR=bsalomon@google.com,robertphillips@google.com,joshualitt@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:

Review URL: https://codereview.chromium.org/1313683002

src/gpu/GrQuad.h

@@ -16,8 +16,6 @@
  */
 class GrQuad {
 public:
-    GrQuad() {}
-
     GrQuad(const GrQuad& that) {
         *this = that;
     }
@@ -52,11 +50,6 @@ public:
         return fPoints;
     }
 
-    const SkPoint& point(int i) const {
-        SkASSERT(i < kNumPoints);
-        return fPoints[i];
-    }
-
 private:
     static const int kNumPoints = 4;
     SkPoint fPoints[kNumPoints];

src/gpu/batches/GrBWFillRectBatch.cpp

@@ -52,16 +52,9 @@ static const GrGeometryProcessor* create_gp(const SkMatrix& viewMatrix,
     Color color(Color::kAttribute_Type);
     Coverage coverage(readsCoverage ? Coverage::kSolid_Type : Coverage::kNone_Type);
 
-    // If we have perspective on the viewMatrix then we won't map on the CPU, nor will we map
+    // if we have a local rect, then we apply the localMatrix directly to the localRect to
-    // the local rect on the cpu (in case the localMatrix also has perspective).
+    // generate vertex local coords
-    // Otherwise, if we have a local rect, then we apply the localMatrix directly to the localRect
+    if (hasExplicitLocalCoords) {
-    // to generate vertex local coords
-    if (viewMatrix.hasPerspective()) {
-        LocalCoords localCoords(hasExplicitLocalCoords ? LocalCoords::kHasExplicit_Type :
-                                                         LocalCoords::kUsePosition_Type,
-                                localMatrix);
-        return GrDefaultGeoProcFactory::Create(color, coverage, localCoords, viewMatrix);
-    } else if (hasExplicitLocalCoords) {
         LocalCoords localCoords(LocalCoords::kHasExplicit_Type);
         return GrDefaultGeoProcFactory::Create(color, coverage, localCoords, SkMatrix::I());
     } else {
@@ -76,25 +69,24 @@ static void tesselate(intptr_t vertices,
                       GrColor color,
                       const SkMatrix& viewMatrix,
                       const SkRect& rect,
-                      const GrQuad* localQuad) {
+                      const SkRect* localRect,
+                      const SkMatrix* localMatrix) {
     SkPoint* positions = reinterpret_cast<SkPoint*>(vertices);
 
     positions->setRectFan(rect.fLeft, rect.fTop,
                           rect.fRight, rect.fBottom, vertexStride);
+    viewMatrix.mapPointsWithStride(positions, vertexStride, BWFillRectBatchBase::kVertsPerInstance);
 
-    if (!viewMatrix.hasPerspective()) {
-        viewMatrix.mapPointsWithStride(positions, vertexStride,
-                                       BWFillRectBatchBase::kVertsPerInstance);
-    }
-
-    // Setup local coords
     // TODO we should only do this if local coords are being read
-    if (localQuad) {
+    if (localRect) {
         static const int kLocalOffset = sizeof(SkPoint) + sizeof(GrColor);
-        for (int i = 0; i < BWFillRectBatchBase::kVertsPerInstance; i++) {
+        SkPoint* coords = reinterpret_cast<SkPoint*>(vertices + kLocalOffset);
-            SkPoint* coords = reinterpret_cast<SkPoint*>(vertices + kLocalOffset +
+        coords->setRectFan(localRect->fLeft, localRect->fTop,
-                              i * vertexStride);
+                           localRect->fRight, localRect->fBottom,
-            *coords = localQuad->point(i);
+                           vertexStride);
+        if (localMatrix) {
+            localMatrix->mapPointsWithStride(coords, vertexStride,
+                                             BWFillRectBatchBase::kVertsPerInstance);
         }
     }
 
@@ -111,12 +103,80 @@ public:
     struct Geometry {
         SkMatrix fViewMatrix;
         SkRect fRect;
-        GrQuad fLocalQuad;
         GrColor fColor;
     };
 
     static const char* Name() { return "BWFillRectBatchNoLocalMatrix"; }
 
+    static bool CanCombine(const Geometry& mine, const Geometry& theirs,
+                           const GrPipelineOptimizations& opts) {
+        // We apply the viewmatrix to the rect points on the cpu.  However, if the pipeline uses
+        // local coords then we won't be able to batch.  We could actually upload the viewmatrix
+        // using vertex attributes in these cases, but haven't investigated that
+        return !opts.readsLocalCoords() || mine.fViewMatrix.cheapEqualTo(theirs.fViewMatrix);
+    }
+
+    static const GrGeometryProcessor* CreateGP(const Geometry& geo,
+                                               const GrPipelineOptimizations& opts) {
+        const GrGeometryProcessor* gp = create_gp(geo.fViewMatrix, opts.readsCoverage(), false,
+                                                  NULL);
+
+        SkASSERT(gp->getVertexStride() == sizeof(GrDefaultGeoProcFactory::PositionColorAttr));
+        return gp;
+    }
+
+    static void Tesselate(intptr_t vertices, size_t vertexStride, const Geometry& geo,
+                          const GrPipelineOptimizations& opts) {
+        tesselate(vertices, vertexStride, geo.fColor, geo.fViewMatrix, geo.fRect, NULL, NULL);
+    }
+};
+
+class BWFillRectBatchLocalMatrixImp : public BWFillRectBatchBase {
+public:
+    struct Geometry {
+        SkMatrix fViewMatrix;
+        SkMatrix fLocalMatrix;
+        SkRect fRect;
+        GrColor fColor;
+    };
+
+    static const char* Name() { return "BWFillRectBatchLocalMatrix"; }
+
+    static bool CanCombine(const Geometry& mine, const Geometry& theirs,
+                           const GrPipelineOptimizations& opts) {
+        // if we read local coords then we have to have the same viewmatrix and localmatrix
+        return !opts.readsLocalCoords() ||
+               (mine.fViewMatrix.cheapEqualTo(theirs.fViewMatrix) &&
+                mine.fLocalMatrix.cheapEqualTo(theirs.fLocalMatrix));
+    }
+
+    static const GrGeometryProcessor* CreateGP(const Geometry& geo,
+                                               const GrPipelineOptimizations& opts) {
+        const GrGeometryProcessor* gp = create_gp(geo.fViewMatrix, opts.readsCoverage(), false,
+                                                  &geo.fLocalMatrix);
+
+        SkASSERT(gp->getVertexStride() == sizeof(GrDefaultGeoProcFactory::PositionColorAttr));
+        return gp;
+    }
+
+    static void Tesselate(intptr_t vertices, size_t vertexStride, const Geometry& geo,
+                          const GrPipelineOptimizations& opts) {
+        tesselate(vertices, vertexStride, geo.fColor, geo.fViewMatrix, geo.fRect, NULL,
+                  &geo.fLocalMatrix);
+    }
+};
+
+class BWFillRectBatchLocalRectImp : public BWFillRectBatchBase {
+public:
+    struct Geometry {
+        SkMatrix fViewMatrix;
+        SkRect fRect;
+        SkRect fLocalRect;
+        GrColor fColor;
+    };
+
+    static const char* Name() { return "BWFillRectBatchLocalRect"; }
+
     static bool CanCombine(const Geometry& mine, const Geometry& theirs,
                            const GrPipelineOptimizations& opts) {
         return true;
@@ -125,7 +185,7 @@ public:
     static const GrGeometryProcessor* CreateGP(const Geometry& geo,
                                                const GrPipelineOptimizations& opts) {
         const GrGeometryProcessor* gp = create_gp(geo.fViewMatrix, opts.readsCoverage(), true,
-                                                  nullptr);
+                                                  NULL);
 
         SkASSERT(gp->getVertexStride() ==
                 sizeof(GrDefaultGeoProcFactory::PositionColorLocalCoordAttr));
@@ -134,12 +194,12 @@ public:
 
     static void Tesselate(intptr_t vertices, size_t vertexStride, const Geometry& geo,
                           const GrPipelineOptimizations& opts) {
-        tesselate(vertices, vertexStride, geo.fColor, geo.fViewMatrix, geo.fRect, &geo.fLocalQuad);
+        tesselate(vertices, vertexStride, geo.fColor, geo.fViewMatrix, geo.fRect, &geo.fLocalRect,
+                  NULL);
     }
 };
 
-// We handle perspective in the local matrix or viewmatrix with special batches
+class BWFillRectBatchLocalMatrixLocalRectImp : public BWFillRectBatchBase {
-class BWFillRectBatchPerspectiveImp : public BWFillRectBatchBase {
 public:
     struct Geometry {
         SkMatrix fViewMatrix;
@@ -147,45 +207,36 @@ public:
         SkRect fRect;
         SkRect fLocalRect;
         GrColor fColor;
-        bool fHasLocalMatrix;
-        bool fHasLocalRect;
     };
 
-    static const char* Name() { return "BWFillRectBatchPerspective"; }
+    static const char* Name() { return "BWFillRectBatchLocalMatrixLocalRect"; }
 
     static bool CanCombine(const Geometry& mine, const Geometry& theirs,
                            const GrPipelineOptimizations& opts) {
-        // We could batch across perspective vm changes if we really wanted to
+        return true;
-        return mine.fViewMatrix.cheapEqualTo(theirs.fViewMatrix) &&
-               (!mine.fHasLocalMatrix || mine.fLocalMatrix.cheapEqualTo(theirs.fLocalMatrix));
     }
 
     static const GrGeometryProcessor* CreateGP(const Geometry& geo,
                                                const GrPipelineOptimizations& opts) {
-        const GrGeometryProcessor* gp = create_gp(geo.fViewMatrix, opts.readsCoverage(),
+        const GrGeometryProcessor* gp = create_gp(geo.fViewMatrix, opts.readsCoverage(), true,
-                                                  geo.fHasLocalRect,
+                                                  NULL);
-                                                  geo.fHasLocalMatrix ? &geo.fLocalMatrix :
-                                                                        nullptr);
 
-        SkASSERT(geo.fHasLocalRect ?
+        SkASSERT(gp->getVertexStride() ==
-             gp->getVertexStride() == sizeof(GrDefaultGeoProcFactory::PositionColorLocalCoordAttr) :
+                sizeof(GrDefaultGeoProcFactory::PositionColorLocalCoordAttr));
-             gp->getVertexStride() == sizeof(GrDefaultGeoProcFactory::PositionColorAttr));
         return gp;
     }
 
     static void Tesselate(intptr_t vertices, size_t vertexStride, const Geometry& geo,
                           const GrPipelineOptimizations& opts) {
-        if (geo.fHasLocalRect) {
+        tesselate(vertices, vertexStride, geo.fColor, geo.fViewMatrix, geo.fRect, &geo.fLocalRect,
-            GrQuad quad(geo.fLocalRect);
+                  &geo.fLocalMatrix);
-            tesselate(vertices, vertexStride, geo.fColor, geo.fViewMatrix, geo.fRect, &quad);
-        } else {
-            tesselate(vertices, vertexStride, geo.fColor, geo.fViewMatrix, geo.fRect, nullptr);
-        }
     }
 };
 
 typedef GrTInstanceBatch<BWFillRectBatchNoLocalMatrixImp> BWFillRectBatchSimple;
-typedef GrTInstanceBatch<BWFillRectBatchPerspectiveImp> BWFillRectBatchPerspective;
+typedef GrTInstanceBatch<BWFillRectBatchLocalMatrixImp> BWFillRectBatchLocalMatrix;
+typedef GrTInstanceBatch<BWFillRectBatchLocalRectImp> BWFillRectBatchLocalRect;
+typedef GrTInstanceBatch<BWFillRectBatchLocalMatrixLocalRectImp> BWFillRectBatchLocalMatrixLocalRect;
 
 namespace GrBWFillRectBatch {
 GrDrawBatch* Create(GrColor color,
@@ -193,45 +244,41 @@ GrDrawBatch* Create(GrColor color,
                     const SkRect& rect,
                     const SkRect* localRect,
                     const SkMatrix* localMatrix) {
-
+    // TODO bubble these up as separate calls
-    /* Perspective has to be handled in a slow path for now */
+    if (localRect && localMatrix) {
-    if (viewMatrix.hasPerspective() || (localMatrix && localMatrix->hasPerspective())) {
+        BWFillRectBatchLocalMatrixLocalRect* batch = BWFillRectBatchLocalMatrixLocalRect::Create();
-        BWFillRectBatchPerspective* batch = BWFillRectBatchPerspective::Create();
+        BWFillRectBatchLocalMatrixLocalRect::Geometry& geo = *batch->geometry();
-        BWFillRectBatchPerspective::Geometry& geo = *batch->geometry();
+        geo.fColor = color;
-
+        geo.fViewMatrix = viewMatrix;
+        geo.fLocalMatrix = *localMatrix;
+        geo.fRect = rect;
+        geo.fLocalRect = *localRect;
+        batch->init();
+        return batch;
+    } else if (localRect) {
+        BWFillRectBatchLocalRect* batch = BWFillRectBatchLocalRect::Create();
+        BWFillRectBatchLocalRect::Geometry& geo = *batch->geometry();
         geo.fColor = color;
         geo.fViewMatrix = viewMatrix;
         geo.fRect = rect;
-        geo.fHasLocalRect = SkToBool(localRect);
-        geo.fHasLocalMatrix = SkToBool(localMatrix);
-        if (localMatrix) {
-            geo.fLocalMatrix = *localMatrix;
-        }
-        if (localRect) {
         geo.fLocalRect = *localRect;
-        }
+        batch->init();
-
+        return batch;
+    } else if (localMatrix) {
+        BWFillRectBatchLocalMatrix* batch = BWFillRectBatchLocalMatrix::Create();
+        BWFillRectBatchLocalMatrix::Geometry& geo = *batch->geometry();
+        geo.fColor = color;
+        geo.fViewMatrix = viewMatrix;
+        geo.fLocalMatrix = *localMatrix;
+        geo.fRect = rect;
         batch->init();
         return batch;
     } else {
-        // TODO bubble these up as separate calls
         BWFillRectBatchSimple* batch = BWFillRectBatchSimple::Create();
         BWFillRectBatchSimple::Geometry& geo = *batch->geometry();
-
         geo.fColor = color;
         geo.fViewMatrix = viewMatrix;
         geo.fRect = rect;
-
-        if (localRect && localMatrix) {
-            geo.fLocalQuad.setFromMappedRect(*localRect, *localMatrix);
-        } else if (localRect) {
-            geo.fLocalQuad.set(*localRect);
-        } else if (localMatrix) {
-            geo.fLocalQuad.setFromMappedRect(rect, *localMatrix);
-        } else {
-            geo.fLocalQuad.set(rect);
-        }
-
         batch->init();
         return batch;
     }