From 8a8dd33e18ce6946913247732273b1cd48ba0433 Mon Sep 17 00:00:00 2001 From: Brian Salomon Date: Thu, 24 May 2018 14:08:31 -0400 Subject: [PATCH] Make SkImage own its GrContext. Change-Id: I86577fab5406ae9ad89d87fa971b0db6e0283cb4 Reviewed-on: https://skia-review.googlesource.com/130020 Commit-Queue: Brian Salomon Reviewed-by: Robert Phillips --- gm/flippity.cpp | 2 +- gm/image_pict.cpp | 6 +++--- src/core/SkSpecialImage.cpp | 9 ++++---- src/image/SkImage_Gpu.cpp | 41 ++++++++++++++++++------------------- src/image/SkImage_Gpu.h | 6 +++--- src/image/SkSurface_Gpu.cpp | 5 ++--- tests/ImageTest.cpp | 39 +++++++++++++++++++++++++++++++++++ tools/gpu/GrTest.cpp | 5 +++-- 8 files changed, 75 insertions(+), 38 deletions(-) diff --git a/gm/flippity.cpp b/gm/flippity.cpp index d730ed58a6..fb0288b191 100644 --- a/gm/flippity.cpp +++ b/gm/flippity.cpp @@ -114,7 +114,7 @@ static sk_sp make_reference_image(GrContext* context, return nullptr; } - return sk_make_sp(context, kNeedNewImageUniqueID, kOpaque_SkAlphaType, + return sk_make_sp(sk_ref_sp(context), kNeedNewImageUniqueID, kOpaque_SkAlphaType, std::move(proxy), nullptr, SkBudgeted::kYes); } diff --git a/gm/image_pict.cpp b/gm/image_pict.cpp index f244fe8f35..11bb902291 100644 --- a/gm/image_pict.cpp +++ b/gm/image_pict.cpp @@ -287,9 +287,9 @@ protected: } // No API to draw a GrTexture directly, so we cheat and create a private image subclass - sk_sp texImage(new SkImage_Gpu(canvas->getGrContext(), image->uniqueID(), - kPremul_SkAlphaType, std::move(proxy), - std::move(texColorSpace), SkBudgeted::kNo)); + sk_sp texImage(new SkImage_Gpu( + sk_ref_sp(canvas->getGrContext()), image->uniqueID(), kPremul_SkAlphaType, + std::move(proxy), std::move(texColorSpace), SkBudgeted::kNo)); canvas->drawImage(texImage.get(), x, y); #endif } diff --git a/src/core/SkSpecialImage.cpp b/src/core/SkSpecialImage.cpp index be9f530396..256ef0f6d6 100644 --- a/src/core/SkSpecialImage.cpp +++ b/src/core/SkSpecialImage.cpp @@ -340,7 +340,7 @@ sk_sp SkSpecialImage::MakeFromRaster(const SkIRect& subset, /////////////////////////////////////////////////////////////////////////////// static sk_sp wrap_proxy_in_image(GrContext* context, sk_sp proxy, SkAlphaType alphaType, sk_sp colorSpace) { - return sk_make_sp(context, kNeedNewImageUniqueID, alphaType, + return sk_make_sp(sk_ref_sp(context), kNeedNewImageUniqueID, alphaType, std::move(proxy), std::move(colorSpace), SkBudgeted::kYes); } @@ -377,10 +377,9 @@ public: // instantiates itself it is going to have to either be okay with having a larger // than expected backing texture (unlikely) or the 'fit' of the SurfaceProxy needs // to be tightened (if it is deferred). - sk_sp img = sk_sp(new SkImage_Gpu(canvas->getGrContext(), - this->uniqueID(), fAlphaType, - fTextureProxy, - fColorSpace, SkBudgeted::kNo)); + sk_sp img = sk_sp( + new SkImage_Gpu(sk_ref_sp(canvas->getGrContext()), this->uniqueID(), fAlphaType, + fTextureProxy, fColorSpace, SkBudgeted::kNo)); canvas->drawImageRect(img, this->subset(), dst, paint, SkCanvas::kStrict_SrcRectConstraint); diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp index d3500e4f4e..f961d153e3 100644 --- a/src/image/SkImage_Gpu.cpp +++ b/src/image/SkImage_Gpu.cpp @@ -43,17 +43,16 @@ #include "SkReadPixelsRec.h" #include "SkTraceEvent.h" -SkImage_Gpu::SkImage_Gpu(GrContext* context, uint32_t uniqueID, SkAlphaType at, - sk_sp proxy, - sk_sp colorSpace, SkBudgeted budgeted) - : INHERITED(proxy->worstCaseWidth(), proxy->worstCaseHeight(), uniqueID) - , fContext(context) - , fProxy(std::move(proxy)) - , fAlphaType(at) - , fBudgeted(budgeted) - , fColorSpace(std::move(colorSpace)) - , fAddedRasterVersionToCache(false) { -} +SkImage_Gpu::SkImage_Gpu(sk_sp context, uint32_t uniqueID, SkAlphaType at, + sk_sp proxy, sk_sp colorSpace, + SkBudgeted budgeted) + : INHERITED(proxy->worstCaseWidth(), proxy->worstCaseHeight(), uniqueID) + , fContext(std::move(context)) + , fProxy(std::move(proxy)) + , fAlphaType(at) + , fBudgeted(budgeted) + , fColorSpace(std::move(colorSpace)) + , fAddedRasterVersionToCache(false) {} SkImage_Gpu::~SkImage_Gpu() { if (fAddedRasterVersionToCache.load()) { @@ -130,12 +129,12 @@ sk_sp SkImage_Gpu::asTextureProxyRef(GrContext* context, SkColorSpace* dstColorSpace, sk_sp* texColorSpace, SkScalar scaleAdjust[2]) const { - if (context != fContext) { + if (context != fContext.get()) { SkASSERT(0); return nullptr; } - GrTextureAdjuster adjuster(fContext, fProxy, this->alphaType(), this->uniqueID(), + GrTextureAdjuster adjuster(fContext.get(), fProxy, this->alphaType(), this->uniqueID(), this->fColorSpace.get()); return adjuster.refTextureProxyForParams(params, dstColorSpace, texColorSpace, scaleAdjust); } @@ -318,8 +317,8 @@ static sk_sp new_wrapped_texture_common(GrContext* ctx, return nullptr; } - return sk_make_sp(ctx, kNeedNewImageUniqueID, - at, std::move(proxy), std::move(colorSpace), SkBudgeted::kNo); + return sk_make_sp(sk_ref_sp(ctx), kNeedNewImageUniqueID, at, std::move(proxy), + std::move(colorSpace), SkBudgeted::kNo); } bool validate_backend_texture(GrContext* ctx, const GrBackendTexture& tex, GrPixelConfig* config, @@ -463,7 +462,7 @@ sk_sp SkImage_Gpu::MakeFromYUVATexturesCopyImpl(GrContext* ctx, ctx->contextPriv().flushSurfaceWrites(renderTargetContext->asSurfaceProxy()); // MDB: this call is okay bc we know 'renderTargetContext' was exact - return sk_make_sp(ctx, kNeedNewImageUniqueID, kOpaque_SkAlphaType, + return sk_make_sp(sk_ref_sp(ctx), kNeedNewImageUniqueID, kOpaque_SkAlphaType, renderTargetContext->asTextureProxyRef(), renderTargetContext->colorSpaceInfo().refColorSpace(), SkBudgeted::kYes); @@ -508,8 +507,8 @@ static sk_sp create_image_from_maker(GrContext* context, GrTextureMaker if (!proxy) { return nullptr; } - return sk_make_sp(context, id, at, - std::move(proxy), std::move(texColorSpace), SkBudgeted::kNo); + return sk_make_sp(sk_ref_sp(context), id, at, std::move(proxy), + std::move(texColorSpace), SkBudgeted::kNo); } sk_sp SkImage::makeTextureImage(GrContext* context, SkColorSpace* dstColorSpace) const { @@ -742,8 +741,8 @@ sk_sp SkImage_Gpu::MakePromiseTexture(GrContext* context, return nullptr; } - return sk_make_sp(context, kNeedNewImageUniqueID, alphaType, std::move(proxy), - std::move(colorSpace), SkBudgeted::kNo); + return sk_make_sp(sk_ref_sp(context), kNeedNewImageUniqueID, alphaType, + std::move(proxy), std::move(colorSpace), SkBudgeted::kNo); } /////////////////////////////////////////////////////////////////////////////////////////////////// @@ -1007,7 +1006,7 @@ bool SkImage_Gpu::onIsValid(GrContext* context) const { return false; } - if (context && context != fContext) { + if (context && context != fContext.get()) { return false; } diff --git a/src/image/SkImage_Gpu.h b/src/image/SkImage_Gpu.h index b0c2785ece..ff2aaaf07c 100644 --- a/src/image/SkImage_Gpu.h +++ b/src/image/SkImage_Gpu.h @@ -23,7 +23,7 @@ class GrTexture; class SkImage_Gpu : public SkImage_Base { public: - SkImage_Gpu(GrContext*, uint32_t uniqueID, SkAlphaType, sk_sp, + SkImage_Gpu(sk_sp, uint32_t uniqueID, SkAlphaType, sk_sp, sk_sp, SkBudgeted); ~SkImage_Gpu() override; @@ -34,7 +34,7 @@ public: bool getROPixels(SkBitmap*, SkColorSpace* dstColorSpace, CachingHint) const override; sk_sp onMakeSubset(const SkIRect&) const override; - GrContext* context() const override { return fContext; } + GrContext* context() const override { return fContext.get(); } GrTextureProxy* peekProxy() const override { return fProxy.get(); } @@ -136,7 +136,7 @@ public: bool onIsValid(GrContext*) const override; private: - GrContext* fContext; + sk_sp fContext; sk_sp fProxy; const SkAlphaType fAlphaType; const SkBudgeted fBudgeted; diff --git a/src/image/SkSurface_Gpu.cpp b/src/image/SkSurface_Gpu.cpp index 2f7b039488..c20608aac2 100644 --- a/src/image/SkSurface_Gpu.cpp +++ b/src/image/SkSurface_Gpu.cpp @@ -118,9 +118,8 @@ sk_sp SkSurface_Gpu::onNewImageSnapshot() { // The renderTargetContext coming out of SkGpuDevice should always be exact and the // above copy creates a kExact surfaceContext. SkASSERT(srcProxy->priv().isExact()); - image = sk_make_sp(ctx, kNeedNewImageUniqueID, - info.alphaType(), std::move(srcProxy), - info.refColorSpace(), budgeted); + image = sk_make_sp(sk_ref_sp(ctx), kNeedNewImageUniqueID, info.alphaType(), + std::move(srcProxy), info.refColorSpace(), budgeted); } return image; } diff --git a/tests/ImageTest.cpp b/tests/ImageTest.cpp index b69dd2e1b2..44e901e3e0 100644 --- a/tests/ImageTest.cpp +++ b/tests/ImageTest.cpp @@ -580,6 +580,45 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(UnpremulTextureImage, reporter, ctxInfo) { } } +DEF_GPUTEST(AbandonedContextImage, reporter, options) { + using Factory = sk_gpu_test::GrContextFactory; + for (int ct = 0; ct < Factory::kContextTypeCnt; ++ct) { + auto type = static_cast(ct); + std::unique_ptr factory(new Factory); + if (!factory->get(type)) { + continue; + } + + sk_sp img; + auto gsurf = SkSurface::MakeRenderTarget( + factory->get(type), SkBudgeted::kYes, + SkImageInfo::Make(100, 100, kRGBA_8888_SkColorType, kPremul_SkAlphaType), 1, + nullptr); + if (!gsurf) { + continue; + } + img = gsurf->makeImageSnapshot(); + gsurf.reset(); + + auto rsurf = SkSurface::MakeRaster(SkImageInfo::MakeN32Premul(100, 100)); + + REPORTER_ASSERT(reporter, img->isValid(factory->get(type))); + REPORTER_ASSERT(reporter, img->isValid(rsurf->getCanvas()->getGrContext())); + + factory->get(type)->abandonContext(); + REPORTER_ASSERT(reporter, !img->isValid(factory->get(type))); + REPORTER_ASSERT(reporter, !img->isValid(rsurf->getCanvas()->getGrContext())); + // This shouldn't crash. + rsurf->getCanvas()->drawImage(img, 0, 0); + + // Give up all other refs on GrContext. + factory.reset(nullptr); + REPORTER_ASSERT(reporter, !img->isValid(rsurf->getCanvas()->getGrContext())); + // This shouldn't crash. + rsurf->getCanvas()->drawImage(img, 0, 0); + } +} + #endif class EmptyGenerator : public SkImageGenerator { diff --git a/tools/gpu/GrTest.cpp b/tools/gpu/GrTest.cpp index ec07758db2..f3630ff721 100644 --- a/tools/gpu/GrTest.cpp +++ b/tools/gpu/GrTest.cpp @@ -137,8 +137,9 @@ sk_sp GrContextPriv::getFontAtlasImage_ForTesting(GrMaskFormat format, } SkASSERT(proxies[index]->priv().isExact()); - sk_sp image(new SkImage_Gpu(fContext, kNeedNewImageUniqueID, kPremul_SkAlphaType, - proxies[index], nullptr, SkBudgeted::kNo)); + sk_sp image(new SkImage_Gpu(sk_ref_sp(fContext), kNeedNewImageUniqueID, + kPremul_SkAlphaType, proxies[index], nullptr, + SkBudgeted::kNo)); return image; }