Increase GrInOrderDrawBuffer's encapsulation of trace markers
Greg - I think this is mainly you
Joshua - I did make a small batching change in onDrawBatch
Review URL: https://codereview.chromium.org/963183002
diff --git a/src/gpu/GrInOrderDrawBuffer.cpp b/src/gpu/GrInOrderDrawBuffer.cpp
index 0807bf3..8c2bea9 100644
--- a/src/gpu/GrInOrderDrawBuffer.cpp
+++ b/src/gpu/GrInOrderDrawBuffer.cpp
@@ -35,6 +35,15 @@
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
@@ -354,15 +363,7 @@
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;
}
@@ -388,7 +389,7 @@
} else {
draw = GrNEW_APPEND_TO_RECORDER(fCmdBuffer, Draw, (info));
}
- this->recordTraceMarkersIfNecessary();
+ this->recordTraceMarkersIfNecessary(draw);
}
void GrInOrderDrawBuffer::onDrawBatch(GrBatch* batch,
@@ -400,17 +401,16 @@
// Check if there is a Batch Draw we can batch with
if (Cmd::kDrawBatch_Cmd != fCmdBuffer.back().type()) {
fDrawBatch = GrNEW_APPEND_TO_RECORDER(fCmdBuffer, DrawBatch, (batch, &fBatchTarget));
+ this->recordTraceMarkersIfNecessary(fDrawBatch);
return;
}
- DrawBatch* draw = static_cast<DrawBatch*>(&fCmdBuffer.back());
- if (draw->fBatch->combineIfPossible(batch)) {
- return;
- } else {
+ SkASSERT(&fCmdBuffer.back() == fDrawBatch);
+ if (!fDrawBatch->fBatch->combineIfPossible(batch)) {
this->closeBatch();
fDrawBatch = GrNEW_APPEND_TO_RECORDER(fCmdBuffer, DrawBatch, (batch, &fBatchTarget));
}
- this->recordTraceMarkersIfNecessary();
+ this->recordTraceMarkersIfNecessary(fDrawBatch);
}
void GrInOrderDrawBuffer::onStencilPath(const GrPipelineBuilder& pipelineBuilder,
@@ -427,7 +427,7 @@
sp->fUseHWAA = pipelineBuilder.isHWAntialias();
sp->fViewMatrix = pathProc->viewMatrix();
sp->fStencil = stencilSettings;
- this->recordTraceMarkersIfNecessary();
+ this->recordTraceMarkersIfNecessary(sp);
}
void GrInOrderDrawBuffer::onDrawPath(const GrPathProcessor* pathProc,
@@ -442,7 +442,7 @@
}
DrawPath* dp = GrNEW_APPEND_TO_RECORDER(fCmdBuffer, DrawPath, (path));
dp->fStencilSettings = stencilSettings;
- this->recordTraceMarkersIfNecessary();
+ this->recordTraceMarkersIfNecessary(dp);
}
void GrInOrderDrawBuffer::onDrawPaths(const GrPathProcessor* pathProc,
@@ -512,7 +512,7 @@
dp->fCount = count;
dp->fStencilSettings = stencilSettings;
- this->recordTraceMarkersIfNecessary();
+ this->recordTraceMarkersIfNecessary(dp);
}
void GrInOrderDrawBuffer::onClear(const SkIRect* rect, GrColor color,
@@ -533,7 +533,7 @@
clr->fColor = color;
clr->fRect = *rect;
clr->fCanIgnoreRect = canIgnoreRect;
- this->recordTraceMarkersIfNecessary();
+ this->recordTraceMarkersIfNecessary(clr);
}
void GrInOrderDrawBuffer::clearStencilClip(const SkIRect& rect,
@@ -545,7 +545,7 @@
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) {
@@ -557,7 +557,7 @@
}
Clear* clr = GrNEW_APPEND_TO_RECORDER(fCmdBuffer, Clear, (renderTarget));
clr->fColor = GrColor_ILLEGAL;
- this->recordTraceMarkersIfNecessary();
+ this->recordTraceMarkersIfNecessary(clr);
}
void GrInOrderDrawBuffer::onReset() {
@@ -595,7 +595,7 @@
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;
@@ -702,7 +702,7 @@
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;
@@ -729,7 +729,7 @@
fCmdBuffer.pop_back();
} else {
fPrevState = ss;
- this->recordTraceMarkersIfNecessary();
+ this->recordTraceMarkersIfNecessary(ss);
}
return true;
}
@@ -752,18 +752,23 @@
} 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);
+ }
}
}
diff --git a/src/gpu/GrInOrderDrawBuffer.h b/src/gpu/GrInOrderDrawBuffer.h
index 5876169..ccda851 100644
--- a/src/gpu/GrInOrderDrawBuffer.h
+++ b/src/gpu/GrInOrderDrawBuffer.h
@@ -298,9 +298,12 @@
// We lazily record clip changes in order to skip clips that have no effect.
void recordClipIfNecessary();
- // Records any trace markers for a command after adding it to the buffer.
- void recordTraceMarkersIfNecessary();
-
+ // Records any trace markers for a command
+ void recordTraceMarkersIfNecessary(Cmd*);
+ SkString getCmdString(int index) const {
+ SkASSERT(index < fGpuCmdMarkers.count());
+ return fGpuCmdMarkers[index].toString();
+ }
bool isIssued(uint32_t drawID) SK_OVERRIDE { return drawID != fDrawID; }
GrBatchTarget* getBatchTarget() { return &fBatchTarget; }
@@ -324,14 +327,7 @@
// This will go away when everything uses batch. However, in the short term anything which
// might be put into the GrInOrderDrawBuffer needs to make sure it closes the last batch
- void closeBatch() {
- if (fDrawBatch) {
- fBatchTarget.resetNumberOfDraws();
- fDrawBatch->execute(this->getGpu(), fPrevState);
- fDrawBatch->fBatch->setNumberOfDraws(fBatchTarget.numberOfDraws());
- fDrawBatch = NULL;
- }
- }
+ inline void closeBatch();
typedef GrFlushToGpuDrawTarget INHERITED;
};