From 7758cd88e17980b6dd17bf68507b07311567870b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= <mail@kkaefer.com>
Date: Thu, 20 Sep 2018 12:07:02 +0200
Subject: [PATCH] [core] don't break ascent in the overscaled tile phase

We optimize our updateRenderable algorithm by breaking ascent when we've already checked a certain tile. So far, we've compared the UnwrappedTileIDs, but they don't include the overscale component. When ascending through overscaled tile IDs, we've stopped the ascent too early, when we should've kept the search going.
---
 src/mbgl/algorithm/update_renderables.hpp  |  9 ++--
 test/algorithm/update_renderables.test.cpp | 50 ++++++++++++++++++++++
 2 files changed, 54 insertions(+), 5 deletions(-)

diff --git a/src/mbgl/algorithm/update_renderables.hpp b/src/mbgl/algorithm/update_renderables.hpp
index 5fbe0d943f1..57cd4f41ec8 100644
--- a/src/mbgl/algorithm/update_renderables.hpp
+++ b/src/mbgl/algorithm/update_renderables.hpp
@@ -21,7 +21,7 @@ void updateRenderables(GetTileFn getTile,
                        const IdealTileIDs& idealTileIDs,
                        const Range<uint8_t>& zoomRange,
                        const uint8_t dataTileZoom) {
-    std::unordered_set<UnwrappedTileID> checked;
+    std::unordered_set<OverscaledTileID> checked;
     bool covered;
     int32_t overscaledZ;
 
@@ -85,14 +85,13 @@ void updateRenderables(GetTileFn getTile,
                 // We couldn't find child tiles that entirely cover the ideal tile.
                 for (overscaledZ = dataTileZoom - 1; overscaledZ >= zoomRange.min; --overscaledZ) {
                     const auto parentDataTileID = idealDataTileID.scaledTo(overscaledZ);
-                    const auto parentRenderTileID = parentDataTileID.toUnwrapped();
 
-                    if (checked.find(parentRenderTileID) != checked.end()) {
+                    if (checked.find(parentDataTileID) != checked.end()) {
                         // Break parent tile ascent, this route has been checked by another child
                         // tile before.
                         break;
                     } else {
-                        checked.emplace(parentRenderTileID);
+                        checked.emplace(parentDataTileID);
                     }
 
                     tile = getTile(parentDataTileID);
@@ -117,7 +116,7 @@ void updateRenderables(GetTileFn getTile,
                         parentIsLoaded = tile->isLoaded();
 
                         if (tile->isRenderable()) {
-                            renderTile(parentRenderTileID, *tile);
+                            renderTile(parentDataTileID.toUnwrapped(), *tile);
                             // Break parent tile ascent, since we found one.
                             break;
                         }
diff --git a/test/algorithm/update_renderables.test.cpp b/test/algorithm/update_renderables.test.cpp
index 7d6a0fcb130..e142758f293 100644
--- a/test/algorithm/update_renderables.test.cpp
+++ b/test/algorithm/update_renderables.test.cpp
@@ -1259,3 +1259,53 @@ TEST(UpdateRenderables, LoadRequiredIfIdealTileCantBeFound) {
               }),
               log);
 }
+
+// Tests overzooming a 0/0/0 tile to zoom level 4, when the maxzoom is 2.
+TEST(UpdateRenderables, LoadOverscaledMaxZoomTile) {
+    ActionLog log;
+    MockSource source;
+    auto getTileData = getTileDataFn(log, source.dataTiles);
+    auto createTileData = createTileDataFn(log, source.dataTiles);
+    auto retainTileData = retainTileDataFn(log);
+    auto renderTile = renderTileFn(log);
+
+    source.zoomRange.max = 2;
+    source.idealTiles.emplace(UnwrappedTileID{ 2, 0, 0 });
+
+    auto tile_4_2_0_0 = source.createTileData(OverscaledTileID{ 4, 0, { 2, 0, 0 } });
+    tile_4_2_0_0->renderable = false;
+    tile_4_2_0_0->triedOptional = true;
+    tile_4_2_0_0->loaded = true;
+
+    auto tile_3_2_0_0 = source.createTileData(OverscaledTileID{ 3, 0, { 2, 0, 0 } });
+    tile_3_2_0_0->renderable = false;
+    tile_3_2_0_0->triedOptional = true;
+    tile_3_2_0_0->loaded = true;
+
+    auto tile_2_2_0_0 = source.createTileData(OverscaledTileID{ 2, 0, { 2, 0, 0 } });
+    tile_2_2_0_0->renderable = false;
+    tile_2_2_0_0->triedOptional = true;
+    tile_2_2_0_0->loaded = true;
+
+    // Tile level 1 won't be overscaled.
+    auto tile_1_1_0_0 = source.createTileData(OverscaledTileID{ 1, 0, { 1, 0, 0 } });
+    tile_1_1_0_0->renderable = true;
+    tile_1_1_0_0->triedOptional = true;
+    tile_1_1_0_0->loaded = true;
+
+    algorithm::updateRenderables(getTileData, createTileData, retainTileData, renderTile,
+                                 source.idealTiles, source.zoomRange, 4);
+    EXPECT_EQ(ActionLog({
+                  GetTileDataAction{ { 4, 0, { 2, 0, 0 } }, Found },
+                  RetainTileDataAction{ { 4, 0, { 2, 0, 0 } }, TileNecessity::Required },
+                  GetTileDataAction{ { 5, 0, { 2, 0, 0 } }, NotFound },
+                  GetTileDataAction{ { 3, 0, { 2, 0, 0 } }, Found },
+                  RetainTileDataAction{ { 3, 0, { 2, 0, 0 } }, TileNecessity::Required },
+                  GetTileDataAction{ { 2, 0, { 2, 0, 0 } }, Found },
+                  RetainTileDataAction{ { 2, 0, { 2, 0, 0 } }, TileNecessity::Required },
+                  GetTileDataAction{ { 1, 0, { 1, 0, 0 } }, Found },
+                  RetainTileDataAction{ { 1, 0, { 1, 0, 0 } }, TileNecessity::Required },
+                  RenderTileAction{ { 1, 0, 0 }, *tile_1_1_0_0 },
+              }),
+              log);
+}