[core] Merge TexturePool into ObjectStore; pool all textures (#5477)

src/mbgl/gl/object_store.cpp

@@ -22,7 +22,11 @@ void BufferDeleter::operator()(GLuint id) const {
 
 void TextureDeleter::operator()(GLuint id) const {
     assert(store);
-    store->abandonedTextures.push_back(id);
+    if (store->pooledTextures.size() >= TextureMax) {
+        store->abandonedTextures.push_back(id);
+    } else {
+        store->pooledTextures.push_back(id);
+    }
 }
 
 void VAODeleter::operator()(GLuint id) const {
@@ -30,14 +34,8 @@ void VAODeleter::operator()(GLuint id) const {
     store->abandonedVAOs.push_back(id);
 }
 
-void TexturePoolDeleter::operator()(ObjectPool ids) const {
-    assert(store);
-    for (const auto& id : ids) {
-        store->abandonedTextures.push_back(id);
-    }
-}
-
 ObjectStore::~ObjectStore() {
+    assert(pooledTextures.empty());
     assert(abandonedPrograms.empty());
     assert(abandonedShaders.empty());
     assert(abandonedBuffers.empty());
@@ -45,6 +43,12 @@ ObjectStore::~ObjectStore() {
     assert(abandonedVAOs.empty());
 }
 
+void ObjectStore::reset() {
+    std::copy(pooledTextures.begin(), pooledTextures.end(), std::back_inserter(abandonedTextures));
+    pooledTextures.resize(0);
+    performCleanup();
+}
+
 void ObjectStore::performCleanup() {
     for (GLuint id : abandonedPrograms) {
         MBGL_CHECK_ERROR(glDeleteProgram(id));

src/mbgl/gl/object_store.hpp

@@ -5,7 +5,6 @@
 
 #include <unique_resource.hpp>
 
-#include <array>
 #include <algorithm>
 #include <memory>
 #include <vector>
@@ -42,19 +41,11 @@ struct VAODeleter {
     void operator()(GLuint) const;
 };
 
-using ObjectPool = std::array<GLuint, TextureMax>;
-
-struct TexturePoolDeleter {
-    ObjectStore* store;
-    void operator()(ObjectPool) const;
-};
-
 using UniqueProgram = std_experimental::unique_resource<GLuint, ProgramDeleter>;
 using UniqueShader = std_experimental::unique_resource<GLuint, ShaderDeleter>;
 using UniqueBuffer = std_experimental::unique_resource<GLuint, BufferDeleter>;
 using UniqueTexture = std_experimental::unique_resource<GLuint, TextureDeleter>;
 using UniqueVAO = std_experimental::unique_resource<GLuint, VAODeleter>;
-using UniqueTexturePool = std_experimental::unique_resource<ObjectPool, TexturePoolDeleter>;
 
 class ObjectStore : private util::noncopyable {
 public:
@@ -75,8 +66,13 @@ public:
     }
 
     UniqueTexture createTexture() {
-        GLuint id = 0;
-        MBGL_CHECK_ERROR(glGenTextures(1, &id));
+        if (pooledTextures.empty()) {
+            pooledTextures.resize(TextureMax);
+            MBGL_CHECK_ERROR(glGenTextures(TextureMax, pooledTextures.data()));
+        }
+
+        GLuint id = pooledTextures.back();
+        pooledTextures.pop_back();
         return UniqueTexture { std::move(id), { this } };
     }
 
@@ -86,19 +82,17 @@ public:
         return UniqueVAO { std::move(id), { this } };
     }
 
-    UniqueTexturePool createTexturePool() {
-        ObjectPool ids;
-        MBGL_CHECK_ERROR(glGenTextures(TextureMax, ids.data()));
-        static_assert(ids.size() == size_t(TextureMax), "Texture ids size mismatch");
-        return UniqueTexturePool { std::move(ids), { this } };
-    }
-
     // 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();
 
+    // Drain pools and remove abandoned objects, in preparation for destroying the store.
+    // Only call this while the OpenGL context is exclusive to this thread.
+    void reset();
+
     bool empty() const {
-        return abandonedPrograms.empty()
+        return pooledTextures.empty()
+            && abandonedPrograms.empty()
             && abandonedShaders.empty()
             && abandonedBuffers.empty()
             && abandonedTextures.empty()
@@ -111,7 +105,8 @@ private:
     friend BufferDeleter;
     friend TextureDeleter;
     friend VAODeleter;
-    friend TexturePoolDeleter;
+
+    std::vector<GLuint> pooledTextures;
 
     std::vector<GLuint> abandonedPrograms;
     std::vector<GLuint> abandonedShaders;

src/mbgl/gl/texture_pool.cpp

@@ -1,75 +0,0 @@
-#include <mbgl/gl/texture_pool.hpp>
-#include <mbgl/gl/object_store.hpp>
-
-#include <algorithm>
-#include <cassert>
-#include <vector>
-
-namespace mbgl {
-namespace gl {
-
-class TexturePool::Impl : private util::noncopyable {
-public:
-    class Pool : private util::noncopyable {
-        public:
-            Pool(gl::ObjectStore& store) : pool(store.createTexturePool()), availableIDs(gl::TextureMax) {
-                std::copy(pool.get().begin(), pool.get().end(), availableIDs.begin());
-            }
-
-            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 acquireTexture(gl::ObjectStore& store) {
-        auto nextAvailableID = [](auto& pool_) {
-            auto it = pool_.availableIDs.begin();
-            GLuint id = *it;
-            pool_.availableIDs.erase(it);
-            return id;
-        };
-
-        for (auto& pool : pools) {
-            if (pool.availableIDs.empty()) continue;
-            return nextAvailableID(pool);
-        }
-
-        // All texture IDs are in use.
-        pools.emplace_back(Pool { store });
-        return nextAvailableID(pools.back());
-    }
-
-    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);
-                if (GLsizei(it->availableIDs.size()) == gl::TextureMax) {
-                    pools.erase(it);
-                }
-                return;
-            }
-        }
-    }
-
-private:
-    std::vector<Pool> pools;
-};
-
-void TextureReleaser::operator()(GLuint id) const {
-    assert(pool);
-    pool->impl->releaseTexture(id);
-}
-
-TexturePool::TexturePool() : impl(std::make_unique<Impl>()) {
-}
-
-TexturePool::~TexturePool() = default;
-
-PooledTexture TexturePool::acquireTexture(gl::ObjectStore& store) {
-    return PooledTexture { impl->acquireTexture(store) , { this } };
-}
-
-} // namespace gl
-} // namespace mbgl

src/mbgl/gl/texture_pool.hpp

@@ -1,38 +0,0 @@
-#pragma once
-
-#include <mbgl/util/noncopyable.hpp>
-#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 PooledTexture = std_experimental::unique_resource<GLuint, TextureReleaser>;
-
-class TexturePool : private util::noncopyable {
-public:
-    TexturePool();
-    ~TexturePool();
-
-    PooledTexture acquireTexture(gl::ObjectStore&);
-
-private:
-    friend TextureReleaser;
-
-    class Impl;
-    const std::unique_ptr<Impl> impl;
-};
-
-} // namespace gl
-} // namespace mbgl

src/mbgl/map/map.cpp

@@ -16,7 +16,6 @@
 #include <mbgl/storage/resource.hpp>
 #include <mbgl/storage/response.hpp>
 #include <mbgl/gl/object_store.hpp>
-#include <mbgl/gl/texture_pool.hpp>
 #include <mbgl/util/projection.hpp>
 #include <mbgl/util/math.hpp>
 #include <mbgl/util/exception.hpp>
@@ -63,7 +62,6 @@ public:
     util::AsyncTask asyncUpdate;
 
     std::unique_ptr<AnnotationManager> annotationManager;
-    std::unique_ptr<gl::TexturePool> texturePool;
     std::unique_ptr<Painter> painter;
     std::unique_ptr<Style> style;
 
@@ -99,8 +97,7 @@ Map::Impl::Impl(View& view_,
       contextMode(contextMode_),
       pixelRatio(view.getPixelRatio()),
       asyncUpdate([this] { update(); }),
-      annotationManager(std::make_unique<AnnotationManager>(pixelRatio)),
-      texturePool(std::make_unique<gl::TexturePool>()) {
+      annotationManager(std::make_unique<AnnotationManager>(pixelRatio)) {
 }
 
 Map::~Map() {
@@ -109,13 +106,11 @@ Map::~Map() {
     impl->styleRequest = nullptr;
 
     // Explicit resets currently necessary because these abandon resources that need to be
-    // cleaned up by store.performCleanup();
+    // cleaned up by store.reset();
     impl->style.reset();
     impl->painter.reset();
-    impl->texturePool.reset();
     impl->annotationManager.reset();
-
-    impl->store.performCleanup();
+    impl->store.reset();
 
     impl->view.deactivate();
 }
@@ -254,7 +249,7 @@ void Map::Impl::update() {
 
 void Map::Impl::render() {
     if (!painter) {
-        painter = std::make_unique<Painter>(transform.getState(), *texturePool, store);
+        painter = std::make_unique<Painter>(transform.getState(), store);
     }
 
     FrameData frameData { view.getFramebufferSize(),

src/mbgl/renderer/bucket.hpp

@@ -16,7 +16,6 @@ class UnwrappedTileID;
 class CollisionTile;
 
 namespace gl {
-class TexturePool;
 class ObjectStore;
 } // namespace gl
 
@@ -30,7 +29,7 @@ public:
 
     // As long as this bucket has a Prepare render pass, this function is getting called. Typically,
     // this only happens once when the bucket is being rendered for the first time.
-    virtual void upload(gl::TexturePool&, gl::ObjectStore&) = 0;
+    virtual void upload(gl::ObjectStore&) = 0;
 
     // Every time this bucket is getting rendered, this function is called. This happens either
     // once or twice (for Opaque and Transparent render passes).

src/mbgl/renderer/circle_bucket.cpp

@@ -16,7 +16,7 @@ CircleBucket::~CircleBucket() {
     // Do not remove. header file only contains forward definitions to unique pointers.
 }
 
-void CircleBucket::upload(gl::TexturePool&, gl::ObjectStore& store) {
+void CircleBucket::upload(gl::ObjectStore& store) {
     vertexBuffer_.upload(store);
     elementsBuffer_.upload(store);
     uploaded = true;

src/mbgl/renderer/circle_bucket.hpp

@@ -18,7 +18,7 @@ public:
     CircleBucket(const MapMode);
     ~CircleBucket() override;
 
-    void upload(gl::TexturePool&, gl::ObjectStore&) override;
+    void upload(gl::ObjectStore&) override;
     void render(Painter&, const style::Layer&, const UnwrappedTileID&, const mat4&) override;
 
     bool hasData() const override;

src/mbgl/renderer/fill_bucket.cpp

@@ -95,7 +95,7 @@ void FillBucket::addGeometry(const GeometryCollection& geometry) {
     }
 }
 
-void FillBucket::upload(gl::TexturePool&, gl::ObjectStore& store) {
+void FillBucket::upload(gl::ObjectStore& store) {
     vertexBuffer.upload(store);
     triangleElementsBuffer.upload(store);
     lineElementsBuffer.upload(store);

src/mbgl/renderer/fill_bucket.hpp

@@ -20,7 +20,7 @@ public:
     FillBucket();
     ~FillBucket() override;
 
-    void upload(gl::TexturePool&, gl::ObjectStore&) override;
+    void upload(gl::ObjectStore&) override;
     void render(Painter&, const style::Layer&, const UnwrappedTileID&, const mat4&) override;
     bool hasData() const override;
     bool needsClipping() const override;

src/mbgl/renderer/line_bucket.cpp

@@ -437,7 +437,7 @@ void LineBucket::addPieSliceVertex(const GeometryCoordinate& currentVertex,
     }
 }
 
-void LineBucket::upload(gl::TexturePool&, gl::ObjectStore& store) {
+void LineBucket::upload(gl::ObjectStore& store) {
     vertexBuffer.upload(store);
     triangleElementsBuffer.upload(store);
 

src/mbgl/renderer/line_bucket.hpp

@@ -24,7 +24,7 @@ public:
     LineBucket(uint32_t overscaling);
     ~LineBucket() override;
 
-    void upload(gl::TexturePool&, gl::ObjectStore&) override;
+    void upload(gl::ObjectStore&) override;
     void render(Painter&, const style::Layer&, const UnwrappedTileID&, const mat4&) override;
     bool hasData() const override;
     bool needsClipping() const override;

src/mbgl/renderer/painter.cpp

@@ -51,9 +51,8 @@ namespace mbgl {
 using namespace style;
 
 Painter::Painter(const TransformState& state_,
-                 gl::TexturePool& texturePool_,
                  gl::ObjectStore& store_)
-    : state(state_), texturePool(texturePool_), store(store_) {
+    : state(state_), store(store_) {
     gl::debugging::enable();
 
     plainShader = std::make_unique<PlainShader>(store);
@@ -129,7 +128,7 @@ void Painter::render(const Style& style, const FrameData& frame_, SpriteAtlas& a
 
         for (const auto& item : order) {
             if (item.bucket && item.bucket->needsUpload()) {
-                item.bucket->upload(texturePool, store);
+                item.bucket->upload(store);
             }
         }
     }

src/mbgl/renderer/painter.hpp

@@ -60,7 +60,6 @@ class CollisionBoxShader;
 struct ClipID;
 
 namespace util {
-class TexturePool;
 class ObjectStore;
 } // namespace util
 
@@ -86,7 +85,7 @@ struct FrameData {
 
 class Painter : private util::noncopyable {
 public:
-    Painter(const TransformState&, gl::TexturePool&, gl::ObjectStore&);
+    Painter(const TransformState&, gl::ObjectStore&);
     ~Painter();
 
     void render(const style::Style&,
@@ -179,7 +178,6 @@ private:
     }();
 
     const TransformState& state;
-    gl::TexturePool& texturePool;
     gl::ObjectStore& store;
 
     FrameData frame;

src/mbgl/renderer/painter_raster.cpp

@@ -37,7 +37,7 @@ void Painter::renderRaster(RasterBucket& bucket,
         config.depthTest = GL_TRUE;
         config.depthMask = GL_FALSE;
         setDepthSublayer(0);
-        bucket.drawRaster(*rasterShader, tileStencilBuffer, coveringRasterArray, texturePool, store);
+        bucket.drawRaster(*rasterShader, tileStencilBuffer, coveringRasterArray, store);
     }
 }
 

src/mbgl/renderer/raster_bucket.cpp

@@ -7,9 +7,9 @@ namespace mbgl {
 
 using namespace style;
 
-void RasterBucket::upload(gl::TexturePool& texturePool, gl::ObjectStore& store) {
+void RasterBucket::upload(gl::ObjectStore& store) {
     if (hasData()) {
-        raster.upload(texturePool, store);
+        raster.upload(store);
         uploaded = true;
     }
 }
@@ -28,9 +28,8 @@ void RasterBucket::setImage(PremultipliedImage image) {
 void RasterBucket::drawRaster(RasterShader& shader,
                               StaticVertexBuffer& vertices,
                               VertexArrayObject& array,
-                              gl::TexturePool& texturePool,
                               gl::ObjectStore& store) {
-    raster.bind(true, texturePool, store);
+    raster.bind(true, store);
     array.bind(shader, vertices, BUFFER_OFFSET_0, store);
     MBGL_CHECK_ERROR(glDrawArrays(GL_TRIANGLES, 0, (GLsizei)vertices.index()));
 }

src/mbgl/renderer/raster_bucket.hpp

@@ -11,14 +11,14 @@ class VertexArrayObject;
 
 class RasterBucket : public Bucket {
 public:
-    void upload(gl::TexturePool&, gl::ObjectStore&) override;
+    void upload(gl::ObjectStore&) override;
     void render(Painter&, const style::Layer&, const UnwrappedTileID&, const mat4&) override;
     bool hasData() const override;
     bool needsClipping() const override;
 
     void setImage(PremultipliedImage);
 
-    void drawRaster(RasterShader&, StaticVertexBuffer&, VertexArrayObject&, gl::TexturePool&, gl::ObjectStore&);
+    void drawRaster(RasterShader&, StaticVertexBuffer&, VertexArrayObject&, gl::ObjectStore&);
 
     Raster raster;
 };

src/mbgl/renderer/symbol_bucket.cpp

@@ -73,7 +73,7 @@ SymbolBucket::~SymbolBucket() {
     // Do not remove. header file only contains forward definitions to unique pointers.
 }
 
-void SymbolBucket::upload(gl::TexturePool&, gl::ObjectStore& store) {
+void SymbolBucket::upload(gl::ObjectStore& store) {
     if (hasTextData()) {
         renderData->text.vertices.upload(store);
         renderData->text.triangles.upload(store);

src/mbgl/renderer/symbol_bucket.hpp

@@ -69,7 +69,7 @@ public:
     SymbolBucket(uint32_t overscaling, float zoom, const MapMode, std::string bucketName_, std::string sourceLayerName_);
     ~SymbolBucket() override;
 
-    void upload(gl::TexturePool&, gl::ObjectStore&) override;
+    void upload(gl::ObjectStore&) override;
     void render(Painter&, const style::Layer&, const UnwrappedTileID&, const mat4&) override;
     bool hasData() const override;
     bool hasTextData() const;

src/mbgl/util/raster.cpp

@@ -26,14 +26,14 @@ void Raster::load(PremultipliedImage image) {
 }
 
 
-void Raster::bind(bool linear, gl::TexturePool& texturePool, gl::ObjectStore& store) {
+void Raster::bind(bool linear, gl::ObjectStore& store) {
     if (!width || !height) {
         Log::Error(Event::OpenGL, "trying to bind texture without dimension");
         return;
     }
 
     if (img.data && !texture) {
-        upload(texturePool, store);
+        upload(store);
     } else if (texture) {
         MBGL_CHECK_ERROR(glBindTexture(GL_TEXTURE_2D, *texture));
     }
@@ -46,9 +46,9 @@ void Raster::bind(bool linear, gl::TexturePool& texturePool, gl::ObjectStore& st
     }
 }
 
-void Raster::upload(gl::TexturePool& texturePool, gl::ObjectStore& store) {
+void Raster::upload(gl::ObjectStore& store) {
     if (img.data && !texture) {
-        texture = texturePool.acquireTexture(store);
+        texture = store.createTexture();
         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));

src/mbgl/util/raster.hpp

@@ -1,7 +1,6 @@
 #pragma once
 
-#include <mbgl/gl/gl.hpp>
-#include <mbgl/gl/texture_pool.hpp>
+#include <mbgl/gl/object_store.hpp>
 #include <mbgl/util/image.hpp>
 #include <mbgl/util/ptr.hpp>
 #include <mbgl/util/chrono.hpp>
@@ -18,10 +17,10 @@ public:
     void load(PremultipliedImage);
 
     // bind current texture
-    void bind(bool linear, gl::TexturePool&, gl::ObjectStore&);
+    void bind(bool linear, gl::ObjectStore&);
 
     // uploads the texture if it hasn't been uploaded yet.
-    void upload(gl::TexturePool&, gl::ObjectStore&);
+    void upload(gl::ObjectStore&);
 
     // loaded status
     bool isLoaded() const;
@@ -32,7 +31,7 @@ public:
     GLsizei height = 0;
 
     // GL buffer object handle.
-    mbgl::optional<gl::PooledTexture> texture;
+    mbgl::optional<gl::UniqueTexture> texture;
 
     // texture opacity
     double opacity = 0;

test/gl/object.cpp

@@ -6,7 +6,6 @@
 #include <mbgl/gl/gl_helper.hpp>
 #include <mbgl/gl/gl_config.hpp>
 #include <mbgl/gl/object_store.hpp>
-#include <mbgl/gl/texture_pool.hpp>
 
 #include <memory>
 
@@ -99,6 +98,8 @@ TEST(GLObject, Store) {
     texture.reset();
     EXPECT_FALSE(store.empty());
     store.performCleanup();
+    EXPECT_FALSE(store.empty());
+    store.reset();
     EXPECT_TRUE(store.empty());
 
     mbgl::gl::UniqueVAO vao = store.createVAO();
@@ -108,84 +109,7 @@ TEST(GLObject, Store) {
     store.performCleanup();
     EXPECT_TRUE(store.empty());
 
-    mbgl::gl::UniqueTexturePool texturePool = store.createTexturePool();
-    for (auto& id : texturePool.get()) {
-        EXPECT_NE(id, 0u);
-    }
-    EXPECT_TRUE(texturePool.get().size() == size_t(mbgl::gl::TextureMax));
-    texturePool.reset();
-    EXPECT_FALSE(store.empty());
-    store.performCleanup();
-    EXPECT_TRUE(store.empty());
-
-    view.deactivate();
-}
-
-TEST(GLObject, TexturePool) {
-    mbgl::HeadlessView view(std::make_shared<mbgl::HeadlessDisplay>(), 1);
-    view.activate();
-
-    mbgl::gl::ObjectStore store;
-    EXPECT_TRUE(store.empty());
-
-    mbgl::gl::TexturePool pool;
-
-    std::vector<mbgl::gl::PooledTexture> ids;
-
-    // Fill an entire texture pool.
-    for (auto i = 0; i != mbgl::gl::TextureMax; ++i) {
-        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) {
-        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.
-    {
-        mbgl::gl::PooledTexture id = pool.acquireTexture(store);
-        EXPECT_EQ(id, GLuint(mbgl::gl::TextureMax + 1));
-        EXPECT_TRUE(store.empty());
-
-        id.reset();
-
-        // Last used texture from pool triggers pool recycling.
-        EXPECT_FALSE(store.empty());
-
-        store.performCleanup();
-        EXPECT_TRUE(store.empty());
-    }
-
-    // First pool is still full, thus creating a new pool.
-    mbgl::gl::PooledTexture id1 = pool.acquireTexture(store);
-    EXPECT_GT(id1.get(), GLuint(mbgl::gl::TextureMax));
-    EXPECT_TRUE(store.empty());
-
-    // Release all textures from the first pool.
-    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.
-    mbgl::gl::PooledTexture 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.
-    id1.reset();
-    EXPECT_FALSE(store.empty());
-
-    store.performCleanup();
+    store.reset();
     EXPECT_TRUE(store.empty());
 
     view.deactivate();