Revert "Make FramebufferAttachmentObject not refcountable"

src/libANGLE/Context.cpp

@@ -36,15 +36,9 @@
 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
@@ -169,11 +163,6 @@ Context::~Context()
     }
     mZeroTextures.clear();
 
-    if (mCurrentSurface != nullptr)
-    {
-        releaseSurface();
-    }
-
     if (mResourceManager)
     {
         mResourceManager->release();
@@ -200,15 +189,6 @@ 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];
 
@@ -250,16 +230,9 @@ void Context::makeCurrent(egl::Surface *surface)
 void Context::releaseSurface()
 {
     Framebuffer *defaultFBO = mFramebufferMap[0];
-    if (defaultFBO)
-    {
-        defaultFBO->resetAttachment(GL_BACK);
-        defaultFBO->resetAttachment(GL_DEPTH);
-        defaultFBO->resetAttachment(GL_STENCIL);
-    }
-
-    ASSERT(mCurrentSurface != nullptr);
-    mCurrentSurface->setIsCurrent(false);
-    mCurrentSurface = nullptr;
+    defaultFBO->resetAttachment(GL_BACK);
+    defaultFBO->resetAttachment(GL_DEPTH);
+    defaultFBO->resetAttachment(GL_STENCIL);
 }
 
 // NOTE: this function should not assume that this context is current!

src/libANGLE/Context.h

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

src/libANGLE/FramebufferAttachment.cpp

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

src/libANGLE/FramebufferAttachment.h

@@ -14,6 +14,7 @@
 #include "common/angleutils.h"
 #include "libANGLE/Error.h"
 #include "libANGLE/ImageIndex.h"
+#include "libANGLE/RefCountObject.h"
 
 namespace egl
 {
@@ -56,8 +57,20 @@ class FramebufferAttachment final
                           const ImageIndex &textureIndex,
                           FramebufferAttachmentObject *resource);
 
-    FramebufferAttachment(const FramebufferAttachment &other);
-    FramebufferAttachment &operator=(const FramebufferAttachment &other);
+    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();
 
@@ -70,7 +83,6 @@ class FramebufferAttachment final
     class Target
     {
       public:
-        Target();
         Target(GLenum binding, const ImageIndex &imageIndex);
         Target(const Target &other);
         Target &operator=(const Target &other);
@@ -103,7 +115,7 @@ class FramebufferAttachment final
     bool isRenderbufferWithId(GLuint renderbufferId) const { return mType == GL_RENDERBUFFER && id() == renderbufferId; }
 
     GLenum getBinding() const { return mTarget.binding(); }
-    GLuint id() const;
+    GLuint id() const { return mResource.id(); }
 
     // These methods are only legal to call on Texture attachments
     const ImageIndex &getTextureImageIndex() const;
@@ -138,24 +150,20 @@ class FramebufferAttachment final
 
     GLenum mType;
     Target mTarget;
-    FramebufferAttachmentObject *mResource;
+    BindingPointer<FramebufferAttachmentObject> mResource;
 };
 
 // A base class for objects that FBO Attachments may point to.
-class FramebufferAttachmentObject
+class FramebufferAttachmentObject : public RefCountObject
 {
   public:
-    FramebufferAttachmentObject() {}
+    FramebufferAttachmentObject(GLuint id) : RefCountObject(id) {}
 
     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,7 +17,8 @@
 
 namespace egl
 {
-ImageSibling::ImageSibling(GLuint id) : RefCountObject(id), mSourcesOf(), mTargetOf()
+ImageSibling::ImageSibling(GLuint id)
+    : gl::FramebufferAttachmentObject(id), mSourcesOf(), mTargetOf()
 {
 }
 

src/libANGLE/Image.h

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

src/libANGLE/Renderbuffer.cpp

@@ -151,18 +151,4 @@ 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, public gl::FramebufferAttachmentObject
+class Renderbuffer : public egl::ImageSibling
 {
   public:
     Renderbuffer(rx::RenderbufferImpl *impl, GLuint id);
@@ -55,10 +55,6 @@ class Renderbuffer : public egl::ImageSibling, public gl::FramebufferAttachmentO
     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,19 +15,12 @@
 
 #include <EGL/eglext.h>
 
-#include <iostream>
-
 namespace egl
 {
 
-Surface::Surface(rx::SurfaceImpl *impl,
-                 EGLint surfaceType,
-                 const egl::Config *config,
-                 const AttributeMap &attributes)
-    : FramebufferAttachmentObject(),
+Surface::Surface(rx::SurfaceImpl *impl, EGLint surfaceType, const egl::Config *config, const AttributeMap &attributes)
+    : FramebufferAttachmentObject(0), // id unused
       mImplementation(impl),
-      mCurrentCount(0),
-      mDestroyed(false),
       mType(surfaceType),
       mConfig(config),
       mPostSubBufferRequested(false),
@@ -41,6 +34,8 @@ Surface::Surface(rx::SurfaceImpl *impl,
       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);
@@ -72,32 +67,6 @@ 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;
@@ -208,9 +177,4 @@ GLsizei Surface::getAttachmentSamples(const gl::FramebufferAttachment::Target &t
     return getConfig()->samples;
 }
 
-GLuint Surface::getId() const
-{
-    UNREACHABLE();
-    return 0;
-}
 }

src/libANGLE/Surface.h

@@ -16,7 +16,6 @@
 #include "common/angleutils.h"
 #include "libANGLE/Error.h"
 #include "libANGLE/FramebufferAttachment.h"
-#include "libANGLE/RefCountObject.h"
 #include "libANGLE/renderer/SurfaceImpl.h"
 
 namespace gl
@@ -49,8 +48,6 @@ class Surface final : public gl::FramebufferAttachmentObject
     EGLint isPostSubBufferSupported() const;
 
     void setSwapInterval(EGLint interval);
-    void setIsCurrent(bool isCurrent);
-    void onDestroy();
 
     const Config *getConfig() const;
 
@@ -73,10 +70,6 @@ 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; }
@@ -86,8 +79,6 @@ class Surface final : public gl::FramebufferAttachmentObject
     void releaseTexImageFromTexture();
 
     rx::SurfaceImpl *mImplementation;
-    int mCurrentCount;
-    bool mDestroyed;
 
     EGLint mType;
 

src/libANGLE/Surface_unittest.cpp

@@ -47,7 +47,7 @@ class SurfaceTest : public testing::Test
 
     virtual void TearDown()
     {
-        mSurface->onDestroy();
+        mSurface->release();
     }
 
     MockSurfaceImpl *mImpl;
@@ -61,7 +61,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->onDestroy();
+    surface->release();
 
     // 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

@@ -691,18 +691,4 @@ 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, public gl::FramebufferAttachmentObject
+class Texture final : public egl::ImageSibling
 {
   public:
     Texture(rx::TextureImpl *impl, GLuint id, GLenum target);
@@ -121,10 +121,6 @@ class Texture final : public egl::ImageSibling, public gl::FramebufferAttachment
     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