Increase GrInOrderDrawBuffer's encapsulation of trace markers

src/gpu/GrInOrderDrawBuffer.cpp

 /*
  * Copyright 2011 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "GrInOrderDrawBuffer.h"
 
 #include "GrBufferAllocPool.h"
@@ skipping 17 matching lines @@
     , fDrawBatch(NULL) {
 
     SkASSERT(vertexPool);
     SkASSERT(indexPool);
 }
 
 GrInOrderDrawBuffer::~GrInOrderDrawBuffer() {
     this->reset();
 }
 
+void GrInOrderDrawBuffer::closeBatch() {
+    if (fDrawBatch) {
+        fBatchTarget.resetNumberOfDraws();
+        fDrawBatch->execute(this->getGpu(), fPrevState);
+        fDrawBatch->fBatch->setNumberOfDraws(fBatchTarget.numberOfDraws());
+        fDrawBatch = NULL;
+    }
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 
 /** We always use per-vertex colors so that rects can be batched across color changes. Sometimes we
     have explicit local coords and sometimes not. We *could* always provide explicit local coords
     and just duplicate the positions when the caller hasn't provided a local coord rect, but we
     haven't seen a use case which frequently switches between local rect and no local rect draws.
 
     The color param is used to determine whether the opaque hint can be set on the draw state.
     The caller must populate the vertex colors itself.
 
@@ skipping 299 matching lines @@
     }
 
     // how many instances can be concat'ed onto draw given the size of the index buffer
     int instancesToConcat = this->indexCountInCurrentSource() / info.indicesPerInstance();
     instancesToConcat -= draw->fInfo.instanceCount();
     instancesToConcat = SkTMin(instancesToConcat, info.instanceCount());
 
     draw->fInfo.adjustInstanceCount(instancesToConcat);
 
     // update last fGpuCmdMarkers to include any additional trace markers that have been added
-    if (this->getActiveTraceMarkers().count() > 0) {
-        if (draw->isTraced()) {
-            fGpuCmdMarkers.back().addSet(this->getActiveTraceMarkers());
-        } else {
-            fGpuCmdMarkers.push_back(this->getActiveTraceMarkers());
-            draw->makeTraced();
-        }
-    }
-
+    this->recordTraceMarkersIfNecessary(draw);
     return instancesToConcat;
 }
 
 void GrInOrderDrawBuffer::onDraw(const GrGeometryProcessor* gp,
                                  const DrawInfo& info,
                                  const PipelineInfo& pipelineInfo) {
     SkASSERT(info.vertexBuffer() && (!info.isIndexed() || info.indexBuffer()));
     this->closeBatch();
 
     if (!this->setupPipelineAndShouldDraw(gp, pipelineInfo)) {
         return;
     }
 
     Draw* draw;
     if (info.isInstanced()) {
         int instancesConcated = this->concatInstancedDraw(info);
         if (info.instanceCount() > instancesConcated) {
             draw = GrNEW_APPEND_TO_RECORDER(fCmdBuffer, Draw, (info));
             draw->fInfo.adjustInstanceCount(-instancesConcated);
         } else {
             return;
         }
     } else {
         draw = GrNEW_APPEND_TO_RECORDER(fCmdBuffer, Draw, (info));
     }
-    this->recordTraceMarkersIfNecessary();
+    this->recordTraceMarkersIfNecessary(draw);
 }
 
 void GrInOrderDrawBuffer::onDrawBatch(GrBatch* batch,
                                       const PipelineInfo& pipelineInfo) {
     if (!this->setupPipelineAndShouldDraw(batch, pipelineInfo)) {
         return;
     }
 
     // Check if there is a Batch Draw we can batch with
     if (Cmd::kDrawBatch_Cmd != fCmdBuffer.back().type()) {
+        this->closeBatch();

[joshualitt, 2015/02/27 19:43:24]

I should have been paying more attention.  This close batch isn't necessary,
because every nonbatch push into the draw buffer will always close the last open
batch.  Was there some kind of an issue here?

'closebatch' goes away once I finish text too(thank God).

[robertphillips, 2015/02/27 20:52:39]

Done.

         fDrawBatch = GrNEW_APPEND_TO_RECORDER(fCmdBuffer, DrawBatch, (batch, &fBatchTarget));
+        this->recordTraceMarkersIfNecessary(fDrawBatch);
         return;
     }
 
     DrawBatch* draw = static_cast<DrawBatch*>(&fCmdBuffer.back());

[egdaniel, 2015/02/27 20:09:36]

I'm pretty sure this draw is fDrawBatch so we can just use that here. Also we
will want to call the recordTMIN for both cases here (when we merge batches we
should fall into the adding to set case on recordTMIN). So just drop this to

if(!combine) {...}
recordTMIN(fDrawBatch)

[robertphillips, 2015/02/27 20:52:39]

Done.

[robertphillips, 2015/02/27 21:06:16]

Done.

     if (draw->fBatch->combineIfPossible(batch)) {
         return;
     } else {
         this->closeBatch();
         fDrawBatch = GrNEW_APPEND_TO_RECORDER(fCmdBuffer, DrawBatch, (batch, &fBatchTarget));
     }
-    this->recordTraceMarkersIfNecessary();
+    this->recordTraceMarkersIfNecessary(fDrawBatch);
 }
 
 void GrInOrderDrawBuffer::onStencilPath(const GrPipelineBuilder& pipelineBuilder,
                                         const GrPathProcessor* pathProc,
                                         const GrPath* path,
                                         const GrScissorState& scissorState,
                                         const GrStencilSettings& stencilSettings) {
     this->closeBatch();
 
     StencilPath* sp = GrNEW_APPEND_TO_RECORDER(fCmdBuffer, StencilPath,
                                                (path, pipelineBuilder.getRenderTarget()));
 
     sp->fScissor = scissorState;
     sp->fUseHWAA = pipelineBuilder.isHWAntialias();
     sp->fViewMatrix = pathProc->viewMatrix();
     sp->fStencil = stencilSettings;
-    this->recordTraceMarkersIfNecessary();
+    this->recordTraceMarkersIfNecessary(sp);
 }
 
 void GrInOrderDrawBuffer::onDrawPath(const GrPathProcessor* pathProc,
                                      const GrPath* path,
                                      const GrStencilSettings& stencilSettings,
                                      const PipelineInfo& pipelineInfo) {
     this->closeBatch();
 
     // TODO: Only compare the subset of GrPipelineBuilder relevant to path covering?
     if (!this->setupPipelineAndShouldDraw(pathProc, pipelineInfo)) {
         return;
     }
     DrawPath* dp = GrNEW_APPEND_TO_RECORDER(fCmdBuffer, DrawPath, (path));
     dp->fStencilSettings = stencilSettings;
-    this->recordTraceMarkersIfNecessary();
+    this->recordTraceMarkersIfNecessary(dp);
 }
 
 void GrInOrderDrawBuffer::onDrawPaths(const GrPathProcessor* pathProc,
                                       const GrPathRange* pathRange,
                                       const void* indices,
                                       PathIndexType indexType,
                                       const float transformValues[],
                                       PathTransformType transformType,
                                       int count,
                                       const GrStencilSettings& stencilSettings,
@@ skipping 49 matching lines @@
     }
 
     DrawPaths* dp = GrNEW_APPEND_TO_RECORDER(fCmdBuffer, DrawPaths, (pathRange));
     dp->fIndices = savedIndices;
     dp->fIndexType = indexType;
     dp->fTransforms = savedTransforms;
     dp->fTransformType = transformType;
     dp->fCount = count;
     dp->fStencilSettings = stencilSettings;
 
-    this->recordTraceMarkersIfNecessary();
+    this->recordTraceMarkersIfNecessary(dp);
 }
 
 void GrInOrderDrawBuffer::onClear(const SkIRect* rect, GrColor color,
                                   bool canIgnoreRect, GrRenderTarget* renderTarget) {
     SkASSERT(renderTarget);
     this->closeBatch();
 
     SkIRect r;
     if (NULL == rect) {
         // We could do something smart and remove previous draws and clears to
         // the current render target. If we get that smart we have to make sure
         // those draws aren't read before this clear (render-to-texture).
         r.setLTRB(0, 0, renderTarget->width(), renderTarget->height());
         rect = &r;
     }
     Clear* clr = GrNEW_APPEND_TO_RECORDER(fCmdBuffer, Clear, (renderTarget));
     GrColorIsPMAssert(color);
     clr->fColor = color;
     clr->fRect = *rect;
     clr->fCanIgnoreRect = canIgnoreRect;
-    this->recordTraceMarkersIfNecessary();
+    this->recordTraceMarkersIfNecessary(clr);
 }
 
 void GrInOrderDrawBuffer::clearStencilClip(const SkIRect& rect,
                                            bool insideClip,
                                            GrRenderTarget* renderTarget) {
     SkASSERT(renderTarget);
     this->closeBatch();
 
     ClearStencilClip* clr = GrNEW_APPEND_TO_RECORDER(fCmdBuffer, ClearStencilClip, (renderTarget));
     clr->fRect = rect;
     clr->fInsideClip = insideClip;
-    this->recordTraceMarkersIfNecessary();
+    this->recordTraceMarkersIfNecessary(clr);
 }
 
 void GrInOrderDrawBuffer::discard(GrRenderTarget* renderTarget) {
     SkASSERT(renderTarget);
     this->closeBatch();
 
     if (!this->caps()->discardRenderTargetSupport()) {
         return;
     }
     Clear* clr = GrNEW_APPEND_TO_RECORDER(fCmdBuffer, Clear, (renderTarget));
     clr->fColor = GrColor_ILLEGAL;
-    this->recordTraceMarkersIfNecessary();
+    this->recordTraceMarkersIfNecessary(clr);
 }
 
 void GrInOrderDrawBuffer::onReset() {
     fCmdBuffer.reset();
     fPrevState = NULL;
     fPathIndexBuffer.rewind();
     fPathTransformBuffer.rewind();
     fGpuCmdMarkers.reset();
     fDrawBatch = NULL;
 }
@@ skipping 17 matching lines @@
     CmdBuffer::Iter iter(fCmdBuffer);
 
     int currCmdMarker = 0;
 
     int i = 0;
     while (iter.next()) {
         i++;
         GrGpuTraceMarker newMarker("", -1);
         SkString traceString;
         if (iter->isTraced()) {
-            traceString = fGpuCmdMarkers[currCmdMarker].toString();
+            traceString = this->getCmdString(currCmdMarker);
             newMarker.fMarker = traceString.c_str();
             this->getGpu()->addGpuTraceMarker(&newMarker);
             ++currCmdMarker;
         }
 
         // TODO temporary hack
         if (Cmd::kDrawBatch_Cmd == iter->type()) {
             DrawBatch* db = reinterpret_cast<DrawBatch*>(iter.get());
             fBatchTarget.flushNext(db->fBatch->numberOfDraws());
             continue;
@@ skipping 86 matching lines @@
 
 bool GrInOrderDrawBuffer::onCopySurface(GrSurface* dst,
                                         GrSurface* src,
                                         const SkIRect& srcRect,
                                         const SkIPoint& dstPoint) {
     if (getGpu()->canCopySurface(dst, src, srcRect, dstPoint)) {
         this->closeBatch();
         CopySurface* cs = GrNEW_APPEND_TO_RECORDER(fCmdBuffer, CopySurface, (dst, src));
         cs->fSrcRect = srcRect;
         cs->fDstPoint = dstPoint;
-        this->recordTraceMarkersIfNecessary();
+        this->recordTraceMarkersIfNecessary(cs);
         return true;
     }
     return false;
 }
 
 bool GrInOrderDrawBuffer::setupPipelineAndShouldDraw(const GrPrimitiveProcessor* primProc,
                                                      const PipelineInfo& pipelineInfo) {
     SetState* ss = GrNEW_APPEND_TO_RECORDER(fCmdBuffer, SetState, (primProc));
     this->setupPipeline(pipelineInfo, ss->pipelineLocation()); 
 
     if (ss->getPipeline()->mustSkip()) {
         fCmdBuffer.pop_back();
         return false;
     }
 
     ss->fPrimitiveProcessor->initBatchTracker(&ss->fBatchTracker,
                                               ss->getPipeline()->getInitBatchTracker());
 
     if (fPrevState && fPrevState->fPrimitiveProcessor.get() &&
         fPrevState->fPrimitiveProcessor->canMakeEqual(fPrevState->fBatchTracker,
                                                       *ss->fPrimitiveProcessor,
                                                       ss->fBatchTracker) &&
         fPrevState->getPipeline()->isEqual(*ss->getPipeline())) {
         fCmdBuffer.pop_back();
     } else {
         fPrevState = ss;
-        this->recordTraceMarkersIfNecessary();
+        this->recordTraceMarkersIfNecessary(ss);
     }
     return true;
 }
 
 bool GrInOrderDrawBuffer::setupPipelineAndShouldDraw(GrBatch* batch,
                                                      const PipelineInfo& pipelineInfo) {
     SetState* ss = GrNEW_APPEND_TO_RECORDER(fCmdBuffer, SetState, ());
     this->setupPipeline(pipelineInfo, ss->pipelineLocation()); 
 
     if (ss->getPipeline()->mustSkip()) {
         fCmdBuffer.pop_back();
         return false;
     }
 
     batch->initBatchTracker(ss->getPipeline()->getInitBatchTracker());
 
     if (fPrevState && !fPrevState->fPrimitiveProcessor.get() &&
         fPrevState->getPipeline()->isEqual(*ss->getPipeline())) {
         fCmdBuffer.pop_back();
     } else {
         this->closeBatch();
         fPrevState = ss;
-        this->recordTraceMarkersIfNecessary();
+        this->recordTraceMarkersIfNecessary(ss);
     }
     return true;
 }
 
-void GrInOrderDrawBuffer::recordTraceMarkersIfNecessary() {
-    SkASSERT(!fCmdBuffer.empty());
-    SkASSERT(!fCmdBuffer.back().isTraced());
+void GrInOrderDrawBuffer::recordTraceMarkersIfNecessary(Cmd* cmd) {
+    if (!cmd) {
+        return;
+    }
     const GrTraceMarkerSet& activeTraceMarkers = this->getActiveTraceMarkers();
     if (activeTraceMarkers.count() > 0) {
-        fCmdBuffer.back().makeTraced();
-        fGpuCmdMarkers.push_back(activeTraceMarkers);
+        if (cmd->isTraced()) {
+            fGpuCmdMarkers.back().addSet(activeTraceMarkers);
+        } else {
+            cmd->makeTraced();
+            fGpuCmdMarkers.push_back(activeTraceMarkers);
+        }
     }
 }
 
 void GrInOrderDrawBuffer::willReserveVertexAndIndexSpace(int vertexCount,
                                                          size_t vertexStride,
                                                          int indexCount) {
     this->closeBatch();
 
     this->INHERITED::willReserveVertexAndIndexSpace(vertexCount, vertexStride, indexCount);
 }