Issue/1648 (#1668)

include/mbgl/renderer/change_request.hpp

@@ -68,14 +68,14 @@ protected:
  */
 class RemoveLayerGroupRequest : public ChangeRequest {
 public:
-    RemoveLayerGroupRequest(int32_t layerIndex_)
-        : layerIndex(layerIndex_) {}
+    RemoveLayerGroupRequest(LayerGroupBasePtr layerGroup_)
+        : layerGroup(std::move(layerGroup_)) {}
     RemoveLayerGroupRequest(const RemoveLayerGroupRequest &) = default;
 
     void execute(RenderOrchestrator &) override;
 
 protected:
-    int32_t layerIndex;
+    LayerGroupBasePtr layerGroup;
 };
 
 class UpdateLayerGroupIndexRequest : public ChangeRequest {

src/mbgl/renderer/change_request.cpp

@@ -14,7 +14,7 @@ void AddLayerGroupRequest::execute(RenderOrchestrator &orchestrator) {
 }
 
 void RemoveLayerGroupRequest::execute(RenderOrchestrator &orchestrator) {
-    orchestrator.removeLayerGroup(layerIndex);
+    orchestrator.removeLayerGroup(layerGroup);
 }
 
 UpdateLayerGroupIndexRequest::UpdateLayerGroupIndexRequest(LayerGroupBasePtr tileLayerGroup_, int32_t newLayerIndex_)

src/mbgl/renderer/layers/render_raster_layer.cpp

@@ -239,9 +239,7 @@ void RenderRasterLayer::layerRemoved(UniqueChangeRequestVec& changes) {
 void RenderRasterLayer::layerIndexChanged(int32_t newLayerIndex, UniqueChangeRequestVec& changes) {
     RenderLayer::layerIndexChanged(newLayerIndex, changes);
 
-    if (imageLayerGroup) {
-        changes.emplace_back(std::make_unique<UpdateLayerGroupIndexRequest>(imageLayerGroup, newLayerIndex));
-    }
+    changeLayerIndex(imageLayerGroup, newLayerIndex, changes);
 }
 
 static const StringIdentity idPosAttribName = StringIndexer::get("a_pos");

src/mbgl/renderer/layers/render_symbol_layer.cpp

@@ -967,9 +967,7 @@ void RenderSymbolLayer::layerRemoved(UniqueChangeRequestVec& changes) {
 void RenderSymbolLayer::layerIndexChanged(int32_t newLayerIndex, UniqueChangeRequestVec& changes) {
     RenderLayer::layerIndexChanged(newLayerIndex, changes);
 
-    if (collisionTileLayerGroup) {
-        changes.emplace_back(std::make_unique<UpdateLayerGroupIndexRequest>(collisionTileLayerGroup, newLayerIndex));
-    }
+    changeLayerIndex(collisionTileLayerGroup, newLayerIndex, changes);
 }
 
 void RenderSymbolLayer::removeTile(RenderPass renderPass, const OverscaledTileID& tileID) {

src/mbgl/renderer/render_layer.cpp

@@ -186,8 +186,12 @@ void RenderLayer::layerIndexChanged(int32_t newLayerIndex, UniqueChangeRequestVe
     layerIndex = newLayerIndex;
 
     // Submit a change request to update the layer index of our tile layer group
-    if (layerGroup) {
-        changes.emplace_back(std::make_unique<UpdateLayerGroupIndexRequest>(layerGroup, newLayerIndex));
+    changeLayerIndex(layerGroup, newLayerIndex, changes);
+}
+
+void RenderLayer::changeLayerIndex(const LayerGroupBasePtr& group, int32_t newIndex, UniqueChangeRequestVec& changes) {
+    if (group && group->getLayerIndex() != newIndex) {
+        changes.emplace_back(std::make_unique<UpdateLayerGroupIndexRequest>(group, newIndex));
     }
 }
 
@@ -218,7 +222,7 @@ void RenderLayer::activateLayerGroup(const LayerGroupBasePtr& layerGroup_,
             changes.emplace_back(std::make_unique<AddLayerGroupRequest>(layerGroup_));
         } else {
             // The RenderTree is informing us we should not render anything
-            changes.emplace_back(std::make_unique<RemoveLayerGroupRequest>(layerGroup_->getLayerIndex()));
+            changes.emplace_back(std::make_unique<RemoveLayerGroupRequest>(layerGroup_));
         }
     }
 }

src/mbgl/renderer/render_layer.hpp

@@ -207,6 +207,9 @@ protected:
     /// (Un-)Register the layer group with the orchestrator
     void activateLayerGroup(const LayerGroupBasePtr&, bool activate, UniqueChangeRequestVec& changes);
 
+    /// Change the layer index on a layer group associated with this layer
+    void changeLayerIndex(const LayerGroupBasePtr&, int32_t newLayerIndex, UniqueChangeRequestVec&);
+
     /// Remove all drawables for the tile from the layer group
     virtual void removeTile(RenderPass, const OverscaledTileID&);
 

src/mbgl/renderer/render_orchestrator.cpp

@@ -808,8 +808,6 @@ void RenderOrchestrator::addChanges(UniqueChangeRequestVec& changes) {
     changes.clear();
 }
 
-void RenderOrchestrator::onRemoveLayerGroup(LayerGroupBase&) {}
-
 void RenderOrchestrator::updateLayerIndex(LayerGroupBasePtr layerGroup, const int32_t newIndex) {
     if (!layerGroup || layerGroup->getLayerIndex() == newIndex) {
         return;
@@ -823,28 +821,17 @@ void RenderOrchestrator::updateLayerIndex(LayerGroupBasePtr layerGroup, const in
             return;
         }
     }
-    // We should have found it, maybe the index was changed manually.
-    assert(!"Missing layer group");
+    // We're not tracking the layer, indicating that it's currently disabled, so update it directly.
     layerGroup->updateLayerIndex(newIndex);
 }
 
-void RenderOrchestrator::updateLayerGroupOrder() {
-    // In the event layer indices change for layer groups, we must re-sort them
-    LayerGroupMap newMap;
-    for (auto& it : layerGroupsByLayerIndex) {
-        newMap.emplace(it.second->getLayerIndex(), it.second);
-    }
-    layerGroupsByLayerIndex = std::move(newMap);
-    layerGroupOrderDirty = false;
-}
-
 bool RenderOrchestrator::addLayerGroup(LayerGroupBasePtr layerGroup) {
-    assert(!layerGroupOrderDirty);
     const auto index = layerGroup->getLayerIndex();
     const auto range = layerGroupsByLayerIndex.equal_range(index);
     bool found = false;
     for (auto it = range.first; it != range.second; ++it) {
         if (it->second == layerGroup) {
+            assert(layerGroup->getLayerIndex() == it->first);
             found = true;
             // not added
             break;
@@ -858,15 +845,18 @@ bool RenderOrchestrator::addLayerGroup(LayerGroupBasePtr layerGroup) {
     }
 }
 
-bool RenderOrchestrator::removeLayerGroup(const int32_t layerIndex) {
-    LayerGroupMap::const_iterator hit;
-    bool removed = false;
-    while ((hit = layerGroupsByLayerIndex.find(layerIndex)) != layerGroupsByLayerIndex.end()) {
-        onRemoveLayerGroup(*hit->second);
-        layerGroupsByLayerIndex.erase(hit);
-        removed = true;
+bool RenderOrchestrator::removeLayerGroup(const LayerGroupBasePtr& layerGroup) {
+    if (!layerGroup) {
+        return false;
     }
-    return removed;
+    const auto range = layerGroupsByLayerIndex.equal_range(layerGroup->getLayerIndex());
+    for (auto it = range.first; it != range.second; ++it) {
+        if (it->second == layerGroup) {
+            layerGroupsByLayerIndex.erase(it);
+            return true;
+        }
+    }
+    return false;
 }
 
 size_t RenderOrchestrator::numLayerGroups() const noexcept {
@@ -875,18 +865,12 @@ size_t RenderOrchestrator::numLayerGroups() const noexcept {
 
 int32_t RenderOrchestrator::maxLayerIndex() const {
     if (!layerGroupsByLayerIndex.empty()) {
-        return layerGroupsByLayerIndex.crbegin()->second->getLayerIndex();
+        assert(layerGroupsByLayerIndex.crbegin()->first == layerGroupsByLayerIndex.crbegin()->second->getLayerIndex());
+        return layerGroupsByLayerIndex.crbegin()->first;
     }
     return -1;
 }
 
-static const LayerGroupBasePtr no_group;
-
-const LayerGroupBasePtr& RenderOrchestrator::getLayerGroup(const int32_t layerIndex) const {
-    const auto hit = layerGroupsByLayerIndex.find(layerIndex);
-    return (hit == layerGroupsByLayerIndex.end()) ? no_group : hit->second;
-}
-
 void RenderOrchestrator::visitLayerGroups(std::function<void(LayerGroupBase&)> f) {
     for (auto& pair : layerGroupsByLayerIndex) {
         if (pair.second) {
@@ -931,14 +915,6 @@ void RenderOrchestrator::processChanges() {
     for (auto& change : localChanges) {
         change->execute(*this);
     }
-
-    if (layerGroupOrderDirty) {
-        updateLayerGroupOrder();
-    }
-}
-
-void RenderOrchestrator::markLayerGroupOrderDirty() {
-    layerGroupOrderDirty = true;
 }
 
 bool RenderOrchestrator::addRenderTarget(RenderTargetPtr renderTarget) {

src/mbgl/renderer/render_orchestrator.hpp

@@ -99,10 +99,9 @@ public:
 
 #if MLN_DRAWABLE_RENDERER
     bool addLayerGroup(LayerGroupBasePtr);
-    bool removeLayerGroup(const int32_t layerIndex);
+    bool removeLayerGroup(const LayerGroupBasePtr&);
     size_t numLayerGroups() const noexcept;
     int32_t maxLayerIndex() const;
-    const LayerGroupBasePtr& getLayerGroup(const int32_t layerIndex) const;
     void visitLayerGroups(std::function<void(LayerGroupBase&)>);
     void visitLayerGroups(std::function<void(const LayerGroupBase&)>) const;
     void updateLayerIndex(LayerGroupBasePtr, int32_t newIndex);
@@ -114,8 +113,6 @@ public:
                       const RenderTree&);
 
     void processChanges();
-    /// @brief Indicate that the orchestrator needs to re-sort layer groups when processing changes
-    void markLayerGroupOrderDirty();
 
     bool addRenderTarget(RenderTargetPtr);
     bool removeRenderTarget(const RenderTargetPtr&);
@@ -161,10 +158,6 @@ private:
 #if MLN_DRAWABLE_RENDERER
     /// Move changes into the pending set, clearing the provided collection
     void addChanges(UniqueChangeRequestVec&);
-
-    void onRemoveLayerGroup(LayerGroupBase&);
-
-    void updateLayerGroupOrder();
 #endif
 
     RendererObserver* observer;
@@ -203,7 +196,6 @@ private:
 
     using LayerGroupMap = std::multimap<int32_t, LayerGroupBasePtr>;
     LayerGroupMap layerGroupsByLayerIndex;
-    bool layerGroupOrderDirty = false;
 
     std::vector<RenderTargetPtr> renderTargets;
     RenderItem::DebugLayerGroupMap debugLayerGroups;

test/util/offscreen_texture.test.cpp

@@ -75,7 +75,11 @@ struct Buffer {
     GLuint buffer = 0;
 };
 
+#if MLN_LEGACY_RENDERER
 TEST(OffscreenTexture, RenderToTexture) {
+#else
+TEST(OffscreenTexture, DISABLED_RenderToTexture) {
+#endif
     if (gfx::Backend::GetType() != gfx::Backend::Type::OpenGL) {
         return;
     }
@@ -167,7 +171,11 @@ void main() {
     test::checkImage("test/fixtures/offscreen_texture/render-to-fbo", image, 0, 0);
 
     // Now, composite the Framebuffer texture we've rendered to onto the main FBO.
+#if MLN_LEGACY_RENDERER
     gl::bindTexture(context, 0, {texture.getTexture().getResource(), gfx::TextureFilterType::Linear});
+#else
+    // TODO: Implement based on gfx::Texture2D
+#endif
     MBGL_CHECK_ERROR(glUseProgram(compositeShader.program));
     MBGL_CHECK_ERROR(glUniform1i(u_texture, 0));
     MBGL_CHECK_ERROR(glBindBuffer(GL_ARRAY_BUFFER, viewportBuffer.buffer));