From c5243786217871fad293f99a48ba406ad84412b6 Mon Sep 17 00:00:00 2001 From: Brian Salomon Date: Thu, 2 Apr 2020 12:50:34 -0400 Subject: [PATCH] Don't require color type to make proxy copies. Bug: skia:10078 Change-Id: I3ce0d97f8ada55403cc3f88bb16659085449ea29 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/281207 Commit-Queue: Brian Salomon Reviewed-by: Greg Daniel Reviewed-by: Michael Ludwig --- gm/image_pict.cpp | 23 ++++---- src/core/SkSpecialImage.cpp | 9 ++-- src/gpu/GrAHardwareBufferImageGenerator.cpp | 5 +- src/gpu/GrBackendTextureImageGenerator.cpp | 5 +- src/gpu/GrBitmapTextureMaker.cpp | 39 +++++++------- src/gpu/GrRenderTargetContext.cpp | 36 +++++++------ src/gpu/GrSurfaceContext.cpp | 34 ++++++------ src/gpu/GrSurfaceContext.h | 14 +++-- src/gpu/GrSurfaceProxy.cpp | 58 ++++++++++----------- src/gpu/GrSurfaceProxy.h | 32 ++++++------ src/gpu/GrSurfaceProxyView.h | 14 +++++ src/gpu/GrTextureAdjuster.cpp | 10 ++-- src/gpu/SkGpuDevice.cpp | 17 +++--- src/gpu/SkGr.cpp | 34 +++++++----- src/gpu/SkGr.h | 16 ++++-- src/image/SkImage_Gpu.cpp | 8 +-- src/image/SkImage_GpuBase.cpp | 9 ++-- src/image/SkImage_GpuYUVA.cpp | 18 +++---- src/image/SkImage_Lazy.cpp | 19 ++++--- src/image/SkSurface_Gpu.cpp | 16 ++---- tests/CopySurfaceTest.cpp | 3 +- tests/GrSurfaceTest.cpp | 4 +- tests/MtlCopySurfaceTest.mm | 14 ++--- tests/RectangleTextureTest.cpp | 2 +- tests/TestUtils.cpp | 10 ++-- 25 files changed, 223 insertions(+), 226 deletions(-) diff --git a/gm/image_pict.cpp b/gm/image_pict.cpp index 629eeea903..c43fe54d6e 100644 --- a/gm/image_pict.cpp +++ b/gm/image_pict.cpp @@ -183,9 +183,11 @@ public: } } protected: - GrSurfaceProxyView onGenerateTexture(GrRecordingContext* ctx, const SkImageInfo& info, - const SkIPoint& origin, GrMipMapped mipMapped, - GrImageTexGenPolicy) override { + GrSurfaceProxyView onGenerateTexture(GrRecordingContext* ctx, + const SkImageInfo& info, + const SkIPoint& origin, + GrMipMapped mipMapped, + GrImageTexGenPolicy policy) override { SkASSERT(ctx); SkASSERT(ctx == fCtx.get()); @@ -193,17 +195,16 @@ protected: return {}; } - if (origin.fX == 0 && origin.fY == 0 && info.dimensions() == fView.proxy()->dimensions()) { + if (origin.fX == 0 && origin.fY == 0 && info.dimensions() == fView.proxy()->dimensions() && + policy == GrImageTexGenPolicy::kDraw) { return fView; } - - // TODO: When we update this function to return a view instead of just a proxy then we can - // remove the extra ref that happens when we call asTextureProxyRef. - return GrSurfaceProxy::Copy( - fCtx.get(), fView.proxy(), fView.origin(), - SkColorTypeToGrColorType(info.colorType()), mipMapped, + auto budgeted = policy == GrImageTexGenPolicy::kNew_Uncached_Budgeted ? SkBudgeted::kYes + : SkBudgeted::kNo; + return GrSurfaceProxyView::Copy( + fCtx.get(), fView, mipMapped, SkIRect::MakeXYWH(origin.x(), origin.y(), info.width(), info.height()), - SkBackingFit::kExact, SkBudgeted::kYes); + SkBackingFit::kExact, budgeted); } private: diff --git a/src/core/SkSpecialImage.cpp b/src/core/SkSpecialImage.cpp index 9dd1e5d085..065d4fe542 100644 --- a/src/core/SkSpecialImage.cpp +++ b/src/core/SkSpecialImage.cpp @@ -484,7 +484,6 @@ public: fAlphaType); } - // TODO: move all the logic here into the subset-flavor GrSurfaceProxy::copy? sk_sp onAsImage(const SkIRect* subset) const override { GrSurfaceProxy* proxy = fView.proxy(); if (subset) { @@ -495,11 +494,9 @@ public: fColorSpace); } - GrSurfaceProxyView subsetView = - GrSurfaceProxy::Copy(fContext, proxy, fView.origin(), fColorType, - GrMipMapped::kNo, *subset, SkBackingFit::kExact, - SkBudgeted::kYes); - if (!subsetView.proxy()) { + auto subsetView = GrSurfaceProxyView::Copy(fContext, fView, GrMipMapped::kNo, *subset, + SkBackingFit::kExact, SkBudgeted::kYes); + if (!subsetView) { return nullptr; } SkASSERT(subsetView.asTextureProxy()); diff --git a/src/gpu/GrAHardwareBufferImageGenerator.cpp b/src/gpu/GrAHardwareBufferImageGenerator.cpp index 22b28402f1..4e603f3231 100644 --- a/src/gpu/GrAHardwareBufferImageGenerator.cpp +++ b/src/gpu/GrAHardwareBufferImageGenerator.cpp @@ -210,9 +210,8 @@ GrSurfaceProxyView GrAHardwareBufferImageGenerator::onGenerateTexture( ? SkBudgeted::kNo : SkBudgeted::kYes; - GrColorType grColorType = SkColorTypeToGrColorType(this->getInfo().colorType()); - return GrSurfaceProxy::Copy(context, texProxyView.proxy(), texProxyView.origin(), grColorType, - mipMapped, subset, SkBackingFit::kExact, budgeted); + return GrSurfaceProxyView::Copy(context, std::move(texProxyView), mipMapped, subset, + SkBackingFit::kExact, budgeted); } bool GrAHardwareBufferImageGenerator::onIsValid(GrContext* context) const { diff --git a/src/gpu/GrBackendTextureImageGenerator.cpp b/src/gpu/GrBackendTextureImageGenerator.cpp index 6f7bae4c99..24ccf07d62 100644 --- a/src/gpu/GrBackendTextureImageGenerator.cpp +++ b/src/gpu/GrBackendTextureImageGenerator.cpp @@ -219,7 +219,8 @@ GrSurfaceProxyView GrBackendTextureImageGenerator::onGenerateTexture( ? SkBudgeted::kNo : SkBudgeted::kYes; - return GrSurfaceProxy::Copy(context, proxy.get(), fSurfaceOrigin, grColorType, mipMapped, - subset, SkBackingFit::kExact, budgeted); + auto copy = GrSurfaceProxy::Copy(context, proxy.get(), fSurfaceOrigin, mipMapped, subset, + SkBackingFit::kExact, budgeted); + return {std::move(copy), fSurfaceOrigin, readSwizzle}; } } diff --git a/src/gpu/GrBitmapTextureMaker.cpp b/src/gpu/GrBitmapTextureMaker.cpp index 5b29ff7c78..25b03f23a3 100644 --- a/src/gpu/GrBitmapTextureMaker.cpp +++ b/src/gpu/GrBitmapTextureMaker.cpp @@ -71,7 +71,7 @@ GrSurfaceProxyView GrBitmapTextureMaker::refOriginalTextureProxyView(GrMipMapped swizzle = this->context()->priv().caps()->getReadSwizzle(proxy->backendFormat(), this->colorType()); if (mipMapped == GrMipMapped::kNo || proxy->mipMapped() == GrMipMapped::kYes) { - return GrSurfaceProxyView(std::move(proxy), kTopLeft_GrSurfaceOrigin, swizzle); + return {std::move(proxy), kTopLeft_GrSurfaceOrigin, swizzle}; } } } @@ -96,7 +96,7 @@ GrSurfaceProxyView GrBitmapTextureMaker::refOriginalTextureProxyView(GrMipMapped if (fKey.isValid()) { installKey(proxy.get()); } - return GrSurfaceProxyView(std::move(proxy), kTopLeft_GrSurfaceOrigin, swizzle); + return {std::move(proxy), kTopLeft_GrSurfaceOrigin, swizzle}; } } @@ -107,26 +107,23 @@ GrSurfaceProxyView GrBitmapTextureMaker::refOriginalTextureProxyView(GrMipMapped // We need a mipped proxy, but we found a proxy earlier that wasn't mipped. Thus we generate // a new mipped surface and copy the original proxy into the base layer. We will then let // the gpu generate the rest of the mips. - GrColorType srcColorType = SkColorTypeToGrColorType(fBitmap.colorType()); - auto mippedView = GrCopyBaseMipMapToTextureProxy(this->context(), proxy.get(), - kTopLeft_GrSurfaceOrigin, srcColorType); - if (auto mippedProxy = mippedView.asTextureProxy()) { - // In this case we are stealing the key from the original proxy which should only happen - // when we have just generated mipmaps for an originally unmipped proxy/texture. This - // means that all future uses of the key will access the mipmapped version. The texture - // backing the unmipped version will remain in the resource cache until the last texture - // proxy referencing it is deleted at which time it too will be deleted or recycled. - SkASSERT(proxy->getUniqueKey() == fKey); - SkASSERT(mippedView.origin() == kTopLeft_GrSurfaceOrigin); - SkASSERT(mippedView.swizzle() == swizzle); - proxyProvider->removeUniqueKeyFromProxy(proxy.get()); - installKey(mippedProxy); - return mippedView; + auto mippedProxy = GrCopyBaseMipMapToTextureProxy(this->context(), proxy.get(), + kTopLeft_GrSurfaceOrigin); + if (!mippedProxy) { + // We failed to make a mipped proxy with the base copied into it. This could have + // been from failure to make the proxy or failure to do the copy. Thus we will fall + // back to just using the non mipped proxy; See skbug.com/7094. + return {std::move(proxy), kTopLeft_GrSurfaceOrigin, swizzle}; } - // We failed to make a mipped proxy with the base copied into it. This could have - // been from failure to make the proxy or failure to do the copy. Thus we will fall - // back to just using the non mipped proxy; See skbug.com/7094. - return GrSurfaceProxyView(std::move(proxy), kTopLeft_GrSurfaceOrigin, swizzle); + // In this case we are stealing the key from the original proxy which should only happen + // when we have just generated mipmaps for an originally unmipped proxy/texture. This + // means that all future uses of the key will access the mipmapped version. The texture + // backing the unmipped version will remain in the resource cache until the last texture + // proxy referencing it is deleted at which time it too will be deleted or recycled. + SkASSERT(proxy->getUniqueKey() == fKey); + proxyProvider->removeUniqueKeyFromProxy(proxy.get()); + installKey(mippedProxy->asTextureProxy()); + return {std::move(mippedProxy), kTopLeft_GrSurfaceOrigin, swizzle}; } return {}; } diff --git a/src/gpu/GrRenderTargetContext.cpp b/src/gpu/GrRenderTargetContext.cpp index a3b93676a8..6730cf2b93 100644 --- a/src/gpu/GrRenderTargetContext.cpp +++ b/src/gpu/GrRenderTargetContext.cpp @@ -152,8 +152,11 @@ std::unique_ptr GrRenderTargetContext::Make( } const GrBackendFormat& format = proxy->backendFormat(); - GrSwizzle readSwizzle = context->priv().caps()->getReadSwizzle(format, colorType); - GrSwizzle writeSwizzle = context->priv().caps()->getWriteSwizzle(format, colorType); + GrSwizzle readSwizzle, writeSwizzle; + if (colorType != GrColorType::kUnknown) { + readSwizzle = context->priv().caps()->getReadSwizzle(format, colorType); + writeSwizzle = context->priv().caps()->getWriteSwizzle(format, colorType); + } GrSurfaceProxyView readView(proxy, origin, readSwizzle); GrSurfaceProxyView writeView(std::move(proxy), origin, writeSwizzle); @@ -1749,14 +1752,14 @@ void GrRenderTargetContext::asyncRescaleAndReadPixels( SkRect srcRectToDraw = SkRect::Make(srcRect); // If the src is not texturable first try to make a copy to a texture. if (!texProxyView.asTextureProxy()) { - texProxyView = GrSurfaceProxy::Copy(fContext, this->asSurfaceProxy(), - this->origin(), this->colorInfo().colorType(), - GrMipMapped::kNo, srcRect, - SkBackingFit::kApprox, SkBudgeted::kNo); - if (!texProxyView.asTextureProxy()) { + texProxyView = + GrSurfaceProxyView::Copy(fContext, texProxyView, GrMipMapped::kNo, srcRect, + SkBackingFit::kApprox, SkBudgeted::kNo); + if (!texProxyView) { callback(context, nullptr); return; } + SkASSERT(texProxyView.asTextureProxy()); srcRectToDraw = SkRect::MakeWH(srcRect.width(), srcRect.height()); } tempRTC = GrRenderTargetContext::Make( @@ -1966,14 +1969,14 @@ void GrRenderTargetContext::asyncRescaleAndReadPixelsYUV420(SkYUVColorSpace yuvC } else { srcView = this->readSurfaceView(); if (!srcView.asTextureProxy()) { - srcView = GrSurfaceProxy::Copy(fContext, fReadView.proxy(), this->origin(), - this->colorInfo().colorType(), GrMipMapped::kNo, - srcRect, SkBackingFit::kApprox, SkBudgeted::kYes); - if (!srcView.asTextureProxy()) { + srcView = GrSurfaceProxyView::Copy(fContext, std::move(srcView), GrMipMapped::kNo, + srcRect, SkBackingFit::kApprox, SkBudgeted::kYes); + if (!srcView) { // If we can't get a texture copy of the contents then give up. callback(context, nullptr); return; } + SkASSERT(srcView.asTextureProxy()); x = y = 0; } // We assume the caller wants kPremul. There is no way to indicate a preference. @@ -2604,13 +2607,12 @@ bool GrRenderTargetContext::setupDstProxyView(const GrClip& clip, const GrOp& op dstOffset = {copyRect.fLeft, copyRect.fTop}; fit = SkBackingFit::kApprox; } - GrSurfaceProxyView newProxyView = - GrSurfaceProxy::Copy(fContext, this->asSurfaceProxy(), this->origin(), colorType, - GrMipMapped::kNo, copyRect, fit, SkBudgeted::kYes, - restrictions.fRectsMustMatch); - SkASSERT(newProxyView.proxy()); + auto copy = + GrSurfaceProxy::Copy(fContext, this->asSurfaceProxy(), this->origin(), GrMipMapped::kNo, + copyRect, fit, SkBudgeted::kYes, restrictions.fRectsMustMatch); + SkASSERT(copy); - dstProxyView->setProxyView(std::move(newProxyView)); + dstProxyView->setProxyView({std::move(copy), this->origin(), this->readSwizzle()}); dstProxyView->setOffset(dstOffset); return true; } diff --git a/src/gpu/GrSurfaceContext.cpp b/src/gpu/GrSurfaceContext.cpp index 2936b11588..1f94a59bb5 100644 --- a/src/gpu/GrSurfaceContext.cpp +++ b/src/gpu/GrSurfaceContext.cpp @@ -47,8 +47,11 @@ std::unique_ptr GrSurfaceContext::Make(GrRecordingContext* con SkASSERT(kPremul_SkAlphaType == alphaType || kOpaque_SkAlphaType == alphaType); // Will we ever want a swizzle that is not the default write swizzle for the format and // colorType here? If so we will need to manually pass that in. - GrSwizzle writeSwizzle = - context->priv().caps()->getWriteSwizzle(proxy->backendFormat(), colorType); + GrSwizzle writeSwizzle; + if (colorType != GrColorType::kUnknown) { + writeSwizzle = + context->priv().caps()->getWriteSwizzle(proxy->backendFormat(), colorType); + } GrSurfaceProxyView writeView(readView.refProxy(), readView.origin(), writeSwizzle); surfaceContext.reset(new GrRenderTargetContext(context, std::move(readView), std::move(writeView), colorType, @@ -74,8 +77,8 @@ std::unique_ptr GrSurfaceContext::Make(GrRecordingContext* con sk_sp colorSpace, SkBackingFit fit, SkBudgeted budgeted) { - GrSwizzle swizzle("rgba"); - if (!context->priv().caps()->isFormatCompressed(format)) { + GrSwizzle swizzle; + if (colorType != GrColorType::kUnknown && !context->priv().caps()->isFormatCompressed(format)) { swizzle = context->priv().caps()->getReadSwizzle(format, colorType); } @@ -428,7 +431,7 @@ bool GrSurfaceContext::writePixels(const GrImageInfo& origSrcInfo, const void* s } else { SkIRect srcRect = SkIRect::MakeWH(srcInfo.width(), srcInfo.height()); SkIPoint dstPoint = SkIPoint::Make(pt.fX, pt.fY); - if (!this->copy(tempProxy.get(), tempOrigin, srcRect, dstPoint)) { + if (!this->copy(tempProxy.get(), srcRect, dstPoint)) { return false; } } @@ -472,8 +475,7 @@ bool GrSurfaceContext::writePixels(const GrImageInfo& origSrcInfo, const void* s srcColorType, src, rowBytes); } -bool GrSurfaceContext::copy(GrSurfaceProxy* src, GrSurfaceOrigin origin, const SkIRect& srcRect, - const SkIPoint& dstPoint) { +bool GrSurfaceContext::copy(GrSurfaceProxy* src, const SkIRect& srcRect, const SkIPoint& dstPoint) { ASSERT_SINGLE_OWNER RETURN_FALSE_IF_ABANDONED SkDEBUGCODE(this->validate();) @@ -483,7 +485,6 @@ bool GrSurfaceContext::copy(GrSurfaceProxy* src, GrSurfaceOrigin origin, const S SkASSERT(src->backendFormat().textureType() != GrTextureType::kExternal); SkASSERT(src->backendFormat() == this->asSurfaceProxy()->backendFormat()); - SkASSERT(origin == this->origin()); if (this->asSurfaceProxy()->framebufferOnly()) { return false; @@ -495,7 +496,7 @@ bool GrSurfaceContext::copy(GrSurfaceProxy* src, GrSurfaceOrigin origin, const S // The swizzle doesn't matter for copies and it is not used. return this->drawingManager()->newCopyRenderTask( - GrSurfaceProxyView(sk_ref_sp(src), origin, GrSwizzle()), srcRect, + GrSurfaceProxyView(sk_ref_sp(src), this->origin(), GrSwizzle("rgba")), srcRect, this->readSurfaceView(), dstPoint); } @@ -528,13 +529,11 @@ std::unique_ptr GrSurfaceContext::rescale( int srcY = srcRect.fTop; GrSurfaceProxyView texView = this->readSurfaceView(); SkCanvas::SrcRectConstraint constraint = SkCanvas::kStrict_SrcRectConstraint; - GrColorType srcColorType = this->colorInfo().colorType(); SkAlphaType srcAlphaType = this->colorInfo().alphaType(); if (!texView.asTextureProxy()) { - texView = GrSurfaceProxy::Copy(fContext, this->asSurfaceProxy(), this->origin(), - srcColorType, GrMipMapped::kNo, srcRect, - SkBackingFit::kApprox, SkBudgeted::kNo); - if (!texView.proxy()) { + texView = GrSurfaceProxyView::Copy(fContext, std::move(texView), GrMipMapped::kNo, srcRect, + SkBackingFit::kApprox, SkBudgeted::kNo); + if (!texView) { return nullptr; } SkASSERT(texView.asTextureProxy()); @@ -737,9 +736,10 @@ GrSurfaceContext::PixelTransferResult GrSurfaceContext::transferPixels(GrColorTy void GrSurfaceContext::validate() const { SkASSERT(fReadView.proxy()); fReadView.proxy()->validate(fContext); - SkASSERT(fContext->priv().caps()->areColorTypeAndFormatCompatible( - this->colorInfo().colorType(), fReadView.proxy()->backendFormat())); - + if (this->colorInfo().colorType() != GrColorType::kUnknown) { + SkASSERT(fContext->priv().caps()->areColorTypeAndFormatCompatible( + this->colorInfo().colorType(), fReadView.proxy()->backendFormat())); + } this->onValidate(); } #endif diff --git a/src/gpu/GrSurfaceContext.h b/src/gpu/GrSurfaceContext.h index 20f67b8b08..028cc52a94 100644 --- a/src/gpu/GrSurfaceContext.h +++ b/src/gpu/GrSurfaceContext.h @@ -65,6 +65,7 @@ public: // make a copy (which refs the proxy) if needed. GrSurfaceProxyView readSurfaceView() { return fReadView; } + SkISize dimensions() const { return fReadView.dimensions(); } int width() const { return fReadView.proxy()->width(); } int height() const { return fReadView.proxy()->height(); } @@ -120,13 +121,12 @@ public: const GrSurfaceContextPriv surfPriv() const; #if GR_TEST_UTILS - bool testCopy(GrSurfaceProxy* src, GrSurfaceOrigin origin, const SkIRect& srcRect, - const SkIPoint& dstPoint) { - return this->copy(src, origin, srcRect, dstPoint); + bool testCopy(GrSurfaceProxy* src, const SkIRect& srcRect, const SkIPoint& dstPoint) { + return this->copy(src, srcRect, dstPoint); } - bool testCopy(GrSurfaceProxy* src, GrSurfaceOrigin origin) { - return this->copy(src, origin, SkIRect::MakeSize(src->dimensions()), SkIPoint::Make(0, 0)); + bool testCopy(GrSurfaceProxy* src) { + return this->copy(src, SkIRect::MakeSize(src->dimensions()), {0, 0}); } #endif @@ -173,7 +173,6 @@ private: * Currently only writePixels and replaceRenderTarget call this directly. All other copies * should go through GrSurfaceProxy::Copy. * @param src src of pixels - * @param srcRect the subset of 'src' to copy * @param dstPoint the origin of the 'srcRect' in the destination coordinate space * @return true if the copy succeeded; false otherwise * @@ -183,8 +182,7 @@ private: * regions will not be shifted. The 'src' must have the same origin as the backing proxy * of fSurfaceContext. */ - bool copy(GrSurfaceProxy* src, GrSurfaceOrigin origin, const SkIRect& srcRect, - const SkIPoint& dstPoint); + bool copy(GrSurfaceProxy* src, const SkIRect& srcRect, const SkIPoint& dstPoint); GrColorInfo fColorInfo; diff --git a/src/gpu/GrSurfaceProxy.cpp b/src/gpu/GrSurfaceProxy.cpp index 37337656f4..b6abd53259 100644 --- a/src/gpu/GrSurfaceProxy.cpp +++ b/src/gpu/GrSurfaceProxy.cpp @@ -264,15 +264,14 @@ void GrSurfaceProxy::validate(GrContext_Base* context) const { } #endif -GrSurfaceProxyView GrSurfaceProxy::Copy(GrRecordingContext* context, - GrSurfaceProxy* src, - GrSurfaceOrigin origin, - GrColorType srcColorType, - GrMipMapped mipMapped, - SkIRect srcRect, - SkBackingFit fit, - SkBudgeted budgeted, - RectsMustMatch rectsMustMatch) { +sk_sp GrSurfaceProxy::Copy(GrRecordingContext* context, + GrSurfaceProxy* src, + GrSurfaceOrigin origin, + GrMipMapped mipMapped, + SkIRect srcRect, + SkBackingFit fit, + SkBudgeted budgeted, + RectsMustMatch rectsMustMatch) { SkASSERT(!src->isFullyLazy()); int width; int height; @@ -295,37 +294,36 @@ GrSurfaceProxyView GrSurfaceProxy::Copy(GrRecordingContext* context, SkASSERT(format.isValid()); if (src->backendFormat().textureType() != GrTextureType::kExternal) { - auto dstContext = GrSurfaceContext::Make(context, {width, height}, format, - GrRenderable::kNo, 1, mipMapped, - src->isProtected(), origin, srcColorType, - kUnknown_SkAlphaType, nullptr, fit, budgeted); - if (dstContext && dstContext->copy(src, origin, srcRect, dstPoint)) { - return dstContext->readSurfaceView(); + auto dstContext = + GrSurfaceContext::Make(context, {width, height}, format, GrRenderable::kNo, 1, + mipMapped, src->isProtected(), origin, GrColorType::kUnknown, + kUnknown_SkAlphaType, nullptr, fit, budgeted); + if (dstContext && dstContext->copy(src, srcRect, dstPoint)) { + return dstContext->asSurfaceProxyRef(); } } if (src->asTextureProxy()) { - auto dstContext = GrRenderTargetContext::Make(context, srcColorType, nullptr, fit, - {width, height}, format, 1, - mipMapped, src->isProtected(), origin, - budgeted, nullptr); - GrSwizzle swizzle = context->priv().caps()->getReadSwizzle(src->backendFormat(), - srcColorType); - GrSurfaceProxyView view(sk_ref_sp(src), origin, swizzle); + auto dstContext = GrRenderTargetContext::Make( + context, GrColorType::kUnknown, nullptr, fit, {width, height}, format, 1, mipMapped, + src->isProtected(), origin, budgeted, nullptr); + GrSurfaceProxyView view(sk_ref_sp(src), origin, GrSwizzle("rgba")); if (dstContext && dstContext->blitTexture(std::move(view), srcRect, dstPoint)) { - return dstContext->readSurfaceView(); + return dstContext->asSurfaceProxyRef(); } } // Can't use backend copies or draws. - return {}; + return nullptr; } -GrSurfaceProxyView GrSurfaceProxy::Copy(GrRecordingContext* context, GrSurfaceProxy* src, - GrSurfaceOrigin origin, GrColorType srcColorType, - GrMipMapped mipMapped, SkBackingFit fit, - SkBudgeted budgeted) { +sk_sp GrSurfaceProxy::Copy(GrRecordingContext* context, + GrSurfaceProxy* src, + GrSurfaceOrigin origin, + GrMipMapped mipMapped, + SkBackingFit fit, + SkBudgeted budgeted) { SkASSERT(!src->isFullyLazy()); - return Copy(context, src, origin, srcColorType, mipMapped, SkIRect::MakeSize(src->dimensions()), - fit, budgeted); + return Copy(context, src, origin, mipMapped, SkIRect::MakeSize(src->dimensions()), fit, + budgeted); } #if GR_TEST_UTILS diff --git a/src/gpu/GrSurfaceProxy.h b/src/gpu/GrSurfaceProxy.h index 064ed91df5..441250595c 100644 --- a/src/gpu/GrSurfaceProxy.h +++ b/src/gpu/GrSurfaceProxy.h @@ -285,23 +285,25 @@ public: // The copy is is not a render target and not multisampled. // // The intended use of this copy call is simply to copy exact pixel values from one proxy to a - // new one. Thus there isn't a need for a swizzle when doing the copy. Also, there shouldn't be - // an assumed "view" of the copy. However, even though not really needed for the swizzle, we - // still pass in a srcColorType since it is required for making a GrSurface/RenderTargetContext. - // Additionally, almost all callers of this will immediately put the resulting proxy into a view - // which is compatible with the srcColorType and origin passed in here. Thus for now we just - // return the GrSurfaceProxyView that is already stored on the internal GrSurfaceContext. If we - // later decide to not pass in a srcColorType (and assume some default color type based on the - // backend format) then we should go back to returning a proxy here and have the callers decide - // what view they want of the proxy. - static GrSurfaceProxyView Copy(GrRecordingContext*, GrSurfaceProxy* src, - GrSurfaceOrigin, GrColorType srcColorType, GrMipMapped, - SkIRect srcRect, SkBackingFit, SkBudgeted, - RectsMustMatch = RectsMustMatch::kNo); + // new one. Thus, there isn't a need for a swizzle when doing the copy. The format of the copy + // will be the same as the src. Therefore, the copy can be used in a view with the same swizzle + // as the original for use with a given color type. + static sk_sp Copy(GrRecordingContext*, + GrSurfaceProxy* src, + GrSurfaceOrigin, + GrMipMapped, + SkIRect srcRect, + SkBackingFit, + SkBudgeted, + RectsMustMatch = RectsMustMatch::kNo); // Same as above Copy but copies the entire 'src' - static GrSurfaceProxyView Copy(GrRecordingContext*, GrSurfaceProxy* src, GrSurfaceOrigin, - GrColorType srcColorType, GrMipMapped, SkBackingFit, SkBudgeted); + static sk_sp Copy(GrRecordingContext*, + GrSurfaceProxy* src, + GrSurfaceOrigin, + GrMipMapped, + SkBackingFit, + SkBudgeted); #if GR_TEST_UTILS int32_t testingOnly_getBackingRefCnt() const; diff --git a/src/gpu/GrSurfaceProxyView.h b/src/gpu/GrSurfaceProxyView.h index 5195d757a5..2a167c562a 100644 --- a/src/gpu/GrSurfaceProxyView.h +++ b/src/gpu/GrSurfaceProxyView.h @@ -76,6 +76,20 @@ public: *this = {}; } + // Helper that copies a rect of a src view'' proxy and then creates a view for the copy with + // the same origin and swizzle as the src view. + static GrSurfaceProxyView Copy(GrRecordingContext* context, + GrSurfaceProxyView src, + GrMipMapped mipMapped, + SkIRect srcRect, + SkBackingFit fit, + SkBudgeted budgeted) { + auto origin = src.origin(); + auto* proxy = src.proxy(); + auto copy = GrSurfaceProxy::Copy(context, proxy, origin, mipMapped, srcRect, fit, budgeted); + return {std::move(copy), src.origin(), src.swizzle()}; + } + // This does not reset the origin or swizzle, so the View can still be used to access those // properties associated with the detached proxy. sk_sp detachProxy() { diff --git a/src/gpu/GrTextureAdjuster.cpp b/src/gpu/GrTextureAdjuster.cpp index 2ce781c659..d3371d759d 100644 --- a/src/gpu/GrTextureAdjuster.cpp +++ b/src/gpu/GrTextureAdjuster.cpp @@ -38,17 +38,15 @@ GrSurfaceProxyView GrTextureAdjuster::makeMippedCopy() { } } - GrSurfaceProxyView copyView = GrCopyBaseMipMapToTextureProxy( - this->context(), fOriginal.proxy(), fOriginal.origin(), this->colorType()); - if (!copyView) { + auto copy = GrCopyBaseMipMapToView(this->context(), fOriginal); + if (!copy) { return {}; } if (mipMappedKey.isValid()) { - SkASSERT(copyView.origin() == fOriginal.origin()); // TODO: If we move listeners up from SkImage_Lazy to SkImage_Base then add one here. - proxyProvider->assignUniqueKeyToProxy(mipMappedKey, copyView.asTextureProxy()); + proxyProvider->assignUniqueKeyToProxy(mipMappedKey, copy.asTextureProxy()); } - return copyView; + return copy; } GrSurfaceProxyView GrTextureAdjuster::onView(GrMipMapped mipMapped) { diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp index e013a772eb..f693cf7c68 100644 --- a/src/gpu/SkGpuDevice.cpp +++ b/src/gpu/SkGpuDevice.cpp @@ -806,21 +806,18 @@ sk_sp SkGpuDevice::snapSpecial(const SkIRect& subset, bool force if (forceCopy || !view.asTextureProxy()) { // When the device doesn't have a texture, or a copy is requested, we create a temporary // texture that matches the device contents - view = GrSurfaceProxy::Copy(fContext.get(), - rtc->asSurfaceProxy(), - view.origin(), - rtc->colorInfo().colorType(), - GrMipMapped::kNo, // Don't auto generate mips - subset, - SkBackingFit::kApprox, - SkBudgeted::kYes); // Always budgeted + view = GrSurfaceProxyView::Copy(fContext.get(), + std::move(view), + GrMipMapped::kNo, // Don't auto generate mips + subset, + SkBackingFit::kApprox, + SkBudgeted::kYes); // Always budgeted if (!view) { return nullptr; } - // Since this copied only the requested subset, the special image wrapping the proxy no // longer needs the original subset. - finalSubset = SkIRect::MakeSize(view.proxy()->dimensions()); + finalSubset = SkIRect::MakeSize(view.dimensions()); } GrColorType ct = SkColorTypeToGrColorType(this->imageInfo().colorType()); diff --git a/src/gpu/SkGr.cpp b/src/gpu/SkGr.cpp index 05f7d6dc4e..d46bc64ae1 100644 --- a/src/gpu/SkGr.cpp +++ b/src/gpu/SkGr.cpp @@ -131,25 +131,31 @@ sk_sp GrMakeUniqueKeyInvalidationListener(GrUniqueKey* key, return std::move(listener); } -GrSurfaceProxyView GrCopyBaseMipMapToTextureProxy(GrRecordingContext* ctx, - GrSurfaceProxy* baseProxy, - GrSurfaceOrigin origin, - GrColorType srcColorType, - SkBudgeted budgeted) { +sk_sp GrCopyBaseMipMapToTextureProxy(GrRecordingContext* ctx, + GrSurfaceProxy* baseProxy, + GrSurfaceOrigin origin, + SkBudgeted budgeted) { SkASSERT(baseProxy); if (!ctx->priv().caps()->isFormatCopyable(baseProxy->backendFormat())) { return {}; } - GrSurfaceProxyView view = GrSurfaceProxy::Copy(ctx, - baseProxy, - origin, - srcColorType, - GrMipMapped::kYes, - SkBackingFit::kExact, - budgeted); - SkASSERT(!view.proxy() || view.asTextureProxy()); - return view; + auto copy = GrSurfaceProxy::Copy(ctx, baseProxy, origin, GrMipMapped::kYes, + SkBackingFit::kExact, budgeted); + if (!copy) { + return {}; + } + SkASSERT(copy->asTextureProxy()); + return copy; +} + +GrSurfaceProxyView GrCopyBaseMipMapToView(GrRecordingContext* context, + GrSurfaceProxyView src, + SkBudgeted budgeted) { + auto origin = src.origin(); + auto swizzle = src.swizzle(); + auto* proxy = src.proxy(); + return {GrCopyBaseMipMapToTextureProxy(context, proxy, origin, budgeted), origin, swizzle}; } GrSurfaceProxyView GrRefCachedBitmapView(GrRecordingContext* ctx, const SkBitmap& bitmap, diff --git a/src/gpu/SkGr.h b/src/gpu/SkGr.h index fec4be4405..e8cfc09a68 100644 --- a/src/gpu/SkGr.h +++ b/src/gpu/SkGr.h @@ -171,11 +171,17 @@ GrSurfaceProxyView GrRefCachedBitmapView(GrRecordingContext*, const SkBitmap&, G /** * Creates a new texture with mipmap levels and copies the baseProxy into the base layer. */ -GrSurfaceProxyView GrCopyBaseMipMapToTextureProxy(GrRecordingContext*, - GrSurfaceProxy* baseProxy, - GrSurfaceOrigin origin, - GrColorType srcColorType, - SkBudgeted = SkBudgeted::kYes); +sk_sp GrCopyBaseMipMapToTextureProxy(GrRecordingContext*, + GrSurfaceProxy* baseProxy, + GrSurfaceOrigin origin, + SkBudgeted = SkBudgeted::kYes); +/** + * Same as GrCopyBaseMipMapToTextureProxy but takes the src as a view and returns a view with same + * origin and swizzle as the src view. + */ +GrSurfaceProxyView GrCopyBaseMipMapToView(GrRecordingContext*, + GrSurfaceProxyView, + SkBudgeted = SkBudgeted::kYes); /* * Create a texture proxy from the provided bitmap and add it to the texture cache diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp index eef6298de8..1f3ff54bdc 100644 --- a/src/image/SkImage_Gpu.cpp +++ b/src/image/SkImage_Gpu.cpp @@ -437,15 +437,11 @@ sk_sp SkImage::makeTextureImage(GrContext* context, !context->priv().caps()->mipMapSupport()) { return sk_ref_sp(const_cast(this)); } - auto copy = GrCopyBaseMipMapToTextureProxy(context->priv().asRecordingContext(), - view->proxy(), - view->origin(), - SkColorTypeToGrColorType(this->colorType()), - budgeted); + auto copy = GrCopyBaseMipMapToView(context->priv().asRecordingContext(), *view, budgeted); if (!copy) { return nullptr; } - return sk_make_sp(sk_ref_sp(context), this->uniqueID(), std::move(copy), + return sk_make_sp(sk_ref_sp(context), this->uniqueID(), copy, this->colorType(), this->alphaType(), this->refColorSpace()); } diff --git a/src/image/SkImage_GpuBase.cpp b/src/image/SkImage_GpuBase.cpp index f9adbee316..5b2a5e2aad 100644 --- a/src/image/SkImage_GpuBase.cpp +++ b/src/image/SkImage_GpuBase.cpp @@ -143,13 +143,10 @@ sk_sp SkImage_GpuBase::onMakeSubset(GrRecordingContext* context, const GrSurfaceProxyView* view = this->view(context); SkASSERT(view && view->proxy()); - GrColorType grColorType = SkColorTypeToGrColorType(this->colorType()); + auto copyView = GrSurfaceProxyView::Copy(context, *view, GrMipMapped::kNo, subset, + SkBackingFit::kExact, view->proxy()->isBudgeted()); - GrSurfaceProxyView copyView = GrSurfaceProxy::Copy( - context, view->proxy(), view->origin(), grColorType, GrMipMapped::kNo, subset, - SkBackingFit::kExact, view->proxy()->isBudgeted()); - - if (!copyView.proxy()) { + if (!copyView) { return nullptr; } diff --git a/src/image/SkImage_GpuYUVA.cpp b/src/image/SkImage_GpuYUVA.cpp index 4d085b057a..df13dcbefe 100644 --- a/src/image/SkImage_GpuYUVA.cpp +++ b/src/image/SkImage_GpuYUVA.cpp @@ -97,9 +97,8 @@ bool SkImage_GpuYUVA::setupMipmapsForPlanes(GrRecordingContext* context) const { if (mipCount && GrGpu::IsACopyNeededForMips(fContext->priv().caps(), fViews[i].asTextureProxy(), GrSamplerState::Filter::kMipMap)) { - auto mippedView = GrCopyBaseMipMapToTextureProxy(context, fViews[i].asTextureProxy(), - fOrigin, fProxyColorTypes[i]); - if (!mippedView.proxy()) { + auto mippedView = GrCopyBaseMipMapToView(context, fViews[i]); + if (!mippedView) { return false; } fViews[i] = std::move(mippedView); @@ -174,16 +173,13 @@ GrSurfaceProxyView SkImage_GpuYUVA::refMippedView(GrRecordingContext* context) c } // need to generate mips for the proxy - GrColorType srcColorType = SkColorTypeToGrColorType(this->colorType()); - GrSurfaceProxyView mippedView = GrCopyBaseMipMapToTextureProxy(context, fRGBView.proxy(), - fRGBView.origin(), srcColorType); - if (mippedView) { - fRGBView = std::move(mippedView); - return fRGBView; + auto mippedView = GrCopyBaseMipMapToView(context, fRGBView); + if (!mippedView) { + return {}; } - // failed to generate mips - return {}; + fRGBView = std::move(mippedView); + return fRGBView; } const GrSurfaceProxyView* SkImage_GpuYUVA::view(GrRecordingContext* context) const { diff --git a/src/image/SkImage_Lazy.cpp b/src/image/SkImage_Lazy.cpp index fadc3342fc..fbfaac6dfa 100644 --- a/src/image/SkImage_Lazy.cpp +++ b/src/image/SkImage_Lazy.cpp @@ -435,17 +435,16 @@ GrSurfaceProxyView SkImage_Lazy::lockTextureProxyView(GrRecordingContext* ctx, // We need a mipped proxy, but we found a cached proxy that wasn't mipped. Thus we // generate a new mipped surface and copy the original proxy into the base layer. We // will then let the gpu generate the rest of the mips. - GrSurfaceProxyView mippedView = GrCopyBaseMipMapToTextureProxy( - ctx, view.proxy(), kTopLeft_GrSurfaceOrigin, ct); - if (mippedView) { - proxyProvider->removeUniqueKeyFromProxy(view.asTextureProxy()); - installKey(mippedView); - return mippedView; + auto mippedView = GrCopyBaseMipMapToView(ctx, view); + if (!mippedView) { + // We failed to make a mipped proxy with the base copied into it. This could + // have been from failure to make the proxy or failure to do the copy. Thus we + // will fall back to just using the non mipped proxy; See skbug.com/7094. + return view; } - // We failed to make a mipped proxy with the base copied into it. This could have - // been from failure to make the proxy or failure to do the copy. Thus we will fall - // back to just using the non mipped proxy; See skbug.com/7094. - return view; + proxyProvider->removeUniqueKeyFromProxy(view.asTextureProxy()); + installKey(mippedView); + return mippedView; } } } diff --git a/src/image/SkSurface_Gpu.cpp b/src/image/SkSurface_Gpu.cpp index 54b8933370..a2a6756201 100644 --- a/src/image/SkSurface_Gpu.cpp +++ b/src/image/SkSurface_Gpu.cpp @@ -101,20 +101,14 @@ sk_sp SkSurface_Gpu::onNewImageSnapshot(const SkIRect* subset) { SkBudgeted budgeted = rtc->asSurfaceProxy()->isBudgeted(); - GrSurfaceProxyView srcView; - if (subset) { - srcView = GrSurfaceProxy::Copy(ctx, rtc->asSurfaceProxy(), rtc->origin(), - rtc->colorInfo().colorType(), rtc->mipMapped(), *subset, - SkBackingFit::kExact, budgeted); - } else if (!rtc->asTextureProxy() || rtc->priv().refsWrappedObjects()) { + GrSurfaceProxyView srcView = rtc->readSurfaceView(); + if (subset || !srcView.asTextureProxy() || rtc->priv().refsWrappedObjects()) { // If the original render target is a buffer originally created by the client, then we don't // want to ever retarget the SkSurface at another buffer we create. Force a copy now to // avoid copy-on-write. - srcView = GrSurfaceProxy::Copy(ctx, rtc->asSurfaceProxy(), rtc->origin(), - rtc->colorInfo().colorType(), rtc->mipMapped(), - SkBackingFit::kExact, budgeted); - } else { - srcView = rtc->readSurfaceView(); + auto rect = subset ? *subset : SkIRect::MakeSize(rtc->dimensions()); + srcView = GrSurfaceProxyView::Copy(ctx, std::move(srcView), rtc->mipMapped(), rect, + SkBackingFit::kExact, budgeted); } const SkImageInfo info = fDevice->imageInfo(); diff --git a/tests/CopySurfaceTest.cpp b/tests/CopySurfaceTest.cpp index 5cb6dbec3b..b3eb1747dd 100644 --- a/tests/CopySurfaceTest.cpp +++ b/tests/CopySurfaceTest.cpp @@ -115,8 +115,7 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(CopySurface, reporter, ctxInfo) { bool result = false; if (sOrigin == dOrigin) { - result = dstContext->testCopy(src.get(), sOrigin, srcRect, - dstPoint); + result = dstContext->testCopy(src.get(), srcRect, dstPoint); } else if (dRenderable == GrRenderable::kYes) { SkASSERT(dstContext->asRenderTargetContext()); GrSwizzle srcSwizzle = context->priv().caps()->getReadSwizzle( diff --git a/tests/GrSurfaceTest.cpp b/tests/GrSurfaceTest.cpp index 5f58ca76e3..53de88c90b 100644 --- a/tests/GrSurfaceTest.cpp +++ b/tests/GrSurfaceTest.cpp @@ -402,7 +402,7 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ReadOnlyTexture, reporter, context_info) { auto copySrc = maker.view(GrMipMapped::kNo); REPORTER_ASSERT(reporter, copySrc.proxy()); - auto copyResult = surfContext->testCopy(copySrc.proxy(), copySrc.origin()); + auto copyResult = surfContext->testCopy(copySrc.proxy()); REPORTER_ASSERT(reporter, copyResult == (ioType == kRW_GrIOType)); // Try the low level copy. context->flush(); @@ -792,7 +792,7 @@ DEF_GPUTEST_FOR_ALL_CONTEXTS(TextureIdleStateTest, reporter, contextInfo) { auto proxy = context->priv().proxyProvider()->testingOnly_createWrapped(std::move(idleTexture)); context->flush(); - SkAssertResult(rtc->testCopy(proxy.get(), rtc->origin())); + SkAssertResult(rtc->testCopy(proxy.get())); proxy.reset(); REPORTER_ASSERT(reporter, flags == 0); diff --git a/tests/MtlCopySurfaceTest.mm b/tests/MtlCopySurfaceTest.mm index e6603f8d53..9b880099e5 100644 --- a/tests/MtlCopySurfaceTest.mm +++ b/tests/MtlCopySurfaceTest.mm @@ -43,16 +43,16 @@ DEF_GPUTEST_FOR_METAL_CONTEXT(MtlCopySurfaceTest, reporter, ctxInfo) { GrProxyProvider* proxyProvider = context->priv().proxyProvider(); sk_sp srcProxy = proxyProvider->wrapBackendRenderTarget(backendRT); - GrSurfaceProxyView dstView = GrSurfaceProxy::Copy(context, srcProxy.get(), - kTopLeft_GrSurfaceOrigin, - GrColorType::kBGRA_8888, - GrMipMapped::kNo, - SkBackingFit::kExact, - SkBudgeted::kYes); + auto dstProxy = GrSurfaceProxy::Copy(context, + srcProxy.get(), + kTopLeft_GrSurfaceOrigin, + GrMipMapped::kNo, + SkBackingFit::kExact, + SkBudgeted::kYes); // TODO: GrSurfaceProxy::Copy doesn't check to see if the framebufferOnly bit is set yet. // Update this when it does -- it should fail. - if (!dstView.proxy()) { + if (!dstProxy) { ERRORF(reporter, "Expected copy to succeed"); } diff --git a/tests/RectangleTextureTest.cpp b/tests/RectangleTextureTest.cpp index 915efbe756..c784b3ec54 100644 --- a/tests/RectangleTextureTest.cpp +++ b/tests/RectangleTextureTest.cpp @@ -116,7 +116,7 @@ static void test_copy_to_surface(skiatest::Reporter* reporter, pixels.get(), 0); // If this assert ever fails we can add a fallback to do copy as draw, but until then we can // be more restrictive. - SkAssertResult(dstContext->testCopy(src.get(), origin)); + SkAssertResult(dstContext->testCopy(src.get())); TestReadPixels(reporter, dstContext, pixels.get(), testName); } } diff --git a/tests/TestUtils.cpp b/tests/TestUtils.cpp index 9dda6fb12e..89ead3b4b2 100644 --- a/tests/TestUtils.cpp +++ b/tests/TestUtils.cpp @@ -81,11 +81,11 @@ void TestCopyFromSurface(skiatest::Reporter* reporter, GrColorType colorType, uint32_t expectedPixelValues[], const char* testName) { - GrSurfaceProxyView view = GrSurfaceProxy::Copy(context, proxy, origin, colorType, - GrMipMapped::kNo, SkBackingFit::kExact, - SkBudgeted::kYes); - SkASSERT(view.asTextureProxy()); - + auto copy = GrSurfaceProxy::Copy(context, proxy, origin, GrMipMapped::kNo, SkBackingFit::kExact, + SkBudgeted::kYes); + SkASSERT(copy && copy->asTextureProxy()); + auto swizzle = context->priv().caps()->getReadSwizzle(copy->backendFormat(), colorType); + GrSurfaceProxyView view(std::move(copy), origin, swizzle); auto dstContext = GrSurfaceContext::Make(context, std::move(view), colorType, kPremul_SkAlphaType, nullptr); SkASSERT(dstContext);