[core] Check all bucket dynamic_castsupstream/fix-12432

A mismatch can occur when a layer changes from one type to another.

15 files changed, 122 insertions, 34 deletions

diff --git a/platform/android/CHANGELOG.md b/platform/android/CHANGELOG.md
index aaeb1f53a0..0216d91a58 100644
--- a/platform/android/CHANGELOG.md
+++ b/platform/android/CHANGELOG.md
@@ -4,6 +4,7 @@ Mapbox welcomes participation and contributions from everyone.  If you'd like to
 
 ## master
   - Token string syntax (`"{token}"`) in `SymbolLayer` `textField` and `iconImage` property values is now correctly converted to the appropriate expression equivalent. ([#11659](https://github.com/mapbox/mapbox-gl-native/issues/11659))
+  - Fixed a crash when switching between two styles having layers with the same identifier but different layer types. ([#12432](https://github.com/mapbox/mapbox-gl-native/issues/12432))
 
 ## 6.3.0 - July 18, 2018
  - Harden map events creation [#12406](https://github.com/mapbox/mapbox-gl-native/pull/12406)
diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md
index 9cc953bd1c..08fc156f91 100644
--- a/platform/ios/CHANGELOG.md
+++ b/platform/ios/CHANGELOG.md
@@ -9,6 +9,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT
 * Fixed a crash that occurred when the user started a gesture before the drift animation for a previous gesture was complete. ([#12148](https://github.com/mapbox/mapbox-gl-native/pull/12148))
 * Token string syntax (`"{token}"`) in `MGLSymbolStyleLayer` `text` and `iconImageName` properties is now correctly converted to the appropriate `NSExpression` equivalent. ([#11659](https://github.com/mapbox/mapbox-gl-native/issues/11659))
 * Added an `MGLMapView.locationManager` property and `MGLLocationManager` protocol for tracking user location using a custom alternative to `CLLocationManager`. ([#12013](https://github.com/mapbox/mapbox-gl-native/pull/12013))
+* Fixed a crash when switching between two styles having layers with the same identifier but different layer types. ([#12432](https://github.com/mapbox/mapbox-gl-native/issues/12432))
 
 
 ## 4.2.0 - July 18, 2018
diff --git a/platform/macos/CHANGELOG.md b/platform/macos/CHANGELOG.md
index 0386b78db0..001b3792d5 100644
--- a/platform/macos/CHANGELOG.md
+++ b/platform/macos/CHANGELOG.md
@@ -5,6 +5,7 @@
 ## Styles and rendering
 
 * Token string syntax (`"{token}"`) in `MGLSymbolStyleLayer` `text` and `iconImageName` properties is now correctly converted to the appropriate `NSExpression` equivalent. ([#11659](https://github.com/mapbox/mapbox-gl-native/issues/11659))
+* Fixed a crash when switching between two styles having layers with the same identifier but different layer types. ([#12432](https://github.com/mapbox/mapbox-gl-native/issues/12432))
 
 # 0.9.0 - July 18, 2018
 
diff --git a/src/mbgl/renderer/layers/render_circle_layer.cpp b/src/mbgl/renderer/layers/render_circle_layer.cpp
index b433a9d3fa..b1b4d22561 100644
--- a/src/mbgl/renderer/layers/render_circle_layer.cpp
+++ b/src/mbgl/renderer/layers/render_circle_layer.cpp
@@ -56,8 +56,11 @@ void RenderCircleLayer::render(PaintParameters& parameters, RenderSource*) {
     const bool pitchWithMap = evaluated.get<CirclePitchAlignment>() == AlignmentType::Map;
 
     for (const RenderTile& tile : renderTiles) {
-        assert(dynamic_cast<CircleBucket*>(tile.tile.getBucket(*baseImpl)));
-        CircleBucket& bucket = *reinterpret_cast<CircleBucket*>(tile.tile.getBucket(*baseImpl));
+        auto bucket_ = dynamic_cast<CircleBucket*>(tile.tile.getBucket(*baseImpl));
+        if (!bucket_) {
+            continue;
+        }
+        CircleBucket& bucket = *bucket_;
 
         const auto& paintPropertyBinders = bucket.paintPropertyBinders.at(getID());
 
diff --git a/src/mbgl/renderer/layers/render_fill_extrusion_layer.cpp b/src/mbgl/renderer/layers/render_fill_extrusion_layer.cpp
index d5282c9b0d..d37f6f528b 100644
--- a/src/mbgl/renderer/layers/render_fill_extrusion_layer.cpp
+++ b/src/mbgl/renderer/layers/render_fill_extrusion_layer.cpp
@@ -100,9 +100,11 @@ void RenderFillExtrusionLayer::render(PaintParameters& parameters, RenderSource*
 
         if (evaluated.get<FillExtrusionPattern>().from.empty()) {
             for (const RenderTile& tile : renderTiles) {
-                assert(dynamic_cast<FillExtrusionBucket*>(tile.tile.getBucket(*baseImpl)));
-                FillExtrusionBucket& bucket =
-                    *reinterpret_cast<FillExtrusionBucket*>(tile.tile.getBucket(*baseImpl));
+                auto bucket_ = dynamic_cast<FillExtrusionBucket*>(tile.tile.getBucket(*baseImpl));
+                if (!bucket_) {
+                    continue;
+                }
+                FillExtrusionBucket& bucket = *bucket_;
 
                 draw(
                     parameters.programs.fillExtrusion.get(evaluated),
@@ -129,9 +131,11 @@ void RenderFillExtrusionLayer::render(PaintParameters& parameters, RenderSource*
             parameters.imageManager.bind(parameters.context, 0);
 
             for (const RenderTile& tile : renderTiles) {
-                assert(dynamic_cast<FillExtrusionBucket*>(tile.tile.getBucket(*baseImpl)));
-                FillExtrusionBucket& bucket =
-                    *reinterpret_cast<FillExtrusionBucket*>(tile.tile.getBucket(*baseImpl));
+                auto bucket_ = dynamic_cast<FillExtrusionBucket*>(tile.tile.getBucket(*baseImpl));
+                if (!bucket_) {
+                    continue;
+                }
+                FillExtrusionBucket& bucket = *bucket_;
 
                 draw(
                     parameters.programs.fillExtrusionPattern.get(evaluated),
diff --git a/src/mbgl/renderer/layers/render_fill_layer.cpp b/src/mbgl/renderer/layers/render_fill_layer.cpp
index c59ca6f906..cce4399262 100644
--- a/src/mbgl/renderer/layers/render_fill_layer.cpp
+++ b/src/mbgl/renderer/layers/render_fill_layer.cpp
@@ -61,8 +61,11 @@ bool RenderFillLayer::hasTransition() const {
 void RenderFillLayer::render(PaintParameters& parameters, RenderSource*) {
     if (evaluated.get<FillPattern>().from.empty()) {
         for (const RenderTile& tile : renderTiles) {
-            assert(dynamic_cast<FillBucket*>(tile.tile.getBucket(*baseImpl)));
-            FillBucket& bucket = *reinterpret_cast<FillBucket*>(tile.tile.getBucket(*baseImpl));
+            auto bucket_ = dynamic_cast<FillBucket*>(tile.tile.getBucket(*baseImpl));
+            if (!bucket_) {
+                continue;
+            }
+            FillBucket& bucket = *bucket_;
 
             auto draw = [&] (auto& program,
                              const auto& drawMode,
@@ -146,8 +149,11 @@ void RenderFillLayer::render(PaintParameters& parameters, RenderSource*) {
         parameters.imageManager.bind(parameters.context, 0);
 
         for (const RenderTile& tile : renderTiles) {
-            assert(dynamic_cast<FillBucket*>(tile.tile.getBucket(*baseImpl)));
-            FillBucket& bucket = *reinterpret_cast<FillBucket*>(tile.tile.getBucket(*baseImpl));
+            auto bucket_ = dynamic_cast<FillBucket*>(tile.tile.getBucket(*baseImpl));
+            if (!bucket_) {
+                continue;
+            }
+            FillBucket& bucket = *bucket_;
 
             auto draw = [&] (auto& program,
                              const auto& drawMode,
diff --git a/src/mbgl/renderer/layers/render_heatmap_layer.cpp b/src/mbgl/renderer/layers/render_heatmap_layer.cpp
index dec9edb318..d2c6f6d970 100644
--- a/src/mbgl/renderer/layers/render_heatmap_layer.cpp
+++ b/src/mbgl/renderer/layers/render_heatmap_layer.cpp
@@ -83,8 +83,11 @@ void RenderHeatmapLayer::render(PaintParameters& parameters, RenderSource*) {
         parameters.context.clear(Color{ 0.0f, 0.0f, 0.0f, 1.0f }, {}, {});
 
         for (const RenderTile& tile : renderTiles) {
-            assert(dynamic_cast<HeatmapBucket*>(tile.tile.getBucket(*baseImpl)));
-            HeatmapBucket& bucket = *reinterpret_cast<HeatmapBucket*>(tile.tile.getBucket(*baseImpl));
+            auto bucket_ = dynamic_cast<HeatmapBucket*>(tile.tile.getBucket(*baseImpl));
+            if (!bucket_) {
+                continue;
+            }
+            HeatmapBucket& bucket = *bucket_;
 
             const auto extrudeScale = tile.id.pixelsToTileUnits(1, parameters.state.getZoom());
 
diff --git a/src/mbgl/renderer/layers/render_hillshade_layer.cpp b/src/mbgl/renderer/layers/render_hillshade_layer.cpp
index 411305edf4..0edc8737c1 100644
--- a/src/mbgl/renderer/layers/render_hillshade_layer.cpp
+++ b/src/mbgl/renderer/layers/render_hillshade_layer.cpp
@@ -114,8 +114,12 @@ void RenderHillshadeLayer::render(PaintParameters& parameters, RenderSource* src
     matrix::translate(mat, mat, 0, -util::EXTENT, 0);
 
     for (const RenderTile& tile : renderTiles) {
-        assert(dynamic_cast<HillshadeBucket*>(tile.tile.getBucket(*baseImpl)));
-        HillshadeBucket& bucket = *reinterpret_cast<HillshadeBucket*>(tile.tile.getBucket(*baseImpl));
+        auto bucket_ = dynamic_cast<HillshadeBucket*>(tile.tile.getBucket(*baseImpl));
+        if (!bucket_) {
+            continue;
+        }
+        HillshadeBucket& bucket = *bucket_;
+
         if (!bucket.hasData()){
             continue;
         }
diff --git a/src/mbgl/renderer/layers/render_line_layer.cpp b/src/mbgl/renderer/layers/render_line_layer.cpp
index 361ad0c76b..ea51705792 100644
--- a/src/mbgl/renderer/layers/render_line_layer.cpp
+++ b/src/mbgl/renderer/layers/render_line_layer.cpp
@@ -58,8 +58,11 @@ void RenderLineLayer::render(PaintParameters& parameters, RenderSource*) {
     }
 
     for (const RenderTile& tile : renderTiles) {
-        assert(dynamic_cast<LineBucket*>(tile.tile.getBucket(*baseImpl)));
-        LineBucket& bucket = *reinterpret_cast<LineBucket*>(tile.tile.getBucket(*baseImpl));
+        auto bucket_ = dynamic_cast<LineBucket*>(tile.tile.getBucket(*baseImpl));
+        if (!bucket_) {
+            continue;
+        }
+        LineBucket& bucket = *bucket_;
 
         auto draw = [&] (auto& program, auto&& uniformValues) {
             auto& programInstance = program.get(evaluated);
diff --git a/src/mbgl/renderer/layers/render_raster_layer.cpp b/src/mbgl/renderer/layers/render_raster_layer.cpp
index f31f53481f..13f2f5e38c 100644
--- a/src/mbgl/renderer/layers/render_raster_layer.cpp
+++ b/src/mbgl/renderer/layers/render_raster_layer.cpp
@@ -142,8 +142,11 @@ void RenderRasterLayer::render(PaintParameters& parameters, RenderSource* source
         }
     } else {
         for (const RenderTile& tile : renderTiles) {
-            assert(dynamic_cast<RasterBucket*>(tile.tile.getBucket(*baseImpl)));
-            RasterBucket& bucket = *reinterpret_cast<RasterBucket*>(tile.tile.getBucket(*baseImpl));
+            auto bucket_ = dynamic_cast<RasterBucket*>(tile.tile.getBucket(*baseImpl));
+            if (!bucket_) {
+                continue;
+            }
+            RasterBucket& bucket = *bucket_;
 
             if (!bucket.hasData())
                 continue;
diff --git a/src/mbgl/renderer/layers/render_symbol_layer.cpp b/src/mbgl/renderer/layers/render_symbol_layer.cpp
index 861112526b..d397ffd724 100644
--- a/src/mbgl/renderer/layers/render_symbol_layer.cpp
+++ b/src/mbgl/renderer/layers/render_symbol_layer.cpp
@@ -75,8 +75,11 @@ void RenderSymbolLayer::render(PaintParameters& parameters, RenderSource*) {
     }
 
     for (const RenderTile& tile : renderTiles) {
-        assert(dynamic_cast<SymbolBucket*>(tile.tile.getBucket(*baseImpl)));
-        SymbolBucket& bucket = *reinterpret_cast<SymbolBucket*>(tile.tile.getBucket(*baseImpl));
+        auto bucket_ = dynamic_cast<SymbolBucket*>(tile.tile.getBucket(*baseImpl));
+        if (!bucket_) {
+            continue;
+        }
+        SymbolBucket& bucket = *bucket_;
 
         const auto& layout = bucket.layout;
 
diff --git a/src/mbgl/text/cross_tile_symbol_index.cpp b/src/mbgl/text/cross_tile_symbol_index.cpp
index 8df26ca117..58dcda46f5 100644
--- a/src/mbgl/text/cross_tile_symbol_index.cpp
+++ b/src/mbgl/text/cross_tile_symbol_index.cpp
@@ -178,9 +178,12 @@ bool CrossTileSymbolIndex::addLayer(RenderSymbolLayer& symbolLayer, float lng) {
             continue;
         }
 
-        auto bucket = renderTile.tile.getBucket(*symbolLayer.baseImpl);
-        assert(dynamic_cast<SymbolBucket*>(bucket));
-        SymbolBucket& symbolBucket = *reinterpret_cast<SymbolBucket*>(bucket);
+        auto bucket = dynamic_cast<SymbolBucket*>(renderTile.tile.getBucket(*symbolLayer.baseImpl));
+        if (!bucket) {
+            continue;
+        }
+        SymbolBucket& symbolBucket = *bucket;
+
         if (symbolBucket.bucketLeaderID != symbolLayer.getID()) {
             // Only add this layer if it's the "group leader" for the bucket
             continue;
diff --git a/src/mbgl/text/placement.cpp b/src/mbgl/text/placement.cpp
index fd0710d959..96c1873e33 100644
--- a/src/mbgl/text/placement.cpp
+++ b/src/mbgl/text/placement.cpp
@@ -50,12 +50,13 @@ void Placement::placeLayer(RenderSymbolLayer& symbolLayer, const mat4& projMatri
         }
         assert(dynamic_cast<GeometryTile*>(&renderTile.tile));
         GeometryTile& geometryTile = static_cast<GeometryTile&>(renderTile.tile);
-        
-        
-        auto bucket = geometryTile.getBucket(*symbolLayer.baseImpl);
-        assert(dynamic_cast<SymbolBucket*>(bucket));
-        SymbolBucket& symbolBucket = *reinterpret_cast<SymbolBucket*>(bucket);
-        
+
+        auto bucket = dynamic_cast<SymbolBucket*>(geometryTile.getBucket(*symbolLayer.baseImpl));
+        if (!bucket) {
+            continue;
+        }
+        SymbolBucket& symbolBucket = *bucket;
+
         if (symbolBucket.bucketLeaderID != symbolLayer.getID()) {
             // Only place this layer if it's the "group leader" for the bucket
             continue;
@@ -231,9 +232,12 @@ void Placement::updateLayerOpacities(RenderSymbolLayer& symbolLayer) {
             continue;
         }
 
-        auto bucket = renderTile.tile.getBucket(*symbolLayer.baseImpl);
-        assert(dynamic_cast<SymbolBucket*>(bucket));
-        SymbolBucket& symbolBucket = *reinterpret_cast<SymbolBucket*>(bucket);
+        auto bucket = dynamic_cast<SymbolBucket*>(renderTile.tile.getBucket(*symbolLayer.baseImpl));
+        if (!bucket) {
+            continue;
+        }
+        SymbolBucket& symbolBucket = *bucket;
+
         if (symbolBucket.bucketLeaderID != symbolLayer.getID()) {
             // Only update opacities this layer if it's the "group leader" for the bucket
             continue;
diff --git a/test/fixtures/map/issue12432/0-0-0.mvt b/test/fixtures/map/issue12432/0-0-0.mvt
new file mode 100644
index 0000000000..a0f049ad43
--- /dev/null
+++ b/test/fixtures/map/issue12432/0-0-0.mvt
diff --git a/test/map/map.test.cpp b/test/map/map.test.cpp
index 4f7bd2df7b..8e2d9cb9cd 100644
--- a/test/map/map.test.cpp
+++ b/test/map/map.test.cpp
@@ -653,3 +653,52 @@ TEST(Map, NoContentTiles) {
                      0.0015,
                      0.1);
 }
+
+// https://github.com/mapbox/mapbox-gl-native/issues/12432
+TEST(Map, Issue12432) {
+    MapTest<> test { 1, MapMode::Continuous };
+
+    test.fileSource.tileResponse = [&](const Resource&) {
+        Response result;
+        result.data = std::make_shared<std::string>(util::read_file("test/fixtures/map/issue12432/0-0-0.mvt"));
+        return result;
+    };
+
+    test.map.getStyle().loadJSON(R"STYLE({
+      "version": 8,
+      "sources": {
+        "mapbox": {
+          "type": "vector",
+          "tiles": ["http://example.com/{z}-{x}-{y}.vector.pbf"]
+        }
+      },
+      "layers": [{
+        "id": "water",
+        "type": "fill",
+        "source": "mapbox",
+        "source-layer": "water"
+      }]
+    })STYLE");
+
+    test.observer.didFinishLoadingMapCallback = [&]() {
+        test.map.getStyle().loadJSON(R"STYLE({
+          "version": 8,
+          "sources": {
+            "mapbox": {
+              "type": "vector",
+              "tiles": ["http://example.com/{z}-{x}-{y}.vector.pbf"]
+            }
+          },
+          "layers": [{
+            "id": "water",
+            "type": "line",
+            "source": "mapbox",
+            "source-layer": "water"
+          }]
+        })STYLE");
+
+        test.runLoop.stop();
+    };
+
+    test.runLoop.run();
+}