Remove budgeted parameter from SkSurface::makeImageSnapshot (take 2)

This unused feature complicates MDB.

Chrome compiles locally for me with this CL.
frameworks/base/libs/hwui compiles locally for me with this CL.

Change-Id: Id3ad64dac72eace52855896df0d7ce3679f15884
Reviewed-on: https://skia-review.googlesource.com/9882
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Matt Sarett <msarett@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>

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();
         }
     }

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);
     }

gn/android_framework_defines.gni

@@ -18,4 +18,5 @@ android_framework_defines = [
   "SK_SUPPORT_LEGACY_CANVAS_HELPERS",
   "SK_SUPPORT_LEGACY_PATHEFFECT_SUBCLASSES",
   "SK_SUPPORT_LEGACY_DRAWVERTICES_VIRTUAL",
+  "SK_SUPPORT_LEGACY_IMG_SNAPSHOT",
 ]

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
+     *  will not be reflected in this image. For the GPU-backend, the budgeting
-     *  parameter controls whether it counts against the resource budget
+     *  decision for the snapped image will match that of the surface.
-     *  (currently for the gpu backend only).
      */
-    sk_sp<SkImage> makeImageSnapshot(SkBudgeted = SkBudgeted::kYes);
+    sk_sp<SkImage> makeImageSnapshot();
 
     /**
      *  Though the caller could get a snapshot image explicitly, and draw that,

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

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;
-}

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

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)) {

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;

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) {
+sk_sp<SkImage> SkSurface::makeImageSnapshot() {
-    return asSB(this)->refCachedImage(budgeted);
+    return asSB(this)->refCachedImage();
 }
 
 sk_sp<SkSurface> SkSurface::makeSurface(const SkImageInfo& info) {

src/image/SkSurface_Base.h

@@ -43,7 +43,16 @@ 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;
+#ifdef SK_SUPPORT_LEGACY_IMG_SNAPSHOT
+    // This entry point is never called but is just here so Android unit tests will compile
+    virtual sk_sp<SkImage> onNewImageSnapshot(SkBudgeted) { return nullptr; }
+
+    // This entry point should be pure virtual. It has a default implementation so Android
+    // unit tests will compile.
+    virtual sk_sp<SkImage> onNewImageSnapshot() { return nullptr; }
+#else
+    virtual sk_sp<SkImage> onNewImageSnapshot() = 0;
+#endif
 
     /**
      *  Default implementation:
@@ -81,7 +90,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 +123,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;

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();
     }

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;

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);

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))->
+        return as_IB(surf->makeImageSnapshot())->peekTexture()->texturePriv().hasMipMaps();
-                peekTexture()->texturePriv().hasMipMaps();
     };
 
     auto mipsAreDirty = [] (SkSurface* surf) {
-        return as_IB(surf->makeImageSnapshot(SkBudgeted::kYes))->
+        return as_IB(surf->makeImageSnapshot())->peekTexture()->texturePriv().mipMapsAreDirty();
-                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()));

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 budgeted : { SkBudgeted::kNo, SkBudgeted::kYes }) {
-        for (auto ibudgeted : { SkBudgeted::kNo, SkBudgeted::kYes }) {
+        auto surface(SkSurface::MakeRenderTarget(ctxInfo.grContext(), budgeted, info));
-            auto surface(SkSurface::MakeRenderTarget(ctxInfo.grContext(), sbudgeted, info));
+        SkASSERT(surface);
-            SkASSERT(surface);
+        REPORTER_ASSERT(reporter, budgeted == is_budgeted(surface));
-            REPORTER_ASSERT(reporter, sbudgeted == is_budgeted(surface));
 
-            sk_sp<SkImage> image(surface->makeImageSnapshot(ibudgeted));
+        sk_sp<SkImage> image(surface->makeImageSnapshot());
 
-            // Initially the image shares a texture with the surface, and the surface decides
+        // Initially the image shares a texture with the surface, and the
-            // whether it is budgeted or not.
+        // the budgets should always match.
-            REPORTER_ASSERT(reporter, sbudgeted == is_budgeted(surface));
+        REPORTER_ASSERT(reporter, budgeted == is_budgeted(surface));
-            REPORTER_ASSERT(reporter, sbudgeted == is_budgeted(image));
+        REPORTER_ASSERT(reporter, budgeted == is_budgeted(image));
 
-            // Now trigger copy-on-write
+        // Now trigger copy-on-write
-            surface->getCanvas()->clear(SK_ColorBLUE);
+        surface->getCanvas()->clear(SK_ColorBLUE);
 
-            // They don't share a texture anymore. They should each have made their own budget
+        // They don't share a texture anymore but the budgets should still match.
-            // decision.
+        REPORTER_ASSERT(reporter, budgeted == is_budgeted(surface));
-            REPORTER_ASSERT(reporter, sbudgeted == is_budgeted(surface));
+        REPORTER_ASSERT(reporter, budgeted == is_budgeted(image));
-            REPORTER_ASSERT(reporter, ibudgeted == is_budgeted(image));
-        }
     }
 }
 #endif