Combine similar DrawPaths calls in GrInOrderDrawBuffer

src/gpu/GrInOrderDrawBuffer.cpp

Index: src/gpu/GrInOrderDrawBuffer.cpp
diff --git a/src/gpu/GrInOrderDrawBuffer.cpp b/src/gpu/GrInOrderDrawBuffer.cpp
index 9835200598d76ddadb1cd94661b8d299b24c961b..a85d1a0f60096c0dc2d229a1301de2840f9a6f04 100644
--- a/src/gpu/GrInOrderDrawBuffer.cpp
+++ b/src/gpu/GrInOrderDrawBuffer.cpp
@@ -23,6 +23,8 @@ GrInOrderDrawBuffer::GrInOrderDrawBuffer(GrGpu* gpu,
     , fDstGpu(gpu)
     , fVertexPool(*vertexPool)
     , fIndexPool(*indexPool)
+    , fPathIndexBuffer(kPathIdxBufferMinAllocCnt)
+    , fPathTransformBuffer(kPathXformBufferMinAllocCnt)
     , fFlushing(false)
     , fDrawID(0) {
 
@@ -101,6 +103,25 @@ static void set_vertex_attributes(GrDrawState* drawState, bool hasLocalCoords, G
     }
 }
 
+static bool path_fill_type_is_winding(const GrStencilSettings& pathStencilSettings) {
+    static const GrStencilSettings::Face pathFace = GrStencilSettings::kFront_Face;
+    bool isWinding = kInvert_StencilOp != pathStencilSettings.passOp(pathFace);
+    if (isWinding) {
+        // Double check that it is in fact winding.
+        SkASSERT(kIncClamp_StencilOp == pathStencilSettings.passOp(pathFace));
+        SkASSERT(kIncClamp_StencilOp == pathStencilSettings.failOp(pathFace));
+        SkASSERT(0x1 != pathStencilSettings.writeMask(pathFace));
+        SkASSERT(!pathStencilSettings.isTwoSided());
+    }
+    return isWinding;
+}
+
+static bool draw_state_is_opaque(const GrDrawState& drawState) {

[egdaniel, 2014/11/11 18:39:07]

I don't believe this function fully captures what your trying to do. For example
there is no check to make sure there is no coverage, or if we are reading the
Dst and blending in fragment code. We will add a call on GrDrawState,
isUnblended(), that should capture the functionality that you are looking for
here.

[Chris Dalton, 2014/11/11 19:29:15]

Ok, so while we wait for isUnblended can I capture all the corner cases from
this file? For example by ensuring dstCopy is null in onDrawPaths?

I'm a bit hazy the concept of coverage, but I believe it wouldn't be used with
nvpr, right?

+    GrBlendCoeff dstBlendCoeff = drawState.getDstBlendCoeff();
+    return (kZero_GrBlendCoeff == dstBlendCoeff ||
+            (kISA_GrBlendCoeff == dstBlendCoeff && drawState.srcAlphaWillBeOne()));
+}
+
 enum {
     kTraceCmdBit = 0x80,
     kCmdMask = 0x7f,
@@ -333,15 +354,36 @@ void GrInOrderDrawBuffer::onDrawPaths(const GrPathRange* pathRange,
 
     this->recordStateIfNecessary();
 
-    int sizeOfIndices = sizeof(uint32_t) * count;
-    int sizeOfTransforms = sizeof(float) * count *
-                           GrPathRendering::PathTransformSize(transformsType);
+    int indicesLocation = fPathIndexBuffer.append(indices, count);
+    int transformsLocation = fPathTransformBuffer.append(transforms,
+                                 GrPathRendering::PathTransformSize(transformsType) * count);
+
+    if (kDrawPaths_Cmd == fCmdBuffer.back().fType) {
+        // The previous command was also DrawPaths. Try to collapse this call into the one
+        // before. Note that stencilling all the paths at once, then covering, may not be
+        // equivalent to two separate draw calls if there is overlap. Blending won't work,
+        // and the combined calls may also cancel each other's winding numbers in some
+        // places. For now the winding numbers are only an issue if the fill is even/odd,
+        // because DrawPaths is currently only used for glyphs, and glyphs in the same
+        // font tend to all wind in the same direction.
+        DrawPaths* previous = static_cast<DrawPaths*>(&fCmdBuffer.back());
+        if (pathRange == previous->pathRange() &&
+            transformsType == previous->fTransformsType &&
+            scissorState == previous->fScissorState &&
+            stencilSettings == previous->fStencilSettings &&
+            path_fill_type_is_winding(stencilSettings) &&
+            draw_state_is_opaque(this->getDrawState())) {

[bsalomon, 2014/11/11 14:41:25]

Could/should you check that the b.boxes don't overlap || the draw is opaque?

[Chris Dalton, 2014/11/11 17:49:07]

In theory, yes. But right now there isn't a way of knowing this, because
GrPathRange doesn't provide or even know the bboxes for individual paths. I
think it's a good optimization though. Is it something we can punt on for now
but keep in mind for the future?

+            // Fold this DrawPaths call into the one previous.
+            SkASSERT(!dstCopy || !dstCopy->texture());
+            previous->fCount += count;
+            return;
+        }
+    }
 
-    DrawPaths* dp = GrNEW_APPEND_WITH_DATA_TO_RECORDER(fCmdBuffer, DrawPaths, (pathRange),
-                                                       sizeOfIndices + sizeOfTransforms);
-    memcpy(dp->indices(), indices, sizeOfIndices);
+    DrawPaths* dp = GrNEW_APPEND_TO_RECORDER(fCmdBuffer, DrawPaths, (pathRange));
+    dp->fIndicesLocation = indicesLocation;
     dp->fCount = count;
-    memcpy(dp->transforms(), transforms, sizeOfTransforms);
+    dp->fTransformsLocation = transformsLocation;
     dp->fTransformsType = transformsType;
     dp->fScissorState = scissorState;
     dp->fStencilSettings = stencilSettings;
@@ -403,6 +445,8 @@ void GrInOrderDrawBuffer::reset() {
     this->resetIndexSource();
 
     fCmdBuffer.reset();
+    fPathIndexBuffer.reset();
+    fPathTransformBuffer.reset();
     fLastState = NULL;
     fVertexPool.reset();
     fIndexPool.reset();
@@ -452,7 +496,7 @@ void GrInOrderDrawBuffer::flush() {
                                   kDrawPaths_Cmd == strip_trace_bit(iter->fType));
         SkASSERT(!isDraw || fDstGpu->drawState() != prevDrawState);
 
-        iter->execute(fDstGpu);
+        iter->execute(this);
 
         if (cmd_has_trace_marker(iter->fType)) {
             fDstGpu->removeGpuTraceMarker(&newMarker);
@@ -468,7 +512,8 @@ void GrInOrderDrawBuffer::flush() {
     ++fDrawID;
 }
 
-void GrInOrderDrawBuffer::Draw::execute(GrGpu* gpu) {
+void GrInOrderDrawBuffer::Draw::execute(GrInOrderDrawBuffer* buf) {
+    GrGpu* gpu = buf->dstGpu();
     gpu->setVertexSourceToBuffer(this->vertexBuffer());
     if (fInfo.isIndexed()) {
         gpu->setIndexSourceToBuffer(this->indexBuffer());
@@ -476,26 +521,31 @@ void GrInOrderDrawBuffer::Draw::execute(GrGpu* gpu) {
     gpu->draw(fInfo, fScissorState);
 }
 
-void GrInOrderDrawBuffer::StencilPath::execute(GrGpu* gpu) {
+void GrInOrderDrawBuffer::StencilPath::execute(GrInOrderDrawBuffer* buf) {
+    GrGpu* gpu = buf->dstGpu();
     gpu->stencilPath(this->path(), fScissorState, fStencilSettings);
 }
 
-void GrInOrderDrawBuffer::DrawPath::execute(GrGpu* gpu) {
+void GrInOrderDrawBuffer::DrawPath::execute(GrInOrderDrawBuffer* buf) {
+    GrGpu* gpu = buf->dstGpu();
     gpu->drawPath(this->path(), fScissorState, fStencilSettings,
-                         fDstCopy.texture() ? &fDstCopy : NULL);
+                  fDstCopy.texture() ? &fDstCopy : NULL);
 }
 
-void GrInOrderDrawBuffer::DrawPaths::execute(GrGpu* gpu) {
-    gpu->drawPaths(this->pathRange(), this->indices(), fCount, this->transforms(),
-                          fTransformsType, fScissorState, fStencilSettings,
-                          fDstCopy.texture() ? &fDstCopy : NULL);
+void GrInOrderDrawBuffer::DrawPaths::execute(GrInOrderDrawBuffer* buf) {
+    GrGpu* gpu = buf->dstGpu();
+    gpu->drawPaths(this->pathRange(), buf->pathIndexBuffer() + fIndicesLocation, fCount,
+                   buf->pathTransformBuffer() + fTransformsLocation, fTransformsType,
+                   fScissorState, fStencilSettings, fDstCopy.texture() ? &fDstCopy : NULL);
 }
 
-void GrInOrderDrawBuffer::SetState::execute(GrGpu* gpu) {
+void GrInOrderDrawBuffer::SetState::execute(GrInOrderDrawBuffer* buf) {
+    GrGpu* gpu = buf->dstGpu();
     gpu->setDrawState(&fState);
 }
 
-void GrInOrderDrawBuffer::Clear::execute(GrGpu* gpu) {
+void GrInOrderDrawBuffer::Clear::execute(GrInOrderDrawBuffer* buf) {
+    GrGpu* gpu = buf->dstGpu();
     if (GrColor_ILLEGAL == fColor) {
         gpu->discard(this->renderTarget());
     } else {
@@ -503,11 +553,13 @@ void GrInOrderDrawBuffer::Clear::execute(GrGpu* gpu) {
     }
 }
 
-void GrInOrderDrawBuffer::ClearStencilClip::execute(GrGpu* gpu) {
-        gpu->clearStencilClip(fRect, fInsideClip, this->renderTarget());
+void GrInOrderDrawBuffer::ClearStencilClip::execute(GrInOrderDrawBuffer* buf) {
+    GrGpu* gpu = buf->dstGpu();
+    gpu->clearStencilClip(fRect, fInsideClip, this->renderTarget());
 }
 
-void GrInOrderDrawBuffer::CopySurface::execute(GrGpu* gpu) {
+void GrInOrderDrawBuffer::CopySurface::execute(GrInOrderDrawBuffer* buf) {
+    GrGpu* gpu = buf->dstGpu();
     gpu->copySurface(this->dst(), this->src(), fSrcRect, fDstPoint);
 }
 
@@ -731,3 +783,25 @@ void GrInOrderDrawBuffer::recordTraceMarkersIfNecessary() {
         fGpuCmdMarkers.push_back(activeTraceMarkers);
     }
 }
+
+template<typename T> int GrInOrderDrawBuffer::DataBuffer<T>::append(const T* array, int count) {
+    if (fCount + count > fAllocCnt) {
+        fAllocCnt = SkTMax(2 * fAllocCnt, (fCount + count) * 3 / 2);
+        fBuffer.realloc(fAllocCnt);
+    }
+    int location = fCount;
+    memcpy(fBuffer + location, array, sizeof(T) * count);
+    fCount += count;
+    return location;
+}
+
+template<typename T> void GrInOrderDrawBuffer::DataBuffer<T>::reset() {
+    // Assume the next time this buffer fills up it will use approximately the same amount
+    // of space as last time. Only resize if we're using less than a third of the
+    // allocated space, and leave enough for 50% growth over last time.
+    if (3 * fCount < fAllocCnt && fAllocCnt > fMinAllocCnt) {
+        fAllocCnt = SkTMax(fMinAllocCnt, fCount * 3 / 2);
+        fBuffer.reset(fAllocCnt);
+    }
+    fCount = 0;
+}