Move ReleaseProc info to GrTexture and for implementations to define it.
authorGreg Daniel <egdaniel@google.com>
Fri, 21 Apr 2017 15:52:27 +0000 (11:52 -0400)
committerSkia Commit-Bot <skia-commit-bot@chromium.org>
Fri, 21 Apr 2017 17:20:27 +0000 (17:20 +0000)
Bug: skia:
Change-Id: I0dbe421ebd17ef7d21fd2f4f027d2a3bdcf04b7b
Reviewed-on: https://skia-review.googlesource.com/14031
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>

13 files changed:
include/gpu/GrSurface.h
include/gpu/GrTexture.h
src/gpu/GrSurface.cpp
src/gpu/gl/GrGLTexture.cpp
src/gpu/gl/GrGLTexture.h
src/gpu/vk/GrVkCommandBuffer.cpp
src/gpu/vk/GrVkCommandBuffer.h
src/gpu/vk/GrVkDescriptorSet.cpp
src/gpu/vk/GrVkDescriptorSet.h
src/gpu/vk/GrVkImage.cpp
src/gpu/vk/GrVkImage.h
src/gpu/vk/GrVkResource.h
src/gpu/vk/GrVkTexture.h

index 65f1e0f..dd37c18 100644 (file)
@@ -70,14 +70,6 @@ public:
     inline GrSurfacePriv surfacePriv();
     inline const GrSurfacePriv surfacePriv() const;
 
-    typedef void* ReleaseCtx;
-    typedef void (*ReleaseProc)(ReleaseCtx);
-
-    void setRelease(ReleaseProc proc, ReleaseCtx ctx) {
-        fReleaseProc = proc;
-        fReleaseCtx = ctx;
-    }
-
     static size_t WorstCaseSize(const GrSurfaceDesc& desc, bool useNextPow2 = false);
     static size_t ComputeSize(const GrSurfaceDesc& desc, int colorSamplesPerPixel,
                               bool hasMIPMaps, bool useNextPow2 = false);
@@ -93,11 +85,9 @@ protected:
 
     GrSurface(GrGpu* gpu, const GrSurfaceDesc& desc)
         : INHERITED(gpu)
-        , fDesc(desc)
-        , fReleaseProc(nullptr)
-        , fReleaseCtx(nullptr) {
+        , fDesc(desc) {
     }
-    ~GrSurface() override;
+    ~GrSurface() override {}
 
     GrSurfaceDesc fDesc;
 
@@ -105,16 +95,6 @@ protected:
     void onAbandon() override;
 
 private:
-    void invokeReleaseProc() {
-        if (fReleaseProc) {
-            fReleaseProc(fReleaseCtx);
-            fReleaseProc = nullptr;
-        }
-    }
-
-    ReleaseProc fReleaseProc;
-    ReleaseCtx  fReleaseCtx;
-
     typedef GrGpuResource INHERITED;
 };
 
index 1f63958..ba7d506 100644 (file)
@@ -41,6 +41,12 @@ public:
     }
 #endif
 
+    // These match the definitions in SkImage, for whence they came
+    typedef void* ReleaseCtx;
+    typedef void (*ReleaseProc)(ReleaseCtx);
+
+    virtual void setRelease(ReleaseProc proc, ReleaseCtx ctx) = 0;
+
     /** Access methods that are only to be used within Skia code. */
     inline GrTexturePriv texturePriv();
     inline const GrTexturePriv texturePriv() const;
index 1762d00..aae3f23 100644 (file)
 #include "SkGr.h"
 #include "SkMathPriv.h"
 
-GrSurface::~GrSurface() {
-    // check that invokeReleaseProc has been called (if needed)
-    SkASSERT(NULL == fReleaseProc);
-}
-
 size_t GrSurface::WorstCaseSize(const GrSurfaceDesc& desc, bool useNextPow2) {
     size_t size;
 
@@ -171,11 +166,9 @@ bool GrSurface::hasPendingIO() const {
 }
 
 void GrSurface::onRelease() {
-    this->invokeReleaseProc();
     this->INHERITED::onRelease();
 }
 
 void GrSurface::onAbandon() {
-    this->invokeReleaseProc();
     this->INHERITED::onAbandon();
 }
index 95c8c8f..fdbcf62 100644 (file)
@@ -102,12 +102,14 @@ void GrGLTexture::onRelease() {
         }
         fInfo.fID = 0;
     }
+    this->invokeReleaseProc();
     INHERITED::onRelease();
 }
 
 void GrGLTexture::onAbandon() {
     fInfo.fTarget = 0;
     fInfo.fID = 0;
+    this->invokeReleaseProc();
     INHERITED::onAbandon();
 }
 
index 16b47f1..7ba8058 100644 (file)
@@ -36,10 +36,20 @@ public:
     GrGLTexture(GrGLGpu*, SkBudgeted, const GrSurfaceDesc&, const IDDesc&,
                 bool wasMipMapDataProvided);
 
+    ~GrGLTexture() override {
+        // check that invokeReleaseProc has been called (if needed)
+        SkASSERT(!fReleaseProc);
+    }
+
     GrBackendObject getTextureHandle() const override;
 
     void textureParamsModified() override { fTexParams.invalidate(); }
 
+    void setRelease(ReleaseProc proc, ReleaseCtx ctx) override {
+        fReleaseProc = proc;
+        fReleaseCtx = ctx;
+    }
+
     // These functions are used to track the texture parameters associated with the texture.
     const TexParams& getCachedTexParams(GrGpu::ResetTimestamp* timestamp) const {
         *timestamp = fTexParamsTimestamp;
@@ -74,6 +84,13 @@ protected:
     std::unique_ptr<GrExternalTextureData> detachBackendTexture() override;
 
 private:
+    void invokeReleaseProc() {
+        if (fReleaseProc) {
+            fReleaseProc(fReleaseCtx);
+            fReleaseProc = nullptr;
+        }
+    }
+
     TexParams                       fTexParams;
     GrGpu::ResetTimestamp           fTexParamsTimestamp;
     // Holds the texture target and ID. A pointer to this may be shared to external clients for
@@ -81,6 +98,9 @@ private:
     GrGLTextureInfo                 fInfo;
     GrBackendObjectOwnership        fTextureIDOwnership;
 
+    ReleaseProc                     fReleaseProc = nullptr;
+    ReleaseCtx                      fReleaseCtx = nullptr;
+
     typedef GrTexture INHERITED;
 };
 
index 64c1c88..cc219f5 100644 (file)
@@ -50,7 +50,7 @@ void GrVkCommandBuffer::freeGPUData(const GrVkGpu* gpu) const {
     this->onFreeGPUData(gpu);
 }
 
-void GrVkCommandBuffer::abandonSubResources() const {
+void GrVkCommandBuffer::abandonGPUData() const {
     for (int i = 0; i < fTrackedResources.count(); ++i) {
         fTrackedResources[i]->unrefAndAbandon();
     }
index e156861..f1de386 100644 (file)
@@ -173,7 +173,7 @@ private:
 
     void freeGPUData(const GrVkGpu* gpu) const override;
     virtual void onFreeGPUData(const GrVkGpu* gpu) const = 0;
-    void abandonSubResources() const override;
+    void abandonGPUData() const override;
 
     virtual void onReset(GrVkGpu* gpu) {}
 
index 47a997f..3cb1035 100644 (file)
@@ -28,7 +28,7 @@ void GrVkDescriptorSet::onRecycle(GrVkGpu* gpu) const {
     gpu->resourceProvider().recycleDescriptorSet(this, fHandle);
 }
 
-void GrVkDescriptorSet::abandonSubResources() const {
+void GrVkDescriptorSet::abandonGPUData() const {
     fPool->unrefAndAbandon();
 }
 
index 69e2d44..cbeb98e 100644 (file)
@@ -33,7 +33,7 @@ public:
 
 private:
     void freeGPUData(const GrVkGpu* gpu) const override;
-    void abandonSubResources() const override;
+    void abandonGPUData() const override;
     void onRecycle(GrVkGpu* gpu) const override;
 
     VkDescriptorSet                          fDescSet;
index b3c103a..5a9f69d 100644 (file)
@@ -152,11 +152,23 @@ void GrVkImage::abandonImage() {
     }
 }
 
+void GrVkImage::setResourceRelease(ReleaseProc proc, ReleaseCtx ctx) {
+    // Forward the release proc on to GrVkImage::Resource
+    fResource->setRelease(proc, ctx);
+}
+
 void GrVkImage::Resource::freeGPUData(const GrVkGpu* gpu) const {
+    SkASSERT(!fReleaseProc);
     VK_CALL(gpu, DestroyImage(gpu->device(), fImage, nullptr));
     bool isLinear = (VK_IMAGE_TILING_LINEAR == fImageTiling);
     GrVkMemory::FreeImageMemory(gpu, isLinear, fAlloc);
 }
 
 void GrVkImage::BorrowedResource::freeGPUData(const GrVkGpu* gpu) const {
+    this->invokeReleaseProc();
 }
+
+void GrVkImage::BorrowedResource::abandonGPUData() const {
+    this->invokeReleaseProc();
+}
+
index 21728c0..336e468 100644 (file)
@@ -84,6 +84,12 @@ public:
     // Destroys the internal VkImage and VkDeviceMemory in the GrVkImageInfo
     static void DestroyImageInfo(const GrVkGpu* gpu, GrVkImageInfo*);
 
+    // These match the definitions in SkImage, for whence they came
+    typedef void* ReleaseCtx;
+    typedef void (*ReleaseProc)(ReleaseCtx);
+
+    void setResourceRelease(ReleaseProc proc, ReleaseCtx ctx);
+
 protected:
     void releaseImage(const GrVkGpu* gpu);
     void abandonImage();
@@ -98,23 +104,42 @@ private:
     public:
         Resource()
             : INHERITED()
+            , fReleaseProc(nullptr)
+            , fReleaseCtx(nullptr)
             , fImage(VK_NULL_HANDLE) {
             fAlloc.fMemory = VK_NULL_HANDLE;
             fAlloc.fOffset = 0;
         }
 
         Resource(VkImage image, const GrVkAlloc& alloc, VkImageTiling tiling)
-            : fImage(image), fAlloc(alloc), fImageTiling(tiling) {}
-
-        ~Resource() override {}
+            : fReleaseProc(nullptr)
+            , fReleaseCtx(nullptr)
+            , fImage(image)
+            , fAlloc(alloc)
+            , fImageTiling(tiling) {}
+
+        ~Resource() override {
+            SkASSERT(!fReleaseProc);
+        }
 
 #ifdef SK_TRACE_VK_RESOURCES
         void dumpInfo() const override {
             SkDebugf("GrVkImage: %d (%d refs)\n", fImage, this->getRefCnt());
         }
 #endif
+        void setRelease(ReleaseProc proc, ReleaseCtx ctx) const {
+            fReleaseProc = proc;
+            fReleaseCtx = ctx;
+        }
+    protected:
+        mutable ReleaseProc fReleaseProc;
+        mutable ReleaseCtx  fReleaseCtx;
+
     private:
         void freeGPUData(const GrVkGpu* gpu) const override;
+        void abandonGPUData() const override {
+            SkASSERT(!fReleaseProc);
+        }
 
         VkImage        fImage;
         GrVkAlloc      fAlloc;
@@ -130,7 +155,15 @@ private:
             : Resource(image, alloc, tiling) {
         }
     private:
+        void invokeReleaseProc() const {
+            if (fReleaseProc) {
+                fReleaseProc(fReleaseCtx);
+                fReleaseProc = nullptr;
+            }
+        }
+
         void freeGPUData(const GrVkGpu* gpu) const override;
+        void abandonGPUData() const override;
     };
 
     const Resource* fResource;
index 9d7212e..9ddde47 100644 (file)
@@ -31,7 +31,7 @@ class GrVkGpu;
 
   This is nearly identical to SkRefCntBase. The exceptions are that unref()
   takes a GrVkGpu, and any derived classes must implement freeGPUData() and
-  possibly abandonSubResources().
+  possibly abandonGPUData().
 */
 
 class GrVkResource : SkNoncopyable {
@@ -153,10 +153,12 @@ private:
      */
     virtual void freeGPUData(const GrVkGpu* gpu) const = 0;
 
-    /** Must be overridden by subclasses that themselves store GrVkResources.
-     *  Will unrefAndAbandon those resources without deleting the underlying Vk data
+    /**
+     * Called from unrefAndAbandon. Resources should do any necessary cleanup without freeing
+     * underlying Vk objects. This must be overridden by subclasses that themselves store
+     * GrVkResources since those resource will need to be unrefed.
      */
-    virtual void abandonSubResources() const {}
+    virtual void abandonGPUData() const {}
 
     /**
      *  Called when the ref count goes to 0. Will free Vk resources.
@@ -175,7 +177,7 @@ private:
      *  Internal_dispose without freeing Vk resources. Used when we've lost context.
      */
     void internal_dispose() const {
-        this->abandonSubResources();
+        this->abandonGPUData();
 #ifdef SK_TRACE_VK_RESOURCES
         fTrace.remove(this);
 #endif
index db7124e..a866748 100644 (file)
@@ -34,6 +34,13 @@ public:
 
     bool reallocForMipmap(GrVkGpu* gpu, uint32_t mipLevels);
 
+    // In Vulkan we call the release proc after we are finished with the underlying
+    // GrVkImage::Resource object (which occurs after the GPU has finsihed all work on it).
+    void setRelease(GrTexture::ReleaseProc proc, GrTexture::ReleaseCtx ctx) override {
+        // Forward the release proc on to GrVkImage
+        this->setResourceRelease(proc, ctx);
+    }
+
 protected:
     GrVkTexture(GrVkGpu*, const GrSurfaceDesc&, const GrVkImageInfo&, const GrVkImageView*,
                 GrVkImage::Wrapped wrapped);