Refactor Framebuffer management into a FramebufferManager class.

src/libANGLE/Context.cpp

@@ -246,7 +246,6 @@ Context::Context(rx::EGLImplFactory *implFactory,
                         mTextureCaps,
                         mExtensions,
                         mLimitations,
-                        mFramebufferMap,
                         GetNoError(attribs)),
       mImplementation(implFactory->createContext(mState)),
       mCompiler(nullptr),
@@ -386,15 +385,6 @@ Context::~Context()
 {
     mGLState.reset();
 
-    for (auto framebuffer : mFramebufferMap)
-    {
-        // Default framebuffer are owned by their respective Surface
-        if (framebuffer.second != nullptr && framebuffer.second->id() != 0)
-        {
-            SafeDelete(framebuffer.second);
-        }
-    }
-
     for (auto fence : mFenceNVMap)
     {
         SafeDelete(fence.second);
@@ -487,7 +477,7 @@ void Context::makeCurrent(egl::Surface *surface)
         {
             mGLState.setDrawFramebufferBinding(newDefault);
         }
-        mFramebufferMap[0] = newDefault;
+        mState.mFramebuffers->setDefaultFramebuffer(newDefault);
     }
 
     // Notify the renderer of a context switch
@@ -509,7 +499,7 @@ void Context::releaseSurface()
         {
             mGLState.setDrawFramebufferBinding(nullptr);
         }
-        mFramebufferMap.erase(0);
+        mState.mFramebuffers->setDefaultFramebuffer(nullptr);
     }
 
     mCurrentSurface->setIsCurrent(false);
@@ -581,11 +571,7 @@ GLuint Context::createTransformFeedback()
 // Returns an unused framebuffer name
 GLuint Context::createFramebuffer()
 {
-    GLuint handle = mFramebufferHandleAllocator.allocate();
-
-    mFramebufferMap[handle] = NULL;
-
-    return handle;
+    return mState.mFramebuffers->createFramebuffer();
 }
 
 GLuint Context::createFenceNV()
@@ -793,16 +779,12 @@ void Context::deleteTransformFeedback(GLuint transformFeedback)
 
 void Context::deleteFramebuffer(GLuint framebuffer)
 {
-    auto framebufferObject = mFramebufferMap.find(framebuffer);
-
-    if (framebufferObject != mFramebufferMap.end())
+    if (mState.mFramebuffers->getFramebuffer(framebuffer))
     {
         detachFramebuffer(framebuffer);
-
-        mFramebufferHandleAllocator.release(framebufferObject->first);
-        delete framebufferObject->second;
-        mFramebufferMap.erase(framebufferObject);
     }
+
+    mState.mFramebuffers->deleteFramebuffer(framebuffer);
 }
 
 void Context::deleteFenceNV(GLuint fence)
@@ -989,13 +971,15 @@ void Context::bindTexture(GLenum target, GLuint handle)
 
 void Context::bindReadFramebuffer(GLuint framebufferHandle)
 {
-    Framebuffer *framebuffer = checkFramebufferAllocation(framebufferHandle);
+    Framebuffer *framebuffer = mState.mFramebuffers->checkFramebufferAllocation(
+        mImplementation.get(), mCaps, framebufferHandle);
     mGLState.setReadFramebufferBinding(framebuffer);
 }
 
 void Context::bindDrawFramebuffer(GLuint framebufferHandle)
 {
-    Framebuffer *framebuffer = checkFramebufferAllocation(framebufferHandle);
+    Framebuffer *framebuffer = mState.mFramebuffers->checkFramebufferAllocation(
+        mImplementation.get(), mCaps, framebufferHandle);
     mGLState.setDrawFramebufferBinding(framebuffer);
 }
 
@@ -1168,10 +1152,9 @@ void Context::getQueryObjectui64v(GLuint id, GLenum pname, GLuint64 *params)
     handleError(GetQueryObjectParameter(getQuery(id), pname, params));
 }
 
-Framebuffer *Context::getFramebuffer(unsigned int handle) const
+Framebuffer *Context::getFramebuffer(GLuint handle) const
 {
-    auto framebufferIt = mFramebufferMap.find(handle);
-    return ((framebufferIt == mFramebufferMap.end()) ? nullptr : framebufferIt->second);
+    return mState.mFramebuffers->getFramebuffer(handle);
 }
 
 FenceNV *Context::getFenceNV(unsigned int handle)
@@ -2049,19 +2032,15 @@ EGLenum Context::getClientType() const
 
 EGLenum Context::getRenderBuffer() const
 {
-    auto framebufferIt = mFramebufferMap.find(0);
-    if (framebufferIt != mFramebufferMap.end())
-    {
-        const Framebuffer *framebuffer              = framebufferIt->second;
-        const FramebufferAttachment *backAttachment = framebuffer->getAttachment(GL_BACK);
-
-        ASSERT(backAttachment != nullptr);
-        return backAttachment->getSurface()->getRenderBuffer();
-    }
-    else
+    const Framebuffer *framebuffer = mState.mFramebuffers->getFramebuffer(0);
+    if (framebuffer == nullptr)
     {
         return EGL_NONE;
     }
+
+    const FramebufferAttachment *backAttachment = framebuffer->getAttachment(GL_BACK);
+    ASSERT(backAttachment != nullptr);
+    return backAttachment->getSurface()->getRenderBuffer();
 }
 
 VertexArray *Context::checkVertexArrayAllocation(GLuint vertexArrayHandle)
@@ -2093,27 +2072,6 @@ TransformFeedback *Context::checkTransformFeedbackAllocation(GLuint transformFee
     return transformFeedback;
 }
 
-Framebuffer *Context::checkFramebufferAllocation(GLuint framebuffer)
-{
-    // Can be called from Bind without a prior call to Gen.
-    auto framebufferIt = mFramebufferMap.find(framebuffer);
-    bool neverCreated = framebufferIt == mFramebufferMap.end();
-    if (neverCreated || framebufferIt->second == nullptr)
-    {
-        Framebuffer *newFBO = new Framebuffer(mCaps, mImplementation.get(), framebuffer);
-        if (neverCreated)
-        {
-            mFramebufferHandleAllocator.reserve(framebuffer);
-            mFramebufferMap[framebuffer] = newFBO;
-            return newFBO;
-        }
-
-        framebufferIt->second = newFBO;
-    }
-
-    return framebufferIt->second;
-}
-
 bool Context::isVertexArrayGenerated(GLuint vertexArray)
 {
     ASSERT(mVertexArrayMap.find(0) != mVertexArrayMap.end());

src/libANGLE/Context.h

@@ -628,7 +628,6 @@ class Context final : public ValidationContext
     void syncStateForBlit();
     VertexArray *checkVertexArrayAllocation(GLuint vertexArrayHandle);
     TransformFeedback *checkTransformFeedbackAllocation(GLuint transformFeedback);
-    Framebuffer *checkFramebufferAllocation(GLuint framebufferHandle);
 
     void detachBuffer(GLuint buffer);
     void detachTexture(GLuint texture);
@@ -667,9 +666,6 @@ class Context final : public ValidationContext
 
     TextureMap mZeroTextures;
 
-    ResourceMap<Framebuffer> mFramebufferMap;
-    HandleAllocator mFramebufferHandleAllocator;
-
     ResourceMap<FenceNV> mFenceNVMap;
     HandleAllocator mFenceNVHandleAllocator;
 

src/libANGLE/ContextState.cpp

@@ -45,8 +45,7 @@ ContextState::ContextState(uintptr_t contextIn,
                            const Caps &capsIn,
                            const TextureCapsMap &textureCapsIn,
                            const Extensions &extensionsIn,
-                           const Limitations &limitationsIn,
-                           const ResourceMap<Framebuffer> &framebufferMap)
+                           const Limitations &limitationsIn)
     : mClientVersion(clientVersion),
       mContext(contextIn),
       mState(stateIn),
@@ -54,7 +53,6 @@ ContextState::ContextState(uintptr_t contextIn,
       mTextureCaps(textureCapsIn),
       mExtensions(extensionsIn),
       mLimitations(limitationsIn),
-      mFramebufferMap(framebufferMap),
       mBuffers(AllocateOrGetSharedResourceManager(shareContextState, &ContextState::mBuffers)),
       mShaderPrograms(
           AllocateOrGetSharedResourceManager(shareContextState, &ContextState::mShaderPrograms)),
@@ -64,7 +62,8 @@ ContextState::ContextState(uintptr_t contextIn,
       mSamplers(AllocateOrGetSharedResourceManager(shareContextState, &ContextState::mSamplers)),
       mFenceSyncs(
           AllocateOrGetSharedResourceManager(shareContextState, &ContextState::mFenceSyncs)),
-      mPaths(AllocateOrGetSharedResourceManager(shareContextState, &ContextState::mPaths))
+      mPaths(AllocateOrGetSharedResourceManager(shareContextState, &ContextState::mPaths)),
+      mFramebuffers(new FramebufferManager())
 {
 }
 
@@ -77,6 +76,7 @@ ContextState::~ContextState()
     mSamplers->release();
     mFenceSyncs->release();
     mPaths->release();
+    mFramebuffers->release();
 }
 
 const TextureCaps &ContextState::getTextureCap(GLenum internalFormat) const
@@ -91,7 +91,6 @@ ValidationContext::ValidationContext(const ValidationContext *shareContext,
                                      const TextureCapsMap &textureCaps,
                                      const Extensions &extensions,
                                      const Limitations &limitations,
-                                     const ResourceMap<Framebuffer> &framebufferMap,
                                      bool skipValidation)
     : mState(reinterpret_cast<uintptr_t>(this),
              shareContext ? &shareContext->mState : nullptr,
@@ -100,8 +99,7 @@ ValidationContext::ValidationContext(const ValidationContext *shareContext,
              caps,
              textureCaps,
              extensions,
-             limitations,
-             framebufferMap),
+             limitations),
       mSkipValidation(skipValidation)
 {
 }
@@ -672,8 +670,7 @@ bool ValidationContext::isRenderbufferGenerated(GLuint renderbuffer) const
 
 bool ValidationContext::isFramebufferGenerated(GLuint framebuffer) const
 {
-    ASSERT(mState.mFramebufferMap.find(0) != mState.mFramebufferMap.end());
-    return mState.mFramebufferMap.find(framebuffer) != mState.mFramebufferMap.end();
+    return mState.mFramebuffers->isFramebufferGenerated(framebuffer);
 }
 
 }  // namespace gl

src/libANGLE/ContextState.h

@@ -18,6 +18,7 @@ namespace gl
 class BufferManager;
 class ContextState;
 class FenceSyncManager;
+class FramebufferManager;
 class PathManager;
 class RenderbufferManager;
 class SamplerManager;
@@ -39,8 +40,7 @@ class ContextState final : public angle::NonCopyable
                  const Caps &caps,
                  const TextureCapsMap &textureCaps,
                  const Extensions &extensions,
-                 const Limitations &limitations,
-                 const ResourceMap<Framebuffer> &framebufferMap);
+                 const Limitations &limitations);
     ~ContextState();
 
     uintptr_t getContext() const { return mContext; }
@@ -66,7 +66,6 @@ class ContextState final : public angle::NonCopyable
     const TextureCapsMap &mTextureCaps;
     const Extensions &mExtensions;
     const Limitations &mLimitations;
-    const ResourceMap<Framebuffer> &mFramebufferMap;
 
     BufferManager *mBuffers;
     ShaderProgramManager *mShaderPrograms;
@@ -75,6 +74,7 @@ class ContextState final : public angle::NonCopyable
     SamplerManager *mSamplers;
     FenceSyncManager *mFenceSyncs;
     PathManager *mPaths;
+    FramebufferManager *mFramebuffers;
 };
 
 class ValidationContext : angle::NonCopyable
@@ -87,7 +87,6 @@ class ValidationContext : angle::NonCopyable
                       const TextureCapsMap &textureCaps,
                       const Extensions &extensions,
                       const Limitations &limitations,
-                      const ResourceMap<Framebuffer> &framebufferMap,
                       bool skipValidation);
     virtual ~ValidationContext() {}
 

src/libANGLE/ResourceManager.cpp

@@ -485,4 +485,70 @@ PathManager::~PathManager()
     }
 }
 
+FramebufferManager::~FramebufferManager()
+{
+    for (auto framebuffer : mFramebuffers)
+    {
+        // Default framebuffer are owned by their respective Surface
+        if (framebuffer.second != nullptr && framebuffer.second->id() != 0)
+        {
+            SafeDelete(framebuffer.second);
+        }
+    }
+}
+
+GLuint FramebufferManager::createFramebuffer()
+{
+    return AllocateEmptyObject(&mHandleAllocator, &mFramebuffers);
+}
+
+void FramebufferManager::deleteFramebuffer(GLuint framebuffer)
+{
+    DeleteObject(&mHandleAllocator, &mFramebuffers, framebuffer, [](Framebuffer *framebuffer) {
+        ASSERT(framebuffer->id() != 0);
+        delete framebuffer;
+    });
+}
+
+Framebuffer *FramebufferManager::checkFramebufferAllocation(rx::GLImplFactory *factory,
+                                                            const Caps &caps,
+                                                            GLuint handle)
+{
+    // Can be called from Bind without a prior call to Gen.
+    auto framebufferIt = mFramebuffers.find(handle);
+    bool neverCreated  = framebufferIt == mFramebuffers.end();
+    if (neverCreated || framebufferIt->second == nullptr)
+    {
+        ASSERT(handle != 0);
+        Framebuffer *newFBO = new Framebuffer(caps, factory, handle);
+        if (neverCreated)
+        {
+            mHandleAllocator.reserve(handle);
+            mFramebuffers[handle] = newFBO;
+            return newFBO;
+        }
+
+        framebufferIt->second = newFBO;
+    }
+
+    return framebufferIt->second;
+}
+
+Framebuffer *FramebufferManager::getFramebuffer(GLuint handle) const
+{
+    return GetObject(mFramebuffers, handle);
+}
+
+void FramebufferManager::setDefaultFramebuffer(Framebuffer *framebuffer)
+{
+    ASSERT(framebuffer == nullptr || framebuffer->id() == 0);
+    mFramebuffers[0] = framebuffer;
+}
+
+bool FramebufferManager::isFramebufferGenerated(GLuint framebuffer)
+{
+    ASSERT(mFramebuffers.find(0) != mFramebuffers.end());
+    return mFramebuffers.find(framebuffer) != mFramebuffers.end();
+}
+
 }  // namespace gl

src/libANGLE/ResourceManager.h

@@ -25,7 +25,9 @@ class GLImplFactory;
 namespace gl
 {
 class Buffer;
+struct Caps;
 class FenceSync;
+class Framebuffer;
 struct Limitations;
 class Path;
 class Program;
@@ -168,6 +170,25 @@ class PathManager : public ResourceManagerBase<HandleRangeAllocator>
     ResourceMap<Path> mPaths;
 };
 
+class FramebufferManager : public ResourceManagerBase<HandleAllocator>
+{
+  public:
+    GLuint createFramebuffer();
+    void deleteFramebuffer(GLuint framebuffer);
+    Framebuffer *checkFramebufferAllocation(rx::GLImplFactory *factory,
+                                            const Caps &caps,
+                                            GLuint handle);
+    Framebuffer *getFramebuffer(GLuint handle) const;
+    void setDefaultFramebuffer(Framebuffer *framebuffer);
+    bool isFramebufferGenerated(GLuint framebuffer);
+
+  protected:
+    ~FramebufferManager() override;
+
+  private:
+    ResourceMap<Framebuffer> mFramebuffers;
+};
+
 }  // namespace gl
 
 #endif // LIBANGLE_RESOURCEMANAGER_H_

src/libANGLE/validationES_unittest.cpp

@@ -37,7 +37,6 @@ class MockValidationContext : public ValidationContext
                           const TextureCapsMap &textureCaps,
                           const Extensions &extensions,
                           const Limitations &limitations,
-                          const ResourceMap<Framebuffer> &framebufferMap,
                           bool skipValidation)
         : ValidationContext(shareContext,
                             version,
@@ -46,7 +45,6 @@ class MockValidationContext : public ValidationContext
                             textureCaps,
                             extensions,
                             limitations,
-                            framebufferMap,
                             skipValidation)
     {
     }
@@ -73,7 +71,6 @@ TEST(ValidationESTest, DrawElementsWithMaxIndexGivesError)
     TextureCapsMap textureCaps;
     Extensions extensions;
     Limitations limitations;
-    ResourceMap<Framebuffer> framebufferMap;
 
     // Set some basic caps.
     caps.maxElementIndex     = 100;
@@ -101,7 +98,7 @@ TEST(ValidationESTest, DrawElementsWithMaxIndexGivesError)
     state.setProgram(program);
 
     NiceMock<MockValidationContext> testContext(nullptr, Version(3, 0), &state, caps, textureCaps,
-                                                extensions, limitations, framebufferMap, false);
+                                                extensions, limitations, false);
 
     // Set the expectation for the validation error here.
     Error expectedError(GL_INVALID_OPERATION, g_ExceedsMaxElementErrorMessage);