[core] Use RAII for TexturePool textures

TexturePool now disposes acquirable ids via SharedTexture, which
guarantees that these are going back to TexturePool once released.

5 files changed, 57 insertions, 62 deletions

diff --git a/src/mbgl/gl/texture_pool.cpp b/src/mbgl/gl/texture_pool.cpp
index 87e5aed256..53e29b907b 100644
--- a/src/mbgl/gl/texture_pool.cpp
+++ b/src/mbgl/gl/texture_pool.cpp
@@ -9,20 +9,20 @@ namespace gl {
 
 class TexturePool::Impl : private util::noncopyable {
 public:
-    class Group : private util::noncopyable {
+    class Pool : private util::noncopyable {
         public:
-            Group(gl::ObjectStore& store) : pool(store.createTexturePool()), availableIDs(gl::TextureMax) {
+            Pool(gl::ObjectStore& store) : pool(store.createTexturePool()), availableIDs(gl::TextureMax) {
                 std::copy(pool.get().begin(), pool.get().end(), availableIDs.begin());
             }
 
-            Group(Group&& o) : pool(std::move(o.pool)), availableIDs(std::move(o.availableIDs)) {}
-            Group& operator=(Group&& o) { pool = std::move(o.pool); availableIDs = std::move(o.availableIDs); return *this; }
+            Pool(Pool&& o) : pool(std::move(o.pool)), availableIDs(std::move(o.availableIDs)) {}
+            Pool& operator=(Pool&& o) { pool = std::move(o.pool); availableIDs = std::move(o.availableIDs); return *this; }
 
             gl::UniqueTexturePool pool;
             std::vector<GLuint> availableIDs;
     };
 
-    GLuint getTextureID(gl::ObjectStore& store) {
+    GLuint acquireTexture(gl::ObjectStore& store) {
         auto nextAvailableID = [](auto& pool_) {
             auto it = pool_.availableIDs.begin();
             GLuint id = *it;
@@ -36,15 +36,14 @@ public:
         }
 
         // All texture IDs are in use.
-        pools.emplace_back(Group { store });
+        pools.emplace_back(Pool { store });
         return nextAvailableID(pools.back());
     }
 
-    void releaseTextureID(GLuint& id) {
+    void releaseTexture(GLuint id) {
         for (auto it = pools.begin(); it != pools.end(); ++it) {
             if (std::find(it->pool.get().begin(), it->pool.get().end(), id) != it->pool.get().end()) {
                 it->availableIDs.push_back(id);
-                id = 0;
                 if (GLsizei(it->availableIDs.size()) == gl::TextureMax) {
                     pools.erase(it);
                 }
@@ -54,21 +53,22 @@ public:
     }
 
 private:
-    std::vector<Group> pools;
+    std::vector<Pool> pools;
 };
 
-TexturePool::TexturePool() : impl(std::make_unique<Impl>()) {
+void TextureReleaser::operator()(GLuint id) const {
+    assert(pool);
+    pool->impl->releaseTexture(id);
 }
 
-TexturePool::~TexturePool() {
+TexturePool::TexturePool() : impl(std::make_unique<Impl>()) {
 }
 
-GLuint TexturePool::getTextureID(gl::ObjectStore& store) {
-    return impl->getTextureID(store);
+TexturePool::~TexturePool() {
 }
 
-void TexturePool::releaseTextureID(GLuint& id) {
-    impl->releaseTextureID(id);
+SharedTexture TexturePool::acquireTexture(gl::ObjectStore& store) {
+    return SharedTexture { impl->acquireTexture(store) , { this } };
 }
 
 } // namespace gl
diff --git a/src/mbgl/gl/texture_pool.hpp b/src/mbgl/gl/texture_pool.hpp
index 9ab7d14ef6..3c38343f62 100644
--- a/src/mbgl/gl/texture_pool.hpp
+++ b/src/mbgl/gl/texture_pool.hpp
@@ -4,20 +4,32 @@
 #include <mbgl/gl/gl.hpp>
 #include <mbgl/gl/object_store.hpp>
 
+#include <unique_resource.hpp>
+
 #include <memory>
 
 namespace mbgl {
 namespace gl {
 
+class TexturePool;
+
+struct TextureReleaser {
+    TexturePool* pool;
+    void operator()(GLuint) const;
+};
+
+using SharedTexture = std_experimental::unique_resource<GLuint, TextureReleaser>;
+
 class TexturePool : private util::noncopyable {
 public:
     TexturePool();
     ~TexturePool();
 
-    GLuint getTextureID(gl::ObjectStore&);
-    void releaseTextureID(GLuint&);
+    SharedTexture acquireTexture(gl::ObjectStore&);
 
 private:
+    friend TextureReleaser;
+
     class Impl;
     const std::unique_ptr<Impl> impl;
 };
diff --git a/src/mbgl/util/raster.cpp b/src/mbgl/util/raster.cpp
index 70fdf7b02a..514472010f 100644
--- a/src/mbgl/util/raster.cpp
+++ b/src/mbgl/util/raster.cpp
@@ -13,12 +13,6 @@ Raster::Raster(gl::TexturePool& texturePool_)
     : texturePool(texturePool_)
 {}
 
-Raster::~Raster() {
-    if (textured) {
-        texturePool.releaseTextureID(textureID);
-    }
-}
-
 bool Raster::isLoaded() const {
     std::lock_guard<std::mutex> lock(mtx);
     return loaded;
@@ -42,10 +36,10 @@ void Raster::bind(bool linear, gl::ObjectStore& store) {
         return;
     }
 
-    if (img.data && !textured) {
+    if (img.data && !texture) {
         upload(store);
-    } else if (textured) {
-        MBGL_CHECK_ERROR(glBindTexture(GL_TEXTURE_2D, textureID));
+    } else if (texture) {
+        MBGL_CHECK_ERROR(glBindTexture(GL_TEXTURE_2D, *texture));
     }
 
     GLint new_filter = linear ? GL_LINEAR : GL_NEAREST;
@@ -57,15 +51,14 @@ void Raster::bind(bool linear, gl::ObjectStore& store) {
 }
 
 void Raster::upload(gl::ObjectStore& store) {
-    if (img.data && !textured) {
-        textureID = texturePool.getTextureID(store);
-        MBGL_CHECK_ERROR(glBindTexture(GL_TEXTURE_2D, textureID));
+    if (img.data && !texture) {
+        texture = texturePool.acquireTexture(store);
+        MBGL_CHECK_ERROR(glBindTexture(GL_TEXTURE_2D, *texture));
 #ifndef GL_ES_VERSION_2_0
         MBGL_CHECK_ERROR(glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAX_LEVEL, 0));
 #endif
         MBGL_CHECK_ERROR(glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE));
         MBGL_CHECK_ERROR(glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE));
         MBGL_CHECK_ERROR(glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, width, height, 0, GL_RGBA, GL_UNSIGNED_BYTE, img.data.release()));
-        textured = true;
     }
 }
diff --git a/src/mbgl/util/raster.hpp b/src/mbgl/util/raster.hpp
index ff16112d90..ecf026b5fa 100644
--- a/src/mbgl/util/raster.hpp
+++ b/src/mbgl/util/raster.hpp
@@ -5,6 +5,7 @@
 #include <mbgl/util/image.hpp>
 #include <mbgl/util/ptr.hpp>
 #include <mbgl/util/chrono.hpp>
+#include <mbgl/util/optional.hpp>
 
 #include <mutex>
 
@@ -14,7 +15,6 @@ class Raster : public std::enable_shared_from_this<Raster> {
 
 public:
     Raster(gl::TexturePool&);
-    ~Raster();
 
     // load image data
     void load(PremultipliedImage);
@@ -33,11 +33,8 @@ public:
     GLsizei width = 0;
     GLsizei height = 0;
 
-    // has been uploaded to texture
-    bool textured = false;
-
-    // the uploaded texture
-    GLuint textureID = 0;
+    // GL buffer object handle.
+    mbgl::optional<gl::SharedTexture> texture;
 
     // texture opacity
     double opacity = 0;
diff --git a/test/gl/object.cpp b/test/gl/object.cpp
index 4511fc4d1c..1ed2bc91d6 100644
--- a/test/gl/object.cpp
+++ b/test/gl/object.cpp
@@ -130,29 +130,31 @@ TEST(GLObject, TexturePool) {
 
     mbgl::gl::TexturePool pool;
 
+    std::vector<mbgl::gl::SharedTexture> ids;
+
     // Fill an entire texture pool.
     for (auto i = 0; i != mbgl::gl::TextureMax; ++i) {
-        EXPECT_EQ(pool.getTextureID(store), GLuint(i + 1));
+        ids.push_back(pool.acquireTexture(store));
+        EXPECT_EQ(ids.back().get(), GLuint(i + 1));
         EXPECT_TRUE(store.empty());
     }
 
     // Reuse texture ids from the same pool.
     for (auto i = 0; i != mbgl::gl::TextureMax; ++i) {
-        GLuint id = i + 1;
-        pool.releaseTextureID(id);
-        EXPECT_EQ(id, 0);
-        EXPECT_EQ(pool.getTextureID(store), GLuint(i + 1));
+        ids[i].reset();
+        ids.push_back(pool.acquireTexture(store));
+        EXPECT_EQ(ids.back().get(), GLuint(i + 1));
         EXPECT_TRUE(store.empty());
     }
 
     // Trigger a new texture pool creation.
     {
-        GLuint id = pool.getTextureID(store);
+        mbgl::gl::SharedTexture id = pool.acquireTexture(store);
         EXPECT_EQ(id, mbgl::gl::TextureMax + 1);
         EXPECT_TRUE(store.empty());
 
-        pool.releaseTextureID(id);
-        EXPECT_EQ(id, 0);
+        id.reset();
+
         // Last used texture from pool triggers pool recycling.
         EXPECT_FALSE(store.empty());
 
@@ -161,35 +163,26 @@ TEST(GLObject, TexturePool) {
     }
 
     // First pool is still full, thus creating a new pool.
-    GLuint id1 = pool.getTextureID(store);
-    EXPECT_GT(id1, mbgl::gl::TextureMax);
+    mbgl::gl::SharedTexture id1 = pool.acquireTexture(store);
+    EXPECT_GT(id1.get(), mbgl::gl::TextureMax);
     EXPECT_TRUE(store.empty());
 
     // Release all textures from the first pool.
-    for (auto i = 0; i != mbgl::gl::TextureMax; ++i) {
-        GLuint id = i + 1;
-        pool.releaseTextureID(id);
-        if (i == mbgl::gl::TextureMax - 1) {
-            // Last texture from pool triggers pool recycling.
-            EXPECT_FALSE(store.empty());
-        } else {
-            EXPECT_TRUE(store.empty());
-        }
-    }
+    ids.clear();
+    EXPECT_FALSE(store.empty());
 
     store.performCleanup();
     EXPECT_TRUE(store.empty());
 
     // The first pool is now gone, the next pool is now in use.
-    GLuint id2 = pool.getTextureID(store);
-    EXPECT_GT(id2, id1);
-    pool.releaseTextureID(id2);
-    EXPECT_EQ(id2, 0);
+    mbgl::gl::SharedTexture id2 = pool.acquireTexture(store);
+    EXPECT_GT(id2.get(), id1.get());
+
+    id2.reset();
     EXPECT_TRUE(store.empty());
 
     // Last used texture from the pool triggers pool recycling.
-    pool.releaseTextureID(id1);
-    EXPECT_EQ(id1, 0);
+    id1.reset();
     EXPECT_FALSE(store.empty());
 
     store.performCleanup();