Make FramebufferAttachmentObject not refcountable

src/libANGLE/Context.cpp

@@ -36,9 +36,15 @@
 namespace gl
 {
 
-Context::Context(const egl::Config *config, int clientVersion, const Context *shareContext, rx::Renderer *renderer, bool notifyResets, bool robustAccess)
+Context::Context(const egl::Config *config,
+                 int clientVersion,
+                 const Context *shareContext,
+                 rx::Renderer *renderer,
+                 bool notifyResets,
+                 bool robustAccess)
     : mRenderer(renderer),
       mConfig(config),
+      mCurrentSurface(nullptr),
       mData(clientVersion, mState, mCaps, mTextureCaps, mExtensions, nullptr)
 {
     ASSERT(robustAccess == false);   // Unimplemented
@@ -163,6 +169,11 @@ Context::~Context()
     }
     mZeroTextures.clear();
 
+    if (mCurrentSurface != nullptr)
+    {
+        releaseSurface();
+    }
+
     if (mResourceManager)
     {
         mResourceManager->release();
@@ -189,6 +200,15 @@ void Context::makeCurrent(egl::Surface *surface)
     // TODO(jmadill): Rework this when we support ContextImpl
     mState.setAllDirtyBits();
 
+    if (mCurrentSurface)
+    {
+        releaseSurface();
+    }
+
+    ASSERT(mCurrentSurface == nullptr);
+    mCurrentSurface = surface;
+    surface->setIsCurrent(true);
+
     // Update default framebuffer
     Framebuffer *defaultFBO = mFramebufferMap[0];
 
@@ -230,9 +250,16 @@ void Context::makeCurrent(egl::Surface *surface)
 void Context::releaseSurface()
 {
     Framebuffer *defaultFBO = mFramebufferMap[0];
-    defaultFBO->resetAttachment(GL_BACK);
-    defaultFBO->resetAttachment(GL_DEPTH);
-    defaultFBO->resetAttachment(GL_STENCIL);
+    if (defaultFBO)
+    {
+        defaultFBO->resetAttachment(GL_BACK);
+        defaultFBO->resetAttachment(GL_DEPTH);
+        defaultFBO->resetAttachment(GL_STENCIL);
+    }
+
+    ASSERT(mCurrentSurface != nullptr);
+    mCurrentSurface->setIsCurrent(false);
+    mCurrentSurface = nullptr;
 }
 
 // NOTE: this function should not assume that this context is current!

src/libANGLE/Context.h

@@ -275,6 +275,7 @@ class Context final : angle::NonCopyable
     GLenum mResetStatus;
     GLenum mResetStrategy;
     bool mRobustAccess;
+    egl::Surface *mCurrentSurface;
 
     ResourceManager *mResourceManager;
 

src/libANGLE/FramebufferAttachment.cpp

@@ -22,6 +22,12 @@ namespace gl
 
 ////// FramebufferAttachment::Target Implementation //////
 
+FramebufferAttachment::Target::Target()
+    : mBinding(GL_NONE),
+      mTextureIndex(ImageIndex::MakeInvalid())
+{
+}
+
 FramebufferAttachment::Target::Target(GLenum binding, const ImageIndex &imageIndex)
     : mBinding(binding),
       mTextureIndex(imageIndex)
@@ -44,8 +50,7 @@ FramebufferAttachment::Target &FramebufferAttachment::Target::operator=(const Ta
 ////// FramebufferAttachment Implementation //////
 
 FramebufferAttachment::FramebufferAttachment()
-    : mType(GL_NONE),
-      mTarget(GL_NONE, ImageIndex::MakeInvalid())
+    : mType(GL_NONE), mResource(nullptr)
 {
 }
 
@@ -53,19 +58,38 @@ FramebufferAttachment::FramebufferAttachment(GLenum type,
                                              GLenum binding,
                                              const ImageIndex &textureIndex,
                                              FramebufferAttachmentObject *resource)
-    : mType(type),
-      mTarget(binding, textureIndex)
+    : mResource(nullptr)
+{
+    attach(type, binding, textureIndex, resource);
+}
+
+FramebufferAttachment::FramebufferAttachment(const FramebufferAttachment &other)
+{
+    attach(other.mType, other.mTarget.binding(), other.mTarget.textureIndex(), other.mResource);
+}
+
+FramebufferAttachment &FramebufferAttachment::operator=(const FramebufferAttachment &other)
+{
+    attach(other.mType, other.mTarget.binding(), other.mTarget.textureIndex(), other.mResource);
+    return *this;
+}
+
+FramebufferAttachment::~FramebufferAttachment()
 {
-    mResource.set(resource);
+    detach();
 }
 
 void FramebufferAttachment::detach()
 {
     mType = GL_NONE;
-    mResource.set(nullptr);
+    if (mResource != nullptr)
+    {
+        mResource->onDetach();
+        mResource = nullptr;
+    }
 
     // not technically necessary, could omit for performance
-    mTarget = Target(GL_NONE, ImageIndex::MakeInvalid());
+    mTarget = Target();
 }
 
 void FramebufferAttachment::attach(GLenum type,
@@ -75,12 +99,16 @@ void FramebufferAttachment::attach(GLenum type,
 {
     mType = type;
     mTarget = Target(binding, textureIndex);
-    mResource.set(resource);
-}
 
-FramebufferAttachment::~FramebufferAttachment()
-{
-    mResource.set(nullptr);
+    if (resource)
+    {
+        resource->onAttach();
+    }
+    if (mResource != nullptr)
+    {
+        mResource->onDetach();
+    }
+    mResource = resource;
 }
 
 GLuint FramebufferAttachment::getRedSize() const
@@ -123,6 +151,11 @@ GLenum FramebufferAttachment::getColorEncoding() const
     return GetInternalFormatInfo(getInternalFormat()).colorEncoding;
 }
 
+GLuint FramebufferAttachment::id() const
+{
+    return mResource->getId();
+}
+
 const ImageIndex &FramebufferAttachment::getTextureImageIndex() const
 {
     ASSERT(type() == GL_TEXTURE);
@@ -158,17 +191,17 @@ GLint FramebufferAttachment::layer() const
 
 Texture *FramebufferAttachment::getTexture() const
 {
-    return rx::GetAs<Texture>(mResource.get());
+    return rx::GetAs<Texture>(mResource);
 }
 
 Renderbuffer *FramebufferAttachment::getRenderbuffer() const
 {
-    return rx::GetAs<Renderbuffer>(mResource.get());
+    return rx::GetAs<Renderbuffer>(mResource);
 }
 
 const egl::Surface *FramebufferAttachment::getSurface() const
 {
-    return rx::GetAs<egl::Surface>(mResource.get());
+    return rx::GetAs<egl::Surface>(mResource);
 }
 
 }

src/libANGLE/FramebufferAttachment.h

@@ -14,7 +14,6 @@
 #include "common/angleutils.h"
 #include "libANGLE/Error.h"
 #include "libANGLE/ImageIndex.h"
-#include "libANGLE/RefCountObject.h"
 
 namespace egl
 {
@@ -57,20 +56,8 @@ class FramebufferAttachment final
                           const ImageIndex &textureIndex,
                           FramebufferAttachmentObject *resource);
 
-    FramebufferAttachment(const FramebufferAttachment &other)
-       : mType(other.mType),
-         mTarget(other.mTarget)
-    {
-        mResource.set(other.mResource.get());
-    }
-
-    FramebufferAttachment &operator=(const FramebufferAttachment &other)
-    {
-        mType = other.mType;
-        mTarget = other.mTarget;
-        mResource.set(other.mResource.get());
-        return *this;
-    }
+    FramebufferAttachment(const FramebufferAttachment &other);
+    FramebufferAttachment &operator=(const FramebufferAttachment &other);
 
     ~FramebufferAttachment();
 
@@ -83,6 +70,7 @@ class FramebufferAttachment final
     class Target
     {
       public:
+        Target();
         Target(GLenum binding, const ImageIndex &imageIndex);
         Target(const Target &other);
         Target &operator=(const Target &other);
@@ -115,7 +103,7 @@ class FramebufferAttachment final
     bool isRenderbufferWithId(GLuint renderbufferId) const { return mType == GL_RENDERBUFFER && id() == renderbufferId; }
 
     GLenum getBinding() const { return mTarget.binding(); }
-    GLuint id() const { return mResource.id(); }
+    GLuint id() const;
 
     // These methods are only legal to call on Texture attachments
     const ImageIndex &getTextureImageIndex() const;
@@ -150,20 +138,24 @@ class FramebufferAttachment final
 
     GLenum mType;
     Target mTarget;
-    BindingPointer<FramebufferAttachmentObject> mResource;
+    FramebufferAttachmentObject *mResource;
 };
 
 // A base class for objects that FBO Attachments may point to.
-class FramebufferAttachmentObject : public RefCountObject
+class FramebufferAttachmentObject
 {
   public:
-    FramebufferAttachmentObject(GLuint id) : RefCountObject(id) {}
+    FramebufferAttachmentObject() {}
 
     virtual GLsizei getAttachmentWidth(const FramebufferAttachment::Target &target) const = 0;
     virtual GLsizei getAttachmentHeight(const FramebufferAttachment::Target &target) const = 0;
     virtual GLenum getAttachmentInternalFormat(const FramebufferAttachment::Target &target) const = 0;
     virtual GLsizei getAttachmentSamples(const FramebufferAttachment::Target &target) const = 0;
 
+    virtual void onAttach() = 0;
+    virtual void onDetach() = 0;
+    virtual GLuint getId() const = 0;
+
     Error getAttachmentRenderTarget(const FramebufferAttachment::Target &target,
                                     rx::FramebufferAttachmentRenderTarget **rtOut) const;
 

src/libANGLE/Image.cpp

@@ -17,8 +17,7 @@
 
 namespace egl
 {
-ImageSibling::ImageSibling(GLuint id)
-    : gl::FramebufferAttachmentObject(id), mSourcesOf(), mTargetOf()
+ImageSibling::ImageSibling(GLuint id) : RefCountObject(id), mSourcesOf(), mTargetOf()
 {
 }
 

src/libANGLE/Image.h

@@ -13,7 +13,6 @@
 #include "libANGLE/AttributeMap.h"
 #include "libANGLE/Error.h"
 #include "libANGLE/RefCountObject.h"
-#include "libANGLE/FramebufferAttachment.h"
 
 #include <set>
 
@@ -26,7 +25,7 @@ namespace egl
 {
 class Image;
 
-class ImageSibling : public gl::FramebufferAttachmentObject
+class ImageSibling : public RefCountObject
 {
   public:
     ImageSibling(GLuint id);

src/libANGLE/Renderbuffer.cpp

@@ -151,4 +151,18 @@ GLuint Renderbuffer::getStencilSize() const
     return GetInternalFormatInfo(mInternalFormat).stencilBits;
 }
 
+void Renderbuffer::onAttach()
+{
+    addRef();
+}
+
+void Renderbuffer::onDetach()
+{
+    release();
+}
+
+GLuint Renderbuffer::getId() const
+{
+    return id();
+}
 }

src/libANGLE/Renderbuffer.h

@@ -25,7 +25,7 @@ namespace gl
 // FramebufferAttachment and Framebuffer for how they are applied to an FBO via an
 // attachment point.
 
-class Renderbuffer : public egl::ImageSibling
+class Renderbuffer : public egl::ImageSibling, public gl::FramebufferAttachmentObject
 {
   public:
     Renderbuffer(rx::RenderbufferImpl *impl, GLuint id);
@@ -55,6 +55,10 @@ class Renderbuffer : public egl::ImageSibling
     GLenum getAttachmentInternalFormat(const FramebufferAttachment::Target &/*target*/) const override { return getInternalFormat(); }
     GLsizei getAttachmentSamples(const FramebufferAttachment::Target &/*target*/) const override { return getSamples(); }
 
+    void onAttach() override;
+    void onDetach() override;
+    GLuint getId() const override;
+
   private:
     rx::FramebufferAttachmentObjectImpl *getAttachmentImpl() const override { return mRenderbuffer; }
 

src/libANGLE/Surface.cpp

@@ -15,12 +15,19 @@
 
 #include <EGL/eglext.h>
 
+#include <iostream>
+
 namespace egl
 {
 
-Surface::Surface(rx::SurfaceImpl *impl, EGLint surfaceType, const egl::Config *config, const AttributeMap &attributes)
-    : FramebufferAttachmentObject(0), // id unused
+Surface::Surface(rx::SurfaceImpl *impl,
+                 EGLint surfaceType,
+                 const egl::Config *config,
+                 const AttributeMap &attributes)
+    : FramebufferAttachmentObject(),
       mImplementation(impl),
+      mCurrentCount(0),
+      mDestroyed(false),
       mType(surfaceType),
       mConfig(config),
       mPostSubBufferRequested(false),
@@ -34,8 +41,6 @@ Surface::Surface(rx::SurfaceImpl *impl, EGLint surfaceType, const egl::Config *c
       mRenderBuffer(EGL_BACK_BUFFER),
       mSwapBehavior(impl->getSwapBehavior())
 {
-    addRef();
-
     mPostSubBufferRequested = (attributes.get(EGL_POST_SUB_BUFFER_SUPPORTED_NV, EGL_FALSE) == EGL_TRUE);
 
     mFixedSize = (attributes.get(EGL_FIXED_SIZE_ANGLE, EGL_FALSE) == EGL_TRUE);
@@ -67,6 +72,32 @@ Surface::~Surface()
     SafeDelete(mImplementation);
 }
 
+void Surface::setIsCurrent(bool isCurrent)
+{
+    if (isCurrent)
+    {
+        mCurrentCount++;
+    }
+    else
+    {
+        ASSERT(mCurrentCount > 0);
+        mCurrentCount--;
+        if (mCurrentCount == 0 && mDestroyed)
+        {
+            delete this;
+        }
+    }
+}
+
+void Surface::onDestroy()
+{
+    mDestroyed = true;
+    if (mCurrentCount == 0)
+    {
+        delete this;
+    }
+}
+
 EGLint Surface::getType() const
 {
     return mType;
@@ -177,4 +208,9 @@ GLsizei Surface::getAttachmentSamples(const gl::FramebufferAttachment::Target &t
     return getConfig()->samples;
 }
 
+GLuint Surface::getId() const
+{
+    UNREACHABLE();
+    return 0;
+}
 }

src/libANGLE/Surface.h

@@ -16,6 +16,7 @@
 #include "common/angleutils.h"
 #include "libANGLE/Error.h"
 #include "libANGLE/FramebufferAttachment.h"
+#include "libANGLE/RefCountObject.h"
 #include "libANGLE/renderer/SurfaceImpl.h"
 
 namespace gl
@@ -48,6 +49,8 @@ class Surface final : public gl::FramebufferAttachmentObject
     EGLint isPostSubBufferSupported() const;
 
     void setSwapInterval(EGLint interval);
+    void setIsCurrent(bool isCurrent);
+    void onDestroy();
 
     const Config *getConfig() const;
 
@@ -70,6 +73,10 @@ class Surface final : public gl::FramebufferAttachmentObject
     GLenum getAttachmentInternalFormat(const gl::FramebufferAttachment::Target &target) const override;
     GLsizei getAttachmentSamples(const gl::FramebufferAttachment::Target &target) const override;
 
+    void onAttach() override {}
+    void onDetach() override {}
+    GLuint getId() const override;
+
   private:
     virtual ~Surface();
     rx::FramebufferAttachmentObjectImpl *getAttachmentImpl() const override { return mImplementation; }
@@ -79,6 +86,8 @@ class Surface final : public gl::FramebufferAttachmentObject
     void releaseTexImageFromTexture();
 
     rx::SurfaceImpl *mImplementation;
+    int mCurrentCount;
+    bool mDestroyed;
 
     EGLint mType;
 

src/libANGLE/Surface_unittest.cpp

@@ -48,7 +48,7 @@ class SurfaceTest : public testing::Test
 
     virtual void TearDown()
     {
-        mSurface->release();
+        mSurface->onDestroy();
     }
 
     MockSurfaceImpl *mImpl;
@@ -63,7 +63,7 @@ TEST_F(SurfaceTest, DestructionDeletesImpl)
     EXPECT_CALL(*impl, destroy()).Times(1).RetiresOnSaturation();
 
     egl::Surface *surface = new egl::Surface(impl, EGL_WINDOW_BIT, &mConfig, egl::AttributeMap());
-    surface->release();
+    surface->onDestroy();
 
     // Only needed because the mock is leaked if bugs are present,
     // which logs an error, but does not cause the test to fail.

src/libANGLE/Texture.cpp

@@ -694,4 +694,18 @@ GLsizei Texture::getAttachmentSamples(const gl::FramebufferAttachment::Target &/
     return 0;
 }
 
+void Texture::onAttach()
+{
+    addRef();
+}
+
+void Texture::onDetach()
+{
+    release();
+}
+
+GLuint Texture::getId() const
+{
+    return id();
+}
 }

src/libANGLE/Texture.h

@@ -35,7 +35,7 @@ struct Data;
 
 bool IsMipmapFiltered(const gl::SamplerState &samplerState);
 
-class Texture final : public egl::ImageSibling
+class Texture final : public egl::ImageSibling, public gl::FramebufferAttachmentObject
 {
   public:
     Texture(rx::TextureImpl *impl, GLuint id, GLenum target);
@@ -121,6 +121,10 @@ class Texture final : public egl::ImageSibling
     GLenum getAttachmentInternalFormat(const FramebufferAttachment::Target &target) const override;
     GLsizei getAttachmentSamples(const FramebufferAttachment::Target &target) const override;
 
+    void onAttach() override;
+    void onDetach() override;
+    GLuint getId() const override;
+
   private:
     rx::FramebufferAttachmentObjectImpl *getAttachmentImpl() const override { return mTexture; }
 

src/libANGLE/renderer/DisplayImpl.cpp

@@ -29,8 +29,8 @@ DisplayImpl::~DisplayImpl()
 
 void DisplayImpl::destroySurface(egl::Surface *surface)
 {
+    surface->onDestroy();
     mSurfaceSet.erase(surface);
-    surface->release();
 }
 
 const egl::DisplayExtensions &DisplayImpl::getExtensions() const