Create GrOptDrawState before recording draw in GrInOrderDrawBuffer

src/gpu/GrOptDrawState.cpp

Index: src/gpu/GrOptDrawState.cpp
diff --git a/src/gpu/GrOptDrawState.cpp b/src/gpu/GrOptDrawState.cpp
index 4a258c2e172bca8fa05f25e6ce0b633aa53d784d..c2e92f35b127c39aaa484968894c71504f0bdf39 100644
--- a/src/gpu/GrOptDrawState.cpp
+++ b/src/gpu/GrOptDrawState.cpp
@@ -19,8 +19,8 @@ GrOptDrawState::GrOptDrawState(const GrDrawState& drawState,
                                GrBlendCoeff optDstCoeff,
                                GrGpu* gpu,
                                const GrDeviceCoordTexture* dstCopy,
-                               GrGpu::DrawType drawType) {
-    fRenderTarget.set(SkSafeRef(drawState.getRenderTarget()), kWrite_GrIOType);
+                               GrGpu::DrawType drawType)
+: fRenderTarget(drawState.fRenderTarget.get()) {
     fViewMatrix = drawState.getViewMatrix();
     fBlendConstant = drawState.getBlendConstant();
     fFlagBits = drawState.getFlagBits();
@@ -66,26 +66,23 @@ GrOptDrawState::GrOptDrawState(const GrDrawState& drawState,
     SkASSERT(GrGpu::IsPathRenderingDrawType(drawType) ||
              GrGpu::kStencilPath_DrawType ||
              drawState.hasGeometryProcessor());
-    if (drawState.hasGeometryProcessor()) {
-        fGeometryProcessor.initAndRef(drawState.fGeometryProcessor);
-    } else {
-        fGeometryProcessor.reset(NULL);
-    }
+    fGeometryProcessor.reset(drawState.getGeometryProcessor());
 
-    // Copy Color Stages from DS to ODS
-    if (firstColorStageIdx < drawState.numColorStages()) {
-        fFragmentStages.reset(&drawState.getColorStage(firstColorStageIdx),
-                              drawState.numColorStages() - firstColorStageIdx);
-    } else {
-        fFragmentStages.reset();
-    }
+    // Copy Stages from DS to ODS
+    SkASSERT(0 == fFragmentStages.count());

[joshualitt, 2014/11/19 14:22:39]

Will this ever be not true?

[bsalomon, 2014/11/19 15:04:43]

I'll remove it... it was for my own sanity as I was working on this. I noticed
that the old code called fFragmentStages.reset() when there were no color stages
to copy and was wondering how that could be necessary. I don't think it was.

+    bool explicitLocalCoords =
+        -1 != descInfo.fFixedFunctionVertexAttribIndices[kLocalCoord_GrVertexAttribBinding];

[joshualitt, 2014/11/19 14:22:39]

descInfo.hasLocalCoordAttribute()

[bsalomon, 2014/11/19 15:04:43]

tx!

 
+    for (int i = firstColorStageIdx; i < drawState.numColorStages(); ++i) {
+        SkNEW_APPEND_TO_TARRAY(&fFragmentStages,
+                               GrPendingFragmentStage,
+                               (drawState.fColorStages[i], explicitLocalCoords));
+    }
     fNumColorStages = fFragmentStages.count();
-
-    // Copy Coverage Stages from DS to ODS
-    if (firstCoverageStageIdx < drawState.numCoverageStages()) {
-        fFragmentStages.push_back_n(drawState.numCoverageStages() - firstCoverageStageIdx,
-                                    &drawState.getCoverageStage(firstCoverageStageIdx));
+    for (int i = firstCoverageStageIdx; i < drawState.numCoverageStages(); ++i) {
+        SkNEW_APPEND_TO_TARRAY(&fFragmentStages,
+                               GrPendingFragmentStage,
+                               (drawState.fCoverageStages[i], explicitLocalCoords));
     }
 
     this->setOutputStateInfo(drawState, *gpu->caps(), &descInfo);
@@ -257,10 +254,6 @@ void GrOptDrawState::getStageStats(const GrDrawState& ds, int firstColorStageIdx
 ////////////////////////////////////////////////////////////////////////////////
 
 bool GrOptDrawState::operator== (const GrOptDrawState& that) const {
-    return this->isEqual(that);
-}
-
-bool GrOptDrawState::isEqual(const GrOptDrawState& that) const {
     if (this->fDesc != that.fDesc) {
         return false;
     }
@@ -298,10 +291,10 @@ bool GrOptDrawState::isEqual(const GrOptDrawState& that) const {
         return false;
     }
 
-    bool explicitLocalCoords = this->fDesc.header().fLocalCoordAttributeIndex != -1;
+    SkASSERT(this->numFragmentStages() == that.numFragmentStages());

[joshualitt, 2014/11/19 14:22:39]

does this get tested with the desc equality test?

[bsalomon, 2014/11/19 15:04:43]

Yeah, it has these fields:

        SkBool8                     fHasGeometryProcessor;
        int8_t                      fColorEffectCnt;
        int8_t                      fCoverageEffectCnt;

Will add a comment to that effect.

     for (int i = 0; i < this->numFragmentStages(); i++) {
-        if (!GrFragmentStage::AreCompatible(this->getFragmentStage(i), that.getFragmentStage(i),
-                                            explicitLocalCoords)) {
+
+        if (this->getFragmentStage(i) != that.getFragmentStage(i)) {
             return false;
         }
     }