[core] RenderTile shall never be created for a not renderable tileupstream/mikhail_renderable_render_tiles

A RenderTile is already never created for a not renderable tile, guarantied by the checks in `updateRenderables()`.

However, the client code had plenty of `isRenderable()` checks in the render path, which complicated the code and affected rendering performance.

This patch removes the unneeded checks from the client code and puts an assertion to `TilePyramid::addRenderTile()`.

5 files changed, 11 insertions, 19 deletions

diff --git a/src/mbgl/renderer/layers/render_symbol_layer.cpp b/src/mbgl/renderer/layers/render_symbol_layer.cpp
index 37c33844ac..5c0aea0eac 100644
--- a/src/mbgl/renderer/layers/render_symbol_layer.cpp
+++ b/src/mbgl/renderer/layers/render_symbol_layer.cpp
@@ -617,8 +617,7 @@ style::TextPaintProperties::PossiblyEvaluated RenderSymbolLayer::textPaintProper
 }
 
 void RenderSymbolLayer::setRenderTiles(RenderTiles tiles, const TransformState& state) {
-    auto filterFn = [](auto& tile){ return !tile.tile.isRenderable(); };
-    renderTiles = RenderLayer::filterRenderTiles(std::move(tiles), filterFn);
+    renderTiles = std::move(tiles);
     // Sort symbol tiles in opposite y position, so tiles with overlapping symbols are drawn
     // on top of each other, with lower symbols being drawn on top of higher symbols.
     std::sort(renderTiles.begin(), renderTiles.end(), [&state](const auto& a, const auto& b) {
diff --git a/src/mbgl/renderer/render_layer.cpp b/src/mbgl/renderer/render_layer.cpp
index 757ee4f241..373a693ffd 100644
--- a/src/mbgl/renderer/render_layer.cpp
+++ b/src/mbgl/renderer/render_layer.cpp
@@ -40,8 +40,7 @@ bool RenderLayer::supportsZoom(float zoom) const {
 }
 
 void RenderLayer::setRenderTiles(RenderTiles tiles, const TransformState&) {
-    auto filterFn = [](auto& tile){ return !tile.tile.isRenderable() || tile.tile.holdForFade(); };
-    renderTiles = filterRenderTiles(std::move(tiles), filterFn);
+    renderTiles = filterRenderTiles(std::move(tiles));
 }
 
 const RenderLayerSymbolInterface* RenderLayer::getSymbolInterface() const {
@@ -52,16 +51,16 @@ optional<Color> RenderLayer::getSolidBackground() const {
     return nullopt;
 }
 
-RenderLayer::RenderTiles RenderLayer::filterRenderTiles(RenderTiles tiles, FilterFunctionPtr filterFn) const {
-    assert(filterFn != nullptr);
+RenderLayer::RenderTiles RenderLayer::filterRenderTiles(RenderTiles tiles) const {
     RenderTiles filtered;
 
     for (auto& tileRef : tiles) {
-        auto& tile = tileRef.get();
-        if (filterFn(tile)) {
+        auto& tile = tileRef.get().tile;
+        assert(tile.isRenderable());
+        if (tile.holdForFade()) {
             continue;
         }
-        filtered.emplace_back(tile);
+        filtered.emplace_back(tileRef);
     }
     return filtered;
 }
diff --git a/src/mbgl/renderer/render_layer.hpp b/src/mbgl/renderer/render_layer.hpp
index 2ecd7c0ae5..2bc2563f52 100644
--- a/src/mbgl/renderer/render_layer.hpp
+++ b/src/mbgl/renderer/render_layer.hpp
@@ -92,9 +92,6 @@ protected:
     // in the console to inform the developer.
     void checkRenderability(const PaintParameters&, uint32_t activeBindingCount);
 
-    using FilterFunctionPtr = bool (*)(RenderTile&);
-    RenderTiles filterRenderTiles(RenderTiles, FilterFunctionPtr) const;
-
 protected:
     // Stores current set of tiles to be rendered for this layer.
     std::vector<std::reference_wrapper<RenderTile>> renderTiles;
@@ -104,6 +101,7 @@ protected:
     RenderPass passes = RenderPass::None;
 
 private:
+    RenderTiles filterRenderTiles(RenderTiles) const;
     // Some layers may not render correctly on some hardware when the vertex attribute limit of
     // that GPU is exceeded. More attributes are used when adding many data driven paint properties
     // to a layer.
diff --git a/src/mbgl/renderer/tile_pyramid.cpp b/src/mbgl/renderer/tile_pyramid.cpp
index 9c73361cb0..32b3ef0dba 100644
--- a/src/mbgl/renderer/tile_pyramid.cpp
+++ b/src/mbgl/renderer/tile_pyramid.cpp
@@ -130,7 +130,6 @@ void TilePyramid::update(const std::vector<Immutable<style::LayerProperties>>& l
     // use because they're still loading. In addition to that, we also need to retain all tiles that
     // we're actively using, e.g. as a replacement for tile that aren't loaded yet.
     std::set<OverscaledTileID> retain;
-    std::set<UnwrappedTileID> rendered;
 
     auto retainTileFn = [&](Tile& tile, TileNecessity necessity) -> void {
         if (retain.emplace(tile.id).second) {
@@ -178,7 +177,6 @@ void TilePyramid::update(const std::vector<Immutable<style::LayerProperties>>& l
 
     auto renderTileFn = [&](const UnwrappedTileID& tileID, Tile& tile) {
         addRenderTile(tileID, tile);
-        rendered.emplace(tileID);
         previouslyRenderedTiles.erase(tileID); // Still rendering this tile, no need for special fading logic.
         tile.markRenderedIdeal();
     };
@@ -201,7 +199,6 @@ void TilePyramid::update(const std::vector<Immutable<style::LayerProperties>>& l
             // Don't mark the tile "Required" to avoid triggering a new network request
             retainTileFn(tile, TileNecessity::Optional);
             addRenderTile(previouslyRenderedTile.first, tile);
-            rendered.emplace(previouslyRenderedTile.first);
         }
     }
 
@@ -242,7 +239,7 @@ void TilePyramid::update(const std::vector<Immutable<style::LayerProperties>>& l
     // Initialize render tiles fields and update the tile contained layer render data.
     for (RenderTile& renderTile : renderTiles) {
         Tile& tile = renderTile.tile;
-        if (!tile.isRenderable()) continue;
+        assert(tile.isRenderable());
 
         const bool holdForFade = tile.holdForFade();
         for (const auto& layerProperties : layers) {
@@ -392,6 +389,7 @@ void TilePyramid::clearAll() {
 }
 
 void TilePyramid::addRenderTile(const UnwrappedTileID& tileID, Tile& tile) {
+    assert(tile.isRenderable());
     auto it = std::lower_bound(renderTiles.begin(), renderTiles.end(), tileID,
         [](const RenderTile& a, const UnwrappedTileID& id) { return a.id < id; });
     renderTiles.emplace(it, tileID, tile);
diff --git a/src/mbgl/text/cross_tile_symbol_index.cpp b/src/mbgl/text/cross_tile_symbol_index.cpp
index 5170bfad75..3af583ebd3 100644
--- a/src/mbgl/text/cross_tile_symbol_index.cpp
+++ b/src/mbgl/text/cross_tile_symbol_index.cpp
@@ -174,9 +174,7 @@ bool CrossTileSymbolIndex::addLayer(const RenderLayerSymbolInterface& symbolInte
     layerIndex.handleWrapJump(lng);
 
     for (const RenderTile& renderTile : symbolInterface.getRenderTiles()) {
-        if (!renderTile.tile.isRenderable()) {
-            continue;
-        }
+        assert(renderTile.tile.isRenderable());
 
         auto bucket = symbolInterface.getSymbolBucket(renderTile);
         if (!bucket) {