Improve tracking of bound FBOs in GrGLGpu.

src/gpu/gl/GrGLRenderTarget.h

Index: src/gpu/gl/GrGLRenderTarget.h
diff --git a/src/gpu/gl/GrGLRenderTarget.h b/src/gpu/gl/GrGLRenderTarget.h
index 7e7349257f054ac44e8a32cd5626c1cf94e1041c..6ab4e71904d1204710aaf44cd82229098a61036a 100644
--- a/src/gpu/gl/GrGLRenderTarget.h
+++ b/src/gpu/gl/GrGLRenderTarget.h
@@ -15,15 +15,78 @@
 
 class GrGLGpu;
 
-class GrGLRenderTarget : public GrRenderTarget {
+/** Represents a GL FBO object. It has a gen ID which is valid whenever the FBO ID owned by the
+    object is valid. The gen IDs are not recycled after FBOs are freed, unlike FBO IDs, and so
+    can be used to uniquely identity FBO ID instantiations. If this object owns an FBO ID, the ID
+    must be deleted or abandoned before this object is freed. FBO IDs should never be owned by

[egdaniel, 2015/02/25 16:21:34]

"FBO IDs should never be owned by more that one instance."

This is contradicted by the id constructor.

+    more than one instance. */
+class GrGLFBO : public SkRefCnt {
 public:
-    // set fTexFBOID to this value to indicate that it is multisampled but
-    // Gr doesn't know how to resolve it.
-    enum { kUnresolvableFBOID = 0 };
+    SK_DECLARE_INST_COUNT(GrGLFBO);
+
+    /** Initializes to invalid state */
+    GrGLFBO() : fGenID(SK_InvalidGenID) {}
+
+    /** Initializes to an FBO. */
+    GrGLFBO(GrGLint id) : fID(id), fGenID(NextGenID()) {}
+
+    /** Initializes to an FBO. */
+    GrGLFBO(const GrGLInterface* gl) {
+        GR_GL_CALL(gl, GenFramebuffers(1, &fID));
+        if (0 != fID) {
+            fGenID = NextGenID();
+        } else {
+            fGenID = SK_InvalidGenID;
+        }
+    }
+
+    ~GrGLFBO() SK_OVERRIDE { SkASSERT(!this->isValid()); }
+
+    /** If this does not own a FBO ID, attempt to create one. Otherwise, no-op. */
+    void generateIfInvalid(const GrGLInterface* gl) {

[egdaniel, 2015/02/25 16:21:34]

I wonder if this should be public. I feel like it would be nice to hide this
inside bindFBO and ctor, but that does add extra conditional check each bind.

+        if (!this->isValid()) {
+            GR_GL_CALL(gl, GenFramebuffers(1, &fID));
+            if (fID) {
+                fGenID = NextGenID();
+            }
+        }
+    }
+
+    /** Does this object own an FBO ID? */
+    bool isValid() const { return SK_InvalidGenID != fGenID; }
+
+    uint32_t genID() const { return fGenID; }
+
+    GrGLint fboID() const { SkASSERT(this->isValid()); return fID; }
+
+    bool isDefaultFramebuffer() const { SkASSERT(this->isValid()); return 0 == fID; }
 
+    /** Give up ownership of the FBO ID owned by this object without deleting it. */
+    void abandon(GrGLGpu* gpu);
+
+    /** Delete and give up ownership of the the FBO ID if it is valid. */
+    void deleteIfValid(GrGLGpu* gpu);
+
+private:
+    static uint32_t NextGenID() {
+        static int32_t gGenID = SK_InvalidGenID + 1;
+        return static_cast<uint32_t>(sk_atomic_inc(&gGenID));
+    }
+
+    GrGLuint    fID;
+    uint32_t    fGenID;
+
+    typedef SkRefCnt INHERITED;
+};
+
+//////////////////////////////////////////////////////////////////////////////
+
+/** GL-specific subclass of GrRenderTarget. */
+class GrGLRenderTarget : public GrRenderTarget {
+public:
     struct IDDesc {
-        GrGLuint                    fRTFBOID;
-        GrGLuint                    fTexFBOID;
+        SkAutoTUnref<GrGLFBO>       fRenderFBO;
+        SkAutoTUnref<GrGLFBO>       fTextureFBO;
         GrGLuint                    fMSColorRenderbufferID;
         GrGpuResource::LifeCycle    fLifeCycle;
     };
@@ -33,21 +96,32 @@ public:
     void setViewport(const GrGLIRect& rect) { fViewport = rect; }
     const GrGLIRect& getViewport() const { return fViewport; }
 
-    // The following two functions return the same ID when a
-    // texture/render target is multisampled, and different IDs when
-    // it is.
-    // FBO ID used to render into
-    GrGLuint renderFBOID() const { return fRTFBOID; }
-    // FBO ID that has texture ID attached.
-    GrGLuint textureFBOID() const { return fTexFBOID; }
+    // For multisampled renderbuffer render targets, these will return different GrGLFBO objects. If
+    // the render target is not texturable, textureFBO() returns NULL. If the render target auto
+    // resolves to a texture, the same object is returned.
+
+    // FBO that should be rendered into. Always non-NULL.
+    const GrGLFBO* renderFBO() const {
+        SkASSERT(fRenderFBO && fRenderFBO->isValid());
+        return fRenderFBO;
+    }
+
+    // FBO that has the target's texture ID attached. The return value may be:
+    //      * NULL when this render target is not a texture,
+    //      * the same as renderFBO() when this surface is not multisampled or auto-resolves,
+    //      * or different than renderFBO() when it requires explicit resolving via
+    //        glBlitFramebuffer.
+    const GrGLFBO* textureFBO() const {
+        SkASSERT(!fTextureFBO || fTextureFBO->isValid());
+        return fTextureFBO;
+    }
 
     // override of GrRenderTarget
     ResolveType getResolveType() const SK_OVERRIDE {
-        if (!this->isMultisampled() ||
-            fRTFBOID == fTexFBOID) {
+        if (!this->isMultisampled() || this->renderFBO() == this->textureFBO()) {
             // catches FBO 0 and non MSAA case
             return kAutoResolves_ResolveType;
-        } else if (kUnresolvableFBOID == fTexFBOID) {
+        } else if (!this->textureFBO()) {
             return kCantResolve_ResolveType;
         } else {
             return kCanResolve_ResolveType;
@@ -73,23 +147,23 @@ protected:
     size_t onGpuMemorySize() const SK_OVERRIDE;
 
 private:
-    GrGLuint      fRTFBOID;
-    GrGLuint      fTexFBOID;
-    GrGLuint      fMSColorRenderbufferID;
+    SkAutoTUnref<GrGLFBO>   fRenderFBO;
+    SkAutoTUnref<GrGLFBO>   fTextureFBO;
+    GrGLuint                fMSColorRenderbufferID;
 
     // We track this separately from GrGpuResource because this may be both a texture and a render
     // target, and the texture may be wrapped while the render target is not.
-    bool fIsWrapped;
+    bool                    fIsWrapped;
 
     // when we switch to this render target we want to set the viewport to
     // only render to content area (as opposed to the whole allocation) and
     // we want the rendering to be at top left (GL has origin in bottom left)
-    GrGLIRect fViewport;
+    GrGLIRect               fViewport;
 
     // onGpuMemorySize() needs to know what how many color values are owned per pixel. However,
     // abandon and release zero out the IDs and the cache needs to know the size even after those
     // actions.
-    uint8_t fColorValuesPerPixel;
+    uint8_t                 fColorValuesPerPixel;
 
     typedef GrRenderTarget INHERITED;
 };