[core] Thread GLObjectStore through to Holder objects

This eliminates the reliance on ThreadContext to provide GLObjectStore, and statically enforces that GL cleanup functions happen only when GLObjectStore::performCleanup is called.

With the elimination of the Map thread, this becomes important because there may be multiple GLObjectStore's per-thread, and Map will need to ensure that the correct context is active when calling GLObjectStore::performCleanup.

4 files changed, 88 insertions, 84 deletions

diff --git a/src/mbgl/gl/gl_object_store.cpp b/src/mbgl/gl/gl_object_store.cpp
index 62d92f3100..4948e20694 100644
--- a/src/mbgl/gl/gl_object_store.cpp
+++ b/src/mbgl/gl/gl_object_store.cpp
@@ -1,115 +1,119 @@
 #include <mbgl/gl/gl_object_store.hpp>
-#include <mbgl/util/thread_context.hpp>
 
 #include <cassert>
 
 namespace mbgl {
 namespace gl {
 
-void ProgramHolder::create() {
+void ProgramHolder::create(GLObjectStore& objectStore_) {
     if (id) return;
-    assert(util::ThreadContext::currentlyOn(util::ThreadType::Map));
+    objectStore = &objectStore_;
     id = MBGL_CHECK_ERROR(glCreateProgram());
 }
 
 void ProgramHolder::reset() {
     if (!id) return;
-    assert(util::ThreadContext::currentlyOn(util::ThreadType::Map));
-    MBGL_CHECK_ERROR(glDeleteProgram(id));
+    objectStore->abandonedPrograms.push_back(id);
     id = 0;
 }
 
-void ShaderHolder::create() {
+void ShaderHolder::create(GLObjectStore& objectStore_) {
     if (id) return;
-    assert(util::ThreadContext::currentlyOn(util::ThreadType::Map));
+    objectStore = &objectStore_;
     id = MBGL_CHECK_ERROR(glCreateShader(type));
 }
 
 void ShaderHolder::reset() {
     if (!id) return;
-    assert(util::ThreadContext::currentlyOn(util::ThreadType::Map));
-    MBGL_CHECK_ERROR(glDeleteShader(id));
+    objectStore->abandonedShaders.push_back(id);
     id = 0;
 }
 
-void BufferHolder::create() {
+void BufferHolder::create(GLObjectStore& objectStore_) {
     if (id) return;
-    assert(util::ThreadContext::currentlyOn(util::ThreadType::Map));
+    objectStore = &objectStore_;
     MBGL_CHECK_ERROR(glGenBuffers(1, &id));
 }
 
 void BufferHolder::reset() {
     if (!id) return;
-    assert(util::ThreadContext::currentlyOn(util::ThreadType::Map));
-    MBGL_CHECK_ERROR(glDeleteBuffers(1, &id));
+    objectStore->abandonedBuffers.push_back(id);
     id = 0;
 }
 
-void TextureHolder::create() {
+void TextureHolder::create(GLObjectStore& objectStore_) {
     if (id) return;
-    assert(util::ThreadContext::currentlyOn(util::ThreadType::Map));
+    objectStore = &objectStore_;
     MBGL_CHECK_ERROR(glGenTextures(1, &id));
 }
 
 void TextureHolder::reset() {
     if (!id) return;
-    assert(util::ThreadContext::currentlyOn(util::ThreadType::Map));
-    MBGL_CHECK_ERROR(glDeleteTextures(1, &id));
+    objectStore->abandonedTextures.push_back(id);
     id = 0;
 }
 
-void TexturePoolHolder::create() {
+void TexturePoolHolder::create(GLObjectStore& objectStore_) {
     if (bool()) return;
-    assert(util::ThreadContext::currentlyOn(util::ThreadType::Map));
+    objectStore = &objectStore_;
     MBGL_CHECK_ERROR(glGenTextures(TextureMax, ids.data()));
 }
 
 void TexturePoolHolder::reset() {
     if (!bool()) return;
-    assert(util::ThreadContext::currentlyOn(util::ThreadType::Map));
-    MBGL_CHECK_ERROR(glDeleteTextures(TextureMax, ids.data()));
+    for (GLuint id : ids) {
+        if (id) {
+            objectStore->abandonedTextures.push_back(id);
+        }
+    }
     ids.fill(0);
 }
 
-void VAOHolder::create() {
+void VAOHolder::create(GLObjectStore& objectStore_) {
     if (id) return;
-    assert(util::ThreadContext::currentlyOn(util::ThreadType::Map));
+    objectStore = &objectStore_;
     MBGL_CHECK_ERROR(gl::GenVertexArrays(1, &id));
 }
 
 void VAOHolder::reset() {
     if (!id) return;
-    assert(util::ThreadContext::currentlyOn(util::ThreadType::Map));
-    MBGL_CHECK_ERROR(gl::DeleteVertexArrays(1, &id));
+    objectStore->abandonedVAOs.push_back(id);
     id = 0;
 }
 
-void GLObjectStore::abandon(BufferHolder&& buffer) {
-    assert(util::ThreadContext::currentlyOn(util::ThreadType::Map));
-    abandonedBuffers.push_back(std::move(buffer));
-}
-
-void GLObjectStore::abandon(TextureHolder&& texture) {
-    assert(util::ThreadContext::currentlyOn(util::ThreadType::Map));
-    abandonedTextures.push_back(std::move(texture));
-}
-
-void GLObjectStore::abandon(TexturePoolHolder&& texture) {
-    assert(util::ThreadContext::currentlyOn(util::ThreadType::Map));
-    abandonedTexturePools.push_back(std::move(texture));
-}
-
-void GLObjectStore::abandon(VAOHolder&& vao) {
-    assert(util::ThreadContext::currentlyOn(util::ThreadType::Map));
-    abandonedVAOs.push_back(std::move(vao));
+GLObjectStore::~GLObjectStore() {
+    assert(abandonedPrograms.empty());
+    assert(abandonedShaders.empty());
+    assert(abandonedBuffers.empty());
+    assert(abandonedTextures.empty());
+    assert(abandonedVAOs.empty());
 }
 
 void GLObjectStore::performCleanup() {
-    assert(util::ThreadContext::currentlyOn(util::ThreadType::Map));
-    abandonedBuffers.clear();
-    abandonedTextures.clear();
-    abandonedTexturePools.clear();
-    abandonedVAOs.clear();
+    for (GLuint id : abandonedPrograms) {
+        MBGL_CHECK_ERROR(glDeleteProgram(id));
+    }
+    abandonedPrograms.clear();
+
+    for (GLuint id : abandonedShaders) {
+        MBGL_CHECK_ERROR(glDeleteShader(id));
+    }
+    abandonedShaders.clear();
+
+    if (!abandonedBuffers.empty()) {
+        MBGL_CHECK_ERROR(glDeleteBuffers(int(abandonedBuffers.size()), abandonedBuffers.data()));
+        abandonedBuffers.clear();
+    }
+
+    if (!abandonedTextures.empty()) {
+        MBGL_CHECK_ERROR(glDeleteTextures(int(abandonedTextures.size()), abandonedTextures.data()));
+        abandonedTextures.clear();
+    }
+
+    if (!abandonedVAOs.empty()) {
+        MBGL_CHECK_ERROR(gl::DeleteVertexArrays(int(abandonedVAOs.size()), abandonedVAOs.data()));
+        abandonedVAOs.clear();
+    }
 }
 
 } // namespace gl
diff --git a/src/mbgl/gl/gl_object_store.hpp b/src/mbgl/gl/gl_object_store.hpp
index 684400d443..832f1d09b3 100644
--- a/src/mbgl/gl/gl_object_store.hpp
+++ b/src/mbgl/gl/gl_object_store.hpp
@@ -12,6 +12,29 @@
 namespace mbgl {
 namespace gl {
 
+class GLObjectStore : private util::noncopyable {
+public:
+    ~GLObjectStore();
+
+    // Actually remove the objects we marked as abandoned with the above methods.
+    // Only call this while the OpenGL context is exclusive to this thread.
+    void performCleanup();
+
+private:
+    friend class ProgramHolder;
+    friend class ShaderHolder;
+    friend class BufferHolder;
+    friend class TextureHolder;
+    friend class TexturePoolHolder;
+    friend class VAOHolder;
+
+    std::vector<GLuint> abandonedPrograms;
+    std::vector<GLuint> abandonedShaders;
+    std::vector<GLuint> abandonedBuffers;
+    std::vector<GLuint> abandonedTextures;
+    std::vector<GLuint> abandonedVAOs;
+};
+
 class GLHolder : private util::noncopyable {
 public:
     GLHolder() {}
@@ -24,6 +47,7 @@ public:
 
 protected:
     GLuint id = 0;
+    GLObjectStore* objectStore = nullptr;
 };
 
 class ProgramHolder : public GLHolder {
@@ -34,7 +58,7 @@ public:
     ProgramHolder(ProgramHolder&& o) noexcept : GLHolder(std::move(o)) {}
     ProgramHolder& operator=(ProgramHolder&& o) noexcept { GLHolder::operator=(std::move(o)); return *this; }
 
-    void create();
+    void create(GLObjectStore&);
     void reset();
 };
 
@@ -46,7 +70,7 @@ public:
     ShaderHolder(ShaderHolder&& o) noexcept : GLHolder(std::move(o)), type(o.type) {}
     ShaderHolder& operator=(ShaderHolder&& o) noexcept { GLHolder::operator=(std::move(o)); type = o.type; return *this; }
 
-    void create();
+    void create(GLObjectStore&);
     void reset();
 
 private:
@@ -61,7 +85,7 @@ public:
     BufferHolder(BufferHolder&& o) noexcept : GLHolder(std::move(o)) {}
     BufferHolder& operator=(BufferHolder&& o) noexcept { GLHolder::operator=(std::move(o)); return *this; }
 
-    void create();
+    void create(GLObjectStore&);
     void reset();
 };
 
@@ -73,7 +97,7 @@ public:
     TextureHolder(TextureHolder&& o) noexcept : GLHolder(std::move(o)) {}
     TextureHolder& operator=(TextureHolder&& o) noexcept { GLHolder::operator=(std::move(o)); return *this; }
 
-    void create();
+    void create(GLObjectStore&);
     void reset();
 };
 
@@ -91,11 +115,12 @@ public:
     const std::array<GLuint, TextureMax>& getIDs() const { return ids; }
     const GLuint& operator[](size_t pos) { return ids[pos]; }
 
-    void create();
+    void create(GLObjectStore&);
     void reset();
 
 private:
     std::array<GLuint, TextureMax> ids;
+    GLObjectStore* objectStore = nullptr;
 };
 
 class VAOHolder : public GLHolder {
@@ -106,31 +131,10 @@ public:
     VAOHolder(VAOHolder&& o) noexcept : GLHolder(std::move(o)) {}
     VAOHolder& operator=(VAOHolder&& o) noexcept { GLHolder::operator=(std::move(o)); return *this; }
 
-    void create();
+    void create(GLObjectStore&);
     void reset();
 };
 
-class GLObjectStore : private util::noncopyable {
-public:
-    // Mark OpenGL objects for deletion
-    void abandon(VAOHolder&&);
-    void abandon(BufferHolder&&);
-    void abandon(TextureHolder&&);
-    void abandon(TexturePoolHolder&&);
-
-    // Actually remove the objects we marked as abandoned with the above methods.
-    // Only call this while the OpenGL context is exclusive to this thread.
-    void performCleanup();
-
-private:
-    // We split the holder objects in separate containers because each
-    // GLHolder-derived object can vary in size.
-    std::vector<VAOHolder> abandonedVAOs;
-    std::vector<BufferHolder> abandonedBuffers;
-    std::vector<TextureHolder> abandonedTextures;
-    std::vector<TexturePoolHolder> abandonedTexturePools;
-};
-
 } // namespace gl
 } // namespace mbgl
 
diff --git a/src/mbgl/gl/texture_pool.cpp b/src/mbgl/gl/texture_pool.cpp
index edde485374..a875f4d579 100644
--- a/src/mbgl/gl/texture_pool.cpp
+++ b/src/mbgl/gl/texture_pool.cpp
@@ -1,15 +1,12 @@
 #include <mbgl/gl/texture_pool.hpp>
 #include <mbgl/gl/gl_object_store.hpp>
-#include <mbgl/util/thread_context.hpp>
 
 #include <vector>
 
 namespace mbgl {
 namespace gl {
 
-GLuint TexturePool::getTextureID() {
-    if (pools.empty()) pools.emplace_back();
-
+GLuint TexturePool::getTextureID(gl::GLObjectStore& glObjectStore) {
     for (auto& impl : pools) {
         if (impl.ids.empty()) continue;
         auto it = impl.ids.begin();
@@ -19,7 +16,7 @@ GLuint TexturePool::getTextureID() {
     }
 
     // All texture IDs are in use.
-    pools.emplace_back();
+    pools.emplace_back(Impl(glObjectStore));
     auto it = pools.back().ids.begin();
     GLuint id = *it;
     pools.back().ids.erase(it);
@@ -32,7 +29,6 @@ void TexturePool::releaseTextureID(GLuint id) {
             if (it->pool[i] == id) {
                 it->ids.push_back(id);
                 if (GLsizei(it->ids.size()) == gl::TexturePoolHolder::TextureMax) {
-                    util::ThreadContext::getGLObjectStore()->abandon(std::move(it->pool));
                     pools.erase(it);
                 }
                 return;
diff --git a/src/mbgl/gl/texture_pool.hpp b/src/mbgl/gl/texture_pool.hpp
index 0992b8398d..10f63bfac9 100644
--- a/src/mbgl/gl/texture_pool.hpp
+++ b/src/mbgl/gl/texture_pool.hpp
@@ -14,14 +14,14 @@ namespace gl {
 
 class TexturePool : private util::noncopyable {
 public:
-    GLuint getTextureID();
+    GLuint getTextureID(gl::GLObjectStore&);
     void releaseTextureID(GLuint);
 
 private:
     class Impl : private util::noncopyable {
     public:
-        Impl() : ids(gl::TexturePoolHolder::TextureMax) {
-            pool.create();
+        Impl(gl::GLObjectStore& glObjectStore) : ids(gl::TexturePoolHolder::TextureMax) {
+            pool.create(glObjectStore);
             std::copy(pool.getIDs().begin(), pool.getIDs().end(), ids.begin());
         }