From b64bcbdc3a5aa7b9e3ff216e4617ddc1db9260b5 Mon Sep 17 00:00:00 2001 From: Robert Phillips Date: Thu, 16 Mar 2017 08:32:56 -0400 Subject: [PATCH] Remove budgeted parameter from SkSurface::makeImageSnapshot This unused feature complicates MDB. Chrome compiles locally for me with this CL. Change-Id: I611e464885fb984030eace43ead42cf39d0e7f72 Reviewed-on: https://skia-review.googlesource.com/9734 Reviewed-by: Brian Salomon Commit-Queue: Robert Phillips --- bench/GrMipMapBench.cpp | 2 +- gm/croppedrects.cpp | 2 +- include/core/SkSurface.h | 7 +++--- src/core/SkImagePriv.h | 6 ----- src/gpu/GrSurfaceProxy.cpp | 15 ------------ src/gpu/GrSurfaceProxyPriv.h | 12 ---------- src/image/SkImage_Gpu.cpp | 6 ----- src/image/SkImage_Gpu.h | 8 ------- src/image/SkSurface.cpp | 6 ++--- src/image/SkSurface_Base.h | 8 +++---- src/image/SkSurface_Gpu.cpp | 11 ++++----- src/image/SkSurface_Gpu.h | 2 +- src/image/SkSurface_Raster.cpp | 6 ++--- tests/GrTextureMipMapInvalidationTest.cpp | 8 +++---- tests/SurfaceTest.cpp | 39 ++++++++++++++----------------- 15 files changed, 42 insertions(+), 96 deletions(-) diff --git a/bench/GrMipMapBench.cpp b/bench/GrMipMapBench.cpp index 7b8ce3f..89100e3 100644 --- a/bench/GrMipMapBench.cpp +++ b/bench/GrMipMapBench.cpp @@ -55,7 +55,7 @@ protected: // Draw reduced version of surface to original canvas, to trigger mip generation canvas->save(); canvas->scale(0.1f, 0.1f); - canvas->drawImage(fSurface->makeImageSnapshot(SkBudgeted::kNo), 0, 0, &paint); + canvas->drawImage(fSurface->makeImageSnapshot(), 0, 0, &paint); canvas->restore(); } } diff --git a/gm/croppedrects.cpp b/gm/croppedrects.cpp index 2d7de95..0ea265c 100644 --- a/gm/croppedrects.cpp +++ b/gm/croppedrects.cpp @@ -46,7 +46,7 @@ private: stroke.setColor(0xff008800); srcCanvas->drawRect(kSrcImageClip.makeInset(kStrokeWidth / 2, kStrokeWidth / 2), stroke); - fSrcImage = srcSurface->makeImageSnapshot(SkBudgeted::kYes); + fSrcImage = srcSurface->makeImageSnapshot(); fSrcImageShader = fSrcImage->makeShader(SkShader::kClamp_TileMode, SkShader::kClamp_TileMode); } diff --git a/include/core/SkSurface.h b/include/core/SkSurface.h index be657b1..52c0e58 100644 --- a/include/core/SkSurface.h +++ b/include/core/SkSurface.h @@ -250,11 +250,10 @@ public: /** * Returns an image of the current state of the surface pixels up to this * point. Subsequent changes to the surface (by drawing into its canvas) - * will not be reflected in this image. If a copy must be made the Budgeted - * parameter controls whether it counts against the resource budget - * (currently for the gpu backend only). + * will not be reflected in this image. For the GPU-backend, the budgeting + * decision for the snapped image will match that of the surface. */ - sk_sp makeImageSnapshot(SkBudgeted = SkBudgeted::kYes); + sk_sp makeImageSnapshot(); /** * Though the caller could get a snapshot image explicitly, and draw that, diff --git a/src/core/SkImagePriv.h b/src/core/SkImagePriv.h index ea44770..921fdc8 100644 --- a/src/core/SkImagePriv.h +++ b/src/core/SkImagePriv.h @@ -50,12 +50,6 @@ extern sk_sp SkMakeImageFromRasterBitmap(const SkBitmap&, SkCopyPixelsM // in which case the surface may need to perform a copy-on-write. extern const SkPixelRef* SkBitmapImageGetPixelRef(const SkImage* rasterImage); -// When a texture is shared by a surface and an image its budgeted status is that of the -// surface. This function is used when the surface makes a new texture for itself in order -// for the orphaned image to determine whether the original texture counts against the -// budget or not. -extern void SkTextureImageApplyBudgetedDecision(SkImage* textureImage); - // Update the texture wrapped by an image created with NewTexture. This // is called when a surface and image share the same GrTexture and the // surface needs to perform a copy-on-write diff --git a/src/gpu/GrSurfaceProxy.cpp b/src/gpu/GrSurfaceProxy.cpp index 727bcb5..ac025dc 100644 --- a/src/gpu/GrSurfaceProxy.cpp +++ b/src/gpu/GrSurfaceProxy.cpp @@ -290,18 +290,3 @@ sk_sp GrSurfaceProxy::TestCopy(GrContext* context, const GrSur return dstContext; } -void GrSurfaceProxyPriv::makeBudgeted() { - if (fProxy->fTarget) { - fProxy->fTarget->resourcePriv().makeBudgeted(); - } - - fProxy->fBudgeted = SkBudgeted::kYes; -} - -void GrSurfaceProxyPriv::makeUnbudgeted() { - if (fProxy->fTarget) { - fProxy->fTarget->resourcePriv().makeUnbudgeted(); - } - - fProxy->fBudgeted = SkBudgeted::kNo; -} diff --git a/src/gpu/GrSurfaceProxyPriv.h b/src/gpu/GrSurfaceProxyPriv.h index 0d10ef6..e5a628c 100644 --- a/src/gpu/GrSurfaceProxyPriv.h +++ b/src/gpu/GrSurfaceProxyPriv.h @@ -23,18 +23,6 @@ public: // Don't abuse these two!!!!!!! bool isExact() const { return SkBackingFit::kExact == fProxy->fFit; } - // These next two are very specialized and wacky - don't use them! - - // In the case where an unbudgeted, deferred SkSurface_Gpu has snapped a budgeted, deferred - // SkImage_Gpu, this serves to propagate the budgeting forward in time. For now, and - // presumably forever, this will not change any flushing decisions but may make Ganesh - // appear to have gone over budget. In the case of non-deferred proxies this will immediately - // propagate the budget decision to the resource, which in itself is dubious. - void makeBudgeted(); - // In the case where a budgeted, deferred SkSurface_Gpu has snapped an unbudgeted, deferred - // SkImage_Gpu, this serves to propagate the lack of budgeting forward in time. - void makeUnbudgeted(); - private: explicit GrSurfaceProxyPriv(GrSurfaceProxy* proxy) : fProxy(proxy) {} GrSurfaceProxyPriv(const GrSurfaceProxyPriv&) {} // unimpl diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp index 10a8c0d..3cc6e2d 100644 --- a/src/image/SkImage_Gpu.cpp +++ b/src/image/SkImage_Gpu.cpp @@ -65,12 +65,6 @@ SkImage_Gpu::~SkImage_Gpu() { } } -extern void SkTextureImageApplyBudgetedDecision(SkImage* image) { - if (image->isTextureBacked()) { - ((SkImage_Gpu*)image)->applyBudgetDecision(); - } -} - SkImageInfo SkImage_Gpu::onImageInfo() const { SkColorType ct; if (!GrPixelConfigToColorType(fProxy->config(), &ct)) { diff --git a/src/image/SkImage_Gpu.h b/src/image/SkImage_Gpu.h index b3a165d..53c38dc 100644 --- a/src/image/SkImage_Gpu.h +++ b/src/image/SkImage_Gpu.h @@ -29,14 +29,6 @@ public: SkImageInfo onImageInfo() const override; SkAlphaType onAlphaType() const override { return fAlphaType; } - void applyBudgetDecision() const { - if (SkBudgeted::kYes == fBudgeted) { - fProxy->priv().makeBudgeted(); - } else { - fProxy->priv().makeUnbudgeted(); - } - } - bool getROPixels(SkBitmap*, SkColorSpace* dstColorSpace, CachingHint) const override; GrTexture* asTextureRef(GrContext*, const GrSamplerParams&, SkColorSpace*, sk_sp*, SkScalar scaleAdjust[2]) const override; diff --git a/src/image/SkSurface.cpp b/src/image/SkSurface.cpp index 08f79ae..55aab3e 100644 --- a/src/image/SkSurface.cpp +++ b/src/image/SkSurface.cpp @@ -71,7 +71,7 @@ SkSurface_Base::~SkSurface_Base() { } void SkSurface_Base::onDraw(SkCanvas* canvas, SkScalar x, SkScalar y, const SkPaint* paint) { - auto image = this->makeImageSnapshot(SkBudgeted::kYes); + auto image = this->makeImageSnapshot(); if (image) { canvas->drawImage(image, x, y, paint); } @@ -153,8 +153,8 @@ SkCanvas* SkSurface::getCanvas() { return asSB(this)->getCachedCanvas(); } -sk_sp SkSurface::makeImageSnapshot(SkBudgeted budgeted) { - return asSB(this)->refCachedImage(budgeted); +sk_sp SkSurface::makeImageSnapshot() { + return asSB(this)->refCachedImage(); } sk_sp SkSurface::makeSurface(const SkImageInfo& info) { diff --git a/src/image/SkSurface_Base.h b/src/image/SkSurface_Base.h index 33aeee2..1b0f9ff 100644 --- a/src/image/SkSurface_Base.h +++ b/src/image/SkSurface_Base.h @@ -43,7 +43,7 @@ public: * must faithfully represent the current contents, even if the surface * is changed after this called (e.g. it is drawn to via its canvas). */ - virtual sk_sp onNewImageSnapshot(SkBudgeted) = 0; + virtual sk_sp onNewImageSnapshot() = 0; /** * Default implementation: @@ -81,7 +81,7 @@ public: virtual void onPrepareForExternalIO() {} inline SkCanvas* getCachedCanvas(); - inline sk_sp refCachedImage(SkBudgeted); + inline sk_sp refCachedImage(); bool hasCachedImage() const { return fCachedImage != nullptr; } @@ -114,12 +114,12 @@ SkCanvas* SkSurface_Base::getCachedCanvas() { return fCachedCanvas.get(); } -sk_sp SkSurface_Base::refCachedImage(SkBudgeted budgeted) { +sk_sp SkSurface_Base::refCachedImage() { if (fCachedImage) { return fCachedImage; } - fCachedImage = this->onNewImageSnapshot(budgeted); + fCachedImage = this->onNewImageSnapshot(); SkASSERT(!fCachedCanvas || fCachedCanvas->getSurfaceBase() == this); return fCachedImage; diff --git a/src/image/SkSurface_Gpu.cpp b/src/image/SkSurface_Gpu.cpp index 0bd34f8..b610c9c 100644 --- a/src/image/SkSurface_Gpu.cpp +++ b/src/image/SkSurface_Gpu.cpp @@ -83,7 +83,7 @@ sk_sp SkSurface_Gpu::onNewSurface(const SkImageInfo& info) { origin, &this->props()); } -sk_sp SkSurface_Gpu::onNewImageSnapshot(SkBudgeted budgeted) { +sk_sp SkSurface_Gpu::onNewImageSnapshot() { GrRenderTargetContext* rtc = fDevice->accessRenderTargetContext(); if (!rtc) { return nullptr; @@ -102,7 +102,7 @@ sk_sp SkSurface_Gpu::onNewImageSnapshot(SkBudgeted budgeted) { copyCtx = ctx->contextPriv().makeDeferredSurfaceContext(desc, SkBackingFit::kExact, - budgeted); + srcProxy->isBudgeted()); if (!copyCtx) { return nullptr; } @@ -122,7 +122,7 @@ sk_sp SkSurface_Gpu::onNewImageSnapshot(SkBudgeted budgeted) { if (tex) { image = sk_make_sp(kNeedNewImageUniqueID, info.alphaType(), sk_ref_sp(tex), - sk_ref_sp(info.colorSpace()), budgeted); + sk_ref_sp(info.colorSpace()), srcProxy->isBudgeted()); } return image; } @@ -137,14 +137,13 @@ void SkSurface_Gpu::onCopyOnWrite(ContentChangeMode mode) { } // are we sharing our render target with the image? Note this call should never create a new // image because onCopyOnWrite is only called when there is a cached image. - sk_sp image(this->refCachedImage(SkBudgeted::kNo)); + sk_sp image(this->refCachedImage()); SkASSERT(image); // MDB TODO: this is unfortunate. The snapping of an Image_Gpu from a surface currently - // funnels down to a GrTexture. Once Image_Gpus are proxy-backed we should be able to + // funnels down to a GrTexture. Once Image_Gpus are proxy-backed we should be able to // compare proxy uniqueIDs. if (rt->asTexture()->getTextureHandle() == image->getTextureHandle(false)) { fDevice->replaceRenderTargetContext(SkSurface::kRetain_ContentChangeMode == mode); - SkTextureImageApplyBudgetedDecision(image.get()); } else if (kDiscard_ContentChangeMode == mode) { this->SkSurface_Gpu::onDiscard(); } diff --git a/src/image/SkSurface_Gpu.h b/src/image/SkSurface_Gpu.h index a625473..5b92eeb 100644 --- a/src/image/SkSurface_Gpu.h +++ b/src/image/SkSurface_Gpu.h @@ -23,7 +23,7 @@ public: bool onGetRenderTargetHandle(GrBackendObject*, BackendHandleAccess) override; SkCanvas* onNewCanvas() override; sk_sp onNewSurface(const SkImageInfo&) override; - sk_sp onNewImageSnapshot(SkBudgeted) override; + sk_sp onNewImageSnapshot() override; void onCopyOnWrite(ContentChangeMode) override; void onDiscard() override; void onPrepareForExternalIO() override; diff --git a/src/image/SkSurface_Raster.cpp b/src/image/SkSurface_Raster.cpp index 47e0dbe..4fae0f0 100644 --- a/src/image/SkSurface_Raster.cpp +++ b/src/image/SkSurface_Raster.cpp @@ -24,7 +24,7 @@ public: SkCanvas* onNewCanvas() override; sk_sp onNewSurface(const SkImageInfo&) override; - sk_sp onNewImageSnapshot(SkBudgeted) override; + sk_sp onNewImageSnapshot() override; void onDraw(SkCanvas*, SkScalar x, SkScalar y, const SkPaint*) override; void onCopyOnWrite(ContentChangeMode) override; void onRestoreBackingMutability() override; @@ -130,7 +130,7 @@ void SkSurface_Raster::onDraw(SkCanvas* canvas, SkScalar x, SkScalar y, canvas->drawBitmap(fBitmap, x, y, paint); } -sk_sp SkSurface_Raster::onNewImageSnapshot(SkBudgeted) { +sk_sp SkSurface_Raster::onNewImageSnapshot() { SkCopyPixelsMode cpm = kIfMutable_SkCopyPixelsMode; if (fWeOwnThePixels) { // SkImage_raster requires these pixels are immutable for its full lifetime. @@ -156,7 +156,7 @@ void SkSurface_Raster::onRestoreBackingMutability() { void SkSurface_Raster::onCopyOnWrite(ContentChangeMode mode) { // are we sharing pixelrefs with the image? - sk_sp cached(this->refCachedImage(SkBudgeted::kNo)); + sk_sp cached(this->refCachedImage()); SkASSERT(cached); if (SkBitmapImageGetPixelRef(cached.get()) == fBitmap.pixelRef()) { SkASSERT(fWeOwnThePixels); diff --git a/tests/GrTextureMipMapInvalidationTest.cpp b/tests/GrTextureMipMapInvalidationTest.cpp index 166c57c..e7573dc 100644 --- a/tests/GrTextureMipMapInvalidationTest.cpp +++ b/tests/GrTextureMipMapInvalidationTest.cpp @@ -20,13 +20,11 @@ // Tests that MIP maps are created and invalidated as expected when drawing to and from GrTextures. DEF_GPUTEST_FOR_NULLGL_CONTEXT(GrTextureMipMapInvalidationTest, reporter, ctxInfo) { auto isMipped = [] (SkSurface* surf) { - return as_IB(surf->makeImageSnapshot(SkBudgeted::kYes))-> - peekTexture()->texturePriv().hasMipMaps(); + return as_IB(surf->makeImageSnapshot())->peekTexture()->texturePriv().hasMipMaps(); }; auto mipsAreDirty = [] (SkSurface* surf) { - return as_IB(surf->makeImageSnapshot(SkBudgeted::kYes))-> - peekTexture()->texturePriv().mipMapsAreDirty(); + return as_IB(surf->makeImageSnapshot())->peekTexture()->texturePriv().mipMapsAreDirty(); }; GrContext* context = ctxInfo.grContext(); @@ -44,7 +42,7 @@ DEF_GPUTEST_FOR_NULLGL_CONTEXT(GrTextureMipMapInvalidationTest, reporter, ctxInf SkPaint paint; paint.setFilterQuality(kMedium_SkFilterQuality); surf2->getCanvas()->scale(0.2f, 0.2f); - surf2->getCanvas()->drawImage(surf1->makeImageSnapshot(SkBudgeted::kYes), 0, 0, &paint); + surf2->getCanvas()->drawImage(surf1->makeImageSnapshot(), 0, 0, &paint); surf2->getCanvas()->flush(); REPORTER_ASSERT(reporter, isMipped(surf1.get())); REPORTER_ASSERT(reporter, !mipsAreDirty(surf1.get())); diff --git a/tests/SurfaceTest.cpp b/tests/SurfaceTest.cpp index b85ac81..6264196 100644 --- a/tests/SurfaceTest.cpp +++ b/tests/SurfaceTest.cpp @@ -460,27 +460,24 @@ static SkBudgeted is_budgeted(const sk_sp image) { DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SurfaceBudget, reporter, ctxInfo) { SkImageInfo info = SkImageInfo::MakeN32Premul(8,8); - for (auto sbudgeted : { SkBudgeted::kNo, SkBudgeted::kYes }) { - for (auto ibudgeted : { SkBudgeted::kNo, SkBudgeted::kYes }) { - auto surface(SkSurface::MakeRenderTarget(ctxInfo.grContext(), sbudgeted, info)); - SkASSERT(surface); - REPORTER_ASSERT(reporter, sbudgeted == is_budgeted(surface)); - - sk_sp image(surface->makeImageSnapshot(ibudgeted)); - - // Initially the image shares a texture with the surface, and the surface decides - // whether it is budgeted or not. - REPORTER_ASSERT(reporter, sbudgeted == is_budgeted(surface)); - REPORTER_ASSERT(reporter, sbudgeted == is_budgeted(image)); - - // Now trigger copy-on-write - surface->getCanvas()->clear(SK_ColorBLUE); - - // They don't share a texture anymore. They should each have made their own budget - // decision. - REPORTER_ASSERT(reporter, sbudgeted == is_budgeted(surface)); - REPORTER_ASSERT(reporter, ibudgeted == is_budgeted(image)); - } + for (auto budgeted : { SkBudgeted::kNo, SkBudgeted::kYes }) { + auto surface(SkSurface::MakeRenderTarget(ctxInfo.grContext(), budgeted, info)); + SkASSERT(surface); + REPORTER_ASSERT(reporter, budgeted == is_budgeted(surface)); + + sk_sp image(surface->makeImageSnapshot()); + + // Initially the image shares a texture with the surface, and the + // the budgets should always match. + REPORTER_ASSERT(reporter, budgeted == is_budgeted(surface)); + REPORTER_ASSERT(reporter, budgeted == is_budgeted(image)); + + // Now trigger copy-on-write + surface->getCanvas()->clear(SK_ColorBLUE); + + // They don't share a texture anymore but the budgets should still match. + REPORTER_ASSERT(reporter, budgeted == is_budgeted(surface)); + REPORTER_ASSERT(reporter, budgeted == is_budgeted(image)); } } #endif -- 2.7.4