From ef7b673d15865843077aaf5fd68fe8d36b8c6e30 Mon Sep 17 00:00:00 2001 From: Asheem Mamoowala Date: Tue, 2 Jan 2018 15:26:06 -0700 Subject: Fixes from code review and update gl-js pin to include render test --- platform/node/test/ignores.json | 1 - src/mbgl/algorithm/update_renderables.hpp | 2 ++ src/mbgl/annotation/render_annotation_source.cpp | 2 +- src/mbgl/renderer/sources/render_geojson_source.cpp | 2 +- src/mbgl/style/conversion/tileset.cpp | 8 ++++++-- src/mbgl/util/tile_range.hpp | 8 ++++---- test/style/conversion/tileset.test.cpp | 2 +- 7 files changed, 15 insertions(+), 10 deletions(-) diff --git a/platform/node/test/ignores.json b/platform/node/test/ignores.json index 10e5e88e79..62e042cd4d 100644 --- 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 index 16c99351b4..5fbe0d943f 100644 --- 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 index 3e3e48fdf3..38ca5ccd0b 100644 --- a/src/mbgl/annotation/render_annotation_source.cpp +++ b/src/mbgl/annotation/render_annotation_source.cpp @@ -41,7 +41,7 @@ void RenderAnnotationSource::update(Immutable baseImpl_, // Zoom level 16 is typically sufficient for annotations. // See https://github.com/mapbox/mapbox-gl-native/issues/10197 { 0, 16 }, - {}, + optional {}, [&] (const OverscaledTileID& tileID) { return std::make_unique(tileID, parameters); }); diff --git a/src/mbgl/renderer/sources/render_geojson_source.cpp b/src/mbgl/renderer/sources/render_geojson_source.cpp index 8a645b62b2..8ea80cd813 100644 --- 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 baseImpl_, SourceType::GeoJSON, util::tileSize, impl().getZoomRange(), - {}, + optional{}, [&] (const OverscaledTileID& tileID) { return std::make_unique(tileID, impl().id, parameters, data->getTile(tileID.canonical)); }); diff --git a/src/mbgl/style/conversion/tileset.cpp b/src/mbgl/style/conversion/tileset.cpp index cafafbf7cd..6e559c0cac 100644 --- a/src/mbgl/style/conversion/tileset.cpp +++ b/src/mbgl/style/conversion/tileset.cpp @@ -85,8 +85,12 @@ optional Converter::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 index eb7d2916c5..f630a49078 100644 --- a/src/mbgl/util/tile_range.hpp +++ b/src/mbgl/util/tile_range.hpp @@ -19,10 +19,10 @@ public: 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 index b70af7a911..8002cd038f 100644 --- 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"); -- cgit v1.2.1