WIP: Don't replace a fully loaded tile with its parent or child until symbols are loaded, to prevent flicker.upstream/start-collision-one-phase

When a tile isn't replacing another tile (e.g. on pan operations), start rendering non-symbol layers before symbols finish loading.

Turned off stencil clipping to get tile combinations working.
Turned off fade animations just to make debugging easier.

9 files changed, 169 insertions, 97 deletions

diff --git a/src/mbgl/algorithm/update_renderables.hpp b/src/mbgl/algorithm/update_renderables.hpp
index c583b6b2b6..cf1ad8af65 100644
--- a/src/mbgl/algorithm/update_renderables.hpp
+++ b/src/mbgl/algorithm/update_renderables.hpp
@@ -1,127 +1,176 @@
 #pragma once
 
+#include <mbgl/tile/tile.hpp>
 #include <mbgl/tile/tile_id.hpp>
 #include <mbgl/tile/tile_necessity.hpp>
 #include <mbgl/util/range.hpp>
 
 #include <unordered_set>
 
+#include <iostream>
+
 namespace mbgl {
 namespace algorithm {
 
+
 template <typename GetTileFn,
           typename CreateTileFn,
           typename RetainTileFn,
-          typename RenderTileFn,
-          typename IdealTileIDs>
-void updateRenderables(GetTileFn getTile,
+          typename RenderTileFn>
+bool findBestTile(GetTileFn getTile,
                        CreateTileFn createTile,
                        RetainTileFn retainTile,
                        RenderTileFn renderTile,
-                       const IdealTileIDs& idealTileIDs,
                        const Range<uint8_t>& zoomRange,
-                       const uint8_t dataTileZoom) {
-    std::unordered_set<UnwrappedTileID> checked;
-    bool covered;
-    int32_t overscaledZ;
+                       const uint8_t dataTileZoom,
+                       const UnwrappedTileID& idealRenderTileID,
+                       const OverscaledTileID& idealDataTileID,
+                       const bool requireFullyRenderable,
+                       std::unordered_set<UnwrappedTileID> checked) {
 
-    // for (all in the set of ideal tiles of the source) {
-    for (const auto& idealRenderTileID : idealTileIDs) {
-        assert(idealRenderTileID.canonical.z >= zoomRange.min);
-        assert(idealRenderTileID.canonical.z <= zoomRange.max);
-        assert(dataTileZoom >= idealRenderTileID.canonical.z);
+    auto tile = getTile(idealDataTileID);
+    if (!tile) {
+        std::cout << "Create tile: " << idealDataTileID << std::endl;
+        tile = createTile(idealDataTileID);
+        assert(tile);
+    }
 
-        const OverscaledTileID idealDataTileID(dataTileZoom, idealRenderTileID.wrap, idealRenderTileID.canonical);
-        auto tile = getTile(idealDataTileID);
-        if (!tile) {
-            tile = createTile(idealDataTileID);
-            assert(tile);
-        }
+    bool covered = true;
+    std::vector<std::pair<UnwrappedTileID,Tile*>> renderTiles;
+    // if (source has the tile and bucket is loaded) {
+    if (requireFullyRenderable ? tile->isFullyRenderable() : tile->isRenderable()) {
+        retainTile(*tile, TileNecessity::Required);
+        renderTiles.emplace_back(std::make_pair(idealRenderTileID, tile));
+    } else {
+        // We are now attempting to load child and parent tiles.
+        bool parentHasTriedOptional = tile->hasTriedCache();
+        bool parentIsLoaded = tile->isLoaded();
+
+        // The tile isn't loaded yet, but retain it anyway because it's an ideal tile.
+        retainTile(*tile, TileNecessity::Required); // TODO: Is it fine to retain this twice for both full and partial render?
 
-        // if (source has the tile and bucket is loaded) {
-        if (tile->isRenderable()) {
-            retainTile(*tile, TileNecessity::Required);
-            renderTile(idealRenderTileID, *tile);
+        
+        int32_t overscaledZ = dataTileZoom + 1;
+        if (overscaledZ > zoomRange.max) {
+            // We're looking for an overzoomed child tile.
+            const auto childDataTileID = idealDataTileID.scaledTo(overscaledZ);
+            tile = getTile(childDataTileID);
+            if (tile && (requireFullyRenderable ? tile->isFullyRenderable() : tile->isRenderable())) {
+                retainTile(*tile, TileNecessity::Optional);
+                renderTiles.emplace_back(std::make_pair(idealRenderTileID, tile));
+            } else {
+                covered = false;
+            }
         } else {
-            // We are now attempting to load child and parent tiles.
-            bool parentHasTriedOptional = tile->hasTriedCache();
-            bool parentIsLoaded = tile->isLoaded();
-
-            // The tile isn't loaded yet, but retain it anyway because it's an ideal tile.
-            retainTile(*tile, TileNecessity::Required);
-            covered = true;
-            overscaledZ = dataTileZoom + 1;
-            if (overscaledZ > zoomRange.max) {
-                // We're looking for an overzoomed child tile.
-                const auto childDataTileID = idealDataTileID.scaledTo(overscaledZ);
+            // Check all four actual child tiles.
+            for (const auto& childTileID : idealDataTileID.canonical.children()) {
+                const OverscaledTileID childDataTileID(overscaledZ, idealRenderTileID.wrap, childTileID);
                 tile = getTile(childDataTileID);
-                if (tile && tile->isRenderable()) {
+                if (tile && (requireFullyRenderable ? tile->isFullyRenderable() : tile->isRenderable())) {
                     retainTile(*tile, TileNecessity::Optional);
-                    renderTile(idealRenderTileID, *tile);
+                    renderTiles.emplace_back(std::make_pair(childDataTileID.toUnwrapped(), tile));
                 } else {
+                    // At least one child tile doesn't exist, so we are going to look for
+                    // parents as well.
                     covered = false;
                 }
-            } else {
-                // Check all four actual child tiles.
-                for (const auto& childTileID : idealDataTileID.canonical.children()) {
-                    const OverscaledTileID childDataTileID(overscaledZ, idealRenderTileID.wrap, childTileID);
-                    tile = getTile(childDataTileID);
-                    if (tile && tile->isRenderable()) {
-                        retainTile(*tile, TileNecessity::Optional);
-                        renderTile(childDataTileID.toUnwrapped(), *tile);
-                    } else {
-                        // At least one child tile doesn't exist, so we are going to look for
-                        // parents as well.
-                        covered = false;
-                    }
-                }
             }
+        }
 
-            if (!covered) {
-                // 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 (!covered) {
+            // 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()) {
-                        // Break parent tile ascent, this route has been checked by another child
-                        // tile before.
-                        break;
+                if (checked.find(parentRenderTileID) != checked.end()) {
+                    // Break parent tile ascent, this route has been checked by another child
+                    // tile before.
+                    break;
+                } else {
+                    checked.emplace(parentRenderTileID);
+                }
+
+                tile = getTile(parentDataTileID);
+                // Don't do cache lookups for parents while we're in the "fully renderable" pass:
+                // Since the cache tiles aren't currently displaying, we don't have to worry about symbol flicker
+                // And we'd rather get a partially rendered tile closer to our ideal zoom level
+                if (!tile && !requireFullyRenderable && (parentHasTriedOptional || parentIsLoaded)) {
+                    tile = createTile(parentDataTileID);
+                }
+
+                if (tile) {
+                    if (!parentIsLoaded) {
+                        // We haven't completed loading the child, so we only do an optional
+                        // (cache) request in an attempt to quickly load data that we can show.
+                        retainTile(*tile, TileNecessity::Optional);
                     } else {
-                        checked.emplace(parentRenderTileID);
+                        // Now that we've checked the child and know for sure that we can't load
+                        // it, we attempt to load the parent from the network.
+                        retainTile(*tile, TileNecessity::Required);
                     }
 
-                    tile = getTile(parentDataTileID);
-                    if (!tile && (parentHasTriedOptional || parentIsLoaded)) {
-                        tile = createTile(parentDataTileID);
-                    }
+                    // Save the current values, since they're the parent of the next iteration
+                    // of the parent tile ascent loop.
+                    parentHasTriedOptional = tile->hasTriedCache();
+                    parentIsLoaded = tile->isLoaded();
 
-                    if (tile) {
-                        if (!parentIsLoaded) {
-                            // We haven't completed loading the child, so we only do an optional
-                            // (cache) request in an attempt to quickly load data that we can show.
-                            retainTile(*tile, TileNecessity::Optional);
-                        } else {
-                            // Now that we've checked the child and know for sure that we can't load
-                            // it, we attempt to load the parent from the network.
-                            retainTile(*tile, TileNecessity::Required);
-                        }
-
-                        // Save the current values, since they're the parent of the next iteration
-                        // of the parent tile ascent loop.
-                        parentHasTriedOptional = tile->hasTriedCache();
-                        parentIsLoaded = tile->isLoaded();
-
-                        if (tile->isRenderable()) {
-                            renderTile(parentRenderTileID, *tile);
-                            // Break parent tile ascent, since we found one.
-                            break;
-                        }
+                    if ((requireFullyRenderable ? tile->isFullyRenderable() : tile->isRenderable())) {
+                        std::cout << "Rendering parent zoom change: " << parentRenderTileID.canonical.z - idealRenderTileID.canonical.z << std::endl;
+                        renderTiles.emplace_back(std::make_pair(parentRenderTileID, tile));
+                        covered = true;
+                        // Break parent tile ascent, since we found one.
+                        break;
                     }
                 }
             }
         }
     }
+    if (covered || !requireFullyRenderable) {
+        // Only add tiles to render tree if we've covered the ideal tile OR we're on our second pass
+        for (auto tilePair : renderTiles) {
+          //  std::cout << "Rendering tile: " << tilePair.first << std::endl;
+            renderTile(tilePair.first, *(tilePair.second));
+        }
+    } else {
+        std::cout << "No tiles added to render tree" << std::endl;
+    }
+    return covered;
+}
+
+template <typename GetTileFn,
+          typename CreateTileFn,
+          typename RetainTileFn,
+          typename RenderTileFn,
+          typename IdealTileIDs>
+void updateRenderables(GetTileFn getTile,
+                       CreateTileFn createTile,
+                       RetainTileFn retainTile,
+                       RenderTileFn renderTile,
+                       const IdealTileIDs& idealTileIDs,
+                       const Range<uint8_t>& zoomRange,
+                       const uint8_t dataTileZoom) {
+    std::unordered_set<UnwrappedTileID> checkedPartial;
+    std::unordered_set<UnwrappedTileID> checkedFull;
+
+    std::cout << "Begin updateRenderables" << std::endl;
+
+    // for (all in the set of ideal tiles of the source) {
+    for (const auto& idealRenderTileID : idealTileIDs) {
+        assert(idealRenderTileID.canonical.z >= zoomRange.min);
+        assert(idealRenderTileID.canonical.z <= zoomRange.max);
+        assert(dataTileZoom >= idealRenderTileID.canonical.z);
+
+        const OverscaledTileID idealDataTileID(dataTileZoom, idealRenderTileID.wrap, idealRenderTileID.canonical);
+        
+        std::cout << "Ideal tile ID: " << idealDataTileID << std::endl;
+
+        // TODO: Two-pass algorithm isn't necessary for tiles that don't have two stage loading (e.g. raster tiles)
+        // TODO: More efficient to write algorithm as single pass?
+        if (!findBestTile(getTile, createTile, retainTile, renderTile, zoomRange, dataTileZoom, idealRenderTileID, idealDataTileID, true, checkedFull)) {
+            findBestTile(getTile, createTile, retainTile, renderTile, zoomRange, dataTileZoom, idealRenderTileID, idealDataTileID, false, checkedPartial);
+        }
+    }
 }
 
 } // namespace algorithm
diff --git a/src/mbgl/renderer/paint_parameters.cpp b/src/mbgl/renderer/paint_parameters.cpp
index beaa9d34f2..ce538b2683 100644
--- a/src/mbgl/renderer/paint_parameters.cpp
+++ b/src/mbgl/renderer/paint_parameters.cpp
@@ -26,7 +26,7 @@ PaintParameters::PaintParameters(gl::Context& context_,
     debugOptions(updateParameters.debugOptions),
     contextMode(contextMode_),
     timePoint(updateParameters.timePoint),
-    symbolFadeChange(updateParameters.mode == MapMode::Still ? 1 : std::chrono::duration<float>(placementCommitTime - updateParameters.timePoint) / Duration(std::chrono::milliseconds(300))), // TODO don't hardcode
+    symbolFadeChange((true || updateParameters.mode == MapMode::Still) ? 1 : std::chrono::duration<float>(placementCommitTime - updateParameters.timePoint) / Duration(std::chrono::milliseconds(300))), // TODO don't hardcode
     pixelRatio(pixelRatio_),
 #ifndef NDEBUG
     programs((debugOptions & MapDebugOptions::Overdraw) ? staticData_.overdrawPrograms : staticData_.programs)
@@ -66,15 +66,16 @@ gl::DepthMode PaintParameters::depthModeFor3D(gl::DepthMode::Mask mask) const {
     return gl::DepthMode { gl::DepthMode::LessEqual, mask, { 0.0, 1.0 } };
 }
 
-gl::StencilMode PaintParameters::stencilModeForClipping(const ClipID& id) const {
-    return gl::StencilMode {
-        gl::StencilMode::Equal { static_cast<uint32_t>(id.mask.to_ulong()) },
-        static_cast<int32_t>(id.reference.to_ulong()),
-        0,
-        gl::StencilMode::Keep,
-        gl::StencilMode::Keep,
-        gl::StencilMode::Replace
-    };
+gl::StencilMode PaintParameters::stencilModeForClipping(const ClipID& ) const {
+    return gl::StencilMode::disabled();
+//    return gl::StencilMode {
+//        gl::StencilMode::Equal { static_cast<uint32_t>(id.mask.to_ulong()) },
+//        static_cast<int32_t>(id.reference.to_ulong()),
+//        0,
+//        gl::StencilMode::Keep,
+//        gl::StencilMode::Keep,
+//        gl::StencilMode::Replace
+//    };
 }
 
 gl::ColorMode PaintParameters::colorModeForRenderPass() const {
diff --git a/src/mbgl/text/placement.cpp b/src/mbgl/text/placement.cpp
index 8fdc26f49a..08d710278e 100644
--- a/src/mbgl/text/placement.cpp
+++ b/src/mbgl/text/placement.cpp
@@ -283,7 +283,7 @@ JointOpacityState Placement::getOpacity(uint32_t crossTileSymbolID) const {
 }
 
 float Placement::symbolFadeChange(TimePoint now) const {
-    if (mapMode == MapMode::Still) {
+    if (true || mapMode == MapMode::Still) {
         return 1.0;
     }
     return std::chrono::duration<float>(now - commitTime) / Duration(std::chrono::milliseconds(300));
diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp
index 6371cbdc0b..ac545b8805 100644
--- a/src/mbgl/tile/geometry_tile.cpp
+++ b/src/mbgl/tile/geometry_tile.cpp
@@ -59,7 +59,8 @@ GeometryTile::GeometryTile(const OverscaledTileID& id_,
       glyphManager(parameters.glyphManager),
       imageManager(parameters.imageManager),
       mode(parameters.mode),
-      showCollisionBoxes(parameters.debugOptions & MapDebugOptions::Collision) {
+      showCollisionBoxes(parameters.debugOptions & MapDebugOptions::Collision),
+      fullyRenderable(false) {
 }
 
 GeometryTile::~GeometryTile() {
@@ -68,6 +69,10 @@ GeometryTile::~GeometryTile() {
     markObsolete();
 }
 
+bool GeometryTile::isFullyRenderable() const {
+    return fullyRenderable;
+}
+
 void GeometryTile::cancel() {
     markObsolete();
 }
@@ -127,6 +132,7 @@ void GeometryTile::setShowCollisionBoxes(const bool showCollisionBoxes_) {
 void GeometryTile::onLayout(LayoutResult result, const uint64_t resultCorrelationID) {
     loaded = true;
     renderable = true;
+    fullyRenderable = false;
     (void)resultCorrelationID;
     nonSymbolBuckets = std::move(result.nonSymbolBuckets);
     featureIndex = std::move(result.featureIndex);
@@ -137,6 +143,7 @@ void GeometryTile::onLayout(LayoutResult result, const uint64_t resultCorrelatio
 void GeometryTile::onPlacement(PlacementResult result, const uint64_t resultCorrelationID) {
     loaded = true;
     renderable = true;
+    fullyRenderable = true;
     if (resultCorrelationID == correlationID) {
         pending = false;
     }
diff --git a/src/mbgl/tile/geometry_tile.hpp b/src/mbgl/tile/geometry_tile.hpp
index 80f2f8c6d7..4362408755 100644
--- a/src/mbgl/tile/geometry_tile.hpp
+++ b/src/mbgl/tile/geometry_tile.hpp
@@ -31,6 +31,8 @@ public:
                  const TileParameters&);
 
     ~GeometryTile() override;
+    
+    bool isFullyRenderable() const override;
 
     void setError(std::exception_ptr);
     void setData(std::unique_ptr<const GeometryTileData>);
@@ -130,6 +132,7 @@ private:
     const MapMode mode;
     
     bool showCollisionBoxes;
+    bool fullyRenderable;
 
 public:
     optional<gl::Texture> glyphAtlasTexture;
diff --git a/src/mbgl/tile/geometry_tile_worker.cpp b/src/mbgl/tile/geometry_tile_worker.cpp
index 969b137c1f..55d3457c3a 100644
--- a/src/mbgl/tile/geometry_tile_worker.cpp
+++ b/src/mbgl/tile/geometry_tile_worker.cpp
@@ -418,6 +418,8 @@ void GeometryTileWorker::attemptPlacement() {
             buckets.emplace(pair.first, bucket);
         }
     }
+    
+    //std::this_thread::sleep_for(Milliseconds(500));
 
     parent.invoke(&GeometryTile::onPlacement, GeometryTile::PlacementResult {
         std::move(buckets),
diff --git a/src/mbgl/tile/tile.hpp b/src/mbgl/tile/tile.hpp
index 06d2a93644..f2fe9c70b2 100644
--- a/src/mbgl/tile/tile.hpp
+++ b/src/mbgl/tile/tile.hpp
@@ -83,6 +83,10 @@ public:
     bool isRenderable() const {
         return renderable;
     }
+    
+    virtual bool isFullyRenderable() const {
+        return renderable;
+    }
 
     // A tile is "Loaded" when we have received a response from a FileSource, and have attempted to
     // parse the tile (if applicable). Tile implementations should set this to true when a load
diff --git a/src/mbgl/tile/tile_cache.cpp b/src/mbgl/tile/tile_cache.cpp
index 3fafb1259c..ed0ef9ae87 100644
--- a/src/mbgl/tile/tile_cache.cpp
+++ b/src/mbgl/tile/tile_cache.cpp
@@ -52,6 +52,7 @@ std::unique_ptr<Tile> TileCache::get(const OverscaledTileID& key) {
     }
 
     return tile;
+    //return std::unique_ptr<Tile>();
 }
 
 bool TileCache::has(const OverscaledTileID& key) {
diff --git a/test/algorithm/mock.hpp b/test/algorithm/mock.hpp
index b8eb020105..dea2162b04 100644
--- a/test/algorithm/mock.hpp
+++ b/test/algorithm/mock.hpp
@@ -33,12 +33,17 @@ struct MockTileData {
     bool isRenderable() const {
         return renderable;
     }
+    
+    bool isFullyRenderable() const {
+        return fullyRenderable;
+    }
 
     bool isLoaded() const {
         return loaded;
     }
 
     bool renderable = false;
+    bool fullyRenderable = false;
     bool triedOptional = false;
     bool loaded = false;
     const mbgl::OverscaledTileID tileID;