From 9e9188f84b15a25e27f63d5f8de3ccd393d9a173 Mon Sep 17 00:00:00 2001 From: Yuqian Li Date: Thu, 16 Mar 2017 15:04:53 +0000 Subject: [PATCH] Revert "Remove budgeted parameter from SkSurface::makeImageSnapshot" This reverts commit b64bcbdc3a5aa7b9e3ff216e4617ddc1db9260b5. Reason for revert: Android build failed as shown below. frameworks/base/libs/hwui/VkLayer.cpp:32:41: error: too many arguments to function call, expected 0, have 1 mImage = surface->makeImageSnapshot(SkBudgeted::kNo); Original change's description: > 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 > TBR=bsalomon@google.com,robertphillips@google.com,reviews@skia.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Change-Id: Iae6e313c15b2352bd0d4fc7b5629de0a51ac398e Reviewed-on: https://skia-review.googlesource.com/9788 Reviewed-by: Yuqian Li Commit-Queue: Yuqian Li --- 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, 96 insertions(+), 42 deletions(-) diff --git a/bench/GrMipMapBench.cpp b/bench/GrMipMapBench.cpp index 89100e3..7b8ce3f 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(), 0, 0, &paint); + canvas->drawImage(fSurface->makeImageSnapshot(SkBudgeted::kNo), 0, 0, &paint); canvas->restore(); } } diff --git a/gm/croppedrects.cpp b/gm/croppedrects.cpp index 0ea265c..2d7de95 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(); + fSrcImage = srcSurface->makeImageSnapshot(SkBudgeted::kYes); fSrcImageShader = fSrcImage->makeShader(SkShader::kClamp_TileMode, SkShader::kClamp_TileMode); } diff --git a/include/core/SkSurface.h b/include/core/SkSurface.h index 52c0e58..be657b1 100644 --- a/include/core/SkSurface.h +++ b/include/core/SkSurface.h @@ -250,10 +250,11 @@ 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. For the GPU-backend, the budgeting - * decision for the snapped image will match that of the surface. + * 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). */ - sk_sp makeImageSnapshot(); + sk_sp makeImageSnapshot(SkBudgeted = SkBudgeted::kYes); /** * 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 921fdc8..ea44770 100644 --- a/src/core/SkImagePriv.h +++ b/src/core/SkImagePriv.h @@ -50,6 +50,12 @@ 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 ac025dc..727bcb5 100644 --- a/src/gpu/GrSurfaceProxy.cpp +++ b/src/gpu/GrSurfaceProxy.cpp @@ -290,3 +290,18 @@ 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 e5a628c..0d10ef6 100644 --- a/src/gpu/GrSurfaceProxyPriv.h +++ b/src/gpu/GrSurfaceProxyPriv.h @@ -23,6 +23,18 @@ 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 3cc6e2d..10a8c0d 100644 --- a/src/image/SkImage_Gpu.cpp +++ b/src/image/SkImage_Gpu.cpp @@ -65,6 +65,12 @@ 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 53c38dc..b3a165d 100644 --- a/src/image/SkImage_Gpu.h +++ b/src/image/SkImage_Gpu.h @@ -29,6 +29,14 @@ 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 55aab3e..08f79ae 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(); + auto image = this->makeImageSnapshot(SkBudgeted::kYes); if (image) { canvas->drawImage(image, x, y, paint); } @@ -153,8 +153,8 @@ SkCanvas* SkSurface::getCanvas() { return asSB(this)->getCachedCanvas(); } -sk_sp SkSurface::makeImageSnapshot() { - return asSB(this)->refCachedImage(); +sk_sp SkSurface::makeImageSnapshot(SkBudgeted budgeted) { + return asSB(this)->refCachedImage(budgeted); } sk_sp SkSurface::makeSurface(const SkImageInfo& info) { diff --git a/src/image/SkSurface_Base.h b/src/image/SkSurface_Base.h index 1b0f9ff..33aeee2 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() = 0; + virtual sk_sp onNewImageSnapshot(SkBudgeted) = 0; /** * Default implementation: @@ -81,7 +81,7 @@ public: virtual void onPrepareForExternalIO() {} inline SkCanvas* getCachedCanvas(); - inline sk_sp refCachedImage(); + inline sk_sp refCachedImage(SkBudgeted); bool hasCachedImage() const { return fCachedImage != nullptr; } @@ -114,12 +114,12 @@ SkCanvas* SkSurface_Base::getCachedCanvas() { return fCachedCanvas.get(); } -sk_sp SkSurface_Base::refCachedImage() { +sk_sp SkSurface_Base::refCachedImage(SkBudgeted budgeted) { if (fCachedImage) { return fCachedImage; } - fCachedImage = this->onNewImageSnapshot(); + fCachedImage = this->onNewImageSnapshot(budgeted); SkASSERT(!fCachedCanvas || fCachedCanvas->getSurfaceBase() == this); return fCachedImage; diff --git a/src/image/SkSurface_Gpu.cpp b/src/image/SkSurface_Gpu.cpp index b610c9c..0bd34f8 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() { +sk_sp SkSurface_Gpu::onNewImageSnapshot(SkBudgeted budgeted) { GrRenderTargetContext* rtc = fDevice->accessRenderTargetContext(); if (!rtc) { return nullptr; @@ -102,7 +102,7 @@ sk_sp SkSurface_Gpu::onNewImageSnapshot() { copyCtx = ctx->contextPriv().makeDeferredSurfaceContext(desc, SkBackingFit::kExact, - srcProxy->isBudgeted()); + budgeted); if (!copyCtx) { return nullptr; } @@ -122,7 +122,7 @@ sk_sp SkSurface_Gpu::onNewImageSnapshot() { if (tex) { image = sk_make_sp(kNeedNewImageUniqueID, info.alphaType(), sk_ref_sp(tex), - sk_ref_sp(info.colorSpace()), srcProxy->isBudgeted()); + sk_ref_sp(info.colorSpace()), budgeted); } return image; } @@ -137,13 +137,14 @@ 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()); + sk_sp image(this->refCachedImage(SkBudgeted::kNo)); 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 5b92eeb..a625473 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() override; + sk_sp onNewImageSnapshot(SkBudgeted) 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 4fae0f0..47e0dbe 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() override; + sk_sp onNewImageSnapshot(SkBudgeted) 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() { +sk_sp SkSurface_Raster::onNewImageSnapshot(SkBudgeted) { 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()); + sk_sp cached(this->refCachedImage(SkBudgeted::kNo)); SkASSERT(cached); if (SkBitmapImageGetPixelRef(cached.get()) == fBitmap.pixelRef()) { SkASSERT(fWeOwnThePixels); diff --git a/tests/GrTextureMipMapInvalidationTest.cpp b/tests/GrTextureMipMapInvalidationTest.cpp index e7573dc..166c57c 100644 --- a/tests/GrTextureMipMapInvalidationTest.cpp +++ b/tests/GrTextureMipMapInvalidationTest.cpp @@ -20,11 +20,13 @@ // 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())->peekTexture()->texturePriv().hasMipMaps(); + return as_IB(surf->makeImageSnapshot(SkBudgeted::kYes))-> + peekTexture()->texturePriv().hasMipMaps(); }; auto mipsAreDirty = [] (SkSurface* surf) { - return as_IB(surf->makeImageSnapshot())->peekTexture()->texturePriv().mipMapsAreDirty(); + return as_IB(surf->makeImageSnapshot(SkBudgeted::kYes))-> + peekTexture()->texturePriv().mipMapsAreDirty(); }; GrContext* context = ctxInfo.grContext(); @@ -42,7 +44,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(), 0, 0, &paint); + surf2->getCanvas()->drawImage(surf1->makeImageSnapshot(SkBudgeted::kYes), 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 6264196..b85ac81 100644 --- a/tests/SurfaceTest.cpp +++ b/tests/SurfaceTest.cpp @@ -460,24 +460,27 @@ static SkBudgeted is_budgeted(const sk_sp image) { DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SurfaceBudget, reporter, ctxInfo) { SkImageInfo info = SkImageInfo::MakeN32Premul(8,8); - 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)); + 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)); + } } } #endif -- 2.7.4