Increase batching for AA fill rects.

src/gpu/batches/GrAAFillRectBatch.cpp

Index: src/gpu/batches/GrAAFillRectBatch.cpp
diff --git a/src/gpu/batches/GrAAFillRectBatch.cpp b/src/gpu/batches/GrAAFillRectBatch.cpp
index ffbab9654e52d61f127a0f8ffa0020fc2ba9ef58..28d1f672c122acda8af5f7e4e023146940f62f16 100644
--- a/src/gpu/batches/GrAAFillRectBatch.cpp
+++ b/src/gpu/batches/GrAAFillRectBatch.cpp
@@ -13,9 +13,9 @@
 #include "GrResourceKey.h"
 #include "GrResourceProvider.h"
 #include "GrTypes.h"
+#include "GrVertexBatch.h"
 #include "SkMatrix.h"
 #include "SkRect.h"
-#include "GrVertexBatch.h"
 
 GR_DECLARE_STATIC_UNIQUE_KEY(gAAFillRectIndexBufferKey);
 
@@ -45,35 +45,6 @@ const GrBuffer* get_index_buffer(GrResourceProvider* resourceProvider) {
         gAAFillRectIndexBufferKey);
 }
 
-static sk_sp<GrGeometryProcessor> create_fill_rect_gp(
-                                       const SkMatrix& viewMatrix,
-                                       const GrXPOverridesForBatch& overrides,
-                                       GrDefaultGeoProcFactory::LocalCoords::Type localCoordsType) {
-    using namespace GrDefaultGeoProcFactory;
-
-    Color color(Color::kAttribute_Type);
-    Coverage::Type coverageType;
-    // TODO remove coverage if coverage is ignored
-    /*if (coverageIgnored) {
-        coverageType = Coverage::kNone_Type;
-    } else*/ if (overrides.canTweakAlphaForCoverage()) {
-        coverageType = Coverage::kSolid_Type;
-    } else {
-        coverageType = Coverage::kAttribute_Type;
-    }
-    Coverage coverage(coverageType);
-
-    // We assume the caller has inverted the viewmatrix
-    if (LocalCoords::kHasExplicit_Type == localCoordsType) {
-        LocalCoords localCoords(localCoordsType);
-        return GrDefaultGeoProcFactory::Make(color, coverage, localCoords, SkMatrix::I());
-    } else {
-        LocalCoords localCoords(overrides.readsLocalCoords() ? localCoordsType :
-                                                               LocalCoords::kUnused_Type);
-        return MakeForDeviceSpace(color, coverage, localCoords, viewMatrix);
-    }
-}
-
 static void generate_aa_fill_rect_geometry(intptr_t verts,
                                            size_t vertexStride,
                                            GrColor color,
@@ -137,7 +108,7 @@ static void generate_aa_fill_rect_geometry(intptr_t verts,
     if (localMatrix) {
         SkMatrix invViewMatrix;
         if (!viewMatrix.invert(&invViewMatrix)) {
-            SkASSERT(false);
+            SkDebugf("View matrix is non-invertible, local coords will be wrong.");
             invViewMatrix = SkMatrix::I();
         }
         SkMatrix localCoordMatrix;
@@ -186,147 +157,37 @@ static void generate_aa_fill_rect_geometry(intptr_t verts,
         }
     }
 }
-
-class AAFillRectNoLocalMatrixBatch : public GrVertexBatch {
-public:
-    DEFINE_BATCH_CLASS_ID
-    AAFillRectNoLocalMatrixBatch(GrColor color,
-                                 const SkMatrix& viewMatrix,
-                                 const SkRect& rect,
-                                 const SkRect& devRect) : INHERITED(ClassID()) {
-        fRects.emplace_back(RectInfo{color, viewMatrix, rect, devRect});
-        fBounds = devRect;
-    }
-
-    const char* name() const override { return "AAFillRectBatchNoLocalMatrix"; }
-
-    SkString dumpInfo() const override {
-        SkString str;
-        str.appendf("# batched: %d\n", fRects.count());
-        for (int i = 0; i < fRects.count(); ++i) {
-            const RectInfo& info = fRects[i];
-            str.appendf("%d: Color: 0x%08x, Rect [L: %.2f, T: %.2f, R: %.2f, B: %.2f]\n",
-                        i, info.fColor,
-                        info.fRect.fLeft, info.fRect.fTop, info.fRect.fRight, info.fRect.fBottom);
-        }
-        str.append(INHERITED::dumpInfo());
-        return str;
-    }
-
-    void computePipelineOptimizations(GrInitInvariantOutput* color,
-                                      GrInitInvariantOutput* coverage,
-                                      GrBatchToXPOverrides* overrides) const override {
-        // When this is called on a batch, there is only one rect
-        color->setKnownFourComponents(fRects[0].fColor);
-        coverage->setUnknownSingleComponent();
-    }
-
-    void initBatchTracker(const GrXPOverridesForBatch& overrides) override {
-        overrides.getOverrideColorIfSet(&fRects[0].fColor);
-        fOverrides = overrides;
-    }
-
-private:
-    AAFillRectNoLocalMatrixBatch() : INHERITED(ClassID()) {}
-
-    void onPrepareDraws(Target* target) const override {
-        sk_sp<GrGeometryProcessor> gp =
-                create_fill_rect_gp(fRects[0].fViewMatrix, fOverrides,
-                                    GrDefaultGeoProcFactory::LocalCoords::kUsePosition_Type);
-        if (!gp) {
-            SkDebugf("Couldn't create GrGeometryProcessor\n");
-            return;
-        }
-        SkASSERT(fOverrides.canTweakAlphaForCoverage() ?
-                     gp->getVertexStride() == sizeof(GrDefaultGeoProcFactory::PositionColorAttr) :
-                     gp->getVertexStride() ==
-                         sizeof(GrDefaultGeoProcFactory::PositionColorCoverageAttr));
-
-        size_t vertexStride = gp->getVertexStride();
-
-        SkAutoTUnref<const GrBuffer> indexBuffer(get_index_buffer(target->resourceProvider()));
-        InstancedHelper helper;
-        void* vertices = helper.init(target, kTriangles_GrPrimitiveType, vertexStride,
-                                     indexBuffer, kVertsPerAAFillRect,
-                                     kIndicesPerAAFillRect, fRects.count());
-        if (!vertices || !indexBuffer) {
-            SkDebugf("Could not allocate vertices\n");
-            return;
-        }
-
-        for (int i = 0; i < fRects.count(); i++) {
-            intptr_t verts = reinterpret_cast<intptr_t>(vertices) +
-                             i * kVertsPerAAFillRect * vertexStride;
-            generate_aa_fill_rect_geometry(verts, vertexStride,
-                                           fRects[i].fColor, fRects[i].fViewMatrix,
-                                           fRects[i].fRect, fRects[i].fDevRect, fOverrides,
-                                           nullptr);
-        }
-        helper.recordDraw(target, gp.get());
-    }
-
-    bool onCombineIfPossible(GrBatch* t, const GrCaps& caps) override {
-        AAFillRectNoLocalMatrixBatch* that = t->cast<AAFillRectNoLocalMatrixBatch>();
-        if (!GrPipeline::CanCombine(*this->pipeline(), this->bounds(), *that->pipeline(),
-                                    that->bounds(), caps)) {
-            return false;
-        }
-
-        // 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
-        if (fOverrides.readsLocalCoords() &&
-            !fRects[0].fViewMatrix.cheapEqualTo(that->fRects[0].fViewMatrix)) {
-            return false;
-        }
-
-        // In the event of two batches, one who can tweak, one who cannot, we just fall back to
-        // not tweaking
-        if (fOverrides.canTweakAlphaForCoverage() && !that->fOverrides.canTweakAlphaForCoverage()) {
-            fOverrides = that->fOverrides;
-        }
-
-        fRects.push_back_n(that->fRects.count(), that->fRects.begin());
-        this->joinBounds(that->bounds());
-        return true;
-    }
-
-    struct RectInfo {
-        GrColor fColor;
-        SkMatrix fViewMatrix;
-        SkRect fRect;
-        SkRect fDevRect;
-    };
-
-    GrXPOverridesForBatch fOverrides;
-    SkSTArray<1, RectInfo, true> fRects;
-
-    typedef GrVertexBatch INHERITED;
-};
-
 class AAFillRectLocalMatrixBatch : public GrVertexBatch {
 public:
     DEFINE_BATCH_CLASS_ID
 
     AAFillRectLocalMatrixBatch(GrColor color,
                                const SkMatrix& viewMatrix,
-                               const SkMatrix& localMatrix,
                                const SkRect& rect,
-                               const SkRect& devRect) : INHERITED(ClassID()) {
-        fRects.emplace_back(RectInfo{color, viewMatrix, localMatrix, rect, devRect});
+                               const SkRect& devRect,
+                               const SkMatrix* localMatrix) : INHERITED(ClassID()) {
+        if (localMatrix) {
+            void* mem = fRectData.append(sizeof(RectWithLocalMatrixInfo));
+            new (mem) RectWithLocalMatrixInfo(color, viewMatrix, rect, devRect, *localMatrix);
+        } else {
+            void* mem = fRectData.append(sizeof(RectInfo));
+            new (mem) RectInfo(color, viewMatrix, rect, devRect);
+        }
         fBounds = devRect;
+        fRectCnt = 1;
     }
 
     const char* name() const override { return "AAFillRectBatchLocalMatrix"; }
 
     SkString dumpInfo() const override {
         SkString str;
-        str.appendf("# batched: %d\n", fRects.count());
-        for (int i = 0; i < fRects.count(); ++i) {
-            const RectInfo& info = fRects[i];
+        str.appendf("# batched: %d\n", fRectCnt);
+        const RectInfo* info = this->first();
+        for (int i = 0; i < fRectCnt; ++i) {
+            const SkRect& rect = info->rect();
             str.appendf("%d: Color: 0x%08x, Rect [L: %.2f, T: %.2f, R: %.2f, B: %.2f]\n",
-                        i, info.fColor,
-                        info.fRect.fLeft, info.fRect.fTop, info.fRect.fRight, info.fRect.fBottom);
+                        i, info->color(), rect.fLeft, rect.fTop, rect.fRight, rect.fBottom);
+            info = this->next(info);
         }
         str.append(INHERITED::dumpInfo());
         return str;
@@ -336,12 +197,15 @@ public:
                                       GrInitInvariantOutput* coverage,
                                       GrBatchToXPOverrides* overrides) const override {
         // When this is called on a batch, there is only one rect
-        color->setKnownFourComponents(fRects[0].fColor);
+        color->setKnownFourComponents(this->first()->color());
         coverage->setUnknownSingleComponent();
     }
 
     void initBatchTracker(const GrXPOverridesForBatch& overrides) override {
-        overrides.getOverrideColorIfSet(&fRects[0].fColor);
+        GrColor color;
+        if (overrides.getOverrideColorIfSet(&color)) {
+            this->first()->setColor(color);
+        }
         fOverrides = overrides;
     }
 
@@ -349,18 +213,25 @@ private:
     AAFillRectLocalMatrixBatch() : INHERITED(ClassID()) {}
 
     void onPrepareDraws(Target* target) const override {
-        sk_sp<GrGeometryProcessor> gp =
-                create_fill_rect_gp(fRects[0].fViewMatrix, fOverrides,
-                                    GrDefaultGeoProcFactory::LocalCoords::kHasExplicit_Type);
+        bool needLocalCoords = fOverrides.readsLocalCoords();
+        using namespace GrDefaultGeoProcFactory;
+
+        Color color(Color::kAttribute_Type);
+        Coverage::Type coverageType;
+        if (fOverrides.canTweakAlphaForCoverage()) {
+            coverageType = Coverage::kSolid_Type;
+        } else {
+            coverageType = Coverage::kAttribute_Type;
+        }
+        Coverage coverage(coverageType);
+        LocalCoords lc = needLocalCoords ? LocalCoords::kHasExplicit_Type
+                                         : LocalCoords::kUnused_Type;
+        sk_sp<GrGeometryProcessor> gp = GrDefaultGeoProcFactory::Make(color, coverage, lc,
+                                                                      SkMatrix::I());
         if (!gp) {
             SkDebugf("Couldn't create GrGeometryProcessor\n");
             return;
         }
-        SkASSERT(fOverrides.canTweakAlphaForCoverage() ?
-                 gp->getVertexStride() ==
-                 sizeof(GrDefaultGeoProcFactory::PositionColorLocalCoordAttr) :
-                 gp->getVertexStride() ==
-                 sizeof(GrDefaultGeoProcFactory::PositionColorLocalCoordCoverage));
 
         size_t vertexStride = gp->getVertexStride();
 
@@ -368,19 +239,28 @@ private:
         InstancedHelper helper;
         void* vertices = helper.init(target, kTriangles_GrPrimitiveType, vertexStride,
                                      indexBuffer, kVertsPerAAFillRect,
-                                     kIndicesPerAAFillRect, fRects.count());
+                                     kIndicesPerAAFillRect, fRectCnt);
         if (!vertices || !indexBuffer) {
             SkDebugf("Could not allocate vertices\n");
             return;
         }
 
-        for (int i = 0; i < fRects.count(); i++) {
+        const RectInfo* info = this->first();
+        const SkMatrix* localMatrix = nullptr;
+        for (int i = 0; i < fRectCnt; i++) {
             intptr_t verts = reinterpret_cast<intptr_t>(vertices) +
                              i * kVertsPerAAFillRect * vertexStride;
-            generate_aa_fill_rect_geometry(verts, vertexStride, fRects[i].fColor,
-                                           fRects[i].fViewMatrix, fRects[i].fRect,
-                                           fRects[i].fDevRect, fOverrides,
-                                           &fRects[i].fLocalMatrix);
+            if (needLocalCoords) {
+                if (HasLocalMatrix::kYes == info->hasLocalMatrix()) {
+                    localMatrix = &static_cast<const RectWithLocalMatrixInfo*>(info)->localMatrix();
+                } else {
+                    localMatrix = &SkMatrix::I();
+                }
+            }
+            generate_aa_fill_rect_geometry(verts, vertexStride, info->color(),
+                                           info->viewMatrix(), info->rect(),
+                                           info->devRect(), fOverrides, localMatrix);
+            info = this->next(info);
         }
         helper.recordDraw(target, gp.get());
     }
@@ -398,21 +278,66 @@ private:
             fOverrides = that->fOverrides;
         }
 
-        fRects.push_back_n(that->fRects.count(), that->fRects.begin());
+        fRectData.append(that->fRectData.count(), that->fRectData.begin());
+        fRectCnt += that->fRectCnt;
         this->joinBounds(that->bounds());
         return true;
     }
 
+    enum class HasLocalMatrix : uint32_t { kNo, kYes };
+
     struct RectInfo {
+    public:
+        RectInfo(GrColor color, const SkMatrix& viewMatrix, const SkRect& rect,
+                 const SkRect& devRect)
+            : RectInfo(color, viewMatrix, rect, devRect, HasLocalMatrix::kNo) {}

[robertphillips, 2016/07/01 16:04:59]

Can this just return bool and hide the 'HasLocalMatrix' enum ?

[bsalomon, 2016/07/01 16:15:14]

Done.

+        HasLocalMatrix hasLocalMatrix() const { return fHasLocalMatrix; }
+        GrColor color() const { return fColor; }
+        const SkMatrix& viewMatrix() const { return fViewMatrix; }
+        const SkRect& rect() const { return fRect; }
+        const SkRect& devRect() const { return fDevRect; }
+
+        void setColor(GrColor color) { fColor = color; }
+    protected:
+        RectInfo(GrColor color, const SkMatrix& viewMatrix, const SkRect& rect,
+                 const SkRect& devRect, HasLocalMatrix hasLM)
+                : fHasLocalMatrix(hasLM)
+                , fColor(color)
+                , fViewMatrix(viewMatrix)
+                , fRect(rect)
+                , fDevRect(devRect) {}
+

[robertphillips, 2016/07/01 16:04:59]

rm ' ' ?

[bsalomon, 2016/07/01 16:15:14]

Done.

+        HasLocalMatrix fHasLocalMatrix ;
         GrColor fColor;
         SkMatrix fViewMatrix;
-        SkMatrix fLocalMatrix;
         SkRect fRect;
         SkRect fDevRect;
     };
 
+    struct RectWithLocalMatrixInfo : public RectInfo {
+    public:
+        RectWithLocalMatrixInfo(GrColor color, const SkMatrix& viewMatrix, const SkRect& rect,

[robertphillips, 2016/07/01 16:04:59]

add a ' ' ?

[bsalomon, 2016/07/01 16:15:14]

Done.

+                               const SkRect& devRect, const SkMatrix& localMatrix)
+            : RectInfo(color, viewMatrix, rect, devRect, HasLocalMatrix::kYes)
+            , fLocalMatrix(localMatrix) {}
+        const SkMatrix& localMatrix() const { return fLocalMatrix; }
+    private:
+        SkMatrix fLocalMatrix;
+    };
+
+    RectInfo* first() { return reinterpret_cast<RectInfo*>(fRectData.begin()); }
+    const RectInfo* first() const { return reinterpret_cast<const RectInfo*>(fRectData.begin()); }
+    const RectInfo* next(const RectInfo* prev) const {
+        intptr_t next = reinterpret_cast<intptr_t>(prev) +
+                (HasLocalMatrix::kYes == prev->hasLocalMatrix()
+                    ? sizeof(RectWithLocalMatrixInfo)
+                    : sizeof(RectInfo));
+        return reinterpret_cast<const RectInfo*>(next);
+    }
+
     GrXPOverridesForBatch fOverrides;
-    SkSTArray<1, RectInfo, true> fRects;
+    SkTDArray<uint8_t> fRectData;
+    int fRectCnt;
 
     typedef GrVertexBatch INHERITED;
 };
@@ -423,7 +348,7 @@ GrDrawBatch* Create(GrColor color,
                     const SkMatrix& viewMatrix,
                     const SkRect& rect,
                     const SkRect& devRect) {
-    return new AAFillRectNoLocalMatrixBatch(color, viewMatrix, rect, devRect);
+    return new AAFillRectLocalMatrixBatch(color, viewMatrix, rect, devRect, nullptr);
 }
 
 GrDrawBatch* Create(GrColor color,
@@ -431,7 +356,7 @@ GrDrawBatch* Create(GrColor color,
                     const SkMatrix& localMatrix,
                     const SkRect& rect,
                     const SkRect& devRect) {
-    return new AAFillRectLocalMatrixBatch(color, viewMatrix, localMatrix, rect, devRect);
+    return new AAFillRectLocalMatrixBatch(color, viewMatrix, rect, devRect, &localMatrix);
 }
 
 GrDrawBatch* Create(GrColor color,