diff options
-rw-r--r-- | include/core/SkImage.h | 8 | ||||
-rw-r--r-- | src/image/SkImage_Gpu.cpp | 42 | ||||
-rw-r--r-- | src/image/SkSurface_Gpu.cpp | 5 | ||||
-rw-r--r-- | tests/SurfaceTest.cpp | 40 |
4 files changed, 55 insertions, 40 deletions
diff --git a/include/core/SkImage.h b/include/core/SkImage.h index c85a2bfc6d..678133db33 100644 --- a/include/core/SkImage.h +++ b/include/core/SkImage.h @@ -65,7 +65,13 @@ public: static SkImage* NewRasterCopy(const Info&, const void* pixels, size_t rowBytes); static SkImage* NewRasterData(const Info&, SkData* pixels, size_t rowBytes); static SkImage* NewEncodedData(SkData*); - static SkImage* NewTexture(GrTexture*); + + /** + * GrTexture is a more logical parameter for this factory, but its + * interactions with scratch cache still has issues, so for now we take + * SkBitmap instead. This will be changed in the future. skbug.com/1449 + */ + static SkImage* NewTexture(const SkBitmap&); int width() const { return fWidth; } int height() const { return fHeight; } diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp index d19d2100c3..036e45bb65 100644 --- a/src/image/SkImage_Gpu.cpp +++ b/src/image/SkImage_Gpu.cpp @@ -17,7 +17,7 @@ class SkImage_Gpu : public SkImage_Base { public: SK_DECLARE_INST_COUNT(SkImage_Gpu) - SkImage_Gpu(GrTexture*); + explicit SkImage_Gpu(const SkBitmap&); virtual ~SkImage_Gpu(); virtual void onDraw(SkCanvas*, SkScalar x, SkScalar y, const SkPaint*) SK_OVERRIDE; @@ -28,12 +28,9 @@ public: return false; } - GrTexture* getTexture() { return fTexture; } - - void setTexture(GrTexture* texture); + GrTexture* getTexture() { return fBitmap.getTexture(); } private: - GrTexture* fTexture; SkBitmap fBitmap; typedef SkImage_Base INHERITED; @@ -43,18 +40,13 @@ SK_DEFINE_INST_COUNT(SkImage_Gpu) /////////////////////////////////////////////////////////////////////////////// -SkImage_Gpu::SkImage_Gpu(GrTexture* texture) - : INHERITED(texture->width(), texture->height()) - , fTexture(texture) { - - SkASSERT(NULL != fTexture); - fTexture->ref(); - fBitmap.setConfig(SkBitmap::kARGB_8888_Config, fTexture->width(), fTexture->height()); - fBitmap.setPixelRef(SkNEW_ARGS(SkGrPixelRef, (fTexture)))->unref(); +SkImage_Gpu::SkImage_Gpu(const SkBitmap& bitmap) + : INHERITED(bitmap.width(), bitmap.height()) + , fBitmap(bitmap) { + SkASSERT(NULL != fBitmap.getTexture()); } SkImage_Gpu::~SkImage_Gpu() { - SkSafeUnref(fTexture); } void SkImage_Gpu::onDraw(SkCanvas* canvas, SkScalar x, SkScalar y, @@ -68,33 +60,19 @@ void SkImage_Gpu::onDrawRectToRect(SkCanvas* canvas, const SkRect* src, const Sk } GrTexture* SkImage_Gpu::onGetTexture() { - return fTexture; -} - -void SkImage_Gpu::setTexture(GrTexture* texture) { - - if (texture == fTexture) { - return; - } - - SkRefCnt_SafeAssign(fTexture, texture); - fBitmap.setPixelRef(new SkGrPixelRef(texture))->unref(); + return fBitmap.getTexture(); } /////////////////////////////////////////////////////////////////////////////// -SkImage* SkImage::NewTexture(GrTexture* texture) { - if (NULL == texture) { +SkImage* SkImage::NewTexture(const SkBitmap& bitmap) { + if (NULL == bitmap.getTexture()) { return NULL; } - return SkNEW_ARGS(SkImage_Gpu, (texture)); + return SkNEW_ARGS(SkImage_Gpu, (bitmap)); } GrTexture* SkTextureImageGetTexture(SkImage* image) { return ((SkImage_Gpu*)image)->getTexture(); } - -void SkTextureImageSetTexture(SkImage* image, GrTexture* texture) { - ((SkImage_Gpu*)image)->setTexture(texture); -} diff --git a/src/image/SkSurface_Gpu.cpp b/src/image/SkSurface_Gpu.cpp index 75c3890eb5..e5b7bd4486 100644 --- a/src/image/SkSurface_Gpu.cpp +++ b/src/image/SkSurface_Gpu.cpp @@ -72,10 +72,7 @@ SkSurface* SkSurface_Gpu::onNewSurface(const SkImage::Info& info) { } SkImage* SkSurface_Gpu::onNewImageSnapshot() { - - GrRenderTarget* rt = fDevice->accessRenderTarget(); - - return SkImage::NewTexture(rt->asTexture()); + return SkImage::NewTexture(fDevice->accessBitmap(false)); } void SkSurface_Gpu::onDraw(SkCanvas* canvas, SkScalar x, SkScalar y, diff --git a/tests/SurfaceTest.cpp b/tests/SurfaceTest.cpp index 07322f950b..049f250667 100644 --- a/tests/SurfaceTest.cpp +++ b/tests/SurfaceTest.cpp @@ -139,15 +139,48 @@ static void TestSurfaceWritableAfterSnapshotRelease(skiatest::Reporter* reporter // This test succeeds by not triggering an assertion. // The test verifies that the surface remains writable (usable) after // acquiring and releasing a snapshot without triggering a copy on write. - SkSurface* surface = createSurface(surfaceType, context); - SkAutoTUnref<SkSurface> aur_surface(surface); + SkAutoTUnref<SkSurface> surface(createSurface(surfaceType, context)); SkCanvas* canvas = surface->getCanvas(); canvas->clear(1); surface->newImageSnapshot()->unref(); // Create and destroy SkImage - canvas->clear(2); + canvas->clear(2); // Must not assert internally } #if SK_SUPPORT_GPU +static void Test_crbug263329(skiatest::Reporter* reporter, + GrContext* context) { + // This is a regression test for crbug.com/263329 + // Bug was caused by onCopyOnWrite releasing the old surface texture + // back to the scratch texture pool even though the texture is used + // by and active SkImage_Gpu. + SkAutoTUnref<SkSurface> surface1(createSurface(kGpu_SurfaceType, context)); + SkAutoTUnref<SkSurface> surface2(createSurface(kGpu_SurfaceType, context)); + SkCanvas* canvas1 = surface1->getCanvas(); + SkCanvas* canvas2 = surface2->getCanvas(); + canvas1->clear(1); + SkAutoTUnref<SkImage> image1(surface1->newImageSnapshot()); + // Trigger copy on write, new backing is a scratch texture + canvas1->clear(2); + SkAutoTUnref<SkImage> image2(surface1->newImageSnapshot()); + // Trigger copy on write, old backing should not be returned to scratch + // pool because it is held by image2 + canvas1->clear(3); + + canvas2->clear(4); + SkAutoTUnref<SkImage> image3(surface2->newImageSnapshot()); + // Trigger copy on write on surface2. The new backing store should not + // be recycling a texture that is held by an existing image. + canvas2->clear(5); + SkAutoTUnref<SkImage> image4(surface2->newImageSnapshot()); + REPORTER_ASSERT(reporter, image4->getTexture() != image3->getTexture()); + // The following assertion checks crbug.com/263329 + REPORTER_ASSERT(reporter, image4->getTexture() != image2->getTexture()); + REPORTER_ASSERT(reporter, image4->getTexture() != image1->getTexture()); + REPORTER_ASSERT(reporter, image3->getTexture() != image2->getTexture()); + REPORTER_ASSERT(reporter, image3->getTexture() != image1->getTexture()); + REPORTER_ASSERT(reporter, image2->getTexture() != image1->getTexture()); +} + static void TestGetTexture(skiatest::Reporter* reporter, SurfaceType surfaceType, GrContext* context) { @@ -209,6 +242,7 @@ static void TestSurface(skiatest::Reporter* reporter, GrContextFactory* factory) TestGetTexture(reporter, kPicture_SurfaceType, NULL); if (NULL != factory) { GrContext* context = factory->get(GrContextFactory::kNative_GLContextType); + Test_crbug263329(reporter, context); TestSurfaceCopyOnWrite(reporter, kGpu_SurfaceType, context); TestSurfaceWritableAfterSnapshotRelease(reporter, kGpu_SurfaceType, context); TestSurfaceNoCanvas(reporter, kGpu_SurfaceType, context, SkSurface::kDiscard_ContentChangeMode); |