Revert "Store value types for FBO attachments."

src/libANGLE/Framebuffer.cpp

@@ -27,19 +27,19 @@ namespace gl
 
 namespace
 {
-void DetachMatchingAttachment(FramebufferAttachment *attachment, GLenum matchType, GLuint matchId)
+void DeleteMatchingAttachment(FramebufferAttachment *&attachment, GLenum matchType, GLuint matchId)
 {
-    if (attachment->isAttached() &&
-        attachment->type() == matchType &&
-        attachment->id() == matchId)
+    if (attachment && attachment->type() == matchType && attachment->id() == matchId)
     {
-        attachment->detach();
+        SafeDelete(attachment);
     }
 }
 }
 
 Framebuffer::Data::Data(const Caps &caps)
-    : mColorAttachments(caps.maxColorAttachments),
+    : mColorAttachments(caps.maxColorAttachments, nullptr),
+      mDepthAttachment(nullptr),
+      mStencilAttachment(nullptr),
       mDrawBufferStates(caps.maxDrawBuffers, GL_NONE),
       mReadBufferState(GL_COLOR_ATTACHMENT0_EXT)
 {
@@ -48,6 +48,12 @@ Framebuffer::Data::Data(const Caps &caps)
 
 Framebuffer::Data::~Data()
 {
+    for (auto &colorAttachment : mColorAttachments)
+    {
+        SafeDelete(colorAttachment);
+    }
+    SafeDelete(mDepthAttachment);
+    SafeDelete(mStencilAttachment);
 }
 
 const FramebufferAttachment *Framebuffer::Data::getReadAttachment() const
@@ -55,16 +61,16 @@ const FramebufferAttachment *Framebuffer::Data::getReadAttachment() const
     ASSERT(mReadBufferState == GL_BACK || (mReadBufferState >= GL_COLOR_ATTACHMENT0 && mReadBufferState <= GL_COLOR_ATTACHMENT15));
     size_t readIndex = (mReadBufferState == GL_BACK ? 0 : static_cast<size_t>(mReadBufferState - GL_COLOR_ATTACHMENT0));
     ASSERT(readIndex < mColorAttachments.size());
-    return mColorAttachments[readIndex].isAttached() ? &mColorAttachments[readIndex] : nullptr;
+    return mColorAttachments[readIndex];
 }
 
 const FramebufferAttachment *Framebuffer::Data::getFirstColorAttachment() const
 {
-    for (const FramebufferAttachment &colorAttachment : mColorAttachments)
+    for (FramebufferAttachment *colorAttachment : mColorAttachments)
     {
-        if (colorAttachment.isAttached())
+        if (colorAttachment != nullptr)
         {
-            return &colorAttachment;
+            return colorAttachment;
         }
     }
 
@@ -73,23 +79,13 @@ const FramebufferAttachment *Framebuffer::Data::getFirstColorAttachment() const
 
 const FramebufferAttachment *Framebuffer::Data::getDepthOrStencilAttachment() const
 {
-    if (mDepthAttachment.isAttached())
-    {
-        return &mDepthAttachment;
-    }
-    if (mStencilAttachment.isAttached())
-    {
-        return &mStencilAttachment;
-    }
-    return nullptr;
+    return (mDepthAttachment != nullptr ? mDepthAttachment : mStencilAttachment);
 }
 
 FramebufferAttachment *Framebuffer::Data::getColorAttachment(unsigned int colorAttachment)
 {
     ASSERT(colorAttachment < mColorAttachments.size());
-    return mColorAttachments[colorAttachment].isAttached() ?
-           &mColorAttachments[colorAttachment] :
-           nullptr;
+    return mColorAttachments[colorAttachment];
 }
 
 const FramebufferAttachment *Framebuffer::Data::getColorAttachment(unsigned int colorAttachment) const
@@ -99,7 +95,7 @@ const FramebufferAttachment *Framebuffer::Data::getColorAttachment(unsigned int
 
 FramebufferAttachment *Framebuffer::Data::getDepthAttachment()
 {
-    return mDepthAttachment.isAttached() ? &mDepthAttachment : nullptr;
+    return mDepthAttachment;
 }
 
 const FramebufferAttachment *Framebuffer::Data::getDepthAttachment() const
@@ -109,7 +105,7 @@ const FramebufferAttachment *Framebuffer::Data::getDepthAttachment() const
 
 FramebufferAttachment *Framebuffer::Data::getStencilAttachment()
 {
-    return mStencilAttachment.isAttached() ? &mStencilAttachment : nullptr;
+    return mStencilAttachment;
 }
 
 const FramebufferAttachment *Framebuffer::Data::getStencilAttachment() const
@@ -121,11 +117,11 @@ FramebufferAttachment *Framebuffer::Data::getDepthStencilAttachment()
 {
     // A valid depth-stencil attachment has the same resource bound to both the
     // depth and stencil attachment points.
-    if (mDepthAttachment.isAttached() && mStencilAttachment.isAttached() &&
-        mDepthAttachment.type() == mStencilAttachment.type() &&
-        mDepthAttachment.id() == mStencilAttachment.id())
+    if (mDepthAttachment && mStencilAttachment &&
+        mDepthAttachment->type() == mStencilAttachment->type() &&
+        mDepthAttachment->id() == mStencilAttachment->id())
     {
-        return &mDepthAttachment;
+        return mDepthAttachment;
     }
 
     return nullptr;
@@ -171,11 +167,11 @@ void Framebuffer::detachResourceById(GLenum resourceType, GLuint resourceId)
 {
     for (auto &colorAttachment : mData.mColorAttachments)
     {
-        DetachMatchingAttachment(&colorAttachment, resourceType, resourceId);
+        DeleteMatchingAttachment(colorAttachment, resourceType, resourceId);
     }
 
-    DetachMatchingAttachment(&mData.mDepthAttachment, resourceType, resourceId);
-    DetachMatchingAttachment(&mData.mStencilAttachment, resourceType, resourceId);
+    DeleteMatchingAttachment(mData.mDepthAttachment, resourceType, resourceId);
+    DeleteMatchingAttachment(mData.mStencilAttachment, resourceType, resourceId);
 }
 
 FramebufferAttachment *Framebuffer::getColorbuffer(unsigned int colorAttachment)
@@ -239,7 +235,7 @@ const FramebufferAttachment *Framebuffer::getFirstColorbuffer() const
     return mData.getFirstColorAttachment();
 }
 
-const FramebufferAttachment *Framebuffer::getAttachment(GLenum attachment) const
+FramebufferAttachment *Framebuffer::getAttachment(GLenum attachment)
 {
     if (attachment >= GL_COLOR_ATTACHMENT0 && attachment <= GL_COLOR_ATTACHMENT15)
     {
@@ -268,6 +264,11 @@ const FramebufferAttachment *Framebuffer::getAttachment(GLenum attachment) const
     }
 }
 
+const FramebufferAttachment *Framebuffer::getAttachment(GLenum attachment) const
+{
+    return const_cast<Framebuffer*>(this)->getAttachment(attachment);
+}
+
 GLenum Framebuffer::getDrawBufferState(unsigned int colorAttachment) const
 {
     ASSERT(colorAttachment < mData.mDrawBufferStates.size());
@@ -301,7 +302,7 @@ void Framebuffer::setReadBuffer(GLenum buffer)
 bool Framebuffer::isEnabledColorAttachment(unsigned int colorAttachment) const
 {
     ASSERT(colorAttachment < mData.mColorAttachments.size());
-    return (mData.mColorAttachments[colorAttachment].isAttached() &&
+    return (mData.mColorAttachments[colorAttachment] &&
             mData.mDrawBufferStates[colorAttachment] != GL_NONE);
 }
 
@@ -320,7 +321,7 @@ bool Framebuffer::hasEnabledColorAttachment() const
 
 bool Framebuffer::hasStencil() const
 {
-    return (mData.mStencilAttachment.isAttached() && mData.mStencilAttachment.getStencilSize() > 0);
+    return (mData.mStencilAttachment && mData.mStencilAttachment->getStencilSize() > 0);
 }
 
 bool Framebuffer::usingExtendedDrawBuffers() const
@@ -351,19 +352,19 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const
     int samples = -1;
     bool missingAttachment = true;
 
-    for (const FramebufferAttachment &colorAttachment : mData.mColorAttachments)
+    for (const FramebufferAttachment *colorAttachment : mData.mColorAttachments)
     {
-        if (colorAttachment.isAttached())
+        if (colorAttachment != nullptr)
         {
-            if (colorAttachment.getWidth() == 0 || colorAttachment.getHeight() == 0)
+            if (colorAttachment->getWidth() == 0 || colorAttachment->getHeight() == 0)
             {
                 return GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT;
             }
 
-            GLenum internalformat = colorAttachment.getInternalFormat();
+            GLenum internalformat = colorAttachment->getInternalFormat();
             const TextureCaps &formatCaps = data.textureCaps->get(internalformat);
             const InternalFormat &formatInfo = GetInternalFormatInfo(internalformat);
-            if (colorAttachment.type() == GL_TEXTURE)
+            if (colorAttachment->type() == GL_TEXTURE)
             {
                 if (!formatCaps.renderable)
                 {
@@ -375,7 +376,7 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const
                     return GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT;
                 }
             }
-            else if (colorAttachment.type() == GL_RENDERBUFFER)
+            else if (colorAttachment->type() == GL_RENDERBUFFER)
             {
                 if (!formatCaps.renderable || formatInfo.depthBits > 0 || formatInfo.stencilBits > 0)
                 {
@@ -386,14 +387,14 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const
             if (!missingAttachment)
             {
                 // all color attachments must have the same width and height
-                if (colorAttachment.getWidth() != width || colorAttachment.getHeight() != height)
+                if (colorAttachment->getWidth() != width || colorAttachment->getHeight() != height)
                 {
                     return GL_FRAMEBUFFER_INCOMPLETE_DIMENSIONS;
                 }
 
                 // APPLE_framebuffer_multisample, which EXT_draw_buffers refers to, requires that
                 // all color attachments have the same number of samples for the FBO to be complete.
-                if (colorAttachment.getSamples() != samples)
+                if (colorAttachment->getSamples() != samples)
                 {
                     return GL_FRAMEBUFFER_INCOMPLETE_MULTISAMPLE_EXT;
                 }
@@ -410,27 +411,27 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const
             }
             else
             {
-                width = colorAttachment.getWidth();
-                height = colorAttachment.getHeight();
-                samples = colorAttachment.getSamples();
+                width = colorAttachment->getWidth();
+                height = colorAttachment->getHeight();
+                samples = colorAttachment->getSamples();
                 colorbufferSize = formatInfo.pixelBytes;
                 missingAttachment = false;
             }
         }
     }
 
-    const FramebufferAttachment &depthAttachment = mData.mDepthAttachment;
-    if (depthAttachment.isAttached())
+    const FramebufferAttachment *depthAttachment = mData.mDepthAttachment;
+    if (depthAttachment != nullptr)
     {
-        if (depthAttachment.getWidth() == 0 || depthAttachment.getHeight() == 0)
+        if (depthAttachment->getWidth() == 0 || depthAttachment->getHeight() == 0)
         {
             return GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT;
         }
 
-        GLenum internalformat = depthAttachment.getInternalFormat();
+        GLenum internalformat = depthAttachment->getInternalFormat();
         const TextureCaps &formatCaps = data.textureCaps->get(internalformat);
         const InternalFormat &formatInfo = GetInternalFormatInfo(internalformat);
-        if (depthAttachment.type() == GL_TEXTURE)
+        if (depthAttachment->type() == GL_TEXTURE)
         {
             // depth texture attachments require OES/ANGLE_depth_texture
             if (!data.extensions->depthTextures)
@@ -448,7 +449,7 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const
                 return GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT;
             }
         }
-        else if (depthAttachment.type() == GL_RENDERBUFFER)
+        else if (depthAttachment->type() == GL_RENDERBUFFER)
         {
             if (!formatCaps.renderable || formatInfo.depthBits == 0)
             {
@@ -458,33 +459,33 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const
 
         if (missingAttachment)
         {
-            width = depthAttachment.getWidth();
-            height = depthAttachment.getHeight();
-            samples = depthAttachment.getSamples();
+            width = depthAttachment->getWidth();
+            height = depthAttachment->getHeight();
+            samples = depthAttachment->getSamples();
             missingAttachment = false;
         }
-        else if (width != depthAttachment.getWidth() || height != depthAttachment.getHeight())
+        else if (width != depthAttachment->getWidth() || height != depthAttachment->getHeight())
         {
             return GL_FRAMEBUFFER_INCOMPLETE_DIMENSIONS;
         }
-        else if (samples != depthAttachment.getSamples())
+        else if (samples != depthAttachment->getSamples())
         {
             return GL_FRAMEBUFFER_INCOMPLETE_MULTISAMPLE_ANGLE;
         }
     }
 
-    const FramebufferAttachment &stencilAttachment = mData.mStencilAttachment;
-    if (stencilAttachment.isAttached())
+    const FramebufferAttachment *stencilAttachment = mData.mStencilAttachment;
+    if (stencilAttachment)
     {
-        if (stencilAttachment.getWidth() == 0 || stencilAttachment.getHeight() == 0)
+        if (stencilAttachment->getWidth() == 0 || stencilAttachment->getHeight() == 0)
         {
             return GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT;
         }
 
-        GLenum internalformat = stencilAttachment.getInternalFormat();
+        GLenum internalformat = stencilAttachment->getInternalFormat();
         const TextureCaps &formatCaps = data.textureCaps->get(internalformat);
         const InternalFormat &formatInfo = GetInternalFormatInfo(internalformat);
-        if (stencilAttachment.type() == GL_TEXTURE)
+        if (stencilAttachment->type() == GL_TEXTURE)
         {
             // texture stencil attachments come along as part
             // of OES_packed_depth_stencil + OES/ANGLE_depth_texture
@@ -503,7 +504,7 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const
                 return GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT;
             }
         }
-        else if (stencilAttachment.type() == GL_RENDERBUFFER)
+        else if (stencilAttachment->type() == GL_RENDERBUFFER)
         {
             if (!formatCaps.renderable || formatInfo.stencilBits == 0)
             {
@@ -513,16 +514,16 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const
 
         if (missingAttachment)
         {
-            width = stencilAttachment.getWidth();
-            height = stencilAttachment.getHeight();
-            samples = stencilAttachment.getSamples();
+            width = stencilAttachment->getWidth();
+            height = stencilAttachment->getHeight();
+            samples = stencilAttachment->getSamples();
             missingAttachment = false;
         }
-        else if (width != stencilAttachment.getWidth() || height != stencilAttachment.getHeight())
+        else if (width != stencilAttachment->getWidth() || height != stencilAttachment->getHeight())
         {
             return GL_FRAMEBUFFER_INCOMPLETE_DIMENSIONS;
         }
-        else if (samples != stencilAttachment.getSamples())
+        else if (samples != stencilAttachment->getSamples())
         {
             return GL_FRAMEBUFFER_INCOMPLETE_MULTISAMPLE_ANGLE;
         }
@@ -530,7 +531,7 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const
 
     // if we have both a depth and stencil buffer, they must refer to the same object
     // since we only support packed_depth_stencil and not separate depth and stencil
-    if (depthAttachment.isAttached() && stencilAttachment.isAttached() && !hasValidDepthStencil())
+    if (depthAttachment && stencilAttachment && !hasValidDepthStencil())
     {
         return GL_FRAMEBUFFER_UNSUPPORTED;
     }
@@ -606,11 +607,11 @@ int Framebuffer::getSamples(const gl::Data &data) const
     {
         // for a complete framebuffer, all attachments must have the same sample count
         // in this case return the first nonzero sample size
-        for (const FramebufferAttachment &colorAttachment : mData.mColorAttachments)
+        for (const FramebufferAttachment *colorAttachment : mData.mColorAttachments)
         {
-            if (colorAttachment.isAttached())
+            if (colorAttachment != nullptr)
             {
-                return colorAttachment.getSamples();
+                return colorAttachment->getSamples();
             }
         }
     }
@@ -623,68 +624,87 @@ bool Framebuffer::hasValidDepthStencil() const
     return mData.getDepthStencilAttachment() != nullptr;
 }
 
-void Framebuffer::setAttachment(GLenum type,
-                                GLenum binding,
-                                const ImageIndex &textureIndex,
-                                FramebufferAttachmentObject *resource)
+void Framebuffer::setTextureAttachment(GLenum attachment, Texture *texture, const ImageIndex &imageIndex)
+{
+    setAttachment(attachment, new FramebufferAttachment(GL_TEXTURE, attachment, imageIndex, texture));
+}
+
+void Framebuffer::setRenderbufferAttachment(GLenum attachment, Renderbuffer *renderbuffer)
 {
-    if (binding == GL_DEPTH_STENCIL || binding == GL_DEPTH_STENCIL_ATTACHMENT)
+    setAttachment(attachment, new FramebufferAttachment(GL_RENDERBUFFER, attachment, ImageIndex::MakeInvalid(), renderbuffer));
+}
+
+void Framebuffer::setNULLAttachment(GLenum attachment)
+{
+    setAttachment(attachment, NULL);
+}
+
+void Framebuffer::setAttachment(GLenum attachment, FramebufferAttachment *attachmentObj)
+{
+    if (attachment >= GL_COLOR_ATTACHMENT0 && attachment < (GL_COLOR_ATTACHMENT0 + mData.mColorAttachments.size()))
     {
-        // ensure this is a legitimate depth+stencil format
-        FramebufferAttachment::Target target(binding, textureIndex);
-        GLenum internalFormat = resource->getAttachmentInternalFormat(target);
-        const InternalFormat &formatInfo = GetInternalFormatInfo(internalFormat);
-        if (resource && formatInfo.depthBits > 0 && formatInfo.stencilBits > 0)
-        {
-            mData.mDepthAttachment.attach(type, binding, textureIndex, resource);
-            mData.mStencilAttachment.attach(type, binding, textureIndex, resource);
-        }
-        else
-        {
-            mData.mDepthAttachment.detach();
-            mData.mStencilAttachment.detach();
-        }
-        mImpl->setDepthStencilAttachment(&mData.mDepthAttachment);
+        size_t colorAttachment = attachment - GL_COLOR_ATTACHMENT0;
+        SafeDelete(mData.mColorAttachments[colorAttachment]);
+        mData.mColorAttachments[colorAttachment] = attachmentObj;
+        mImpl->setColorAttachment(colorAttachment, attachmentObj);
     }
-    else
+    else if (attachment == GL_BACK)
+    {
+        SafeDelete(mData.mColorAttachments[0]);
+        mData.mColorAttachments[0] = attachmentObj;
+        mImpl->setColorAttachment(0, attachmentObj);
+    }
+    else if (attachment == GL_DEPTH_ATTACHMENT || attachment == GL_DEPTH)
+    {
+        SafeDelete(mData.mDepthAttachment);
+        mData.mDepthAttachment = attachmentObj;
+        mImpl->setDepthAttachment(attachmentObj);
+    }
+    else if (attachment == GL_STENCIL_ATTACHMENT || attachment == GL_STENCIL)
+    {
+        SafeDelete(mData.mStencilAttachment);
+        mData.mStencilAttachment = attachmentObj;
+        mImpl->setStencilAttachment(attachmentObj);
+    }
+    else if (attachment == GL_DEPTH_STENCIL_ATTACHMENT || attachment == GL_DEPTH_STENCIL)
     {
-        FramebufferAttachment *attachment = nullptr;
+        SafeDelete(mData.mDepthAttachment);
+        SafeDelete(mData.mStencilAttachment);
 
-        switch (binding)
+        // ensure this is a legitimate depth+stencil format
+        if (attachmentObj && attachmentObj->getDepthSize() > 0 && attachmentObj->getStencilSize() > 0)
         {
-          case GL_DEPTH:
-          case GL_DEPTH_ATTACHMENT:
-            attachment = &mData.mDepthAttachment;
-            attachment->attach(type, binding, textureIndex, resource);
-            mImpl->setDepthAttachment(attachment);
-            break;
-          case GL_STENCIL:
-          case GL_STENCIL_ATTACHMENT:
-            attachment = &mData.mStencilAttachment;
-            attachment->attach(type, binding, textureIndex, resource);
-            mImpl->setStencilAttachment(attachment);
-            break;
-          case GL_BACK:
-            attachment = &mData.mColorAttachments[0];
-            attachment->attach(type, binding, textureIndex, resource);
-            mImpl->setColorAttachment(0, attachment);
-            break;
-          default:
+            mData.mDepthAttachment = attachmentObj;
+            mImpl->setDepthAttachment(attachmentObj);
+
+            // Make a new attachment object to ensure we do not double-delete
+            // See angle issue 686
+            if (attachmentObj->type() == GL_TEXTURE)
+            {
+                mData.mStencilAttachment = new FramebufferAttachment(GL_TEXTURE,
+                                                                     GL_DEPTH_STENCIL_ATTACHMENT,
+                                                                     attachmentObj->getTextureImageIndex(),
+                                                                     attachmentObj->getTexture());
+                mImpl->setStencilAttachment(mData.mStencilAttachment);
+            }
+            else if (attachmentObj->type() == GL_RENDERBUFFER)
+            {
+                mData.mStencilAttachment = new FramebufferAttachment(GL_RENDERBUFFER,
+                                                                     GL_DEPTH_STENCIL_ATTACHMENT,
+                                                                     ImageIndex::MakeInvalid(),
+                                                                     attachmentObj->getRenderbuffer());
+                mImpl->setStencilAttachment(mData.mStencilAttachment);
+            }
+            else
             {
-                size_t colorIndex = binding - GL_COLOR_ATTACHMENT0;
-                ASSERT(colorIndex < mData.mColorAttachments.size());
-                attachment = &mData.mColorAttachments[colorIndex];
-                attachment->attach(type, binding, textureIndex, resource);
-                mImpl->setColorAttachment(colorIndex, attachment);
+                UNREACHABLE();
             }
-            break;
         }
     }
-}
-
-void Framebuffer::resetAttachment(GLenum binding)
-{
-    setAttachment(GL_NONE, binding, ImageIndex::MakeInvalid(), nullptr);
+    else
+    {
+        UNREACHABLE();
+    }
 }
 
 DefaultFramebuffer::DefaultFramebuffer(const Caps &caps, rx::ImplFactory *factory, egl::Surface *surface)
@@ -692,15 +712,15 @@ DefaultFramebuffer::DefaultFramebuffer(const Caps &caps, rx::ImplFactory *factor
 {
     const egl::Config *config = surface->getConfig();
 
-    setAttachment(GL_FRAMEBUFFER_DEFAULT, GL_BACK, ImageIndex::MakeInvalid(), surface);
+    setAttachment(GL_BACK, new FramebufferAttachment(GL_FRAMEBUFFER_DEFAULT, GL_BACK, ImageIndex::MakeInvalid(), surface));
 
     if (config->depthSize > 0)
     {
-        setAttachment(GL_FRAMEBUFFER_DEFAULT, GL_DEPTH, ImageIndex::MakeInvalid(), surface);
+        setAttachment(GL_DEPTH, new FramebufferAttachment(GL_FRAMEBUFFER_DEFAULT, GL_DEPTH, ImageIndex::MakeInvalid(), surface));
     }
     if (config->stencilSize > 0)
     {
-        setAttachment(GL_FRAMEBUFFER_DEFAULT, GL_STENCIL, ImageIndex::MakeInvalid(), surface);
+        setAttachment(GL_STENCIL, new FramebufferAttachment(GL_FRAMEBUFFER_DEFAULT, GL_STENCIL, ImageIndex::MakeInvalid(), surface));
     }
 
     GLenum drawBufferState = GL_BACK;

src/libANGLE/Framebuffer.h

@@ -15,7 +15,6 @@
 #include "common/angleutils.h"
 #include "libANGLE/Constants.h"
 #include "libANGLE/Error.h"
-#include "libANGLE/FramebufferAttachment.h"
 #include "libANGLE/RefCountObject.h"
 
 namespace rx
@@ -33,6 +32,7 @@ class Surface;
 
 namespace gl
 {
+class FramebufferAttachment;
 class Renderbuffer;
 class State;
 class Texture;
@@ -43,6 +43,8 @@ struct Extensions;
 struct ImageIndex;
 struct Rectangle;
 
+typedef std::vector<const FramebufferAttachment *> AttachmentList;
+
 class Framebuffer
 {
   public:
@@ -62,7 +64,7 @@ class Framebuffer
         const FramebufferAttachment *getDepthStencilAttachment() const;
 
         const std::vector<GLenum> &getDrawBufferStates() const { return mDrawBufferStates; }
-        const std::vector<FramebufferAttachment> &getColorAttachments() const { return mColorAttachments; }
+        const std::vector<FramebufferAttachment *> &getColorAttachments() const { return mColorAttachments; }
 
       private:
         friend class Framebuffer;
@@ -71,9 +73,9 @@ class Framebuffer
         FramebufferAttachment *getStencilAttachment();
         FramebufferAttachment *getDepthStencilAttachment();
 
-        std::vector<FramebufferAttachment> mColorAttachments;
-        FramebufferAttachment mDepthAttachment;
-        FramebufferAttachment mStencilAttachment;
+        std::vector<FramebufferAttachment *> mColorAttachments;
+        FramebufferAttachment *mDepthAttachment;
+        FramebufferAttachment *mStencilAttachment;
 
         std::vector<GLenum> mDrawBufferStates;
         GLenum mReadBufferState;
@@ -87,11 +89,9 @@ class Framebuffer
 
     GLuint id() const { return mId; }
 
-    void setAttachment(GLenum type,
-                       GLenum binding,
-                       const ImageIndex &textureIndex,
-                       FramebufferAttachmentObject *resource);
-    void resetAttachment(GLenum binding);
+    void setTextureAttachment(GLenum attachment, Texture *texture, const ImageIndex &imageIndex);
+    void setRenderbufferAttachment(GLenum attachment, Renderbuffer *renderbuffer);
+    void setNULLAttachment(GLenum attachment);
 
     void detachTexture(GLuint texture);
     void detachRenderbuffer(GLuint renderbuffer);
@@ -109,6 +109,7 @@ class Framebuffer
     GLenum getReadColorbufferType() const;
     const FramebufferAttachment *getFirstColorbuffer() const;
 
+    FramebufferAttachment *getAttachment(GLenum attachment);
     const FramebufferAttachment *getAttachment(GLenum attachment) const;
 
     GLenum getDrawBufferState(unsigned int colorAttachment) const;
@@ -143,6 +144,7 @@ class Framebuffer
                GLbitfield mask, GLenum filter, const gl::Framebuffer *sourceFramebuffer);
 
   protected:
+    void setAttachment(GLenum attachment, FramebufferAttachment *attachmentObj);
     void detachResourceById(GLenum resourceType, GLuint resourceId);
 
     Data mData;

src/libANGLE/FramebufferAttachment.cpp

@@ -43,12 +43,6 @@ FramebufferAttachment::Target &FramebufferAttachment::Target::operator=(const Ta
 
 ////// FramebufferAttachment Implementation //////
 
-FramebufferAttachment::FramebufferAttachment()
-    : mType(GL_NONE),
-      mTarget(GL_NONE, ImageIndex::MakeInvalid())
-{
-}
-
 FramebufferAttachment::FramebufferAttachment(GLenum type,
                                              GLenum binding,
                                              const ImageIndex &textureIndex,
@@ -59,25 +53,6 @@ FramebufferAttachment::FramebufferAttachment(GLenum type,
     mResource.set(resource);
 }
 
-void FramebufferAttachment::detach()
-{
-    mType = GL_NONE;
-    mResource.set(nullptr);
-
-    // not technically necessary, could omit for performance
-    mTarget = Target(GL_NONE, ImageIndex::MakeInvalid());
-}
-
-void FramebufferAttachment::attach(GLenum type,
-                                   GLenum binding,
-                                   const ImageIndex &textureIndex,
-                                   FramebufferAttachmentObject *resource)
-{
-    mType = type;
-    mTarget = Target(binding, textureIndex);
-    mResource.set(resource);
-}
-
 FramebufferAttachment::~FramebufferAttachment()
 {
     mResource.set(nullptr);

src/libANGLE/FramebufferAttachment.h

@@ -35,8 +35,6 @@ class Texture;
 class FramebufferAttachment final : angle::NonCopyable
 {
   public:
-    FramebufferAttachment();
-
     FramebufferAttachment(GLenum type,
                           GLenum binding,
                           const ImageIndex &textureIndex,
@@ -64,12 +62,6 @@ class FramebufferAttachment final : angle::NonCopyable
         ImageIndex mTextureIndex;
     };
 
-    void detach();
-    void attach(GLenum type,
-                GLenum binding,
-                const ImageIndex &textureIndex,
-                FramebufferAttachmentObject *resource);
-
     // Helper methods
     GLuint getRedSize() const;
     GLuint getGreenSize() const;
@@ -97,7 +89,6 @@ class FramebufferAttachment final : angle::NonCopyable
     GLenum getInternalFormat() const;
     GLsizei getSamples() const;
     GLenum type() const { return mType; }
-    bool isAttached() const { return mType != GL_NONE; }
 
     Renderbuffer *getRenderbuffer() const;
     Texture *getTexture() const;

src/libANGLE/renderer/d3d/FramebufferD3D.cpp

@@ -324,15 +324,15 @@ GLenum FramebufferD3D::checkStatus() const
     const auto &colorAttachments = mData.getColorAttachments();
     for (size_t colorAttachment = 0; colorAttachment < colorAttachments.size(); colorAttachment++)
     {
-        const gl::FramebufferAttachment &attachment = colorAttachments[colorAttachment];
-        if (attachment.isAttached())
+        const gl::FramebufferAttachment *attachment = colorAttachments[colorAttachment];
+        if (attachment != nullptr)
         {
             for (size_t prevColorAttachment = 0; prevColorAttachment < colorAttachment; prevColorAttachment++)
             {
-                const gl::FramebufferAttachment &prevAttachment = colorAttachments[prevColorAttachment];
-                if (prevAttachment.isAttached() &&
-                    (attachment.id() == prevAttachment.id() &&
-                     attachment.type() == prevAttachment.type()))
+                const gl::FramebufferAttachment *prevAttachment = colorAttachments[prevColorAttachment];
+                if (prevAttachment != nullptr &&
+                    (attachment->id() == prevAttachment->id() &&
+                     attachment->type() == prevAttachment->type()))
                 {
                     return GL_FRAMEBUFFER_UNSUPPORTED;
                 }
@@ -355,16 +355,15 @@ const gl::AttachmentList &FramebufferD3D::getColorAttachmentsForRender(const Wor
 
     const auto &colorAttachments = mData.getColorAttachments();
     const auto &drawBufferStates = mData.getDrawBufferStates();
-
     for (size_t attachmentIndex = 0; attachmentIndex < colorAttachments.size(); ++attachmentIndex)
     {
         GLenum drawBufferState = drawBufferStates[attachmentIndex];
-        const gl::FramebufferAttachment &colorAttachment = colorAttachments[attachmentIndex];
+        const gl::FramebufferAttachment *colorAttachment = colorAttachments[attachmentIndex];
 
-        if (colorAttachment.isAttached() && drawBufferState != GL_NONE)
+        if (colorAttachment != nullptr && drawBufferState != GL_NONE)
         {
             ASSERT(drawBufferState == GL_BACK || drawBufferState == (GL_COLOR_ATTACHMENT0_EXT + attachmentIndex));
-            mColorAttachmentsForRender.push_back(&colorAttachment);
+            mColorAttachmentsForRender.push_back(colorAttachment);
         }
         else if (!workarounds.mrtPerfWorkaround)
         {

src/libANGLE/renderer/d3d/FramebufferD3D.h

@@ -19,9 +19,6 @@ namespace gl
 {
 class FramebufferAttachment;
 struct PixelPackState;
-
-typedef std::vector<const FramebufferAttachment *> AttachmentList;
-
 }
 
 namespace rx

src/libANGLE/renderer/d3d/d3d11/Clear11.cpp

@@ -253,27 +253,27 @@ gl::Error Clear11::clearFramebuffer(const ClearParameters &clearParams, const gl
 
     for (size_t colorAttachment = 0; colorAttachment < colorAttachments.size(); colorAttachment++)
     {
-        const gl::FramebufferAttachment &attachment = colorAttachments[colorAttachment];
-
         if (clearParams.clearColor[colorAttachment] &&
-            attachment.isAttached() &&
+            colorAttachments[colorAttachment] != nullptr &&
             drawBufferStates[colorAttachment] != GL_NONE)
         {
+            const gl::FramebufferAttachment *attachment = colorAttachments[colorAttachment];
+
             RenderTarget11 *renderTarget = NULL;
-            gl::Error error = d3d11::GetAttachmentRenderTarget(&attachment, &renderTarget);
+            gl::Error error = d3d11::GetAttachmentRenderTarget(attachment, &renderTarget);
             if (error.isError())
             {
                 return error;
             }
 
-            const gl::InternalFormat &formatInfo = gl::GetInternalFormatInfo(attachment.getInternalFormat());
+            const gl::InternalFormat &formatInfo = gl::GetInternalFormatInfo(attachment->getInternalFormat());
 
             if (clearParams.colorClearType == GL_FLOAT &&
                 !(formatInfo.componentType == GL_FLOAT || formatInfo.componentType == GL_UNSIGNED_NORMALIZED || formatInfo.componentType == GL_SIGNED_NORMALIZED))
             {
                 ERR("It is undefined behaviour to clear a render buffer which is not normalized fixed point or floating-"
                     "point to floating point values (color attachment %u has internal format 0x%X).", colorAttachment,
-                    attachment.getInternalFormat());
+                    attachment->getInternalFormat());
             }
 
             if ((formatInfo.redBits == 0 || !clearParams.colorMaskRed) &&

src/libANGLE/renderer/d3d/d3d11/Framebuffer11.cpp

@@ -64,15 +64,12 @@ static gl::Error InvalidateAttachmentSwizzles(const gl::FramebufferAttachment *a
 
 gl::Error Framebuffer11::invalidateSwizzles() const
 {
-    for (const auto &colorAttachment : mData.getColorAttachments())
+    for (const auto *colorAttachment : mData.getColorAttachments())
     {
-        if (colorAttachment.isAttached())
+        gl::Error error = InvalidateAttachmentSwizzles(colorAttachment);
+        if (error.isError())
         {
-            gl::Error error = InvalidateAttachmentSwizzles(&colorAttachment);
-            if (error.isError())
-            {
-                return error;
-            }
+            return error;
         }
     }
 
@@ -188,7 +185,7 @@ gl::Error Framebuffer11::blit(const gl::Rectangle &sourceArea, const gl::Rectang
         const gl::FramebufferAttachment *readBuffer = sourceFramebuffer->getReadColorbuffer();
         ASSERT(readBuffer);
 
-        RenderTargetD3D *readRenderTarget = nullptr;
+        RenderTargetD3D *readRenderTarget = NULL;
         gl::Error error = GetAttachmentRenderTarget(readBuffer, &readRenderTarget);
         if (error.isError())
         {
@@ -198,16 +195,15 @@ gl::Error Framebuffer11::blit(const gl::Rectangle &sourceArea, const gl::Rectang
 
         const auto &colorAttachments = mData.getColorAttachments();
         const auto &drawBufferStates = mData.getDrawBufferStates();
-
         for (size_t colorAttachment = 0; colorAttachment < colorAttachments.size(); colorAttachment++)
         {
-            const gl::FramebufferAttachment &drawBuffer = colorAttachments[colorAttachment];
-
-            if (drawBuffer.isAttached() &&
+            if (colorAttachments[colorAttachment] != nullptr &&
                 drawBufferStates[colorAttachment] != GL_NONE)
             {
-                RenderTargetD3D *drawRenderTarget = nullptr;
-                error = GetAttachmentRenderTarget(&drawBuffer, &drawRenderTarget);
+                const gl::FramebufferAttachment *drawBuffer = colorAttachments[colorAttachment];
+
+                RenderTargetD3D *drawRenderTarget = NULL;
+                error = GetAttachmentRenderTarget(drawBuffer, &drawRenderTarget);
                 if (error.isError())
                 {
                     return error;

src/libANGLE/validationES.cpp

@@ -636,8 +636,8 @@ bool ValidateBlitFramebufferParameters(gl::Context *context, GLint srcX0, GLint
     {
         if (mask & masks[i])
         {
-            const gl::FramebufferAttachment *readBuffer = readFramebuffer->getAttachment(attachments[i]);
-            const gl::FramebufferAttachment *drawBuffer = drawFramebuffer->getAttachment(attachments[i]);
+            gl::FramebufferAttachment *readBuffer = readFramebuffer->getAttachment(attachments[i]);
+            gl::FramebufferAttachment *drawBuffer = drawFramebuffer->getAttachment(attachments[i]);
 
             if (readBuffer && drawBuffer)
             {

src/libGLESv2/entry_points_gles_2_0.cpp

@@ -1342,11 +1342,11 @@ void GL_APIENTRY FramebufferRenderbuffer(GLenum target, GLenum attachment, GLenu
         if (renderbuffer != 0)
         {
             Renderbuffer *renderbufferObject = context->getRenderbuffer(renderbuffer);
-            framebuffer->setAttachment(GL_RENDERBUFFER, attachment, gl::ImageIndex::MakeInvalid(), renderbufferObject);
+            framebuffer->setRenderbufferAttachment(attachment, renderbufferObject);
         }
         else
         {
-            framebuffer->resetAttachment(attachment);
+            framebuffer->setNULLAttachment(attachment);
         }
     }
 }
@@ -1383,11 +1383,11 @@ void GL_APIENTRY FramebufferTexture2D(GLenum target, GLenum attachment, GLenum t
                 index = ImageIndex::MakeCube(textarget, level);
             }
 
-            framebuffer->setAttachment(GL_TEXTURE, attachment, index, textureObj);
+            framebuffer->setTextureAttachment(attachment, textureObj, index);
         }
         else
         {
-            framebuffer->resetAttachment(attachment);
+            framebuffer->setNULLAttachment(attachment);
         }
     }
 }

src/libGLESv2/entry_points_gles_3_0.cpp

@@ -763,11 +763,11 @@ void GL_APIENTRY FramebufferTextureLayer(GLenum target, GLenum attachment, GLuin
                 index = ImageIndex::Make2DArray(level, layer);
             }
 
-            framebuffer->setAttachment(GL_TEXTURE, attachment, index, textureObject);
+            framebuffer->setTextureAttachment(attachment, textureObject, index);
         }
         else
         {
-            framebuffer->resetAttachment(attachment);
+            framebuffer->setNULLAttachment(attachment);
         }
     }
 }