remove unused ForceUnique option from makeImageSnapshot

BUG=skia:

Change-Id: I2555ceb86b597f7bb34c8fc48b3e07eb7115ea82
Reviewed-on: https://skia-review.googlesource.com/6481
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>

diff --git a/gm/croppedrects.cpp b/gm/croppedrects.cpp
index ce7ff4b..2d7de95 100644 (file)
--- 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, SkSurface::kNo_ForceUnique);
+        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 eb8f6ae..be657b1 100644 (file)
--- a/include/core/SkSurface.h
+++ b/include/core/SkSurface.h
@@ -257,18 +257,6 @@ public:
     sk_sp<SkImage> makeImageSnapshot(SkBudgeted = SkBudgeted::kYes);
 
     /**
-     * In rare instances a client may want a unique copy of the SkSurface's contents in an image
-     * snapshot. This enum can be used to enforce that the image snapshot's backing store is not
-     * shared with another image snapshot or the surface's backing store. This is generally more
-     * expensive. This was added for Chromium bug 585250.
-     */
-    enum ForceUnique {
-        kNo_ForceUnique,
-        kYes_ForceUnique
-    };
-    sk_sp<SkImage> makeImageSnapshot(SkBudgeted, ForceUnique);
-
-    /**
      *  Though the caller could get a snapshot image explicitly, and draw that,
      *  it seems that directly drawing a surface into another canvas might be
      *  a common pattern, and that we could possibly be more efficient, since

diff --git a/src/image/SkSurface.cpp b/src/image/SkSurface.cpp
index 3d6670f..cb85708 100644 (file)
--- a/src/image/SkSurface.cpp
+++ b/src/image/SkSurface.cpp
@@ -161,13 +161,7 @@ SkCanvas* SkSurface::getCanvas() {
 }
 
 sk_sp<SkImage> SkSurface::makeImageSnapshot(SkBudgeted budgeted) {
-    // the caller will call unref() to balance this
-    return asSB(this)->refCachedImage(budgeted, kNo_ForceUnique);
-}
-
-sk_sp<SkImage> SkSurface::makeImageSnapshot(SkBudgeted budgeted, ForceUnique unique) {
-    // the caller will call unref() to balance this
-    return asSB(this)->refCachedImage(budgeted, unique);
+    return asSB(this)->refCachedImage(budgeted);
 }
 
 sk_sp<SkSurface> SkSurface::makeSurface(const SkImageInfo& info) {

diff --git a/src/image/SkSurface_Base.h b/src/image/SkSurface_Base.h
index a8c1d8f..8c41ef5 100644 (file)
--- a/src/image/SkSurface_Base.h
+++ b/src/image/SkSurface_Base.h
@@ -81,7 +81,7 @@ public:
     virtual void onPrepareForExternalIO() {}
 
     inline SkCanvas* getCachedCanvas();
-    inline sk_sp<SkImage> refCachedImage(SkBudgeted, ForceUnique);
+    inline sk_sp<SkImage> refCachedImage(SkBudgeted);
 
     bool hasCachedImage() const { return fCachedImage != nullptr; }
 
@@ -114,21 +114,16 @@ SkCanvas* SkSurface_Base::getCachedCanvas() {
     return fCachedCanvas.get();
 }
 
-sk_sp<SkImage> SkSurface_Base::refCachedImage(SkBudgeted budgeted, ForceUnique unique) {
+sk_sp<SkImage> SkSurface_Base::refCachedImage(SkBudgeted budgeted) {
     SkImage* snap = fCachedImage;
-    if (kYes_ForceUnique == unique && snap && !snap->unique()) {
-        snap = nullptr;
-    }
     if (snap) {
         return sk_ref_sp(snap);
     }
-    SkCopyPixelsMode cpm = (kYes_ForceUnique == unique) ? kAlways_SkCopyPixelsMode :
-                                                          kIfMutable_SkCopyPixelsMode;
-    snap = this->onNewImageSnapshot(budgeted, cpm).release();
-    if (kNo_ForceUnique == unique) {
-        SkASSERT(!fCachedImage);
-        fCachedImage = SkSafeRef(snap);
-    }
+
+    snap = this->onNewImageSnapshot(budgeted, kIfMutable_SkCopyPixelsMode).release();
+    SkASSERT(!fCachedImage);
+    fCachedImage = SkSafeRef(snap);
+
     SkASSERT(!fCachedCanvas || fCachedCanvas->getSurfaceBase() == this);
     return sk_sp<SkImage>(snap);
 }

diff --git a/src/image/SkSurface_Gpu.cpp b/src/image/SkSurface_Gpu.cpp
index c81c06e..17376ea 100644 (file)
--- a/src/image/SkSurface_Gpu.cpp
+++ b/src/image/SkSurface_Gpu.cpp
@@ -136,7 +136,7 @@ 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, kNo_ForceUnique));
+    sk_sp<SkImage> image(this->refCachedImage(SkBudgeted::kNo));
     SkASSERT(image);
     if (rt->asTexture() == as_IB(image)->peekTexture()) {
         this->fDevice->replaceRenderTargetContext(SkSurface::kRetain_ContentChangeMode == mode);

diff --git a/src/image/SkSurface_Raster.cpp b/src/image/SkSurface_Raster.cpp
index 308b072..2f26e7f 100644 (file)
--- a/src/image/SkSurface_Raster.cpp
+++ b/src/image/SkSurface_Raster.cpp
@@ -155,7 +155,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, kNo_ForceUnique));
+    sk_sp<SkImage> cached(this->refCachedImage(SkBudgeted::kNo));
     SkASSERT(cached);
     if (SkBitmapImageGetPixelRef(cached.get()) == fBitmap.pixelRef()) {
         SkASSERT(fWeOwnThePixels);

diff --git a/tests/SurfaceTest.cpp b/tests/SurfaceTest.cpp
index ec8e4a9..1f04de5 100644 (file)
--- a/tests/SurfaceTest.cpp
+++ b/tests/SurfaceTest.cpp
@@ -221,134 +221,6 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SurfaceBackendHandleAccessCopyOnWrite_Gpu, re
 }
 #endif
 
-static bool same_image(SkImage* a, SkImage* b,
-                       std::function<intptr_t(SkImage*)> getImageBackingStore) {
-    return getImageBackingStore(a) == getImageBackingStore(b);
-}
-
-static bool same_image_surf(SkImage* a, SkSurface* b,
-                            std::function<intptr_t(SkImage*)> getImageBackingStore,
-                            std::function<intptr_t(SkSurface*)> getSurfaceBackingStore) {
-    return getImageBackingStore(a) == getSurfaceBackingStore(b);
-}
-
-static void test_unique_image_snap(skiatest::Reporter* reporter, SkSurface* surface,
-                                   bool surfaceIsDirect,
-                                   std::function<intptr_t(SkImage*)> imageBackingStore,
-                                   std::function<intptr_t(SkSurface*)> surfaceBackingStore) {
-    std::function<intptr_t(SkImage*)> ibs = imageBackingStore;
-    std::function<intptr_t(SkSurface*)> sbs = surfaceBackingStore;
-    static const SkBudgeted kB = SkBudgeted::kNo;
-    {
-        sk_sp<SkImage> image(surface->makeImageSnapshot(kB, SkSurface::kYes_ForceUnique));
-        REPORTER_ASSERT(reporter, !same_image_surf(image.get(), surface, ibs, sbs));
-        REPORTER_ASSERT(reporter, image->unique());
-    }
-    {
-        sk_sp<SkImage> image1(surface->makeImageSnapshot(kB, SkSurface::kYes_ForceUnique));
-        REPORTER_ASSERT(reporter, !same_image_surf(image1.get(), surface, ibs, sbs));
-        REPORTER_ASSERT(reporter, image1->unique());
-        sk_sp<SkImage> image2(surface->makeImageSnapshot(kB, SkSurface::kYes_ForceUnique));
-        REPORTER_ASSERT(reporter, !same_image_surf(image2.get(), surface, ibs, sbs));
-        REPORTER_ASSERT(reporter, !same_image(image1.get(), image2.get(), ibs));
-        REPORTER_ASSERT(reporter, image2->unique());
-    }
-    {
-        sk_sp<SkImage> image1(surface->makeImageSnapshot(kB, SkSurface::kNo_ForceUnique));
-        sk_sp<SkImage> image2(surface->makeImageSnapshot(kB, SkSurface::kYes_ForceUnique));
-        sk_sp<SkImage> image3(surface->makeImageSnapshot(kB, SkSurface::kNo_ForceUnique));
-        sk_sp<SkImage> image4(surface->makeImageSnapshot(kB, SkSurface::kYes_ForceUnique));
-        // Image 1 and 3 ought to be the same (or we're missing an optimization).
-        REPORTER_ASSERT(reporter, same_image(image1.get(), image3.get(), ibs));
-        // If the surface is not direct then images 1 and 3 should alias the surface's
-        // store.
-        REPORTER_ASSERT(reporter, !surfaceIsDirect == same_image_surf(image1.get(), surface, ibs, sbs));
-        // Image 2 should not be shared with any other image.
-        REPORTER_ASSERT(reporter, !same_image(image1.get(), image2.get(), ibs) &&
-                                  !same_image(image3.get(), image2.get(), ibs) &&
-                                  !same_image(image4.get(), image2.get(), ibs));
-        REPORTER_ASSERT(reporter, image2->unique());
-        REPORTER_ASSERT(reporter, !same_image_surf(image2.get(), surface, ibs, sbs));
-        // Image 4 should not be shared with any other image.
-        REPORTER_ASSERT(reporter, !same_image(image1.get(), image4.get(), ibs) &&
-                                  !same_image(image3.get(), image4.get(), ibs));
-        REPORTER_ASSERT(reporter, !same_image_surf(image4.get(), surface, ibs, sbs));
-        REPORTER_ASSERT(reporter, image4->unique());
-    }
-}
-
-DEF_TEST(UniqueImageSnapshot, reporter) {
-    auto getImageBackingStore = [reporter](SkImage* image) {
-        SkPixmap pm;
-        bool success = image->peekPixels(&pm);
-        REPORTER_ASSERT(reporter, success);
-        return reinterpret_cast<intptr_t>(pm.addr());
-    };
-    auto getSufaceBackingStore = [reporter](SkSurface* surface) {
-        SkPixmap pmap;
-        const void* pixels = surface->getCanvas()->peekPixels(&pmap) ? pmap.addr() : nullptr;
-        REPORTER_ASSERT(reporter, pixels);
-        return reinterpret_cast<intptr_t>(pixels);
-    };
-
-    auto surface(create_surface());
-    test_unique_image_snap(reporter, surface.get(), false, getImageBackingStore,
-                           getSufaceBackingStore);
-    surface = create_direct_surface();
-    test_unique_image_snap(reporter, surface.get(), true, getImageBackingStore,
-                           getSufaceBackingStore);
-}
-
-#if SK_SUPPORT_GPU
-DEF_GPUTEST_FOR_RENDERING_CONTEXTS(UniqueImageSnapshot_Gpu, reporter, ctxInfo) {
-    GrContext* context = ctxInfo.grContext();
-    for (auto& surface_func : { &create_gpu_surface, &create_gpu_scratch_surface }) {
-        auto surface(surface_func(context, kOpaque_SkAlphaType, nullptr));
-
-        auto imageBackingStore = [reporter](SkImage* image) {
-            GrTexture* texture = as_IB(image)->peekTexture();
-            if (!texture) {
-                ERRORF(reporter, "Not texture backed.");
-                return static_cast<intptr_t>(0);
-            }
-            return static_cast<intptr_t>(texture->uniqueID().asUInt());
-        };
-
-        auto surfaceBackingStore = [reporter](SkSurface* surface) {
-            GrRenderTargetContext* rtc =
-                surface->getCanvas()->internal_private_accessTopLayerRenderTargetContext();
-            GrRenderTarget* rt = rtc->accessRenderTarget();
-            if (!rt) {
-                ERRORF(reporter, "Not render target backed.");
-                return static_cast<intptr_t>(0);
-            }
-            return static_cast<intptr_t>(rt->uniqueID().asUInt());
-        };
-
-        test_unique_image_snap(reporter, surface.get(), false, imageBackingStore,
-                               surfaceBackingStore);
-
-        // Test again with a "direct" render target;
-        GrBackendObject textureObject = context->getGpu()->createTestingOnlyBackendTexture(nullptr,
-            10, 10, kRGBA_8888_GrPixelConfig, true);
-        GrBackendTextureDesc desc;
-        desc.fConfig = kRGBA_8888_GrPixelConfig;
-        desc.fWidth = 10;
-        desc.fHeight = 10;
-        desc.fFlags = kRenderTarget_GrBackendTextureFlag;
-        desc.fTextureHandle = textureObject;
-
-        {
-            sk_sp<SkSurface> surface(SkSurface::MakeFromBackendTexture(context, desc, nullptr));
-            test_unique_image_snap(reporter, surface.get(), true, imageBackingStore,
-                                   surfaceBackingStore);
-        }
-
-        context->getGpu()->deleteTestingOnlyBackendTexture(textureObject);
-    }
-}
-#endif
-
 #if SK_SUPPORT_GPU
 
 static void test_backend_handle_unique_id(