Store value types for FBO attachments.

*re-land with fix for Mac*

This prevents us from re-allocating FBO attachments every set.
This change requires quite a bit of refactoring.

BUG=angleproject:963

Change-Id: Ia1f83e3c427d446ddbe16c6703db136942149e91
Reviewed-on: https://chromium-review.googlesource.com/266691
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Tested-by: Jamie Madill <jmadill@chromium.org>
diff --git a/src/libANGLE/Framebuffer.cpp b/src/libANGLE/Framebuffer.cpp
index 78a06a8..3e005b3 100644
--- a/src/libANGLE/Framebuffer.cpp
+++ b/src/libANGLE/Framebuffer.cpp
@@ -27,19 +27,19 @@
 
 namespace
 {
-void DeleteMatchingAttachment(FramebufferAttachment *&attachment, GLenum matchType, GLuint matchId)
+void DetachMatchingAttachment(FramebufferAttachment *attachment, GLenum matchType, GLuint matchId)
 {
-    if (attachment && attachment->type() == matchType && attachment->id() == matchId)
+    if (attachment->isAttached() &&
+        attachment->type() == matchType &&
+        attachment->id() == matchId)
     {
-        SafeDelete(attachment);
+        attachment->detach();
     }
 }
 }
 
 Framebuffer::Data::Data(const Caps &caps)
-    : mColorAttachments(caps.maxColorAttachments, nullptr),
-      mDepthAttachment(nullptr),
-      mStencilAttachment(nullptr),
+    : mColorAttachments(caps.maxColorAttachments),
       mDrawBufferStates(caps.maxDrawBuffers, GL_NONE),
       mReadBufferState(GL_COLOR_ATTACHMENT0_EXT)
 {
@@ -48,12 +48,6 @@
 
 Framebuffer::Data::~Data()
 {
-    for (auto &colorAttachment : mColorAttachments)
-    {
-        SafeDelete(colorAttachment);
-    }
-    SafeDelete(mDepthAttachment);
-    SafeDelete(mStencilAttachment);
 }
 
 const FramebufferAttachment *Framebuffer::Data::getReadAttachment() const
@@ -61,16 +55,16 @@
     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];
+    return mColorAttachments[readIndex].isAttached() ? &mColorAttachments[readIndex] : nullptr;
 }
 
 const FramebufferAttachment *Framebuffer::Data::getFirstColorAttachment() const
 {
-    for (FramebufferAttachment *colorAttachment : mColorAttachments)
+    for (const FramebufferAttachment &colorAttachment : mColorAttachments)
     {
-        if (colorAttachment != nullptr)
+        if (colorAttachment.isAttached())
         {
-            return colorAttachment;
+            return &colorAttachment;
         }
     }
 
@@ -79,13 +73,23 @@
 
 const FramebufferAttachment *Framebuffer::Data::getDepthOrStencilAttachment() const
 {
-    return (mDepthAttachment != nullptr ? mDepthAttachment : mStencilAttachment);
+    if (mDepthAttachment.isAttached())
+    {
+        return &mDepthAttachment;
+    }
+    if (mStencilAttachment.isAttached())
+    {
+        return &mStencilAttachment;
+    }
+    return nullptr;
 }
 
 FramebufferAttachment *Framebuffer::Data::getColorAttachment(unsigned int colorAttachment)
 {
     ASSERT(colorAttachment < mColorAttachments.size());
-    return mColorAttachments[colorAttachment];
+    return mColorAttachments[colorAttachment].isAttached() ?
+           &mColorAttachments[colorAttachment] :
+           nullptr;
 }
 
 const FramebufferAttachment *Framebuffer::Data::getColorAttachment(unsigned int colorAttachment) const
@@ -95,7 +99,7 @@
 
 FramebufferAttachment *Framebuffer::Data::getDepthAttachment()
 {
-    return mDepthAttachment;
+    return mDepthAttachment.isAttached() ? &mDepthAttachment : nullptr;
 }
 
 const FramebufferAttachment *Framebuffer::Data::getDepthAttachment() const
@@ -105,7 +109,7 @@
 
 FramebufferAttachment *Framebuffer::Data::getStencilAttachment()
 {
-    return mStencilAttachment;
+    return mStencilAttachment.isAttached() ? &mStencilAttachment : nullptr;
 }
 
 const FramebufferAttachment *Framebuffer::Data::getStencilAttachment() const
@@ -117,11 +121,11 @@
 {
     // A valid depth-stencil attachment has the same resource bound to both the
     // depth and stencil attachment points.
-    if (mDepthAttachment && mStencilAttachment &&
-        mDepthAttachment->type() == mStencilAttachment->type() &&
-        mDepthAttachment->id() == mStencilAttachment->id())
+    if (mDepthAttachment.isAttached() && mStencilAttachment.isAttached() &&
+        mDepthAttachment.type() == mStencilAttachment.type() &&
+        mDepthAttachment.id() == mStencilAttachment.id())
     {
-        return mDepthAttachment;
+        return &mDepthAttachment;
     }
 
     return nullptr;
@@ -167,11 +171,11 @@
 {
     for (auto &colorAttachment : mData.mColorAttachments)
     {
-        DeleteMatchingAttachment(colorAttachment, resourceType, resourceId);
+        DetachMatchingAttachment(&colorAttachment, resourceType, resourceId);
     }
 
-    DeleteMatchingAttachment(mData.mDepthAttachment, resourceType, resourceId);
-    DeleteMatchingAttachment(mData.mStencilAttachment, resourceType, resourceId);
+    DetachMatchingAttachment(&mData.mDepthAttachment, resourceType, resourceId);
+    DetachMatchingAttachment(&mData.mStencilAttachment, resourceType, resourceId);
 }
 
 FramebufferAttachment *Framebuffer::getColorbuffer(unsigned int colorAttachment)
@@ -235,7 +239,7 @@
     return mData.getFirstColorAttachment();
 }
 
-FramebufferAttachment *Framebuffer::getAttachment(GLenum attachment)
+const FramebufferAttachment *Framebuffer::getAttachment(GLenum attachment) const
 {
     if (attachment >= GL_COLOR_ATTACHMENT0 && attachment <= GL_COLOR_ATTACHMENT15)
     {
@@ -264,11 +268,6 @@
     }
 }
 
-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());
@@ -302,7 +301,7 @@
 bool Framebuffer::isEnabledColorAttachment(unsigned int colorAttachment) const
 {
     ASSERT(colorAttachment < mData.mColorAttachments.size());
-    return (mData.mColorAttachments[colorAttachment] &&
+    return (mData.mColorAttachments[colorAttachment].isAttached() &&
             mData.mDrawBufferStates[colorAttachment] != GL_NONE);
 }
 
@@ -321,7 +320,7 @@
 
 bool Framebuffer::hasStencil() const
 {
-    return (mData.mStencilAttachment && mData.mStencilAttachment->getStencilSize() > 0);
+    return (mData.mStencilAttachment.isAttached() && mData.mStencilAttachment.getStencilSize() > 0);
 }
 
 bool Framebuffer::usingExtendedDrawBuffers() const
@@ -352,19 +351,19 @@
     int samples = -1;
     bool missingAttachment = true;
 
-    for (const FramebufferAttachment *colorAttachment : mData.mColorAttachments)
+    for (const FramebufferAttachment &colorAttachment : mData.mColorAttachments)
     {
-        if (colorAttachment != nullptr)
+        if (colorAttachment.isAttached())
         {
-            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)
                 {
@@ -376,7 +375,7 @@
                     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)
                 {
@@ -387,14 +386,14 @@
             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;
                 }
@@ -411,27 +410,27 @@
             }
             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 != nullptr)
+    const FramebufferAttachment &depthAttachment = mData.mDepthAttachment;
+    if (depthAttachment.isAttached())
     {
-        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)
@@ -449,7 +448,7 @@
                 return GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT;
             }
         }
-        else if (depthAttachment->type() == GL_RENDERBUFFER)
+        else if (depthAttachment.type() == GL_RENDERBUFFER)
         {
             if (!formatCaps.renderable || formatInfo.depthBits == 0)
             {
@@ -459,33 +458,33 @@
 
         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)
+    const FramebufferAttachment &stencilAttachment = mData.mStencilAttachment;
+    if (stencilAttachment.isAttached())
     {
-        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
@@ -504,7 +503,7 @@
                 return GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT;
             }
         }
-        else if (stencilAttachment->type() == GL_RENDERBUFFER)
+        else if (stencilAttachment.type() == GL_RENDERBUFFER)
         {
             if (!formatCaps.renderable || formatInfo.stencilBits == 0)
             {
@@ -514,16 +513,16 @@
 
         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;
         }
@@ -531,7 +530,7 @@
 
     // 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 && stencilAttachment && !hasValidDepthStencil())
+    if (depthAttachment.isAttached() && stencilAttachment.isAttached() && !hasValidDepthStencil())
     {
         return GL_FRAMEBUFFER_UNSUPPORTED;
     }
@@ -607,11 +606,11 @@
     {
         // 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 != nullptr)
+            if (colorAttachment.isAttached())
             {
-                return colorAttachment->getSamples();
+                return colorAttachment.getSamples();
             }
         }
     }
@@ -624,103 +623,84 @@
     return mData.getDepthStencilAttachment() != nullptr;
 }
 
-void Framebuffer::setTextureAttachment(GLenum attachment, Texture *texture, const ImageIndex &imageIndex)
+void Framebuffer::setAttachment(GLenum type,
+                                GLenum binding,
+                                const ImageIndex &textureIndex,
+                                FramebufferAttachmentObject *resource)
 {
-    setAttachment(attachment, new FramebufferAttachment(GL_TEXTURE, attachment, imageIndex, texture));
-}
-
-void Framebuffer::setRenderbufferAttachment(GLenum attachment, Renderbuffer *renderbuffer)
-{
-    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()))
+    if (binding == GL_DEPTH_STENCIL || binding == GL_DEPTH_STENCIL_ATTACHMENT)
     {
-        size_t colorAttachment = attachment - GL_COLOR_ATTACHMENT0;
-        SafeDelete(mData.mColorAttachments[colorAttachment]);
-        mData.mColorAttachments[colorAttachment] = attachmentObj;
-        mImpl->setColorAttachment(colorAttachment, attachmentObj);
-    }
-    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)
-    {
-        SafeDelete(mData.mDepthAttachment);
-        SafeDelete(mData.mStencilAttachment);
-
         // ensure this is a legitimate depth+stencil format
-        if (attachmentObj && attachmentObj->getDepthSize() > 0 && attachmentObj->getStencilSize() > 0)
+        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 = 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
-            {
-                UNREACHABLE();
-            }
+            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);
     }
     else
     {
-        UNREACHABLE();
+        FramebufferAttachment *attachment = nullptr;
+
+        switch (binding)
+        {
+          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:
+            {
+                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);
+            }
+            break;
+        }
     }
 }
 
+void Framebuffer::resetAttachment(GLenum binding)
+{
+    setAttachment(GL_NONE, binding, ImageIndex::MakeInvalid(), nullptr);
+}
+
 DefaultFramebuffer::DefaultFramebuffer(const Caps &caps, rx::ImplFactory *factory, egl::Surface *surface)
     : Framebuffer(caps, factory, 0)
 {
     const egl::Config *config = surface->getConfig();
 
-    setAttachment(GL_BACK, new FramebufferAttachment(GL_FRAMEBUFFER_DEFAULT, GL_BACK, ImageIndex::MakeInvalid(), surface));
+    setAttachment(GL_FRAMEBUFFER_DEFAULT, GL_BACK, ImageIndex::MakeInvalid(), surface);
 
     if (config->depthSize > 0)
     {
-        setAttachment(GL_DEPTH, new FramebufferAttachment(GL_FRAMEBUFFER_DEFAULT, GL_DEPTH, ImageIndex::MakeInvalid(), surface));
+        setAttachment(GL_FRAMEBUFFER_DEFAULT, GL_DEPTH, ImageIndex::MakeInvalid(), surface);
     }
     if (config->stencilSize > 0)
     {
-        setAttachment(GL_STENCIL, new FramebufferAttachment(GL_FRAMEBUFFER_DEFAULT, GL_STENCIL, ImageIndex::MakeInvalid(), surface));
+        setAttachment(GL_FRAMEBUFFER_DEFAULT, GL_STENCIL, ImageIndex::MakeInvalid(), surface);
     }
 
     GLenum drawBufferState = GL_BACK;
diff --git a/src/libANGLE/Framebuffer.h b/src/libANGLE/Framebuffer.h
index dffb1e7..486d8c8 100644
--- a/src/libANGLE/Framebuffer.h
+++ b/src/libANGLE/Framebuffer.h
@@ -15,6 +15,7 @@
 #include "common/angleutils.h"
 #include "libANGLE/Constants.h"
 #include "libANGLE/Error.h"
+#include "libANGLE/FramebufferAttachment.h"
 #include "libANGLE/RefCountObject.h"
 
 namespace rx
@@ -32,7 +33,6 @@
 
 namespace gl
 {
-class FramebufferAttachment;
 class Renderbuffer;
 class State;
 class Texture;
@@ -43,8 +43,6 @@
 struct ImageIndex;
 struct Rectangle;
 
-typedef std::vector<const FramebufferAttachment *> AttachmentList;
-
 class Framebuffer
 {
   public:
@@ -64,7 +62,7 @@
         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;
@@ -73,9 +71,9 @@
         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;
@@ -89,9 +87,11 @@
 
     GLuint id() const { return mId; }
 
-    void setTextureAttachment(GLenum attachment, Texture *texture, const ImageIndex &imageIndex);
-    void setRenderbufferAttachment(GLenum attachment, Renderbuffer *renderbuffer);
-    void setNULLAttachment(GLenum attachment);
+    void setAttachment(GLenum type,
+                       GLenum binding,
+                       const ImageIndex &textureIndex,
+                       FramebufferAttachmentObject *resource);
+    void resetAttachment(GLenum binding);
 
     void detachTexture(GLuint texture);
     void detachRenderbuffer(GLuint renderbuffer);
@@ -109,7 +109,6 @@
     GLenum getReadColorbufferType() const;
     const FramebufferAttachment *getFirstColorbuffer() const;
 
-    FramebufferAttachment *getAttachment(GLenum attachment);
     const FramebufferAttachment *getAttachment(GLenum attachment) const;
 
     GLenum getDrawBufferState(unsigned int colorAttachment) const;
@@ -144,7 +143,6 @@
                GLbitfield mask, GLenum filter, const gl::Framebuffer *sourceFramebuffer);
 
   protected:
-    void setAttachment(GLenum attachment, FramebufferAttachment *attachmentObj);
     void detachResourceById(GLenum resourceType, GLuint resourceId);
 
     Data mData;
diff --git a/src/libANGLE/FramebufferAttachment.cpp b/src/libANGLE/FramebufferAttachment.cpp
index 23f409d..8e3a978 100644
--- a/src/libANGLE/FramebufferAttachment.cpp
+++ b/src/libANGLE/FramebufferAttachment.cpp
@@ -43,6 +43,12 @@
 
 ////// FramebufferAttachment Implementation //////
 
+FramebufferAttachment::FramebufferAttachment()
+    : mType(GL_NONE),
+      mTarget(GL_NONE, ImageIndex::MakeInvalid())
+{
+}
+
 FramebufferAttachment::FramebufferAttachment(GLenum type,
                                              GLenum binding,
                                              const ImageIndex &textureIndex,
@@ -53,6 +59,25 @@
     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);
diff --git a/src/libANGLE/FramebufferAttachment.h b/src/libANGLE/FramebufferAttachment.h
index 7926f09..3372e86 100644
--- a/src/libANGLE/FramebufferAttachment.h
+++ b/src/libANGLE/FramebufferAttachment.h
@@ -32,13 +32,31 @@
 // Note: Our old naming scheme used the term "Renderbuffer" for both GL renderbuffers and for
 // framebuffer attachments, which confused their usage.
 
-class FramebufferAttachment final : angle::NonCopyable
+class FramebufferAttachment final
 {
   public:
+    FramebufferAttachment();
+
     FramebufferAttachment(GLenum type,
                           GLenum binding,
                           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();
 
     // A framebuffer attachment points to one of three types of resources: Renderbuffers,
@@ -62,6 +80,12 @@
         ImageIndex mTextureIndex;
     };
 
+    void detach();
+    void attach(GLenum type,
+                GLenum binding,
+                const ImageIndex &textureIndex,
+                FramebufferAttachmentObject *resource);
+
     // Helper methods
     GLuint getRedSize() const;
     GLuint getGreenSize() const;
@@ -89,6 +113,7 @@
     GLenum getInternalFormat() const;
     GLsizei getSamples() const;
     GLenum type() const { return mType; }
+    bool isAttached() const { return mType != GL_NONE; }
 
     Renderbuffer *getRenderbuffer() const;
     Texture *getTexture() const;
diff --git a/src/libANGLE/renderer/d3d/FramebufferD3D.cpp b/src/libANGLE/renderer/d3d/FramebufferD3D.cpp
index 5dbc102..0006ba5 100644
--- a/src/libANGLE/renderer/d3d/FramebufferD3D.cpp
+++ b/src/libANGLE/renderer/d3d/FramebufferD3D.cpp
@@ -324,15 +324,15 @@
     const auto &colorAttachments = mData.getColorAttachments();
     for (size_t colorAttachment = 0; colorAttachment < colorAttachments.size(); colorAttachment++)
     {
-        const gl::FramebufferAttachment *attachment = colorAttachments[colorAttachment];
-        if (attachment != nullptr)
+        const gl::FramebufferAttachment &attachment = colorAttachments[colorAttachment];
+        if (attachment.isAttached())
         {
             for (size_t prevColorAttachment = 0; prevColorAttachment < colorAttachment; prevColorAttachment++)
             {
-                const gl::FramebufferAttachment *prevAttachment = colorAttachments[prevColorAttachment];
-                if (prevAttachment != nullptr &&
-                    (attachment->id() == prevAttachment->id() &&
-                     attachment->type() == prevAttachment->type()))
+                const gl::FramebufferAttachment &prevAttachment = colorAttachments[prevColorAttachment];
+                if (prevAttachment.isAttached() &&
+                    (attachment.id() == prevAttachment.id() &&
+                     attachment.type() == prevAttachment.type()))
                 {
                     return GL_FRAMEBUFFER_UNSUPPORTED;
                 }
@@ -355,15 +355,16 @@
 
     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 != nullptr && drawBufferState != GL_NONE)
+        if (colorAttachment.isAttached() && 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)
         {
diff --git a/src/libANGLE/renderer/d3d/FramebufferD3D.h b/src/libANGLE/renderer/d3d/FramebufferD3D.h
index d5d2dae..49bc161 100644
--- a/src/libANGLE/renderer/d3d/FramebufferD3D.h
+++ b/src/libANGLE/renderer/d3d/FramebufferD3D.h
@@ -19,6 +19,9 @@
 {
 class FramebufferAttachment;
 struct PixelPackState;
+
+typedef std::vector<const FramebufferAttachment *> AttachmentList;
+
 }
 
 namespace rx
diff --git a/src/libANGLE/renderer/d3d/d3d11/Clear11.cpp b/src/libANGLE/renderer/d3d/d3d11/Clear11.cpp
index e52582d..8c31267 100644
--- a/src/libANGLE/renderer/d3d/d3d11/Clear11.cpp
+++ b/src/libANGLE/renderer/d3d/d3d11/Clear11.cpp
@@ -253,27 +253,27 @@
 
     for (size_t colorAttachment = 0; colorAttachment < colorAttachments.size(); colorAttachment++)
     {
+        const gl::FramebufferAttachment &attachment = colorAttachments[colorAttachment];
+
         if (clearParams.clearColor[colorAttachment] &&
-            colorAttachments[colorAttachment] != nullptr &&
+            attachment.isAttached() &&
             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) &&
diff --git a/src/libANGLE/renderer/d3d/d3d11/Framebuffer11.cpp b/src/libANGLE/renderer/d3d/d3d11/Framebuffer11.cpp
index e2a52e3..51295e7 100644
--- a/src/libANGLE/renderer/d3d/d3d11/Framebuffer11.cpp
+++ b/src/libANGLE/renderer/d3d/d3d11/Framebuffer11.cpp
@@ -64,12 +64,15 @@
 
 gl::Error Framebuffer11::invalidateSwizzles() const
 {
-    for (const auto *colorAttachment : mData.getColorAttachments())
+    for (const auto &colorAttachment : mData.getColorAttachments())
     {
-        gl::Error error = InvalidateAttachmentSwizzles(colorAttachment);
-        if (error.isError())
+        if (colorAttachment.isAttached())
         {
-            return error;
+            gl::Error error = InvalidateAttachmentSwizzles(&colorAttachment);
+            if (error.isError())
+            {
+                return error;
+            }
         }
     }
 
@@ -185,7 +188,7 @@
         const gl::FramebufferAttachment *readBuffer = sourceFramebuffer->getReadColorbuffer();
         ASSERT(readBuffer);
 
-        RenderTargetD3D *readRenderTarget = NULL;
+        RenderTargetD3D *readRenderTarget = nullptr;
         gl::Error error = GetAttachmentRenderTarget(readBuffer, &readRenderTarget);
         if (error.isError())
         {
@@ -195,15 +198,16 @@
 
         const auto &colorAttachments = mData.getColorAttachments();
         const auto &drawBufferStates = mData.getDrawBufferStates();
+
         for (size_t colorAttachment = 0; colorAttachment < colorAttachments.size(); colorAttachment++)
         {
-            if (colorAttachments[colorAttachment] != nullptr &&
+            const gl::FramebufferAttachment &drawBuffer = colorAttachments[colorAttachment];
+
+            if (drawBuffer.isAttached() &&
                 drawBufferStates[colorAttachment] != GL_NONE)
             {
-                const gl::FramebufferAttachment *drawBuffer = colorAttachments[colorAttachment];
-
-                RenderTargetD3D *drawRenderTarget = NULL;
-                error = GetAttachmentRenderTarget(drawBuffer, &drawRenderTarget);
+                RenderTargetD3D *drawRenderTarget = nullptr;
+                error = GetAttachmentRenderTarget(&drawBuffer, &drawRenderTarget);
                 if (error.isError())
                 {
                     return error;
diff --git a/src/libANGLE/validationES.cpp b/src/libANGLE/validationES.cpp
index 4ddc9e4..71a9007 100644
--- a/src/libANGLE/validationES.cpp
+++ b/src/libANGLE/validationES.cpp
@@ -636,8 +636,8 @@
     {
         if (mask & masks[i])
         {
-            gl::FramebufferAttachment *readBuffer = readFramebuffer->getAttachment(attachments[i]);
-            gl::FramebufferAttachment *drawBuffer = drawFramebuffer->getAttachment(attachments[i]);
+            const gl::FramebufferAttachment *readBuffer = readFramebuffer->getAttachment(attachments[i]);
+            const gl::FramebufferAttachment *drawBuffer = drawFramebuffer->getAttachment(attachments[i]);
 
             if (readBuffer && drawBuffer)
             {
diff --git a/src/libGLESv2/entry_points_gles_2_0.cpp b/src/libGLESv2/entry_points_gles_2_0.cpp
index 0989e6c..aa36cb7 100644
--- a/src/libGLESv2/entry_points_gles_2_0.cpp
+++ b/src/libGLESv2/entry_points_gles_2_0.cpp
@@ -1342,11 +1342,11 @@
         if (renderbuffer != 0)
         {
             Renderbuffer *renderbufferObject = context->getRenderbuffer(renderbuffer);
-            framebuffer->setRenderbufferAttachment(attachment, renderbufferObject);
+            framebuffer->setAttachment(GL_RENDERBUFFER, attachment, gl::ImageIndex::MakeInvalid(), renderbufferObject);
         }
         else
         {
-            framebuffer->setNULLAttachment(attachment);
+            framebuffer->resetAttachment(attachment);
         }
     }
 }
@@ -1383,11 +1383,11 @@
                 index = ImageIndex::MakeCube(textarget, level);
             }
 
-            framebuffer->setTextureAttachment(attachment, textureObj, index);
+            framebuffer->setAttachment(GL_TEXTURE, attachment, index, textureObj);
         }
         else
         {
-            framebuffer->setNULLAttachment(attachment);
+            framebuffer->resetAttachment(attachment);
         }
     }
 }
diff --git a/src/libGLESv2/entry_points_gles_3_0.cpp b/src/libGLESv2/entry_points_gles_3_0.cpp
index 50dee1e..0ab42ea 100644
--- a/src/libGLESv2/entry_points_gles_3_0.cpp
+++ b/src/libGLESv2/entry_points_gles_3_0.cpp
@@ -763,11 +763,11 @@
                 index = ImageIndex::Make2DArray(level, layer);
             }
 
-            framebuffer->setTextureAttachment(attachment, textureObject, index);
+            framebuffer->setAttachment(GL_TEXTURE, attachment, index, textureObject);
         }
         else
         {
-            framebuffer->setNULLAttachment(attachment);
+            framebuffer->resetAttachment(attachment);
         }
     }
 }