Fixes from code review and update gl-js pin to include render test

mapbox · Jan 4, 2018 · ef7b673 · ef7b673
1 parent ef39e8c
commit ef7b673
Show file tree

Hide file tree

Showing 7 changed files with 15 additions and 10 deletions.
diff --git a/platform/node/test/ignores.json b/platform/node/test/ignores.json
@@ -85,7 +85,6 @@
   "render-tests/text-pitch-alignment/map-text-rotation-alignment-map": "https://github.com/mapbox/mapbox-gl-native/issues/9732",
   "render-tests/text-pitch-alignment/viewport-text-rotation-alignment-map": "https://github.com/mapbox/mapbox-gl-native/issues/9732",
   "render-tests/text-pitch-scaling/line-half": "https://github.com/mapbox/mapbox-gl-native/issues/9732",
-  "render-tests/tilejson-bounds/default": "https://github.com/mapbox/mapbox-gl-native/pull/10701",
   "render-tests/video/default": "skip - https://github.com/mapbox/mapbox-gl-native/issues/601",
   "render-tests/background-color/colorSpace-hcl": "needs issue",
   "render-tests/hillshade-accent-color/default": "skip - https://github.com/mapbox/mapbox-gl-native/pull/10642",

diff --git a/src/mbgl/algorithm/update_renderables.hpp b/src/mbgl/algorithm/update_renderables.hpp
@@ -35,6 +35,8 @@ void updateRenderables(GetTileFn getTile,
         auto tile = getTile(idealDataTileID);
         if (!tile) {
             tile = createTile(idealDataTileID);
+            // For source types where TileJSON.bounds is set, tiles outside the
+            // bounds are not created
             if(tile == nullptr) {
                 continue;
             }

diff --git a/src/mbgl/annotation/render_annotation_source.cpp b/src/mbgl/annotation/render_annotation_source.cpp
@@ -41,7 +41,7 @@ void RenderAnnotationSource::update(Immutable<style::Source::Impl> baseImpl_,
                        // Zoom level 16 is typically sufficient for annotations.
                        // See https://github.com/mapbox/mapbox-gl-native/issues/10197
                        { 0, 16 },
-                       {},
+                       optional<LatLngBounds> {},
                        [&] (const OverscaledTileID& tileID) {
                            return std::make_unique<AnnotationTile>(tileID, parameters);
                        });

diff --git a/src/mbgl/renderer/sources/render_geojson_source.cpp b/src/mbgl/renderer/sources/render_geojson_source.cpp
@@ -62,7 +62,7 @@ void RenderGeoJSONSource::update(Immutable<style::Source::Impl> baseImpl_,
                        SourceType::GeoJSON,
                        util::tileSize,
                        impl().getZoomRange(),
-                       {},
+                       optional<LatLngBounds>{},
                        [&] (const OverscaledTileID& tileID) {
                            return std::make_unique<GeoJSONTile>(tileID, impl().id, parameters, data->getTile(tileID.canonical));
                        });

diff --git a/src/mbgl/style/conversion/tileset.cpp b/src/mbgl/style/conversion/tileset.cpp
@@ -85,8 +85,12 @@ optional<Tileset> Converter<Tileset>::operator()(const Convertible& value, Error
             error = { "bounds array must contain numeric longitude and latitude values" };
             return {};
         }
-        if (!validateLatitude(*bottom) || !validateLatitude(*top)){
-            error = { "bounds latitude values must be between -90 and 90" };
+        if (!validateLatitude(*bottom) || !validateLatitude(*top) || top <= bottom){
+            error = { "bounds latitude values must be between -90 and 90 with bottom less than top" };
+            return {};
+        }
+        if(*left >= *right) {
+            error = { "bounds left longitude should be less than right longitude" };
             return {};
         }
         result.bounds = LatLngBounds::hull({ *bottom, *left }, { *top, *right });

diff --git a/src/mbgl/util/tile_range.hpp b/src/mbgl/util/tile_range.hpp
@@ -19,10 +19,10 @@ class TileRange {
         auto swProj = Projection::project(bounds.southwest().wrapped(), z);
         auto ne = bounds.northeast();
         auto neProj = Projection::project(ne.longitude() > util::LONGITUDE_MAX ? ne.wrapped() : ne , z);
-        auto minX = std::floor(swProj.x);
-        auto maxX = std::ceil(neProj.x);
-        auto minY = std::floor(neProj.y);
-        auto maxY = std::ceil(swProj.y);
+        const auto minX = std::floor(swProj.x);
+        const auto maxX = std::ceil(neProj.x);
+        const auto minY = std::floor(neProj.y);
+        const auto maxY = std::ceil(swProj.y);
         return TileRange({ {minX, minY}, {maxX, maxY} }, z);
     }
 

diff --git a/test/style/conversion/tileset.test.cpp b/test/style/conversion/tileset.test.cpp
@@ -60,7 +60,7 @@ TEST(Tileset, FullConversion) {
         "minzoom": 1,
         "maxzoom": 2,
         "attribution": "mapbox",
-        "bounds": [-180, 73, -120, -73]
+        "bounds": [-180, -73, -120, 73]
     })JSON", error);
 
     EXPECT_EQ(converted.tiles[0], "http://mytiles");