Revert "Revert "Revert "Remove budgeted parameter from SkSurface::makeImageSnapshot"""

author Yuqian Li <liyuqian@google.com>

Fri, 17 Mar 2017 20:50:06 +0000 (20:50 +0000)

committer Skia Commit-Bot <skia-commit-bot@chromium.org>

Fri, 17 Mar 2017 20:50:19 +0000 (20:50 +0000)
author Yuqian Li <liyuqian@google.com>
Fri, 17 Mar 2017 20:50:06 +0000 (20:50 +0000)
committer Skia Commit-Bot <skia-commit-bot@chromium.org>
Fri, 17 Mar 2017 20:50:19 +0000 (20:50 +0000)
diff --git a/bench/GrMipMapBench.cpp b/bench/GrMipMapBench.cpp

index 89100e3..7b8ce3f 100644 (file)
--- a/bench/GrMipMapBench.cpp
+++ b/bench/GrMipMapBench.cpp
@@ -55,7 +55,7 @@ protected:
              // Draw reduced version of surface to original canvas, to trigger mip generation
              canvas->save();
              canvas->scale(0.1f, 0.1f);
-            canvas->drawImage(fSurface->makeImageSnapshot(), 0, 0, &paint);
+            canvas->drawImage(fSurface->makeImageSnapshot(SkBudgeted::kNo), 0, 0, &paint);
              canvas->restore();
          }
      }
diff --git a/gm/croppedrects.cpp b/gm/croppedrects.cpp

index 0ea265c..2d7de95 100644 (file)
--- 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();
+        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 52c0e58..be657b1 100644 (file)
--- a/include/core/SkSurface.h
+++ b/include/core/SkSurface.h
@@ -250,10 +250,11 @@ 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. For the GPU-backend, the budgeting
-     *  decision for the snapped image will match that of the surface.
+     *  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).
       */
-    sk_sp<SkImage> makeImageSnapshot();
+    sk_sp<SkImage> makeImageSnapshot(SkBudgeted = SkBudgeted::kYes);
  
      /**
       *  Though the caller could get a snapshot image explicitly, and draw that,
diff --git a/src/core/SkImagePriv.h b/src/core/SkImagePriv.h

index 921fdc8..ea44770 100644 (file)
--- a/src/core/SkImagePriv.h
+++ b/src/core/SkImagePriv.h
@@ -50,6 +50,12 @@ extern sk_sp<SkImage> SkMakeImageFromRasterBitmap(const SkBitmap&, SkCopyPixelsM
  // in which case the surface may need to perform a copy-on-write.
  extern const SkPixelRef* SkBitmapImageGetPixelRef(const SkImage* rasterImage);
  
+// 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
diff --git a/src/gpu/GrSurfaceProxy.cpp b/src/gpu/GrSurfaceProxy.cpp

index ac025dc..727bcb5 100644 (file)
--- a/src/gpu/GrSurfaceProxy.cpp
+++ b/src/gpu/GrSurfaceProxy.cpp
@@ -290,3 +290,18 @@ sk_sp<GrSurfaceContext> GrSurfaceProxy::TestCopy(GrContext* context, const GrSur
      return dstContext;
  }
  
+void GrSurfaceProxyPriv::makeBudgeted() {
+    if (fProxy->fTarget) {
+        fProxy->fTarget->resourcePriv().makeBudgeted();
+    }
+
+    fProxy->fBudgeted = SkBudgeted::kYes;
+}
+
+void GrSurfaceProxyPriv::makeUnbudgeted() {
+    if (fProxy->fTarget) {
+        fProxy->fTarget->resourcePriv().makeUnbudgeted();
+    }
+
+    fProxy->fBudgeted = SkBudgeted::kNo;
+}
diff --git a/src/gpu/GrSurfaceProxyPriv.h b/src/gpu/GrSurfaceProxyPriv.h

index e5a628c..0d10ef6 100644 (file)
--- a/src/gpu/GrSurfaceProxyPriv.h
+++ b/src/gpu/GrSurfaceProxyPriv.h
@@ -23,6 +23,18 @@ public:
      // Don't abuse these two!!!!!!!
      bool isExact() const { return SkBackingFit::kExact == fProxy->fFit; }
  
+    // These next two are very specialized and wacky - don't use them!
+
+    // In the case where an unbudgeted, deferred SkSurface_Gpu has snapped a budgeted, deferred
+    // SkImage_Gpu, this serves to propagate the budgeting forward in time. For now, and
+    // presumably forever, this will not change any flushing decisions but may make Ganesh
+    // appear to have gone over budget. In the case of non-deferred proxies this will immediately
+    // propagate the budget decision to the resource, which in itself is dubious.
+    void makeBudgeted();
+    // In the case where a budgeted, deferred SkSurface_Gpu has snapped an unbudgeted, deferred
+    // SkImage_Gpu, this serves to propagate the lack of budgeting forward in time.
+    void makeUnbudgeted();
+
  private:
      explicit GrSurfaceProxyPriv(GrSurfaceProxy* proxy) : fProxy(proxy) {}
      GrSurfaceProxyPriv(const GrSurfaceProxyPriv&) {} // unimpl
diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp

index 20abd88..b8d9dea 100644 (file)
--- a/src/image/SkImage_Gpu.cpp
+++ b/src/image/SkImage_Gpu.cpp
@@ -65,6 +65,12 @@ SkImage_Gpu::~SkImage_Gpu() {
      }
  }
  
+extern void SkTextureImageApplyBudgetedDecision(SkImage* image) {
+    if (image->isTextureBacked()) {
+        ((SkImage_Gpu*)image)->applyBudgetDecision();
+    }
+}
+
  SkImageInfo SkImage_Gpu::onImageInfo() const {
      SkColorType ct;
      if (!GrPixelConfigToColorType(fProxy->config(), &ct)) {
diff --git a/src/image/SkImage_Gpu.h b/src/image/SkImage_Gpu.h

index 53c38dc..b3a165d 100644 (file)
--- a/src/image/SkImage_Gpu.h
+++ b/src/image/SkImage_Gpu.h
@@ -29,6 +29,14 @@ public:
      SkImageInfo onImageInfo() const override;
      SkAlphaType onAlphaType() const override { return fAlphaType; }
  
+    void applyBudgetDecision() const {
+        if (SkBudgeted::kYes == fBudgeted) {
+            fProxy->priv().makeBudgeted();
+        } else {
+            fProxy->priv().makeUnbudgeted();
+        }
+    }
+
      bool getROPixels(SkBitmap*, SkColorSpace* dstColorSpace, CachingHint) const override;
      GrTexture* asTextureRef(GrContext*, const GrSamplerParams&, SkColorSpace*,
                              sk_sp<SkColorSpace>*, SkScalar scaleAdjust[2]) const override;
diff --git a/src/image/SkSurface.cpp b/src/image/SkSurface.cpp

index 55aab3e..08f79ae 100644 (file)
--- a/src/image/SkSurface.cpp
+++ b/src/image/SkSurface.cpp
@@ -71,7 +71,7 @@ SkSurface_Base::~SkSurface_Base() {
  }
  
  void SkSurface_Base::onDraw(SkCanvas* canvas, SkScalar x, SkScalar y, const SkPaint* paint) {
-    auto image = this->makeImageSnapshot();
+    auto image = this->makeImageSnapshot(SkBudgeted::kYes);
      if (image) {
          canvas->drawImage(image, x, y, paint);
      }
@@ -153,8 +153,8 @@ SkCanvas* SkSurface::getCanvas() {
      return asSB(this)->getCachedCanvas();
  }
  
-sk_sp<SkImage> SkSurface::makeImageSnapshot() {
-    return asSB(this)->refCachedImage();
+sk_sp<SkImage> SkSurface::makeImageSnapshot(SkBudgeted budgeted) {
+    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 1b0f9ff..33aeee2 100644 (file)
--- a/src/image/SkSurface_Base.h
+++ b/src/image/SkSurface_Base.h
@@ -43,7 +43,7 @@ public:
       *  must faithfully represent the current contents, even if the surface
       *  is changed after this called (e.g. it is drawn to via its canvas).
       */
-    virtual sk_sp<SkImage> onNewImageSnapshot() = 0;
+    virtual sk_sp<SkImage> onNewImageSnapshot(SkBudgeted) = 0;
  
      /**
       *  Default implementation:
@@ -81,7 +81,7 @@ public:
      virtual void onPrepareForExternalIO() {}
  
      inline SkCanvas* getCachedCanvas();
-    inline sk_sp<SkImage> refCachedImage();
+    inline sk_sp<SkImage> refCachedImage(SkBudgeted);
  
      bool hasCachedImage() const { return fCachedImage != nullptr; }
  
@@ -114,12 +114,12 @@ SkCanvas* SkSurface_Base::getCachedCanvas() {
      return fCachedCanvas.get();
  }
  
-sk_sp<SkImage> SkSurface_Base::refCachedImage() {
+sk_sp<SkImage> SkSurface_Base::refCachedImage(SkBudgeted budgeted) {
      if (fCachedImage) {
          return 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 b610c9c..0bd34f8 100644 (file)
--- a/src/image/SkSurface_Gpu.cpp
+++ b/src/image/SkSurface_Gpu.cpp
@@ -83,7 +83,7 @@ sk_sp<SkSurface> SkSurface_Gpu::onNewSurface(const SkImageInfo& info) {
                                         origin, &this->props());
  }
  
-sk_sp<SkImage> SkSurface_Gpu::onNewImageSnapshot() {
+sk_sp<SkImage> SkSurface_Gpu::onNewImageSnapshot(SkBudgeted budgeted) {
      GrRenderTargetContext* rtc = fDevice->accessRenderTargetContext();
      if (!rtc) {
          return nullptr;
@@ -102,7 +102,7 @@ sk_sp<SkImage> SkSurface_Gpu::onNewImageSnapshot() {
  
          copyCtx = ctx->contextPriv().makeDeferredSurfaceContext(desc,
                                                                  SkBackingFit::kExact,
-                                                                srcProxy->isBudgeted());
+                                                                budgeted);
          if (!copyCtx) {
              return nullptr;
          }
@@ -122,7 +122,7 @@ sk_sp<SkImage> SkSurface_Gpu::onNewImageSnapshot() {
      if (tex) {
          image = sk_make_sp<SkImage_Gpu>(kNeedNewImageUniqueID,
                                          info.alphaType(), sk_ref_sp(tex),
-                                        sk_ref_sp(info.colorSpace()), srcProxy->isBudgeted());
+                                        sk_ref_sp(info.colorSpace()), budgeted);
      }
      return image;
  }
@@ -137,13 +137,14 @@ 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());
+    sk_sp<SkImage> image(this->refCachedImage(SkBudgeted::kNo));
      SkASSERT(image);
      // MDB TODO: this is unfortunate. The snapping of an Image_Gpu from a surface currently
-    // funnels down to a GrTexture. Once Image_Gpus are proxy-backed we should be able to
+    // funnels down to a GrTexture. Once Image_Gpus are proxy-backed we should be able to 
      // compare proxy uniqueIDs.
      if (rt->asTexture()->getTextureHandle() == image->getTextureHandle(false)) {
          fDevice->replaceRenderTargetContext(SkSurface::kRetain_ContentChangeMode == mode);
+        SkTextureImageApplyBudgetedDecision(image.get());
      } else if (kDiscard_ContentChangeMode == mode) {
          this->SkSurface_Gpu::onDiscard();
      }
diff --git a/src/image/SkSurface_Gpu.h b/src/image/SkSurface_Gpu.h

index 5b92eeb..a625473 100644 (file)
--- a/src/image/SkSurface_Gpu.h
+++ b/src/image/SkSurface_Gpu.h
@@ -23,7 +23,7 @@ public:
      bool onGetRenderTargetHandle(GrBackendObject*, BackendHandleAccess) override;
      SkCanvas* onNewCanvas() override;
      sk_sp<SkSurface> onNewSurface(const SkImageInfo&) override;
-    sk_sp<SkImage> onNewImageSnapshot() override;
+    sk_sp<SkImage> onNewImageSnapshot(SkBudgeted) override;
      void onCopyOnWrite(ContentChangeMode) override;
      void onDiscard() override;
      void onPrepareForExternalIO() override;
diff --git a/src/image/SkSurface_Raster.cpp b/src/image/SkSurface_Raster.cpp

index 4fae0f0..47e0dbe 100644 (file)
--- a/src/image/SkSurface_Raster.cpp
+++ b/src/image/SkSurface_Raster.cpp
@@ -24,7 +24,7 @@ public:
  
      SkCanvas* onNewCanvas() override;
      sk_sp<SkSurface> onNewSurface(const SkImageInfo&) override;
-    sk_sp<SkImage> onNewImageSnapshot() override;
+    sk_sp<SkImage> onNewImageSnapshot(SkBudgeted) override;
      void onDraw(SkCanvas*, SkScalar x, SkScalar y, const SkPaint*) override;
      void onCopyOnWrite(ContentChangeMode) override;
      void onRestoreBackingMutability() override;
@@ -130,7 +130,7 @@ void SkSurface_Raster::onDraw(SkCanvas* canvas, SkScalar x, SkScalar y,
      canvas->drawBitmap(fBitmap, x, y, paint);
  }
  
-sk_sp<SkImage> SkSurface_Raster::onNewImageSnapshot() {
+sk_sp<SkImage> SkSurface_Raster::onNewImageSnapshot(SkBudgeted) {
      SkCopyPixelsMode cpm = kIfMutable_SkCopyPixelsMode;
      if (fWeOwnThePixels) {
          // SkImage_raster requires these pixels are immutable for its full lifetime.
@@ -156,7 +156,7 @@ void SkSurface_Raster::onRestoreBackingMutability() {
  
  void SkSurface_Raster::onCopyOnWrite(ContentChangeMode mode) {
      // are we sharing pixelrefs with the image?
-    sk_sp<SkImage> cached(this->refCachedImage());
+    sk_sp<SkImage> cached(this->refCachedImage(SkBudgeted::kNo));
      SkASSERT(cached);
      if (SkBitmapImageGetPixelRef(cached.get()) == fBitmap.pixelRef()) {
          SkASSERT(fWeOwnThePixels);
diff --git a/tests/GrTextureMipMapInvalidationTest.cpp b/tests/GrTextureMipMapInvalidationTest.cpp

index e7573dc..166c57c 100644 (file)
--- a/tests/GrTextureMipMapInvalidationTest.cpp
+++ b/tests/GrTextureMipMapInvalidationTest.cpp
@@ -20,11 +20,13 @@
  // Tests that MIP maps are created and invalidated as expected when drawing to and from GrTextures.
  DEF_GPUTEST_FOR_NULLGL_CONTEXT(GrTextureMipMapInvalidationTest, reporter, ctxInfo) {
      auto isMipped = [] (SkSurface* surf) {
-        return as_IB(surf->makeImageSnapshot())->peekTexture()->texturePriv().hasMipMaps();
+        return as_IB(surf->makeImageSnapshot(SkBudgeted::kYes))->
+                peekTexture()->texturePriv().hasMipMaps();
      };
  
      auto mipsAreDirty = [] (SkSurface* surf) {
-        return as_IB(surf->makeImageSnapshot())->peekTexture()->texturePriv().mipMapsAreDirty();
+        return as_IB(surf->makeImageSnapshot(SkBudgeted::kYes))->
+                peekTexture()->texturePriv().mipMapsAreDirty();
      };
  
      GrContext* context = ctxInfo.grContext();
@@ -42,7 +44,7 @@ DEF_GPUTEST_FOR_NULLGL_CONTEXT(GrTextureMipMapInvalidationTest, reporter, ctxInf
      SkPaint paint;
      paint.setFilterQuality(kMedium_SkFilterQuality);
      surf2->getCanvas()->scale(0.2f, 0.2f);
-    surf2->getCanvas()->drawImage(surf1->makeImageSnapshot(), 0, 0, &paint);
+    surf2->getCanvas()->drawImage(surf1->makeImageSnapshot(SkBudgeted::kYes), 0, 0, &paint);
      surf2->getCanvas()->flush();
      REPORTER_ASSERT(reporter, isMipped(surf1.get()));
      REPORTER_ASSERT(reporter, !mipsAreDirty(surf1.get()));
diff --git a/tests/SurfaceTest.cpp b/tests/SurfaceTest.cpp

index 6264196..b85ac81 100644 (file)
--- a/tests/SurfaceTest.cpp
+++ b/tests/SurfaceTest.cpp
@@ -460,24 +460,27 @@ static SkBudgeted is_budgeted(const sk_sp<SkImage> image) {
  
  DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SurfaceBudget, reporter, ctxInfo) {
      SkImageInfo info = SkImageInfo::MakeN32Premul(8,8);
-    for (auto budgeted : { SkBudgeted::kNo, SkBudgeted::kYes }) {
-        auto surface(SkSurface::MakeRenderTarget(ctxInfo.grContext(), budgeted, info));
-        SkASSERT(surface);
-        REPORTER_ASSERT(reporter, budgeted == is_budgeted(surface));
-
-        sk_sp<SkImage> image(surface->makeImageSnapshot());
-
-        // Initially the image shares a texture with the surface, and the
-        // the budgets should always match.
-        REPORTER_ASSERT(reporter, budgeted == is_budgeted(surface));
-        REPORTER_ASSERT(reporter, budgeted == is_budgeted(image));
-
-        // Now trigger copy-on-write
-        surface->getCanvas()->clear(SK_ColorBLUE);
-
-        // They don't share a texture anymore but the budgets should still match.
-        REPORTER_ASSERT(reporter, budgeted == is_budgeted(surface));
-        REPORTER_ASSERT(reporter, budgeted == is_budgeted(image));
+    for (auto sbudgeted : { SkBudgeted::kNo, SkBudgeted::kYes }) {
+        for (auto ibudgeted : { SkBudgeted::kNo, SkBudgeted::kYes }) {
+            auto surface(SkSurface::MakeRenderTarget(ctxInfo.grContext(), sbudgeted, info));
+            SkASSERT(surface);
+            REPORTER_ASSERT(reporter, sbudgeted == is_budgeted(surface));
+
+            sk_sp<SkImage> image(surface->makeImageSnapshot(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
author	Yuqian Li <liyuqian@google.com>
	Fri, 17 Mar 2017 20:50:06 +0000 (20:50 +0000)
committer	Skia Commit-Bot <skia-commit-bot@chromium.org>
	Fri, 17 Mar 2017 20:50:19 +0000 (20:50 +0000)
bench/GrMipMapBench.cpp		patch \| blob \| history
gm/croppedrects.cpp		patch \| blob \| history
include/core/SkSurface.h		patch \| blob \| history
src/core/SkImagePriv.h		patch \| blob \| history
src/gpu/GrSurfaceProxy.cpp		patch \| blob \| history
src/gpu/GrSurfaceProxyPriv.h		patch \| blob \| history
src/image/SkImage_Gpu.cpp		patch \| blob \| history
src/image/SkImage_Gpu.h		patch \| blob \| history
src/image/SkSurface.cpp		patch \| blob \| history
src/image/SkSurface_Base.h		patch \| blob \| history
src/image/SkSurface_Gpu.cpp		patch \| blob \| history
src/image/SkSurface_Gpu.h		patch \| blob \| history
src/image/SkSurface_Raster.cpp		patch \| blob \| history
tests/GrTextureMipMapInvalidationTest.cpp		patch \| blob \| history
tests/SurfaceTest.cpp		patch \| blob \| history