From a702415d9e80f5631181143c293498de924e8ae4 Mon Sep 17 00:00:00 2001 From: joshualitt Date: Mon, 3 Nov 2014 14:16:35 -0800 Subject: Temporary fix to remove drawrect call from GpuGL BUG=skia: Committed: https://skia.googlesource.com/skia/+/d4a5c2028117c100ccf44263c0118a0b4745f627 Review URL: https://codereview.chromium.org/694933002 --- src/gpu/GrDrawTarget.cpp | 67 ++++++++++++++++------------------------- src/gpu/GrDrawTarget.h | 39 ++++++------------------ src/gpu/GrInOrderDrawBuffer.cpp | 22 ++++++++------ src/gpu/GrInOrderDrawBuffer.h | 18 ++++++----- src/gpu/gl/GrGpuGL.cpp | 39 +++++++++++------------- src/gpu/gl/GrGpuGL.h | 19 ++++++------ 6 files changed, 86 insertions(+), 118 deletions(-) diff --git a/src/gpu/GrDrawTarget.cpp b/src/gpu/GrDrawTarget.cpp index 2f25272e31..cd422ea525 100644 --- a/src/gpu/GrDrawTarget.cpp +++ b/src/gpu/GrDrawTarget.cpp @@ -916,13 +916,30 @@ bool GrDrawTarget::copySurface(GrSurface* dst, dstPoint, &clippedSrcRect, &clippedDstPoint)) { - SkASSERT(this->canCopySurface(dst, src, srcRect, dstPoint)); + SkASSERT(GrDrawTarget::canCopySurface(dst, src, srcRect, dstPoint)); return true; } - bool result = this->onCopySurface(dst, src, clippedSrcRect, clippedDstPoint); - SkASSERT(result == this->canCopySurface(dst, src, clippedSrcRect, clippedDstPoint)); - return result; + if (!GrDrawTarget::canCopySurface(dst, src, clippedSrcRect, clippedDstPoint)) { + return false; + } + + GrRenderTarget* rt = dst->asRenderTarget(); + GrTexture* tex = src->asTexture(); + + GrDrawTarget::AutoStateRestore asr(this, kReset_ASRInit); + this->drawState()->setRenderTarget(rt); + SkMatrix matrix; + matrix.setTranslate(SkIntToScalar(clippedSrcRect.fLeft - clippedDstPoint.fX), + SkIntToScalar(clippedSrcRect.fTop - clippedDstPoint.fY)); + matrix.postIDiv(tex->width(), tex->height()); + this->drawState()->addColorTextureProcessor(tex, matrix); + SkIRect dstRect = SkIRect::MakeXYWH(clippedDstPoint.fX, + clippedDstPoint.fY, + clippedSrcRect.width(), + clippedSrcRect.height()); + this->drawSimpleRect(dstRect); + return true; } bool GrDrawTarget::canCopySurface(GrSurface* dst, @@ -943,49 +960,17 @@ bool GrDrawTarget::canCopySurface(GrSurface* dst, &clippedDstPoint)) { return true; } - return this->onCanCopySurface(dst, src, clippedSrcRect, clippedDstPoint); -} -bool GrDrawTarget::onCanCopySurface(GrSurface* dst, - GrSurface* src, - const SkIRect& srcRect, - const SkIPoint& dstPoint) { // Check that the read/write rects are contained within the src/dst bounds. - SkASSERT(!srcRect.isEmpty()); - SkASSERT(SkIRect::MakeWH(src->width(), src->height()).contains(srcRect)); - SkASSERT(dstPoint.fX >= 0 && dstPoint.fY >= 0); - SkASSERT(dstPoint.fX + srcRect.width() <= dst->width() && - dstPoint.fY + srcRect.height() <= dst->height()); + SkASSERT(!clippedSrcRect.isEmpty()); + SkASSERT(SkIRect::MakeWH(src->width(), src->height()).contains(clippedSrcRect)); + SkASSERT(clippedDstPoint.fX >= 0 && clippedDstPoint.fY >= 0); + SkASSERT(clippedDstPoint.fX + clippedSrcRect.width() <= dst->width() && + clippedDstPoint.fY + clippedSrcRect.height() <= dst->height()); return !dst->surfacePriv().isSameAs(src) && dst->asRenderTarget() && src->asTexture(); } -bool GrDrawTarget::onCopySurface(GrSurface* dst, - GrSurface* src, - const SkIRect& srcRect, - const SkIPoint& dstPoint) { - if (!GrDrawTarget::onCanCopySurface(dst, src, srcRect, dstPoint)) { - return false; - } - - GrRenderTarget* rt = dst->asRenderTarget(); - GrTexture* tex = src->asTexture(); - - GrDrawTarget::AutoStateRestore asr(this, kReset_ASRInit); - this->drawState()->setRenderTarget(rt); - SkMatrix matrix; - matrix.setTranslate(SkIntToScalar(srcRect.fLeft - dstPoint.fX), - SkIntToScalar(srcRect.fTop - dstPoint.fY)); - matrix.postIDiv(tex->width(), tex->height()); - this->drawState()->addColorTextureProcessor(tex, matrix); - SkIRect dstRect = SkIRect::MakeXYWH(dstPoint.fX, - dstPoint.fY, - srcRect.width(), - srcRect.height()); - this->drawSimpleRect(dstRect); - return true; -} - 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; diff --git a/src/gpu/GrDrawTarget.h b/src/gpu/GrDrawTarget.h index 955581b683..4e922d723d 100644 --- a/src/gpu/GrDrawTarget.h +++ b/src/gpu/GrDrawTarget.h @@ -452,18 +452,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. */ - bool copySurface(GrSurface* dst, - GrSurface* src, - const SkIRect& srcRect, - const SkIPoint& dstPoint); + virtual bool copySurface(GrSurface* dst, + GrSurface* src, + const SkIRect& srcRect, + const SkIPoint& dstPoint); /** * Function that determines whether a copySurface call would succeed without * performing the copy. */ - bool canCopySurface(GrSurface* dst, - GrSurface* src, - const SkIRect& srcRect, - const SkIPoint& dstPoint); + virtual bool canCopySurface(GrSurface* dst, + 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 @@ -740,25 +740,6 @@ protected: } } - // 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. - virtual bool onCopySurface(GrSurface* dst, - GrSurface* src, - const SkIRect& srcRect, - const SkIPoint& dstPoint); - - // 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. - virtual bool onCanCopySurface(GrSurface* dst, - GrSurface* src, - const SkIRect& srcRect, - const SkIPoint& dstPoint); - GrContext* getContext() { return fContext; } const GrContext* getContext() const { return fContext; } @@ -921,7 +902,7 @@ private: // Check to see if this set of draw commands has been sent out virtual bool isIssued(uint32_t drawID) { return true; } - virtual GrClipMaskManager* getClipMaskManager() = 0; + virtual GrClipMaskManager* clipMaskManager() = 0; enum { kPreallocGeoSrcStateStackCnt = 4, @@ -977,7 +958,7 @@ protected: GrClipMaskManager fClipMaskManager; private: - GrClipMaskManager* getClipMaskManager() { return &fClipMaskManager; } + GrClipMaskManager* clipMaskManager() { return &fClipMaskManager; } typedef GrDrawTarget INHERITED; }; diff --git a/src/gpu/GrInOrderDrawBuffer.cpp b/src/gpu/GrInOrderDrawBuffer.cpp index 04ce17b8b6..04b9ba23d8 100644 --- a/src/gpu/GrInOrderDrawBuffer.cpp +++ b/src/gpu/GrInOrderDrawBuffer.cpp @@ -582,26 +582,30 @@ void GrInOrderDrawBuffer::CopySurface::execute(GrClipTarget* gpu) { gpu->copySurface(this->dst(), this->src(), fSrcRect, fDstPoint); } -bool GrInOrderDrawBuffer::onCopySurface(GrSurface* dst, - GrSurface* src, - const SkIRect& srcRect, - const SkIPoint& dstPoint) { +bool GrInOrderDrawBuffer::copySurface(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; } } -bool GrInOrderDrawBuffer::onCanCopySurface(GrSurface* dst, - GrSurface* src, - const SkIRect& srcRect, - const SkIPoint& dstPoint) { - return fDstGpu->canCopySurface(dst, src, srcRect, dstPoint); +bool GrInOrderDrawBuffer::canCopySurface(GrSurface* dst, + GrSurface* src, + const SkIRect& srcRect, + const SkIPoint& dstPoint) { + return fDstGpu->canCopySurface(dst, src, srcRect, dstPoint) || + GrDrawTarget::canCopySurface(dst, src, srcRect, dstPoint); } void GrInOrderDrawBuffer::initCopySurfaceDstDesc(const GrSurface* src, GrSurfaceDesc* desc) { diff --git a/src/gpu/GrInOrderDrawBuffer.h b/src/gpu/GrInOrderDrawBuffer.h index e6971a9e47..1e8b5a6d3c 100644 --- a/src/gpu/GrInOrderDrawBuffer.h +++ b/src/gpu/GrInOrderDrawBuffer.h @@ -76,6 +76,16 @@ public: virtual bool geometryHints(int* vertexCount, int* indexCount) const SK_OVERRIDE; + virtual bool copySurface(GrSurface* dst, + GrSurface* src, + const SkIRect& srcRect, + const SkIPoint& dstPoint) SK_OVERRIDE; + + virtual bool canCopySurface(GrSurface* dst, + GrSurface* src, + const SkIRect& srcRect, + const SkIPoint& dstPoint) SK_OVERRIDE; + virtual void clear(const SkIRect* rect, GrColor color, bool canIgnoreRect, @@ -282,14 +292,6 @@ private: virtual void geometrySourceWillPop(const GeometrySrcState& restoredState) SK_OVERRIDE; virtual void willReserveVertexAndIndexSpace(int vertexCount, int indexCount) SK_OVERRIDE; - virtual bool onCopySurface(GrSurface* dst, - GrSurface* src, - const SkIRect& srcRect, - const SkIPoint& dstPoint) SK_OVERRIDE; - virtual bool onCanCopySurface(GrSurface* dst, - GrSurface* src, - const SkIRect& srcRect, - const SkIPoint& dstPoint) SK_OVERRIDE; bool quickInsideClip(const SkRect& devBounds); diff --git a/src/gpu/gl/GrGpuGL.cpp b/src/gpu/gl/GrGpuGL.cpp index 621bef8f87..1a5048aa32 100644 --- a/src/gpu/gl/GrGpuGL.cpp +++ b/src/gpu/gl/GrGpuGL.cpp @@ -2388,15 +2388,12 @@ void GrGpuGL::initCopySurfaceDstDesc(const GrSurface* src, GrSurfaceDesc* desc) } } -bool GrGpuGL::onCopySurface(GrSurface* dst, - GrSurface* src, - const SkIRect& srcRect, - const SkIPoint& dstPoint) { - bool inheritedCouldCopy = INHERITED::onCanCopySurface(dst, src, srcRect, dstPoint); +bool GrGpuGL::copySurface(GrSurface* dst, + GrSurface* src, + const SkIRect& srcRect, + const SkIPoint& dstPoint) { bool copied = false; - bool wouldNeedTempFBO = false; - if (can_copy_texsubimage(dst, src, this, &wouldNeedTempFBO) && - (!wouldNeedTempFBO || !inheritedCouldCopy)) { + if (can_copy_texsubimage(dst, src, this)) { GrGLuint srcFBO; GrGLIRect srcVP; srcFBO = this->bindSurfaceAsFBO(src, GR_GL_FRAMEBUFFER, &srcVP); @@ -2428,8 +2425,7 @@ bool GrGpuGL::onCopySurface(GrSurface* dst, if (srcFBO) { GL_CALL(DeleteFramebuffers(1, &srcFBO)); } - } else if (can_blit_framebuffer(dst, src, this, &wouldNeedTempFBO) && - (!wouldNeedTempFBO || !inheritedCouldCopy)) { + } else if (can_blit_framebuffer(dst, src, this)) { SkIRect dstRect = SkIRect::MakeXYWH(dstPoint.fX, dstPoint.fY, srcRect.width(), srcRect.height()); bool selfOverlap = false; @@ -2492,22 +2488,21 @@ bool GrGpuGL::onCopySurface(GrSurface* dst, copied = true; } } - if (!copied && inheritedCouldCopy) { - copied = INHERITED::onCopySurface(dst, src, srcRect, dstPoint); - SkASSERT(copied); - } return copied; } -bool GrGpuGL::onCanCopySurface(GrSurface* dst, - GrSurface* src, - const SkIRect& srcRect, - const SkIPoint& dstPoint) { - // This mirrors the logic in onCopySurface. - if (can_copy_texsubimage(dst, src, this)) { +bool GrGpuGL::canCopySurface(GrSurface* dst, + GrSurface* src, + 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 + bool wouldNeedTempFBO = false; + if (can_copy_texsubimage(dst, src, this, &wouldNeedTempFBO) && !wouldNeedTempFBO) { return true; } - if (can_blit_framebuffer(dst, src, this)) { + if (can_blit_framebuffer(dst, src, this, &wouldNeedTempFBO) && !wouldNeedTempFBO) { if (dst->surfacePriv().isSameAs(src)) { SkIRect dstRect = SkIRect::MakeXYWH(dstPoint.fX, dstPoint.fY, srcRect.width(), srcRect.height()); @@ -2518,7 +2513,7 @@ bool GrGpuGL::onCanCopySurface(GrSurface* dst, return true; } } - return INHERITED::onCanCopySurface(dst, src, srcRect, dstPoint); + return false; } void GrGpuGL::didAddGpuTraceMarker() { diff --git a/src/gpu/gl/GrGpuGL.h b/src/gpu/gl/GrGpuGL.h index e7266f3fa2..63fcea6e1f 100644 --- a/src/gpu/gl/GrGpuGL.h +++ b/src/gpu/gl/GrGpuGL.h @@ -96,17 +96,18 @@ public: fHWGeometryState.notifyIndexBufferDelete(id); } -protected: - virtual bool onCopySurface(GrSurface* dst, - GrSurface* src, - const SkIRect& srcRect, - const SkIPoint& dstPoint) SK_OVERRIDE; + // DrawTarget overrides + virtual bool copySurface(GrSurface* dst, + GrSurface* src, + const SkIRect& srcRect, + const SkIPoint& dstPoint) SK_OVERRIDE; - virtual bool onCanCopySurface(GrSurface* dst, - GrSurface* src, - const SkIRect& srcRect, - const SkIPoint& dstPoint) SK_OVERRIDE; + virtual bool canCopySurface(GrSurface* dst, + GrSurface* src, + const SkIRect& srcRect, + const SkIPoint& dstPoint) SK_OVERRIDE; +protected: virtual void buildProgramDesc(const GrOptDrawState&, const GrProgramDesc::DescInfo&, GrGpu::DrawType, -- cgit v1.2.3