Fixed bug with SkImage leaving canvas backing store in an immutable state after destroy.

tests/SurfaceTest.cpp

 
 /*
  * Copyright 2013 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
+
+#include "GrContextFactory.h"

[Justin Novosad, 2013/04/03 14:31:26]

This is OK, even when SK_SUPPORT_GPU == 0, right?

[bsalomon, 2013/04/03 14:35:47]

Actually, no, the #include needs to be guarded. We could consider changing the
header to simply forward declare the class when SK_SUPPORT_GPU == 0.

 #include "SkCanvas.h"
 #include "SkRRect.h"
 #include "SkSurface.h"
 #include "Test.h"
 
-#if SK_SUPPORT_GPU
-#include "GrContextFactory.h"
-#else
-class GrContextFactory;
-class GrContext;
-#endif
-
 enum SurfaceType {
     kRaster_SurfaceType,
     kGpu_SurfaceType,
     kPicture_SurfaceType
 };
 
 static SkSurface* createSurface(SurfaceType surfaceType, GrContext* context) {
     static const SkImage::Info imageSpec = {
         10,  // width
         10,  // height
         SkImage::kPMColor_ColorType,
         SkImage::kPremul_AlphaType
     };
 
     switch (surfaceType) {
     case kRaster_SurfaceType:
         return SkSurface::NewRaster(imageSpec);
     case kGpu_SurfaceType:
-#if SK_SUPPORT_GPU
         SkASSERT(NULL != context);
         return SkSurface::NewRenderTarget(context, imageSpec);
-#else
-        SkASSERT(0);
-#endif
     case kPicture_SurfaceType:
         return SkSurface::NewPicture(10, 10);
     }
     SkASSERT(0);
     return NULL;
 }
 
 static void TestSurfaceCopyOnWrite(skiatest::Reporter* reporter, SurfaceType surfaceType,
                                    GrContext* context) {
     // Verify that the right canvas commands trigger a copy on write
@@ skipping 72 matching lines @@
     EXPECT_COPY_ON_WRITE(drawBitmapMatrix(testBitmap, testMatrix, NULL))
     EXPECT_COPY_ON_WRITE(drawBitmapNine(testBitmap, testIRect, testRect, NULL))
     EXPECT_COPY_ON_WRITE(drawSprite(testBitmap, 0, 0, NULL))
     EXPECT_COPY_ON_WRITE(drawText(testText.c_str(), testText.size(), 0, 1, testPaint))
     EXPECT_COPY_ON_WRITE(drawPosText(testText.c_str(), testText.size(), testPoints2, \
         testPaint))
     EXPECT_COPY_ON_WRITE(drawTextOnPath(testText.c_str(), testText.size(), testPath, NULL, \
         testPaint))
 }
 
+static void TestSurfaceWritableAfterSnapshotRelease(skiatest::Reporter* reporter,
+                                                    SurfaceType surfaceType,
+                                                    GrContext* context) {
+    // 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);
+    SkCanvas* canvas = surface->getCanvas();
+    canvas->clear(1);
+    surface->newImageShapshot()->unref();  // Create and destroy SkImage
+    canvas->clear(2);
+}
+
 static void TestSurface(skiatest::Reporter* reporter) {

[bsalomon, 2013/04/03 14:35:47]

Do we need this function anymore?

     TestSurfaceCopyOnWrite(reporter, kRaster_SurfaceType, NULL);
     TestSurfaceCopyOnWrite(reporter, kPicture_SurfaceType, NULL);
+    TestSurfaceWritableAfterSnapshotRelease(reporter, kRaster_SurfaceType, NULL);
+    TestSurfaceWritableAfterSnapshotRelease(reporter, kPicture_SurfaceType, NULL);
 }
 
 static void TestSurfaceGpu(skiatest::Reporter* reporter, GrContextFactory* factory) {
-#if SK_SUPPORT_GPU
-    GrContext* context = factory->get(GrContextFactory::kNative_GLContextType);
-    TestSurfaceCopyOnWrite(reporter, kGpu_SurfaceType, context);
-#endif
+    TestSurfaceCopyOnWrite(reporter, kRaster_SurfaceType, NULL);
+    TestSurfaceCopyOnWrite(reporter, kPicture_SurfaceType, NULL);
+    TestSurfaceWritableAfterSnapshotRelease(reporter, kRaster_SurfaceType, NULL);
+    TestSurfaceWritableAfterSnapshotRelease(reporter, kPicture_SurfaceType, NULL);
+    if (NULL != factory) {
+        GrContext* context = factory->get(GrContextFactory::kNative_GLContextType);
+        TestSurfaceCopyOnWrite(reporter, kGpu_SurfaceType, context);
+        TestSurfaceWritableAfterSnapshotRelease(reporter, kGpu_SurfaceType, context);
+    }
 }
 
 #include "TestClassDef.h"
-DEFINE_TESTCLASS("Surface", SurfaceTestClass, TestSurface)
 DEFINE_GPUTESTCLASS("SurfaceGpu", SurfaceGpuTestClass, TestSurfaceGpu)