From: Mike Reed Date: Thu, 29 Dec 2016 14:36:20 +0000 (-0500) Subject: remove unused ForceUnique option from makeImageSnapshot X-Git-Tag: accepted/tizen/5.0/unified/20181102.025319~55^2~1052 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=85ff84821d5f6c8d48f2af12bdffdd3b5f8baf2c;p=platform%2Fupstream%2FlibSkiaSharp.git remove unused ForceUnique option from makeImageSnapshot BUG=skia: Change-Id: I2555ceb86b597f7bb34c8fc48b3e07eb7115ea82 Reviewed-on: https://skia-review.googlesource.com/6481 Commit-Queue: Mike Reed Reviewed-by: Florin Malita --- diff --git a/gm/croppedrects.cpp b/gm/croppedrects.cpp index ce7ff4b..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(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 --- a/include/core/SkSurface.h +++ b/include/core/SkSurface.h @@ -257,18 +257,6 @@ public: sk_sp 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 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 --- a/src/image/SkSurface.cpp +++ b/src/image/SkSurface.cpp @@ -161,13 +161,7 @@ SkCanvas* SkSurface::getCanvas() { } sk_sp SkSurface::makeImageSnapshot(SkBudgeted budgeted) { - // the caller will call unref() to balance this - return asSB(this)->refCachedImage(budgeted, kNo_ForceUnique); -} - -sk_sp 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::makeSurface(const SkImageInfo& info) { diff --git a/src/image/SkSurface_Base.h b/src/image/SkSurface_Base.h index a8c1d8f..8c41ef5 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 refCachedImage(SkBudgeted, ForceUnique); + inline sk_sp refCachedImage(SkBudgeted); bool hasCachedImage() const { return fCachedImage != nullptr; } @@ -114,21 +114,16 @@ SkCanvas* SkSurface_Base::getCachedCanvas() { return fCachedCanvas.get(); } -sk_sp SkSurface_Base::refCachedImage(SkBudgeted budgeted, ForceUnique unique) { +sk_sp 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(snap); } diff --git a/src/image/SkSurface_Gpu.cpp b/src/image/SkSurface_Gpu.cpp index c81c06e..17376ea 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 image(this->refCachedImage(SkBudgeted::kNo, kNo_ForceUnique)); + sk_sp 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 --- 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 cached(this->refCachedImage(SkBudgeted::kNo, kNo_ForceUnique)); + sk_sp 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 --- 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 getImageBackingStore) { - return getImageBackingStore(a) == getImageBackingStore(b); -} - -static bool same_image_surf(SkImage* a, SkSurface* b, - std::function getImageBackingStore, - std::function getSurfaceBackingStore) { - return getImageBackingStore(a) == getSurfaceBackingStore(b); -} - -static void test_unique_image_snap(skiatest::Reporter* reporter, SkSurface* surface, - bool surfaceIsDirect, - std::function imageBackingStore, - std::function surfaceBackingStore) { - std::function ibs = imageBackingStore; - std::function sbs = surfaceBackingStore; - static const SkBudgeted kB = SkBudgeted::kNo; - { - sk_sp 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 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 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 image1(surface->makeImageSnapshot(kB, SkSurface::kNo_ForceUnique)); - sk_sp image2(surface->makeImageSnapshot(kB, SkSurface::kYes_ForceUnique)); - sk_sp image3(surface->makeImageSnapshot(kB, SkSurface::kNo_ForceUnique)); - sk_sp 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(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(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(0); - } - return static_cast(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(0); - } - return static_cast(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 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(