diff options
author | Mike Reed <reed@google.com> | 2016-12-29 09:36:20 -0500 |
---|---|---|
committer | Skia Commit-Bot <skia-commit-bot@chromium.org> | 2016-12-29 16:01:42 +0000 |
commit | 85ff84821d5f6c8d48f2af12bdffdd3b5f8baf2c (patch) | |
tree | 73199a7a61951d83c961b0df24cfd72dc21bd7f4 | |
parent | db90e26e2bb7f0bf767e04129673bd2fa48545a6 (diff) |
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>
-rw-r--r-- | gm/croppedrects.cpp | 2 | ||||
-rw-r--r-- | include/core/SkSurface.h | 12 | ||||
-rw-r--r-- | src/image/SkSurface.cpp | 8 | ||||
-rw-r--r-- | src/image/SkSurface_Base.h | 19 | ||||
-rw-r--r-- | src/image/SkSurface_Gpu.cpp | 2 | ||||
-rw-r--r-- | src/image/SkSurface_Raster.cpp | 2 | ||||
-rw-r--r-- | tests/SurfaceTest.cpp | 128 |
7 files changed, 11 insertions, 162 deletions
diff --git a/gm/croppedrects.cpp b/gm/croppedrects.cpp index ce7ff4b519..2d7de95c55 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, 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 eb8f6aefc8..be657b1c62 100644 --- 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 3d6670f169..cb85708bf5 100644 --- 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 a8c1d8f0a4..8c41ef5946 100644 --- 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 c81c06ed53..17376ea7d7 100644 --- 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 308b072ea4..2f26e7fd83 100644 --- 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 ec8e4a9059..1f04de5903 100644 --- 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( |