diff options
author | Robert Phillips <robertphillips@google.com> | 2017-03-17 17:11:37 +0000 |
---|---|---|
committer | Skia Commit-Bot <skia-commit-bot@chromium.org> | 2017-03-17 17:44:01 +0000 |
commit | 31249bb2df023868e4585f14519bf17c835f8700 (patch) | |
tree | 2490cadaa74df7ffad4b41fdf33b8be4bae91bb4 | |
parent | b2082afc1d2076576327d9d1de5fbec6b13403fe (diff) |
Revert "Revert "Remove budgeted parameter from SkSurface::makeImageSnapshot""
This reverts commit 9e9188f84b15a25e27f63d5f8de3ccd393d9a173.
Reason for revert: Android-side fix has landed
Original change's description:
> 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 <bsalomon@google.com>
> > Commit-Queue: Robert Phillips <robertphillips@google.com>
> >
>
> 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 <liyuqian@google.com>
> Commit-Queue: Yuqian Li <liyuqian@google.com>
>
TBR=bsalomon@google.com,robertphillips@google.com,reviews@skia.org,liyuqian@google.com
# Not skipping CQ checks because original CL landed > 1 day ago.
Change-Id: If07d1b5db6e6c618d37445a0cf127780ed243a92
Reviewed-on: https://skia-review.googlesource.com/9843
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
-rw-r--r-- | bench/GrMipMapBench.cpp | 2 | ||||
-rw-r--r-- | gm/croppedrects.cpp | 2 | ||||
-rw-r--r-- | include/core/SkSurface.h | 7 | ||||
-rw-r--r-- | src/core/SkImagePriv.h | 6 | ||||
-rw-r--r-- | src/gpu/GrSurfaceProxy.cpp | 15 | ||||
-rw-r--r-- | src/gpu/GrSurfaceProxyPriv.h | 12 | ||||
-rw-r--r-- | src/image/SkImage_Gpu.cpp | 6 | ||||
-rw-r--r-- | src/image/SkImage_Gpu.h | 8 | ||||
-rw-r--r-- | src/image/SkSurface.cpp | 6 | ||||
-rw-r--r-- | src/image/SkSurface_Base.h | 8 | ||||
-rw-r--r-- | src/image/SkSurface_Gpu.cpp | 11 | ||||
-rw-r--r-- | src/image/SkSurface_Gpu.h | 2 | ||||
-rw-r--r-- | src/image/SkSurface_Raster.cpp | 6 | ||||
-rw-r--r-- | tests/GrTextureMipMapInvalidationTest.cpp | 8 | ||||
-rw-r--r-- | tests/SurfaceTest.cpp | 39 |
15 files changed, 42 insertions, 96 deletions
diff --git a/bench/GrMipMapBench.cpp b/bench/GrMipMapBench.cpp index 7b8ce3f3c9..89100e346d 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 2d7de95c55..0ea265c2fb 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 be657b1c62..52c0e5841a 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<SkImage> makeImageSnapshot(SkBudgeted = SkBudgeted::kYes); + sk_sp<SkImage> 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 ea44770d78..921fdc85c8 100644 --- a/src/core/SkImagePriv.h +++ b/src/core/SkImagePriv.h @@ -50,12 +50,6 @@ extern sk_sp<SkImage> 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 727bcb51aa..ac025dcc59 100644 --- a/src/gpu/GrSurfaceProxy.cpp +++ b/src/gpu/GrSurfaceProxy.cpp @@ -290,18 +290,3 @@ sk_sp<GrSurfaceContext> 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 0d10ef6504..e5a628c1e7 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 b8d9dea951..20abd88345 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 b3a165d079..53c38dc278 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<SkColorSpace>*, SkScalar scaleAdjust[2]) const override; diff --git a/src/image/SkSurface.cpp b/src/image/SkSurface.cpp index 08f79aea28..55aab3e992 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<SkImage> SkSurface::makeImageSnapshot(SkBudgeted budgeted) { - return asSB(this)->refCachedImage(budgeted); +sk_sp<SkImage> SkSurface::makeImageSnapshot() { + return asSB(this)->refCachedImage(); } sk_sp<SkSurface> SkSurface::makeSurface(const SkImageInfo& info) { diff --git a/src/image/SkSurface_Base.h b/src/image/SkSurface_Base.h index 33aeee2dd6..1b0f9ff975 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<SkImage> onNewImageSnapshot(SkBudgeted) = 0; + virtual sk_sp<SkImage> onNewImageSnapshot() = 0; /** * Default implementation: @@ -81,7 +81,7 @@ public: virtual void onPrepareForExternalIO() {} inline SkCanvas* getCachedCanvas(); - inline sk_sp<SkImage> refCachedImage(SkBudgeted); + inline sk_sp<SkImage> refCachedImage(); bool hasCachedImage() const { return fCachedImage != nullptr; } @@ -114,12 +114,12 @@ SkCanvas* SkSurface_Base::getCachedCanvas() { return fCachedCanvas.get(); } -sk_sp<SkImage> SkSurface_Base::refCachedImage(SkBudgeted budgeted) { +sk_sp<SkImage> 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 0bd34f8150..b610c9c9a1 100644 --- a/src/image/SkSurface_Gpu.cpp +++ b/src/image/SkSurface_Gpu.cpp @@ -83,7 +83,7 @@ sk_sp<SkSurface> SkSurface_Gpu::onNewSurface(const SkImageInfo& info) { origin, &this->props()); } -sk_sp<SkImage> SkSurface_Gpu::onNewImageSnapshot(SkBudgeted budgeted) { +sk_sp<SkImage> SkSurface_Gpu::onNewImageSnapshot() { GrRenderTargetContext* rtc = fDevice->accessRenderTargetContext(); if (!rtc) { return nullptr; @@ -102,7 +102,7 @@ sk_sp<SkImage> 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<SkImage> SkSurface_Gpu::onNewImageSnapshot(SkBudgeted budgeted) { if (tex) { image = sk_make_sp<SkImage_Gpu>(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<SkImage> image(this->refCachedImage(SkBudgeted::kNo)); + sk_sp<SkImage> 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 a625473639..5b92eebe3a 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<SkSurface> onNewSurface(const SkImageInfo&) override; - sk_sp<SkImage> onNewImageSnapshot(SkBudgeted) override; + sk_sp<SkImage> 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 47e0dbed33..4fae0f03c0 100644 --- a/src/image/SkSurface_Raster.cpp +++ b/src/image/SkSurface_Raster.cpp @@ -24,7 +24,7 @@ public: SkCanvas* onNewCanvas() override; sk_sp<SkSurface> onNewSurface(const SkImageInfo&) override; - sk_sp<SkImage> onNewImageSnapshot(SkBudgeted) override; + sk_sp<SkImage> 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<SkImage> SkSurface_Raster::onNewImageSnapshot(SkBudgeted) { +sk_sp<SkImage> 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<SkImage> cached(this->refCachedImage(SkBudgeted::kNo)); + sk_sp<SkImage> cached(this->refCachedImage()); SkASSERT(cached); if (SkBitmapImageGetPixelRef(cached.get()) == fBitmap.pixelRef()) { SkASSERT(fWeOwnThePixels); diff --git a/tests/GrTextureMipMapInvalidationTest.cpp b/tests/GrTextureMipMapInvalidationTest.cpp index 166c57c066..e7573dc010 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 b85ac81788..6264196640 100644 --- a/tests/SurfaceTest.cpp +++ b/tests/SurfaceTest.cpp @@ -460,27 +460,24 @@ static SkBudgeted is_budgeted(const sk_sp<SkImage> 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<SkImage> 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<SkImage> 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 |