Replace op== with CombineIfPossible in GrDrawState.

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

Author: bsalomon@google.com

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

4 files changed, 72 insertions, 51 deletions

diff --git a/src/gpu/GrDrawState.h b/src/gpu/GrDrawState.h
index b894cb5a29..a4db89d643 100644
--- a/src/gpu/GrDrawState.h
+++ b/src/gpu/GrDrawState.h
@@ -876,43 +876,58 @@ public:
 
     ///////////////////////////////////////////////////////////////////////////
 
-    bool operator ==(const GrDrawState& that) const {
-        if (fRenderTarget.get() != that.fRenderTarget.get() ||
-            fColorStages.count() != that.fColorStages.count() ||
-            fCoverageStages.count() != that.fCoverageStages.count() ||
-            fColor != that.fColor ||
-            !fViewMatrix.cheapEqualTo(that.fViewMatrix) ||
-            fSrcBlend != that.fSrcBlend ||
-            fDstBlend != that.fDstBlend ||
-            fBlendConstant != that.fBlendConstant ||
-            fFlagBits != that.fFlagBits ||
-            fVACount != that.fVACount ||
-            memcmp(fVAPtr, that.fVAPtr, fVACount * sizeof(GrVertexAttrib)) ||
-            fStencilSettings != that.fStencilSettings ||
-            fCoverage != that.fCoverage ||
-            fDrawFace != that.fDrawFace) {
-            return false;
+    /** Return type for CombineIfPossible. */
+    enum CombinedState {
+        /** The GrDrawStates cannot be combined. */
+        kIncompatible_CombinedState,
+        /** Either draw state can be used in place of the other. */
+        kAOrB_CombinedState,
+        /** Use the first draw state. */
+        kA_CombinedState,
+        /** Use the second draw state. */
+        kB_CombinedState,
+    };
+
+    /** This function determines whether the GrDrawStates used for two draws can be combined into
+        a single GrDrawState. This is used to avoid storing redundant GrDrawStates and to determine
+        if draws can be batched. The return value indicates whether combining is possible and, if
+        so, which of the two inputs should be used. */
+    static CombinedState CombineIfPossible(const GrDrawState& a, const GrDrawState& b) {
+        if (a.fRenderTarget.get() != b.fRenderTarget.get() ||
+            a.fColorStages.count() != b.fColorStages.count() ||
+            a.fCoverageStages.count() != b.fCoverageStages.count() ||
+            a.fColor != b.fColor ||
+            !a.fViewMatrix.cheapEqualTo(b.fViewMatrix) ||
+            a.fSrcBlend != b.fSrcBlend ||
+            a.fDstBlend != b.fDstBlend ||
+            a.fBlendConstant != b.fBlendConstant ||
+            a.fFlagBits != b.fFlagBits ||
+            a.fVACount != b.fVACount ||
+            memcmp(a.fVAPtr, b.fVAPtr, a.fVACount * sizeof(GrVertexAttrib)) ||
+            a.fStencilSettings != b.fStencilSettings ||
+            a.fCoverage != b.fCoverage ||
+            a.fDrawFace != b.fDrawFace) {
+            return kIncompatible_CombinedState;
         }
 
-        bool explicitLocalCoords = this->hasLocalCoordAttribute();
-        for (int i = 0; i < fColorStages.count(); i++) {
-            if (!GrEffectStage::AreCompatible(fColorStages[i], that.fColorStages[i],
+        bool explicitLocalCoords = a.hasLocalCoordAttribute();
+        for (int i = 0; i < a.fColorStages.count(); i++) {
+            if (!GrEffectStage::AreCompatible(a.fColorStages[i], b.fColorStages[i],
                 explicitLocalCoords)) {
-                return false;
+                return kIncompatible_CombinedState;
             }
         }
-        for (int i = 0; i < fCoverageStages.count(); i++) {
-            if (!GrEffectStage::AreCompatible(fCoverageStages[i], that.fCoverageStages[i],
+        for (int i = 0; i < a.fCoverageStages.count(); i++) {
+            if (!GrEffectStage::AreCompatible(a.fCoverageStages[i], b.fCoverageStages[i],
                 explicitLocalCoords)) {
-                return false;
+                return kIncompatible_CombinedState;
             }
         }
-        SkASSERT(0 == memcmp(fFixedFunctionVertexAttribIndices,
-                             that.fFixedFunctionVertexAttribIndices,
-                             sizeof(fFixedFunctionVertexAttribIndices)));
-        return true;
+        SkASSERT(0 == memcmp(a.fFixedFunctionVertexAttribIndices,
+                             b.fFixedFunctionVertexAttribIndices,
+                             sizeof(a.fFixedFunctionVertexAttribIndices)));
+        return kAOrB_CombinedState;
     }
-    bool operator !=(const GrDrawState& s) const { return !(*this == s); }
 
     GrDrawState& operator= (const GrDrawState& that) {
         SkASSERT(0 == fBlockEffectRemovalCnt || 0 == this->numTotalStages());
diff --git a/src/gpu/GrInOrderDrawBuffer.cpp b/src/gpu/GrInOrderDrawBuffer.cpp
index 0cab19ff04..6e447590b0 100644
--- a/src/gpu/GrInOrderDrawBuffer.cpp
+++ b/src/gpu/GrInOrderDrawBuffer.cpp
@@ -350,9 +350,7 @@ void GrInOrderDrawBuffer::onDraw(const DrawInfo& info) {
     if (this->needsNewClip()) {
        this->recordClip();
     }
-    if (this->needsNewState()) {
-        this->recordState();
-    }
+    this->recordStateIfNecessary();
 
     DrawRecord* draw;
     if (info.isInstanced()) {
@@ -424,9 +422,7 @@ void GrInOrderDrawBuffer::onStencilPath(const GrPath* path, SkPath::FillType fil
         this->recordClip();
     }
     // Only compare the subset of GrDrawState relevant to path stenciling?
-    if (this->needsNewState()) {
-        this->recordState();
-    }
+    this->recordStateIfNecessary();
     StencilPath* sp = this->recordStencilPath();
     sp->fPath.reset(path);
     path->ref();
@@ -439,9 +435,7 @@ void GrInOrderDrawBuffer::onDrawPath(const GrPath* path,
         this->recordClip();
     }
     // TODO: Only compare the subset of GrDrawState relevant to path covering?
-    if (this->needsNewState()) {
-        this->recordState();
-    }
+    this->recordStateIfNecessary();
     DrawPath* cp = this->recordDrawPath();
     cp->fPath.reset(path);
     path->ref();
@@ -462,9 +456,7 @@ void GrInOrderDrawBuffer::onDrawPaths(const GrPathRange* pathRange,
     if (this->needsNewClip()) {
         this->recordClip();
     }
-    if (this->needsNewState()) {
-        this->recordState();
-    }
+    this->recordStateIfNecessary();
     DrawPaths* dp = this->recordDrawPaths();
     dp->fPathRange.reset(SkRef(pathRange));
     dp->fIndices = SkNEW_ARRAY(uint32_t, count); // TODO: Accomplish this without a malloc
@@ -919,8 +911,26 @@ void GrInOrderDrawBuffer::geometrySourceWillPop(
     }
 }
 
-bool GrInOrderDrawBuffer::needsNewState() const {
-    return fStates.empty() || fStates.back() != this->getDrawState();
+void GrInOrderDrawBuffer::recordStateIfNecessary() {
+    if (fStates.empty()) {
+        fStates.push_back() = this->getDrawState();
+        this->addToCmdBuffer(kSetState_Cmd);
+        return;
+    }
+    const GrDrawState& curr = this->getDrawState();
+    GrDrawState& prev = fStates.back();
+    switch (GrDrawState::CombineIfPossible(prev, curr)) {
+        case GrDrawState::kIncompatible_CombinedState:
+            fStates.push_back() = this->getDrawState();
+            this->addToCmdBuffer(kSetState_Cmd);
+            break;
+        case GrDrawState::kA_CombinedState:
+        case GrDrawState::kAOrB_CombinedState: // Treat the same as kA.
+            break;
+        case GrDrawState::kB_CombinedState:
+            prev = curr;
+            break;
+    }
 }
 
 bool GrInOrderDrawBuffer::needsNewClip() const {
@@ -953,11 +963,6 @@ void GrInOrderDrawBuffer::recordClip() {
     this->addToCmdBuffer(kSetClip_Cmd);
 }
 
-void GrInOrderDrawBuffer::recordState() {
-    fStates.push_back() = this->getDrawState();
-    this->addToCmdBuffer(kSetState_Cmd);
-}
-
 GrInOrderDrawBuffer::DrawRecord* GrInOrderDrawBuffer::recordDraw(const DrawInfo& info) {
     this->addToCmdBuffer(kDraw_Cmd);
     return &fDraws.push_back(info);
diff --git a/src/gpu/GrInOrderDrawBuffer.h b/src/gpu/GrInOrderDrawBuffer.h
index b678518a5d..491da5dbfb 100644
--- a/src/gpu/GrInOrderDrawBuffer.h
+++ b/src/gpu/GrInOrderDrawBuffer.h
@@ -201,9 +201,9 @@ private:
     // instanced draw. The caller must have already recorded a new draw state and clip if necessary.
     int concatInstancedDraw(const DrawInfo& info);
 
-    // we lazily record state and clip changes in order to skip clips and states that have no
-    // effect.
-    bool needsNewState() const;
+    // Determines whether the current draw operation requieres a new drawstate and if so records it.
+    void recordStateIfNecessary();
+    // We lazily record clip changes in order to skip clips that have no effect.
     bool needsNewClip() const;
 
     // these functions record a command
diff --git a/src/gpu/effects/GrBicubicEffect.cpp b/src/gpu/effects/GrBicubicEffect.cpp
index cc621ddec2..fb98b4ac4c 100644
--- a/src/gpu/effects/GrBicubicEffect.cpp
+++ b/src/gpu/effects/GrBicubicEffect.cpp
@@ -164,7 +164,8 @@ const GrBackendEffectFactory& GrBicubicEffect::getFactory() const {
 bool GrBicubicEffect::onIsEqual(const GrEffect& sBase) const {
     const GrBicubicEffect& s = CastEffect<GrBicubicEffect>(sBase);
     return this->textureAccess(0) == s.textureAccess(0) &&
-           !memcmp(fCoefficients, s.coefficients(), 16);
+           !memcmp(fCoefficients, s.coefficients(), 16) &&
+           fDomain == s.fDomain;
 }
 
 void GrBicubicEffect::getConstantColorComponents(GrColor* color, uint32_t* validFlags) const {