some cleanup around GrGpu/GrDrawTarget copySurface

Committed: https://skia.googlesource.com/skia/+/e9aa5dc4d5906788eaf691d7c69f1494928f401d

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

9 files changed, 213 insertions, 189 deletions

diff --git a/src/gpu/GrDrawTarget.cpp b/src/gpu/GrDrawTarget.cpp
index bbf3eca05b..9ac2d6a3b9 100644
--- a/src/gpu/GrDrawTarget.cpp
+++ b/src/gpu/GrDrawTarget.cpp
@@ -921,17 +921,20 @@ bool GrDrawTarget::copySurface(GrSurface* dst,
                                    dstPoint,
                                    &clippedSrcRect,
                                    &clippedDstPoint)) {
-        SkASSERT(GrDrawTarget::canCopySurface(dst, src, srcRect, dstPoint));
         return true;
     }
 
-    if (!GrDrawTarget::canCopySurface(dst, src, clippedSrcRect, clippedDstPoint)) {
-        return false;
+    if (this->onCopySurface(dst, src, clippedSrcRect, clippedDstPoint)) {
+        return true;
     }
 
     GrRenderTarget* rt = dst->asRenderTarget();
     GrTexture* tex = src->asTexture();
 
+    if ((dst == src) || !rt || !tex) {
+        return false;
+    }
+
     GrDrawState drawState;
     drawState.setRenderTarget(rt);
     SkMatrix matrix;
@@ -965,7 +968,13 @@ bool GrDrawTarget::canCopySurface(const GrSurface* dst,
                                    &clippedDstPoint)) {
         return true;
     }
+    return this->internalCanCopySurface(dst, src, clippedSrcRect, clippedDstPoint);
+}
 
+bool GrDrawTarget::internalCanCopySurface(const GrSurface* dst,
+                                          const GrSurface* src,
+                                          const SkIRect& clippedSrcRect,
+                                          const SkIPoint& clippedDstPoint) {
     // Check that the read/write rects are contained within the src/dst bounds.
     SkASSERT(!clippedSrcRect.isEmpty());
     SkASSERT(SkIRect::MakeWH(src->width(), src->height()).contains(clippedSrcRect));
@@ -973,14 +982,9 @@ bool GrDrawTarget::canCopySurface(const GrSurface* dst,
     SkASSERT(clippedDstPoint.fX + clippedSrcRect.width() <= dst->width() &&
              clippedDstPoint.fY + clippedSrcRect.height() <= dst->height());
 
-    return (dst != src) && dst->asRenderTarget() && src->asTexture();
-}
-
-void GrDrawTarget::initCopySurfaceDstDesc(const GrSurface* src, GrSurfaceDesc* desc) {
-    // Make the dst of the copy be a render target because the default copySurface draws to the dst.
-    desc->fOrigin = kDefault_GrSurfaceOrigin;
-    desc->fFlags = kRenderTarget_GrSurfaceFlag | kNoStencil_GrSurfaceFlag;
-    desc->fConfig = src->config();
+    // The base class can do it as a draw or the subclass may be able to handle it.
+    return ((dst != src) && dst->asRenderTarget() && src->asTexture()) ||
+           this->onCanCopySurface(dst, src, clippedSrcRect, clippedDstPoint);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
diff --git a/src/gpu/GrDrawTarget.h b/src/gpu/GrDrawTarget.h
index 0afbe66537..7695365503 100644
--- a/src/gpu/GrDrawTarget.h
+++ b/src/gpu/GrDrawTarget.h
@@ -68,7 +68,7 @@ public:
     const GrClipData* getClip() const;
 
     /**
-     * There are three types of "sources" of geometry (vertices and indices) for
+     * There are two types of "sources" of geometry (vertices and indices) for
      * draw calls made on the target. When performing an indexed draw, the
      * indices and vertices can use different source types. Once a source is
      * specified it can be used for multiple draws. However, the time at which
@@ -84,15 +84,9 @@ public:
      * is set or resetVertexSource / resetIndexSource is called. Drawing from
      * a reset source is an error.
      *
-     * The three types of sources are:
+     * The two types of sources are:
      *
-     * 1. A cpu array (set*SourceToArray). This is useful when the caller
-     *    already provided vertex data in a format compatible with a
-     *    GrVertexLayout. The data in the array is consumed at the time that
-     *    set*SourceToArray is called and subsequent edits to the array will not
-     *    be reflected in draws.
-     *
-     * 2. Reserve. This is most useful when the caller has data it must
+     * 1. Reserve. This is most useful when the caller has data it must
      *    transform before drawing and is not long-lived. The caller requests
      *    that the draw target make room for some amount of vertex and/or index
      *    data. The target provides ptrs to hold the vertex and/or index data.
@@ -104,13 +98,11 @@ public:
      *    Where the space is allocated and how it is uploaded to the GPU is
      *    subclass-dependent.
      *
-     * 3. Vertex and Index Buffers. This is most useful for geometry that will
+     * 2. Vertex and Index Buffers. This is most useful for geometry that will
      *    is long-lived. When the data in the buffer is consumed depends on the
      *    GrDrawTarget subclass. For deferred subclasses the caller has to
      *    guarantee that the data is still available in the buffers at playback.
      *    (TODO: Make this more automatic as we have done for read/write pixels)
-     *
-     * The size of each vertex is determined by querying the current GrDrawState.
      */
 
     /**
@@ -413,26 +405,18 @@ public:
      * limitations. If rect is clipped out entirely by the src or dst bounds then
      * true is returned since there is no actual copy necessary to succeed.
      */
-    virtual bool copySurface(GrSurface* dst,
-                             GrSurface* src,
-                             const SkIRect& srcRect,
-                             const SkIPoint& dstPoint);
+    bool copySurface(GrSurface* dst,
+                     GrSurface* src,
+                     const SkIRect& srcRect,
+                     const SkIPoint& dstPoint);
     /**
-     * Function that determines whether a copySurface call would succeed without
+     * Function that determines whether a copySurface call would succeed without actually
      * performing the copy.
      */
-    virtual bool canCopySurface(const GrSurface* dst,
-                                const GrSurface* src,
-                                const SkIRect& srcRect,
-                                const SkIPoint& dstPoint);
-
-    /**
-     * This is can be called before allocating a texture to be a dst for copySurface. It will
-     * populate the origin, config, and flags fields of the desc such that copySurface is more
-     * likely to succeed and be efficient.
-     */
-    virtual void initCopySurfaceDstDesc(const GrSurface* src, GrSurfaceDesc* desc);
-
+    bool canCopySurface(const GrSurface* dst,
+                        const GrSurface* src,
+                        const SkIRect& srcRect,
+                        const SkIPoint& dstPoint);
 
     /**
      * Release any resources that are cached but not currently in use. This
@@ -670,6 +654,25 @@ protected:
                                  const SkRect* drawBounds);
 
 private:
+    /**
+     * This will be called before allocating a texture as a dst for copySurface. This function
+     * populates the dstDesc's config, flags, and origin so as to maximize efficiency and guarantee
+     * success of the copySurface call.
+     */
+    void initCopySurfaceDstDesc(const GrSurface* src, GrSurfaceDesc* dstDesc) {
+        if (!this->onInitCopySurfaceDstDesc(src, dstDesc)) {
+            dstDesc->fOrigin = kDefault_GrSurfaceOrigin;
+            dstDesc->fFlags = kRenderTarget_GrSurfaceFlag | kNoStencil_GrSurfaceFlag;
+            dstDesc->fConfig = src->config();
+        }
+    }
+
+    /** Internal implementation of canCopySurface. */
+    bool internalCanCopySurface(const GrSurface* dst,
+                                const GrSurface* src,
+                                const SkIRect& clippedSrcRect,
+                                const SkIPoint& clippedDstRect);
+
     // A subclass can optionally overload this function to be notified before
     // vertex and index space is reserved.
     virtual void willReserveVertexAndIndexSpace(int vertexCount,
@@ -719,6 +722,29 @@ private:
     virtual void onClear(const SkIRect* rect, GrColor color, bool canIgnoreRect,
                          GrRenderTarget* renderTarget) = 0;
 
+    /** The subclass will get a chance to copy the surface for falling back to the default
+        implementation, which simply draws a rectangle (and fails if dst isn't a render target). It
+        should assume that any clipping has already been performed on the rect and point. It won't
+        be called if the copy can be skipped. */
+    virtual bool onCopySurface(GrSurface* dst,
+                               GrSurface* src,
+                               const SkIRect& srcRect,
+                               const SkIPoint& dstPoint) = 0;
+
+    /** Indicates whether onCopySurface would succeed. It should assume that any clipping has
+        already been performed on the rect and point. It won't be called if the copy can be
+        skipped. */
+    virtual bool onCanCopySurface(const GrSurface* dst,
+                                  const GrSurface* src,
+                                  const SkIRect& srcRect,
+                                  const SkIPoint& dstPoint) = 0;
+    /**
+     * This will be called before allocating a texture to be a dst for onCopySurface. Only the
+     * dstDesc's config, flags, and origin need be set by the function. If the subclass cannot
+     * create a surface that would succeed its implementation of onCopySurface, it should return
+     * false. The base class will fall back to creating a render target to draw into using the src.
+     */
+    virtual bool onInitCopySurfaceDstDesc(const GrSurface* src, GrSurfaceDesc* dstDesc) = 0;
 
     // helpers for reserving vertex and index space.
     bool reserveVertexSpace(size_t vertexSize,
diff --git a/src/gpu/GrGpu.cpp b/src/gpu/GrGpu.cpp
index 74e9419e74..d97809f952 100644
--- a/src/gpu/GrGpu.cpp
+++ b/src/gpu/GrGpu.cpp
@@ -209,13 +209,6 @@ void GrGpu::resolveRenderTarget(GrRenderTarget* target) {
     this->onResolveRenderTarget(target);
 }
 
-void GrGpu::initCopySurfaceDstDesc(const GrSurface* src, GrSurfaceDesc* desc) {
-    // Make the dst of the copy be a render target because the default copySurface draws to the dst.
-    desc->fOrigin = kDefault_GrSurfaceOrigin;
-    desc->fFlags = kRenderTarget_GrSurfaceFlag | kNoStencil_GrSurfaceFlag;
-    desc->fConfig = src->config();
-}
-
 typedef GrTraceMarkerSet::Iter TMIter;
 void GrGpu::saveActiveTraceMarkers() {
     if (this->caps()->gpuTracingSupport()) {
diff --git a/src/gpu/GrGpu.h b/src/gpu/GrGpu.h
index b10a63f2df..d0bff30262 100644
--- a/src/gpu/GrGpu.h
+++ b/src/gpu/GrGpu.h
@@ -278,10 +278,11 @@ public:
 
     /**
      * This is can be called before allocating a texture to be a dst for copySurface. It will
-     * populate the origin, config, and flags fields of the desc such that copySurface is more
-     * likely to succeed and be efficient.
+     * populate the origin, config, and flags fields of the desc such that copySurface can
+     * efficiently succeed. It should only succeed if it can allow copySurface to perform a copy
+     * that would be more effecient than drawing the src to a dst render target.
      */
-    virtual void initCopySurfaceDstDesc(const GrSurface* src, GrSurfaceDesc* desc);
+    virtual bool initCopySurfaceDstDesc(const GrSurface* src, GrSurfaceDesc* desc) = 0;
 
     // After the client interacts directly with the 3D context state the GrGpu
     // must resync its internal state and assumptions about 3D context state.
@@ -337,20 +338,20 @@ public:
     void saveActiveTraceMarkers();
     void restoreActiveTraceMarkers();
 
-    // Called to determine whether an onCopySurface call would succeed or not. This is useful for
-    // proxy subclasses to test whether the copy would succeed without executing it yet. Derived
-    // classes must keep this consistent with their implementation of onCopySurface(). The inputs
-    // are the same as onCopySurface(), i.e. srcRect and dstPoint are clipped to be inside the src
-    // and dst bounds.
+    // Called to determine whether a copySurface call would succeed or not. Derived
+    // classes must keep this consistent with their implementation of onCopySurface(). Fallbacks
+    // to issuing a draw from the src to dst take place at the GrDrawTarget level and this function
+    // should only return true if a faster copy path exists. The rect and point are pre-clipped. The
+    // src rect and implied dst rect are guaranteed to be within the src/dst bounds and non-empty.
     virtual bool canCopySurface(const GrSurface* dst,
                                 const GrSurface* src,
                                 const SkIRect& srcRect,
                                 const SkIPoint& dstPoint) = 0;
 
-    // This method is called by copySurface  The srcRect is guaranteed to be entirely within the
-    // src bounds. Likewise, the dst rect implied by dstPoint and srcRect's width and height falls
-    // entirely within the dst. The default implementation will draw a rect from the src to the
-    // dst if the src is a texture and the dst is a render target and fail otherwise.
+    // Called to perform a surface to surface copy. Fallbacks to issuing a draw from the src to dst
+    // take place at the GrDrawTarget level and this function implement faster copy paths. The rect
+    // and point are pre-clipped. The src rect and implied dst rect are guaranteed to be within the
+    // src/dst bounds and non-empty.
     virtual bool copySurface(GrSurface* dst,
                              GrSurface* src,
                              const SkIRect& srcRect,
diff --git a/src/gpu/GrInOrderDrawBuffer.cpp b/src/gpu/GrInOrderDrawBuffer.cpp
index 624c774763..45dd16d9a6 100644
--- a/src/gpu/GrInOrderDrawBuffer.cpp
+++ b/src/gpu/GrInOrderDrawBuffer.cpp
@@ -557,34 +557,29 @@ void GrInOrderDrawBuffer::CopySurface::execute(GrInOrderDrawBuffer* buf, const G
     buf->fDstGpu->copySurface(this->dst(), this->src(), fSrcRect, fDstPoint);
 }
 
-bool GrInOrderDrawBuffer::copySurface(GrSurface* dst,
-                                      GrSurface* src,
-                                      const SkIRect& srcRect,
-                                      const SkIPoint& dstPoint) {
+bool GrInOrderDrawBuffer::onCopySurface(GrSurface* dst,
+                                        GrSurface* src,
+                                        const SkIRect& srcRect,
+                                        const SkIPoint& dstPoint) {
     if (fDstGpu->canCopySurface(dst, src, srcRect, dstPoint)) {
         CopySurface* cs = GrNEW_APPEND_TO_RECORDER(fCmdBuffer, CopySurface, (dst, src));
         cs->fSrcRect = srcRect;
         cs->fDstPoint = dstPoint;
         this->recordTraceMarkersIfNecessary();
         return true;
-    } else if (GrDrawTarget::canCopySurface(dst, src, srcRect, dstPoint)) {
-        GrDrawTarget::copySurface(dst, src, srcRect, dstPoint);
-        return true;
-    } else {
-        return false;
     }
+    return false;
 }
 
-bool GrInOrderDrawBuffer::canCopySurface(const GrSurface* dst,
+bool GrInOrderDrawBuffer::onCanCopySurface(const GrSurface* dst,
                                          const GrSurface* src,
                                          const SkIRect& srcRect,
                                          const SkIPoint& dstPoint) {
-    return fDstGpu->canCopySurface(dst, src, srcRect, dstPoint) ||
-           GrDrawTarget::canCopySurface(dst, src, srcRect, dstPoint);
+    return fDstGpu->canCopySurface(dst, src, srcRect, dstPoint);
 }
 
-void GrInOrderDrawBuffer::initCopySurfaceDstDesc(const GrSurface* src, GrSurfaceDesc* desc) {
-    fDstGpu->initCopySurfaceDstDesc(src, desc);
+bool GrInOrderDrawBuffer::onInitCopySurfaceDstDesc(const GrSurface* src, GrSurfaceDesc* desc) {
+    return fDstGpu->initCopySurfaceDstDesc(src, desc);
 }
 
 void GrInOrderDrawBuffer::willReserveVertexAndIndexSpace(int vertexCount,
diff --git a/src/gpu/GrInOrderDrawBuffer.h b/src/gpu/GrInOrderDrawBuffer.h
index 373c5b4d59..415c3e1d1d 100644
--- a/src/gpu/GrInOrderDrawBuffer.h
+++ b/src/gpu/GrInOrderDrawBuffer.h
@@ -52,7 +52,7 @@ public:
                         GrVertexBufferAllocPool* vertexPool,
                         GrIndexBufferAllocPool* indexPool);
 
-    virtual ~GrInOrderDrawBuffer();
+    ~GrInOrderDrawBuffer() SK_OVERRIDE;
 
     /**
      * Empties the draw buffer of any queued up draws. This must not be called while inside an
@@ -69,30 +69,18 @@ public:
     void flush();
 
     // tracking for draws
-    virtual DrawToken getCurrentDrawToken() { return DrawToken(this, fDrawID); }
+    DrawToken getCurrentDrawToken() { return DrawToken(this, fDrawID); }
 
     // overrides from GrDrawTarget
-    virtual bool geometryHints(size_t vertexStride,
-                               int* vertexCount,
-                               int* indexCount) const SK_OVERRIDE;
+    bool geometryHints(size_t vertexStride,
+                       int* vertexCount,
+                       int* indexCount) const SK_OVERRIDE;
 
-    virtual bool copySurface(GrSurface* dst,
-                             GrSurface* src,
-                             const SkIRect& srcRect,
-                             const SkIPoint& dstPoint)  SK_OVERRIDE;
+    void clearStencilClip(const SkIRect& rect,
+                          bool insideClip,
+                          GrRenderTarget* renderTarget) SK_OVERRIDE;
 
-    virtual bool canCopySurface(const GrSurface* dst,
-                                const GrSurface* src,
-                                const SkIRect& srcRect,
-                                const SkIPoint& dstPoint) SK_OVERRIDE;
-
-    virtual void clearStencilClip(const SkIRect& rect,
-                                  bool insideClip,
-                                  GrRenderTarget* renderTarget) SK_OVERRIDE;
-
-    virtual void discard(GrRenderTarget*) SK_OVERRIDE;
-
-    virtual void initCopySurfaceDstDesc(const GrSurface* src, GrSurfaceDesc* desc) SK_OVERRIDE;
+    void discard(GrRenderTarget*) SK_OVERRIDE;
 
 private:
     typedef GrClipMaskManager::ScissorState ScissorState;
@@ -273,6 +261,15 @@ private:
     void willReserveVertexAndIndexSpace(int vertexCount,
                                         size_t vertexStride,
                                         int indexCount) SK_OVERRIDE;
+    bool onCopySurface(GrSurface* dst,
+                       GrSurface* src,
+                       const SkIRect& srcRect,
+                       const SkIPoint& dstPoint) SK_OVERRIDE;
+    bool onCanCopySurface(const GrSurface* dst,
+                          const GrSurface* src,
+                          const SkIRect& srcRect,
+                          const SkIPoint& dstPoint) SK_OVERRIDE;
+    bool onInitCopySurfaceDstDesc(const GrSurface* src, GrSurfaceDesc* desc) SK_OVERRIDE;
 
     // Attempts to concat instances from info onto the previous draw. info must represent an
     // instanced draw. The caller must have already recorded a new draw state and clip if necessary.
diff --git a/src/gpu/GrTest.cpp b/src/gpu/GrTest.cpp
index f861e42abe..48f72a7e4c 100644
--- a/src/gpu/GrTest.cpp
+++ b/src/gpu/GrTest.cpp
@@ -53,108 +53,100 @@ class GrOptDrawState;
 class MockGpu : public GrGpu {
 public:
     MockGpu(GrContext* context) : INHERITED(context) { fCaps.reset(SkNEW(GrDrawTargetCaps)); }
-    virtual ~MockGpu() { }
-    virtual bool canWriteTexturePixels(const GrTexture*,
-                                       GrPixelConfig srcConfig) const SK_OVERRIDE {
+    ~MockGpu() SK_OVERRIDE {}
+    bool canWriteTexturePixels(const GrTexture*, GrPixelConfig srcConfig) const SK_OVERRIDE {
         return true;
     }
 
-    virtual bool readPixelsWillPayForYFlip(GrRenderTarget* renderTarget,
-                                           int left, int top,
-                                           int width, int height,
-                                           GrPixelConfig config,
-                                           size_t rowBytes) const SK_OVERRIDE { return false; }
-    virtual void buildProgramDesc(const GrOptDrawState&,
-                                  const GrProgramDesc::DescInfo&,
-                                  GrGpu::DrawType,
-                                  GrProgramDesc* desc) SK_OVERRIDE { }
-
-    virtual void discard(GrRenderTarget*) SK_OVERRIDE { }
-
-    virtual bool canCopySurface(const GrSurface* dst,
-                                const GrSurface* src,
-                                const SkIRect& srcRect,
-                                const SkIPoint& dstPoint) SK_OVERRIDE { return false; };
-
-    virtual bool copySurface(GrSurface* dst,
-                             GrSurface* src,
-                             const SkIRect& srcRect,
-                             const SkIPoint& dstPoint) SK_OVERRIDE { return false; };
+    bool readPixelsWillPayForYFlip(GrRenderTarget* renderTarget,
+                                   int left, int top,
+                                   int width, int height,
+                                   GrPixelConfig config,
+                                   size_t rowBytes) const SK_OVERRIDE { return false; }
+    void buildProgramDesc(const GrOptDrawState&,
+                          const GrProgramDesc::DescInfo&,
+                          GrGpu::DrawType,
+                          GrProgramDesc* desc) SK_OVERRIDE {}
+
+    void discard(GrRenderTarget*) SK_OVERRIDE {}
+
+    bool canCopySurface(const GrSurface* dst,
+                        const GrSurface* src,
+                        const SkIRect& srcRect,
+                        const SkIPoint& dstPoint) SK_OVERRIDE { return false; };
+
+    bool copySurface(GrSurface* dst,
+                     GrSurface* src,
+                     const SkIRect& srcRect,
+                     const SkIPoint& dstPoint) SK_OVERRIDE { return false; };
+
+    bool initCopySurfaceDstDesc(const GrSurface* src, GrSurfaceDesc* desc) SK_OVERRIDE {
+        return false;
+    }
 
 private:
-    virtual void onResetContext(uint32_t resetBits) { };
-    virtual GrTexture* onCreateTexture(const GrSurfaceDesc& desc,
-                                       const void* srcData,
-                                       size_t rowBytes)  SK_OVERRIDE {
-        return NULL;
-    }
+    void onResetContext(uint32_t resetBits) SK_OVERRIDE {}
 
-    virtual GrTexture* onCreateCompressedTexture(const GrSurfaceDesc& desc,
-                                                 const void* srcData)  SK_OVERRIDE {
+    GrTexture* onCreateTexture(const GrSurfaceDesc& desc,
+                               const void* srcData,
+                               size_t rowBytes) SK_OVERRIDE {
         return NULL;
     }
 
-    virtual GrTexture* onWrapBackendTexture(const GrBackendTextureDesc&)  SK_OVERRIDE {
+    GrTexture* onCreateCompressedTexture(const GrSurfaceDesc& desc,
+                                         const void* srcData) SK_OVERRIDE {
         return NULL;
     }
 
-    virtual GrRenderTarget* onWrapBackendRenderTarget(
-                                    const GrBackendRenderTargetDesc&) SK_OVERRIDE {
-        return NULL;
-    }
+    GrTexture* onWrapBackendTexture(const GrBackendTextureDesc&) SK_OVERRIDE { return NULL; }
 
-    virtual GrVertexBuffer* onCreateVertexBuffer(size_t size, bool dynamic)  SK_OVERRIDE {
+    GrRenderTarget* onWrapBackendRenderTarget(const GrBackendRenderTargetDesc&) SK_OVERRIDE {
         return NULL;
     }
 
-    virtual GrIndexBuffer* onCreateIndexBuffer(size_t size, bool dynamic)  SK_OVERRIDE {
-        return NULL;
-    }
+    GrVertexBuffer* onCreateVertexBuffer(size_t size, bool dynamic) SK_OVERRIDE { return NULL; }
 
-    virtual void onClear(GrRenderTarget*, const SkIRect* rect, GrColor color,
-                         bool canIgnoreRect)  SK_OVERRIDE { }
+    GrIndexBuffer* onCreateIndexBuffer(size_t size, bool dynamic) SK_OVERRIDE { return NULL; }
 
-    virtual void onClearStencilClip(GrRenderTarget*,
-                                    const SkIRect& rect,
-                                    bool insideClip)  SK_OVERRIDE { }
+    void onClear(GrRenderTarget*, const SkIRect* rect, GrColor color,
+                         bool canIgnoreRect) SK_OVERRIDE {}
 
-    virtual void onDraw(const GrOptDrawState&, const GrDrawTarget::DrawInfo&)  SK_OVERRIDE { }
-    virtual bool onReadPixels(GrRenderTarget* target,
-                              int left, int top, int width, int height,
-                              GrPixelConfig,
-                              void* buffer,
-                              size_t rowBytes)  SK_OVERRIDE {
+    void onClearStencilClip(GrRenderTarget*, const SkIRect& rect, bool insideClip) SK_OVERRIDE {}
+
+    void onDraw(const GrOptDrawState&, const GrDrawTarget::DrawInfo&) SK_OVERRIDE {}
+
+    bool onReadPixels(GrRenderTarget* target,
+                      int left, int top, int width, int height,
+                      GrPixelConfig,
+                      void* buffer,
+                      size_t rowBytes) SK_OVERRIDE {
         return false;
     }
 
-    virtual bool onWriteTexturePixels(GrTexture* texture,
-                                      int left, int top, int width, int height,
-                                      GrPixelConfig config, const void* buffer,
-                                      size_t rowBytes)  SK_OVERRIDE {
+    bool onWriteTexturePixels(GrTexture* texture,
+                              int left, int top, int width, int height,
+                              GrPixelConfig config, const void* buffer,
+                              size_t rowBytes) SK_OVERRIDE {
         return false;
     }
 
-    virtual void onResolveRenderTarget(GrRenderTarget* target)  SK_OVERRIDE {
-        return;
-    }
+    void onResolveRenderTarget(GrRenderTarget* target) SK_OVERRIDE { return; }
 
-    virtual bool createStencilBufferForRenderTarget(GrRenderTarget*, int width,
-                                                    int height) SK_OVERRIDE {
+    bool createStencilBufferForRenderTarget(GrRenderTarget*, int width, int height) SK_OVERRIDE {
         return false;
     }
 
-    virtual bool attachStencilBufferToRenderTarget(GrStencilBuffer*, GrRenderTarget*)  SK_OVERRIDE {
+    bool attachStencilBufferToRenderTarget(GrStencilBuffer*, GrRenderTarget*) SK_OVERRIDE {
         return false;
     }
 
-    virtual bool flushGraphicsState(const GrOptDrawState&, DrawType)  SK_OVERRIDE {
-        return false;
-    }
+    bool flushGraphicsState(const GrOptDrawState&, DrawType) SK_OVERRIDE { return false; }
+
+    void clearStencil(GrRenderTarget* target) SK_OVERRIDE  {}
 
-    virtual void clearStencil(GrRenderTarget* target)  SK_OVERRIDE  { }
+    void didAddGpuTraceMarker() SK_OVERRIDE { }
 
-    virtual void didAddGpuTraceMarker() SK_OVERRIDE { }
-    virtual void didRemoveGpuTraceMarker() SK_OVERRIDE { }
+    void didRemoveGpuTraceMarker() SK_OVERRIDE { }
 
     typedef GrGpu INHERITED;
 };
diff --git a/src/gpu/gl/GrGpuGL.cpp b/src/gpu/gl/GrGpuGL.cpp
index 8bf35c1690..32da5d7798 100644
--- a/src/gpu/gl/GrGpuGL.cpp
+++ b/src/gpu/gl/GrGpuGL.cpp
@@ -2362,30 +2362,47 @@ GrGLuint GrGpuGL::bindSurfaceAsFBO(GrSurface* surface, GrGLenum fboTarget, GrGLI
     return tempFBOID;
 }
 
-void GrGpuGL::initCopySurfaceDstDesc(const GrSurface* src, GrSurfaceDesc* desc) {
+bool GrGpuGL::initCopySurfaceDstDesc(const GrSurface* src, GrSurfaceDesc* desc) {
+    // In here we look for opportunities to use CopyTexSubImage, or fbo blit. If neither are
+    // possible and we return false to fallback to creating a render target dst for render-to-
+    // texture. This code prefers CopyTexSubImage to fbo blit and avoids triggering temporary fbo
+    // creation. It isn't clear that avoiding temporary fbo creation is actually optimal.
+
     // Check for format issues with glCopyTexSubImage2D
     if (kGLES_GrGLStandard == this->glStandard() && this->glCaps().bgraIsInternalFormat() &&
         kBGRA_8888_GrPixelConfig == src->config()) {
-        // glCopyTexSubImage2D doesn't work with this config. We'll want to make it a render target
-        // in order to call glBlitFramebuffer or to copy to it by rendering.
-        INHERITED::initCopySurfaceDstDesc(src, desc);
-        return;
+        // glCopyTexSubImage2D doesn't work with this config. If the bgra can be used with fbo blit
+        // then we set up for that, otherwise fail.
+        if (this->caps()->isConfigRenderable(kBGRA_8888_GrPixelConfig, false)) {
+            desc->fOrigin = kDefault_GrSurfaceOrigin;
+            desc->fFlags = kRenderTarget_GrSurfaceFlag | kNoStencil_GrSurfaceFlag;
+            desc->fConfig = kBGRA_8888_GrPixelConfig;
+            return true;
+        }
+        return false;
     } else if (NULL == src->asRenderTarget()) {
-        // We don't want to have to create an FBO just to use glCopyTexSubImage2D. Let the base
-        // class handle it by rendering.
-        INHERITED::initCopySurfaceDstDesc(src, desc);
-        return;
+        // CopyTexSubImage2D or fbo blit would require creating a temp fbo for the src.
+        return false;
     }
 
     const GrGLRenderTarget* srcRT = static_cast<const GrGLRenderTarget*>(src->asRenderTarget());
     if (srcRT && srcRT->renderFBOID() != srcRT->textureFBOID()) {
-        // It's illegal to call CopyTexSubImage2D on a MSAA renderbuffer.
-        INHERITED::initCopySurfaceDstDesc(src, desc);
-    } else {
-        desc->fConfig = src->config();
-        desc->fOrigin = src->origin();
-        desc->fFlags = kNone_GrSurfaceFlags;
+        // It's illegal to call CopyTexSubImage2D on a MSAA renderbuffer. Set up for FBO blit or
+        // fail.
+        if (this->caps()->isConfigRenderable(src->config(), false)) {
+            desc->fOrigin = kDefault_GrSurfaceOrigin;
+            desc->fFlags = kRenderTarget_GrSurfaceFlag | kNoStencil_GrSurfaceFlag;
+            desc->fConfig = src->config();
+            return true;
+        }
+        return false;
     }
+
+    // We'll do a CopyTexSubImage. Make the dst a plain old texture.
+    desc->fConfig = src->config();
+    desc->fOrigin = src->origin();
+    desc->fFlags = kNone_GrSurfaceFlags;
+    return true;
 }
 
 bool GrGpuGL::copySurface(GrSurface* dst,
@@ -2496,8 +2513,8 @@ bool GrGpuGL::canCopySurface(const GrSurface* dst,
                              const SkIRect& srcRect,
                              const SkIPoint& dstPoint) {
     // This mirrors the logic in onCopySurface.  We prefer our base makes the copy if we need to
-    // create a temp fbo
-    // TODO verify this assumption, it may not be true at all
+    // create a temp fbo. TODO verify the assumption that temp fbos are expensive; it may not be
+    // true at all.
     bool wouldNeedTempFBO = false;
     if (can_copy_texsubimage(dst, src, this, &wouldNeedTempFBO) && !wouldNeedTempFBO) {
         return true;
diff --git a/src/gpu/gl/GrGpuGL.h b/src/gpu/gl/GrGpuGL.h
index a176c95bca..4224ba64fd 100644
--- a/src/gpu/gl/GrGpuGL.h
+++ b/src/gpu/gl/GrGpuGL.h
@@ -68,7 +68,7 @@ public:
                                     size_t rowBytes) const SK_OVERRIDE;
     virtual bool fullReadPixelsIsFasterThanPartial() const SK_OVERRIDE;
 
-    virtual void initCopySurfaceDstDesc(const GrSurface* src, GrSurfaceDesc* desc) SK_OVERRIDE;
+    virtual bool initCopySurfaceDstDesc(const GrSurface* src, GrSurfaceDesc* desc) SK_OVERRIDE;
 
     // These functions should be used to bind GL objects. They track the GL state and skip redundant
     // bindings. Making the equivalent glBind calls directly will confuse the state tracking.
@@ -94,16 +94,15 @@ public:
         fHWGeometryState.notifyIndexBufferDelete(id);
     }
 
-    // DrawTarget overrides
-    virtual bool copySurface(GrSurface* dst,
-                             GrSurface* src,
-                             const SkIRect& srcRect,
-                             const SkIPoint& dstPoint) SK_OVERRIDE;
+    bool copySurface(GrSurface* dst,
+                     GrSurface* src,
+                     const SkIRect& srcRect,
+                     const SkIPoint& dstPoint) SK_OVERRIDE;
 
-    virtual bool canCopySurface(const GrSurface* dst,
-                                const GrSurface* src,
-                                const SkIRect& srcRect,
-                                const SkIPoint& dstPoint) SK_OVERRIDE;
+    bool canCopySurface(const GrSurface* dst,
+                        const GrSurface* src,
+                        const SkIRect& srcRect,
+                        const SkIPoint& dstPoint) SK_OVERRIDE;
 
 protected:
     virtual void buildProgramDesc(const GrOptDrawState&,