Take budgeted param when snapping new image.

Review URL: https://codereview.chromium.org/872543002

diff --git a/gyp/gpu.gypi b/gyp/gpu.gypi
index 26fc5cd..4110c7e 100644 (file)
--- a/gyp/gpu.gypi
+++ b/gyp/gpu.gypi
@@ -300,6 +300,7 @@
       '<(skia_src_path)/gpu/SkGrPixelRef.cpp',
       '<(skia_src_path)/gpu/SkGrTexturePixelRef.cpp',
 
+      '<(skia_src_path)/image/SkImage_Gpu.h',
       '<(skia_src_path)/image/SkImage_Gpu.cpp',
       '<(skia_src_path)/image/SkSurface_Gpu.h',
       '<(skia_src_path)/image/SkSurface_Gpu.cpp',

diff --git a/include/core/SkSurface.h b/include/core/SkSurface.h
index 1fd345a..ce27783 100644 (file)
--- a/include/core/SkSurface.h
+++ b/include/core/SkSurface.h
@@ -183,12 +183,14 @@ public:
     /**
      *  Returns an image of the current state of the surface pixels up to this
      *  point. Subsequent changes to the surface (by drawing into its canvas)
-     *  will not be reflected in this image.
+     *  will not be reflected in this image. If a copy must be made the Budgeted
+     *  parameter controls whether it counts against the resource budget
+     *  (currently for the gpu backend only).
      */
-    SkImage* newImageSnapshot();
+    SkImage* newImageSnapshot(Budgeted = kYes_Budgeted);
 
     /**
-     *  Thought the caller could get a snapshot image explicitly, and draw that,
+     *  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
      *  we'd know that the "snapshot" need only live until we've handed it off

diff --git a/src/image/SkImagePriv.h b/src/image/SkImagePriv.h
index 7306934..874476b 100644 (file)
--- a/src/image/SkImagePriv.h
+++ b/src/image/SkImagePriv.h
@@ -9,6 +9,7 @@
 #define SkImagePriv_DEFINED
 
 #include "SkImage.h"
+#include "SkSurface.h"
 
 // Call this if you explicitly want to use/share this pixelRef in the image
 extern SkImage* SkNewImageFromPixelRef(const SkImageInfo&, SkPixelRef*, size_t rowBytes,
@@ -40,11 +41,18 @@ extern const SkPixelRef* SkBitmapImageGetPixelRef(const SkImage* rasterImage);
 // in which case the surface may need to perform a copy-on-write.
 extern GrTexture* SkTextureImageGetTexture(SkImage* textureImage);
 
+// When a texture is shared by a surface and an image its budgeted status is that of the
+// surface. This function is used when the surface makes a new texture for itself in order
+// for the orphaned image to determine whether the original texture counts against the
+// budget or not.
+extern void SkTextureImageApplyBudgetedDecision(SkImage* textureImage);
+
 // Update the texture wrapped by an image created with NewTexture. This
 // is called when a surface and image share the same GrTexture and the
 // surface needs to perform a copy-on-write
 extern void SkTextureImageSetTexture(SkImage* image, GrTexture* texture);
 
-extern SkImage* SkNewImageFromBitmapTexture(const SkBitmap&, int sampleCountForNewSurfaces);
+extern SkImage* SkNewImageFromBitmapTexture(const SkBitmap&, int sampleCountForNewSurfaces,
+                                            SkSurface::Budgeted);
 
 #endif

diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp
index 8169b2a..7ccff17 100644 (file)
--- a/src/image/SkImage_Gpu.cpp
+++ b/src/image/SkImage_Gpu.cpp
@@ -5,48 +5,16 @@
  * found in the LICENSE file.
  */
 
-#include "SkImage_Base.h"
-#include "SkImagePriv.h"
-#include "SkBitmap.h"
+#include "SkImage_Gpu.h"
 #include "SkCanvas.h"
-#include "SkSurface.h"
 #include "GrContext.h"
-#include "GrTexture.h"
 
-class SkImage_Gpu : public SkImage_Base {
-public:
-    SK_DECLARE_INST_COUNT(SkImage_Gpu)
-
-    SkImage_Gpu(const SkBitmap&, int sampleCountForNewSurfaces);
-
-    void onDraw(SkCanvas*, SkScalar x, SkScalar y, const SkPaint*) const SK_OVERRIDE;
-    void onDrawRect(SkCanvas*, const SkRect* src, const SkRect& dst,
-                    const SkPaint*) const SK_OVERRIDE;
-    SkSurface* onNewSurface(const SkImageInfo&, const SkSurfaceProps&) const SK_OVERRIDE;
-    GrTexture* onGetTexture() const SK_OVERRIDE;
-    bool getROPixels(SkBitmap*) const SK_OVERRIDE;
-
-    GrTexture* getTexture() const { return fBitmap.getTexture(); }
-
-    SkShader* onNewShader(SkShader::TileMode,
-                                  SkShader::TileMode,
-                                  const SkMatrix* localMatrix) const SK_OVERRIDE;
-
-    bool isOpaque() const SK_OVERRIDE;
-
-private:
-    SkBitmap    fBitmap;
-    const int   fSampleCountForNewSurfaces;   // 0 if we don't know
-
-    typedef SkImage_Base INHERITED;
-};
-
-///////////////////////////////////////////////////////////////////////////////
-
-SkImage_Gpu::SkImage_Gpu(const SkBitmap& bitmap, int sampleCountForNewSurfaces)
+SkImage_Gpu::SkImage_Gpu(const SkBitmap& bitmap, int sampleCountForNewSurfaces,
+                         SkSurface::Budgeted budgeted)
     : INHERITED(bitmap.width(), bitmap.height(), NULL)
     , fBitmap(bitmap)
     , fSampleCountForNewSurfaces(sampleCountForNewSurfaces)
+    , fBudgeted(budgeted)
 {
     SkASSERT(fBitmap.getTexture());
 }
@@ -88,14 +56,18 @@ bool SkImage_Gpu::isOpaque() const {
 
 ///////////////////////////////////////////////////////////////////////////////
 
-SkImage* SkNewImageFromBitmapTexture(const SkBitmap& bitmap, int sampleCountForNewSurfaces) {
+SkImage* SkNewImageFromBitmapTexture(const SkBitmap& bitmap, int sampleCountForNewSurfaces,
+                                     SkSurface::Budgeted budgeted) {
     if (0 == bitmap.width() || 0 == bitmap.height() || NULL == bitmap.getTexture()) {
         return NULL;
     }
-    return SkNEW_ARGS(SkImage_Gpu, (bitmap, sampleCountForNewSurfaces));
+    return SkNEW_ARGS(SkImage_Gpu, (bitmap, sampleCountForNewSurfaces, budgeted));
 }
 
 GrTexture* SkTextureImageGetTexture(SkImage* image) {
     return ((SkImage_Gpu*)image)->getTexture();
 }
 
+extern void SkTextureImageApplyBudgetedDecision(SkImage* image) {
+    ((SkImage_Gpu*)image)->applyBudgetDecision();
+}

diff --git a/src/image/SkImage_Gpu.h b/src/image/SkImage_Gpu.h
new file mode 100644 (file)
index 0000000..f20625a
--- /dev/null
+++ b/src/image/SkImage_Gpu.h
@@ -0,0 +1,55 @@
+/*
+ * Copyright 2015 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef SkImage_Gpu_DEFINED
+#define SkImage_Gpu_DEFINED
+
+#include "GrTexture.h"
+#include "GrGpuResourceCacheAccess.h"
+#include "SkBitmap.h"
+#include "SkImage_Base.h"
+#include "SkImagePriv.h"
+#include "SkSurface.h"
+
+class SkImage_Gpu : public SkImage_Base {
+public:
+    SK_DECLARE_INST_COUNT(SkImage_Gpu)
+
+    SkImage_Gpu(const SkBitmap&, int sampleCountForNewSurfaces, SkSurface::Budgeted);
+
+    void onDraw(SkCanvas*, SkScalar x, SkScalar y, const SkPaint*) const SK_OVERRIDE;
+    void onDrawRect(SkCanvas*, const SkRect* src, const SkRect& dst,
+                    const SkPaint*) const SK_OVERRIDE;
+    SkSurface* onNewSurface(const SkImageInfo&, const SkSurfaceProps&) const SK_OVERRIDE;
+    GrTexture* onGetTexture() const SK_OVERRIDE;
+    bool getROPixels(SkBitmap*) const SK_OVERRIDE;
+
+    GrTexture* getTexture() const { return fBitmap.getTexture(); }
+
+    SkShader* onNewShader(SkShader::TileMode,
+                                  SkShader::TileMode,
+                                  const SkMatrix* localMatrix) const SK_OVERRIDE;
+
+    bool isOpaque() const SK_OVERRIDE;
+
+    void applyBudgetDecision() const {
+        if (fBudgeted) {
+            fBitmap.getTexture()->cacheAccess().makeBudgeted();
+        } else {
+            fBitmap.getTexture()->cacheAccess().makeUnbudgeted();
+        }
+    }
+
+private:
+    SkBitmap            fBitmap;
+    const int           fSampleCountForNewSurfaces;   // 0 if we don't know
+    SkSurface::Budgeted fBudgeted;
+
+    typedef SkImage_Base INHERITED;
+};
+
+#endif

diff --git a/src/image/SkSurface.cpp b/src/image/SkSurface.cpp
index 81b348f..2d3fa2b 100644 (file)
--- a/src/image/SkSurface.cpp
+++ b/src/image/SkSurface.cpp
@@ -79,7 +79,7 @@ SkSurface_Base::~SkSurface_Base() {
 }
 
 void SkSurface_Base::onDraw(SkCanvas* canvas, SkScalar x, SkScalar y, const SkPaint* paint) {
-    SkImage* image = this->newImageSnapshot();
+    SkImage* image = this->newImageSnapshot(kYes_Budgeted);
     if (image) {
         canvas->drawImage(image, x, y, paint);
         image->unref();
@@ -151,8 +151,8 @@ SkCanvas* SkSurface::getCanvas() {
     return asSB(this)->getCachedCanvas();
 }
 
-SkImage* SkSurface::newImageSnapshot() {
-    SkImage* image = asSB(this)->getCachedImage();
+SkImage* SkSurface::newImageSnapshot(Budgeted budgeted) {
+    SkImage* image = asSB(this)->getCachedImage(budgeted);
     SkSafeRef(image);   // the caller will call unref() to balance this
     return image;
 }

diff --git a/src/image/SkSurface_Base.h b/src/image/SkSurface_Base.h
index 4da4cfb..30af976 100644 (file)
--- a/src/image/SkSurface_Base.h
+++ b/src/image/SkSurface_Base.h
@@ -32,9 +32,9 @@ public:
      *  Allocate an SkImage that represents the current contents of the surface.
      *  This needs to be able to outlive the surface itself (if need be), and
      *  must faithfully represent the current contents, even if the surface
-     *  is chaged after this calle (e.g. it is drawn to via its canvas).
+     *  is changed after this called (e.g. it is drawn to via its canvas).
      */
-    virtual SkImage* onNewImageSnapshot() = 0;
+    virtual SkImage* onNewImageSnapshot(Budgeted) = 0;
 
     /**
      *  Default implementation:
@@ -61,7 +61,7 @@ public:
     virtual void onCopyOnWrite(ContentChangeMode) = 0;
 
     inline SkCanvas* getCachedCanvas();
-    inline SkImage* getCachedImage();
+    inline SkImage* getCachedImage(Budgeted);
 
     // called by SkSurface to compute a new genID
     uint32_t newGenerationID();
@@ -87,9 +87,9 @@ SkCanvas* SkSurface_Base::getCachedCanvas() {
     return fCachedCanvas;
 }
 
-SkImage* SkSurface_Base::getCachedImage() {
+SkImage* SkSurface_Base::getCachedImage(Budgeted budgeted) {
     if (NULL == fCachedImage) {
-        fCachedImage = this->onNewImageSnapshot();
+        fCachedImage = this->onNewImageSnapshot(budgeted);
         SkASSERT(!fCachedCanvas || fCachedCanvas->getSurfaceBase() == this);
     }
     return fCachedImage;

diff --git a/src/image/SkSurface_Gpu.cpp b/src/image/SkSurface_Gpu.cpp
index 6cd741d..34aec2b 100644 (file)
--- a/src/image/SkSurface_Gpu.cpp
+++ b/src/image/SkSurface_Gpu.cpp
@@ -44,9 +44,10 @@ SkSurface* SkSurface_Gpu::onNewSurface(const SkImageInfo& info) {
                                       &this->props());
 }
 
-SkImage* SkSurface_Gpu::onNewImageSnapshot() {
+SkImage* SkSurface_Gpu::onNewImageSnapshot(Budgeted budgeted) {
     const int sampleCount = fDevice->accessRenderTarget()->numSamples();
-    SkImage* image = SkNewImageFromBitmapTexture(fDevice->accessBitmap(false), sampleCount);
+    SkImage* image = SkNewImageFromBitmapTexture(fDevice->accessBitmap(false), sampleCount,
+                                                 budgeted);
     if (image) {
         as_IB(image)->initWithProps(this->props());
     }
@@ -63,9 +64,11 @@ void SkSurface_Gpu::onDraw(SkCanvas* canvas, SkScalar x, SkScalar y,
 // doesn't force an OpenGL flush.
 void SkSurface_Gpu::onCopyOnWrite(ContentChangeMode mode) {
     GrRenderTarget* rt = fDevice->accessRenderTarget();
-    // are we sharing our render target with the image?
-    SkASSERT(this->getCachedImage());
-    if (rt->asTexture() == SkTextureImageGetTexture(this->getCachedImage())) {
+    // 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.
+    SkImage* image = this->getCachedImage(kNo_Budgeted);
+    SkASSERT(image);
+    if (rt->asTexture() == SkTextureImageGetTexture(image)) {
         GrRenderTarget* oldRT = this->fDevice->accessRenderTarget();
         SkSurface::Budgeted budgeted = oldRT->cacheAccess().isBudgeted() ? kYes_Budgeted :
                                                                            kNo_Budgeted;
@@ -82,10 +85,7 @@ void SkSurface_Gpu::onCopyOnWrite(ContentChangeMode mode) {
         this->getCachedCanvas()->setRootDevice(newDevice);
         SkRefCnt_SafeAssign(fDevice, newDevice.get());
 
-        // For now we always treat the image snapshots as budgeted. We could make newImageSnapshot
-        // take a Budgeted param.
-        oldRT->cacheAccess().makeBudgeted();
-
+        SkTextureImageApplyBudgetedDecision(image);
     } else if (kDiscard_ContentChangeMode == mode) {
         this->SkSurface_Gpu::onDiscard();
     }

diff --git a/src/image/SkSurface_Gpu.h b/src/image/SkSurface_Gpu.h
index f87c5d7..90c00c4 100644 (file)
--- a/src/image/SkSurface_Gpu.h
+++ b/src/image/SkSurface_Gpu.h
@@ -23,7 +23,7 @@ public:
 
     SkCanvas* onNewCanvas() SK_OVERRIDE;
     SkSurface* onNewSurface(const SkImageInfo&) SK_OVERRIDE;
-    SkImage* onNewImageSnapshot() SK_OVERRIDE;
+    SkImage* onNewImageSnapshot(Budgeted) SK_OVERRIDE;
     virtual void onDraw(SkCanvas*, SkScalar x, SkScalar y,
                         const SkPaint*) SK_OVERRIDE;
     void onCopyOnWrite(ContentChangeMode) SK_OVERRIDE;

diff --git a/src/image/SkSurface_Raster.cpp b/src/image/SkSurface_Raster.cpp
index 0752383..9c38640 100644 (file)
--- a/src/image/SkSurface_Raster.cpp
+++ b/src/image/SkSurface_Raster.cpp
@@ -24,7 +24,7 @@ public:
 
     SkCanvas* onNewCanvas() SK_OVERRIDE;
     SkSurface* onNewSurface(const SkImageInfo&) SK_OVERRIDE;
-    SkImage* onNewImageSnapshot() SK_OVERRIDE;
+    SkImage* onNewImageSnapshot(Budgeted) SK_OVERRIDE;
     virtual void onDraw(SkCanvas*, SkScalar x, SkScalar y,
                         const SkPaint*) SK_OVERRIDE;
     void onCopyOnWrite(ContentChangeMode) SK_OVERRIDE;
@@ -118,14 +118,14 @@ void SkSurface_Raster::onDraw(SkCanvas* canvas, SkScalar x, SkScalar y,
     canvas->drawBitmap(fBitmap, x, y, paint);
 }
 
-SkImage* SkSurface_Raster::onNewImageSnapshot() {
+SkImage* SkSurface_Raster::onNewImageSnapshot(Budgeted) {
     return SkNewImageFromBitmap(fBitmap, fWeOwnThePixels, &this->props());
 }
 
 void SkSurface_Raster::onCopyOnWrite(ContentChangeMode mode) {
     // are we sharing pixelrefs with the image?
-    SkASSERT(this->getCachedImage());
-    if (SkBitmapImageGetPixelRef(this->getCachedImage()) == fBitmap.pixelRef()) {
+    SkASSERT(this->getCachedImage(kNo_Budgeted));
+    if (SkBitmapImageGetPixelRef(this->getCachedImage(kNo_Budgeted)) == fBitmap.pixelRef()) {
         SkASSERT(fWeOwnThePixels);
         if (kDiscard_ContentChangeMode == mode) {
             fBitmap.setPixelRef(NULL);

diff --git a/tests/DeferredCanvasTest.cpp b/tests/DeferredCanvasTest.cpp
index 3089dc0..6756f86 100644 (file)
--- a/tests/DeferredCanvasTest.cpp
+++ b/tests/DeferredCanvasTest.cpp
@@ -64,7 +64,7 @@ public:
         return NULL;
     }
 
-    SkImage* onNewImageSnapshot() SK_OVERRIDE {
+    SkImage* onNewImageSnapshot(Budgeted) SK_OVERRIDE {
         return SkNewImageFromBitmap(fBitmap, true, &this->props());
     }
 

diff --git a/tests/SurfaceTest.cpp b/tests/SurfaceTest.cpp
index 4816ffc..9449014 100644 (file)
--- a/tests/SurfaceTest.cpp
+++ b/tests/SurfaceTest.cpp
@@ -455,6 +455,51 @@ static void TestGetTexture(skiatest::Reporter* reporter,
     surface->notifyContentWillChange(SkSurface::kDiscard_ContentChangeMode);
     REPORTER_ASSERT(reporter, image->getTexture() == texture);
 }
+
+#include "GrGpuResourceCacheAccess.h"
+#include "SkGpuDevice.h"
+#include "SkImage_Gpu.h"
+#include "SkSurface_Gpu.h"
+
+SkSurface::Budgeted is_budgeted(SkSurface* surf) {
+    return ((SkSurface_Gpu*)surf)->getDevice()->accessRenderTarget()->cacheAccess().isBudgeted() ?
+        SkSurface::kYes_Budgeted : SkSurface::kNo_Budgeted;
+}
+
+SkSurface::Budgeted is_budgeted(SkImage* image) {
+    return ((SkImage_Gpu*)image)->getTexture()->cacheAccess().isBudgeted() ?
+        SkSurface::kYes_Budgeted : SkSurface::kNo_Budgeted;
+}
+
+static void test_surface_budget(skiatest::Reporter* reporter, GrContext* context) {
+    SkImageInfo info = SkImageInfo::MakeN32Premul(8,8);
+    for (int i = 0; i < 2; ++i) {
+        SkSurface::Budgeted sbudgeted = i ? SkSurface::kYes_Budgeted : SkSurface::kNo_Budgeted;
+        for (int j = 0; j < 2; ++j) {
+            SkSurface::Budgeted ibudgeted = j ? SkSurface::kYes_Budgeted : SkSurface::kNo_Budgeted;
+            SkAutoTUnref<SkSurface>
+                surface(SkSurface::NewRenderTarget(context, sbudgeted, info, 0));
+            SkASSERT(surface);
+            REPORTER_ASSERT(reporter, sbudgeted == is_budgeted(surface));
+
+            SkImage* image = surface->newImageSnapshot(ibudgeted);
+
+            // Initially the image shares a texture with the surface, and the surface decides
+            // whether it is budgeted or not.
+            REPORTER_ASSERT(reporter, sbudgeted == is_budgeted(surface));
+            REPORTER_ASSERT(reporter, sbudgeted == is_budgeted(image));
+
+            // Now trigger copy-on-write
+            surface->getCanvas()->clear(SK_ColorBLUE);
+
+            // They don't share a texture anymore. They should each have made their own budget
+            // decision.
+            REPORTER_ASSERT(reporter, sbudgeted == is_budgeted(surface));
+            REPORTER_ASSERT(reporter, ibudgeted == is_budgeted(image));
+        }
+    }
+}
+
 #endif
 
 static void TestSurfaceNoCanvas(skiatest::Reporter* reporter,
@@ -526,6 +571,7 @@ DEF_GPUTEST(Surface, reporter, factory) {
                 TestGetTexture(reporter, kGpu_SurfaceType, context);
                 TestGetTexture(reporter, kGpuScratch_SurfaceType, context);
                 test_empty_surface(reporter, context);
+                test_surface_budget(reporter, context);
             }
         }
     }