From 57cdaaa040d6ad90655f42aa4a37c9949be36f5d Mon Sep 17 00:00:00 2001 From: Asheem Mamoowala Date: Mon, 11 Dec 2017 16:58:04 -0800 Subject: [PATCH 1/6] [core] Use z0 tile coordinates instead of screen coordinates to compute ideal zoom for image sources --- .../renderer/sources/render_image_source.cpp | 48 ++++++++----------- 1 file changed, 21 insertions(+), 27 deletions(-) diff --git a/src/mbgl/renderer/sources/render_image_source.cpp b/src/mbgl/renderer/sources/render_image_source.cpp index b5c42584e02..0844db39d9b 100644 --- a/src/mbgl/renderer/sources/render_image_source.cpp +++ b/src/mbgl/renderer/sources/render_image_source.cpp @@ -114,44 +114,39 @@ void RenderImageSource::update(Immutable baseImpl_, return; } - auto size = transformState.getSize(); - const double viewportHeight = size.height; - - // Compute the screen coordinates at wrap=0 for the given LatLng - ScreenCoordinate nePixel = { -INFINITY, -INFINITY }; - ScreenCoordinate swPixel = { INFINITY, INFINITY }; - + // Compute the z0 tile coordinates for the given LatLngs + TileCoordinatePoint nePoint = { -INFINITY, -INFINITY }; + TileCoordinatePoint swPoint = { INFINITY, INFINITY }; + std::vector tileCoordinates; for (LatLng latLng : coords) { - ScreenCoordinate pixel = transformState.latLngToScreenCoordinate(latLng); - swPixel.x = std::min(swPixel.x, pixel.x); - nePixel.x = std::max(nePixel.x, pixel.x); - swPixel.y = std::min(swPixel.y, viewportHeight - pixel.y); - nePixel.y = std::max(nePixel.y, viewportHeight - pixel.y); - } - const double width = nePixel.x - swPixel.x; - const double height = nePixel.y - swPixel.y; + auto point = TileCoordinate::fromLatLng(0, latLng).p; + tileCoordinates.push_back(point); + swPoint.x = std::min(swPoint.x, point.x); + nePoint.x = std::max(nePoint.x, point.x); + swPoint.y = std::min(swPoint.y, point.y); + nePoint.y = std::max(nePoint.y, point.y); + } + + // Calculate the optimum zoom level to determine the tile ids to use for transforms + auto dx = nePoint.x - swPoint.x; + auto dy = nePoint.y - swPoint.y; + auto dMax = std::max(dx, dy); + double zoom = std::max(0.0, std::floor(-util::log2(dMax))); // Don't bother drawing the ImageSource unless it occupies >4 screen pixels - enabled = (width * height > 4); + enabled = dMax * std::pow(2.0, transformState.getZoom()) > 2.0 / util::tileSize; if (!enabled) { return; } - // Calculate the optimum zoom level to determine the tile ids to use for transforms - double minScale = INFINITY; - double scaleX = double(size.width) / width; - double scaleY = double(size.height) / height; - minScale = util::min(scaleX, scaleY); - double zoom = transformState.getZoom() + util::log2(minScale); - zoom = std::floor(util::clamp(zoom, transformState.getMinZoom(), transformState.getMaxZoom())); auto imageBounds = LatLngBounds::hull(coords[0], coords[1]); imageBounds.extend(coords[2]); imageBounds.extend(coords[3]); auto tileCover = util::tileCover(imageBounds, zoom); tileIds.clear(); tileIds.push_back(tileCover[0]); - bool hasVisibleTile = false; + bool hasVisibleTile = false; // Add additional wrapped tile ids if neccessary auto idealTiles = util::tileCover(transformState, transformState.getZoom()); for (auto tile : idealTiles) { @@ -177,9 +172,8 @@ void RenderImageSource::update(Immutable baseImpl_, // Calculate Geometry Coordinates based on tile cover at ideal zoom GeometryCoordinates geomCoords; - for (auto latLng : coords) { - auto tc = TileCoordinate::fromLatLng(0, latLng); - auto gc = TileCoordinate::toGeometryCoordinate(tileIds[0], tc.p); + for (auto tileCoords : tileCoordinates) { + auto gc = TileCoordinate::toGeometryCoordinate(tileIds[0], tileCoords); geomCoords.push_back(gc); } if (!bucket) { From 2571f344dbc5caaaead32fbec420a080588311a4 Mon Sep 17 00:00:00 2001 From: Asheem Mamoowala Date: Tue, 12 Dec 2017 13:22:33 -0800 Subject: [PATCH 2/6] Add comment --- src/mbgl/renderer/sources/render_image_source.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/mbgl/renderer/sources/render_image_source.cpp b/src/mbgl/renderer/sources/render_image_source.cpp index 0844db39d9b..e2e742167bb 100644 --- a/src/mbgl/renderer/sources/render_image_source.cpp +++ b/src/mbgl/renderer/sources/render_image_source.cpp @@ -133,8 +133,12 @@ void RenderImageSource::update(Immutable baseImpl_, auto dMax = std::max(dx, dy); double zoom = std::max(0.0, std::floor(-util::log2(dMax))); - // Don't bother drawing the ImageSource unless it occupies >4 screen pixels - enabled = dMax * std::pow(2.0, transformState.getZoom()) > 2.0 / util::tileSize; + // Only enable if the long side of the image is > 2 pixels. Resulting in a + // display of at least 2 x 1 px image + // dMax is in tile coordinate units at z0, so scale by util::EXTENT and then + // by 2^z to get geometry coordinates at the current zoom. + // 1 gc unit = tileSize / extent pixels. + enabled = dMax * std::pow( 2.0, transformState.getZoom()) * util::tileSize > 2.0; if (!enabled) { return; } From 48bcdf3be5956575dcc5b1af6e7cf926d002995e Mon Sep 17 00:00:00 2001 From: Asheem Mamoowala Date: Tue, 12 Dec 2017 13:56:28 -0800 Subject: [PATCH 3/6] Update gl-js submodule to include render test --- mapbox-gl-js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mapbox-gl-js b/mapbox-gl-js index 98d8fce6dc3..2d6d9b0193b 160000 --- a/mapbox-gl-js +++ b/mapbox-gl-js @@ -1 +1 @@ -Subproject commit 98d8fce6dc317951a65962036f896c0d1d54b97c +Subproject commit 2d6d9b0193bb5d5f8bd39c6306e6bbad093d28ab From a08a655245b30fd338c93cc874af0f5d504b30ca Mon Sep 17 00:00:00 2001 From: Asheem Mamoowala Date: Tue, 12 Dec 2017 14:44:10 -0800 Subject: [PATCH 4/6] Update ignore file for render tests --- platform/node/test/ignores.json | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/platform/node/test/ignores.json b/platform/node/test/ignores.json index 4b362086521..d9e259a7df0 100644 --- a/platform/node/test/ignores.json +++ b/platform/node/test/ignores.json @@ -12,10 +12,10 @@ "expression-tests/equal/object": "https://github.com/mapbox/mapbox-gl-native/issues/10678", "expression-tests/equal/string": "https://github.com/mapbox/mapbox-gl-native/issues/10678", "expression-tests/equal/value": "https://github.com/mapbox/mapbox-gl-native/issues/10678", - "expression-tests/not-equal/mismatch": "https://github.com/mapbox/mapbox-gl-native/issues/10678", - "expression-tests/not-equal/number": "https://github.com/mapbox/mapbox-gl-native/issues/10678", - "expression-tests/not-equal/string": "https://github.com/mapbox/mapbox-gl-native/issues/10678", - "expression-tests/not-equal/value": "https://github.com/mapbox/mapbox-gl-native/issues/10678", + "expression-tests/not_equal/mismatch": "https://github.com/mapbox/mapbox-gl-native/issues/10678", + "expression-tests/not_equal/number": "https://github.com/mapbox/mapbox-gl-native/issues/10678", + "expression-tests/not_equal/string": "https://github.com/mapbox/mapbox-gl-native/issues/10678", + "expression-tests/not_equal/value": "https://github.com/mapbox/mapbox-gl-native/issues/10678", "expression-tests/interpolate/linear-color": "https://github.com/mapbox/mapbox-gl-native/issues/10604", "expression-tests/to-rgba/basic": "https://github.com/mapbox/mapbox-gl-native/issues/10604", "query-tests/circle-stroke-width/inside": "https://github.com/mapbox/mapbox-gl-native/issues/10307", @@ -83,5 +83,15 @@ "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/video/default": "skip - https://github.com/mapbox/mapbox-gl-native/issues/601" + "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", + "render-tests/hillshade-accent-color/literal": "skip - https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/hillshade-accent-color/zoom-function": "skip - https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/hillshade-highlight-color/default": "skip - https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/hillshade-highlight-color/literal": "skip - https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/hillshade-highlight-color/zoom-function": "skip - https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/hillshade-shadow-color/default": "skip - https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/hillshade-shadow-color/literal": "skip - https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/hillshade-shadow-color/zoom-function": "skip - https://github.com/mapbox/mapbox-gl-native/pull/10642" } From f53a0f7e14ce0711e857b10d67963eaf474f346b Mon Sep 17 00:00:00 2001 From: Asheem Mamoowala Date: Tue, 12 Dec 2017 15:53:06 -0800 Subject: [PATCH 5/6] Update comment --- src/mbgl/renderer/sources/render_image_source.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/mbgl/renderer/sources/render_image_source.cpp b/src/mbgl/renderer/sources/render_image_source.cpp index e2e742167bb..d215dc8d131 100644 --- a/src/mbgl/renderer/sources/render_image_source.cpp +++ b/src/mbgl/renderer/sources/render_image_source.cpp @@ -135,10 +135,10 @@ void RenderImageSource::update(Immutable baseImpl_, // Only enable if the long side of the image is > 2 pixels. Resulting in a // display of at least 2 x 1 px image - // dMax is in tile coordinate units at z0, so scale by util::EXTENT and then - // by 2^z to get geometry coordinates at the current zoom. - // 1 gc unit = tileSize / extent pixels. - enabled = dMax * std::pow( 2.0, transformState.getZoom()) * util::tileSize > 2.0; + // A tile coordinate unit represents the length of one tile (tileSize) at a given zoom. + // To convert a tile coordinate to pixels, multiply by tileSize. + // Here dMax is in z0 tile units, so we also scale by 2^z to match current zoom. + enabled = dMax * std::pow(2.0, transformState.getZoom()) * util::tileSize > 2.0; if (!enabled) { return; } From 9ce7c49bbd62e21f2c65eb726bb001916652201d Mon Sep 17 00:00:00 2001 From: Asheem Mamoowala Date: Tue, 12 Dec 2017 16:38:04 -0800 Subject: [PATCH 6/6] update submodule pin and add ignores --- mapbox-gl-js | 2 +- platform/node/test/ignores.json | 56 ++++++++++++++++++++++++++++++++- 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/mapbox-gl-js b/mapbox-gl-js index 2d6d9b0193b..cca8ba41dff 160000 --- a/mapbox-gl-js +++ b/mapbox-gl-js @@ -1 +1 @@ -Subproject commit 2d6d9b0193bb5d5f8bd39c6306e6bbad093d28ab +Subproject commit cca8ba41dffd09c55a33ae207b734bf5631a9ad8 diff --git a/platform/node/test/ignores.json b/platform/node/test/ignores.json index d9e259a7df0..2ed72f7bfd1 100644 --- a/platform/node/test/ignores.json +++ b/platform/node/test/ignores.json @@ -93,5 +93,59 @@ "render-tests/hillshade-highlight-color/zoom-function": "skip - https://github.com/mapbox/mapbox-gl-native/pull/10642", "render-tests/hillshade-shadow-color/default": "skip - https://github.com/mapbox/mapbox-gl-native/pull/10642", "render-tests/hillshade-shadow-color/literal": "skip - https://github.com/mapbox/mapbox-gl-native/pull/10642", - "render-tests/hillshade-shadow-color/zoom-function": "skip - https://github.com/mapbox/mapbox-gl-native/pull/10642" + "render-tests/hillshade-shadow-color/zoom-function": "skip - https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/combinations/background-opaque--heatmap-translucent": "https://github.com/mapbox/mapbox-gl-native/issues/10146", + "render-tests/combinations/background-translucent--heatmap-translucent": "https://github.com/mapbox/mapbox-gl-native/issues/10146", + "render-tests/combinations/circle-translucent--heatmap-translucent": "https://github.com/mapbox/mapbox-gl-native/issues/10146", + "render-tests/combinations/fill-extrusion-translucent--heatmap-translucent": "https://github.com/mapbox/mapbox-gl-native/issues/10146", + "render-tests/combinations/fill-opaque--heatmap-translucent": "https://github.com/mapbox/mapbox-gl-native/issues/10146", + "render-tests/combinations/fill-translucent--heatmap-translucent": "https://github.com/mapbox/mapbox-gl-native/issues/10146", + "render-tests/combinations/heatmap-translucent--background-opaque": "https://github.com/mapbox/mapbox-gl-native/issues/10146", + "render-tests/combinations/heatmap-translucent--background-translucent": "https://github.com/mapbox/mapbox-gl-native/issues/10146", + "render-tests/combinations/heatmap-translucent--circle-translucent": "https://github.com/mapbox/mapbox-gl-native/issues/10146", + "render-tests/combinations/heatmap-translucent--fill-extrusion-translucent": "https://github.com/mapbox/mapbox-gl-native/issues/10146", + "render-tests/combinations/heatmap-translucent--fill-opaque": "https://github.com/mapbox/mapbox-gl-native/issues/10146", + "render-tests/combinations/heatmap-translucent--fill-translucent": "https://github.com/mapbox/mapbox-gl-native/issues/10146", + "render-tests/combinations/heatmap-translucent--heatmap-translucent": "https://github.com/mapbox/mapbox-gl-native/issues/10146", + "render-tests/combinations/heatmap-translucent--hillshade-translucent": "https://github.com/mapbox/mapbox-gl-native/issues/10146", + "render-tests/combinations/heatmap-translucent--line-translucent": "https://github.com/mapbox/mapbox-gl-native/issues/10146", + "render-tests/combinations/heatmap-translucent--raster-translucent": "https://github.com/mapbox/mapbox-gl-native/issues/10146", + "render-tests/combinations/heatmap-translucent--symbol-translucent": "https://github.com/mapbox/mapbox-gl-native/issues/10146", + "render-tests/combinations/line-translucent--heatmap-translucent": "https://github.com/mapbox/mapbox-gl-native/issues/10146", + "render-tests/combinations/raster-translucent--heatmap-translucent": "https://github.com/mapbox/mapbox-gl-native/issues/10146", + "render-tests/combinations/symbol-translucent--heatmap-translucent": "https://github.com/mapbox/mapbox-gl-native/issues/10146", + "render-tests/combinations/background-opaque--hillshade-translucent": "https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/combinations/background-translucent--hillshade-translucent": "https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/combinations/circle-translucent--hillshade-translucent": "https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/combinations/fill-extrusion-translucent--hillshade-translucent": "https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/combinations/fill-opaque--hillshade-translucent": "https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/combinations/fill-translucent--hillshade-translucent": "https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/combinations/hillshade-translucent--background-opaque": "https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/combinations/hillshade-translucent--background-translucent": "https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/combinations/hillshade-translucent--circle-translucent": "https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/combinations/hillshade-translucent--fill-extrusion-translucent": "https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/combinations/hillshade-translucent--fill-opaque": "https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/combinations/hillshade-translucent--fill-translucent": "https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/combinations/hillshade-translucent--heatmap-translucent": "https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/combinations/hillshade-translucent--hillshade-translucent": "https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/combinations/hillshade-translucent--line-translucent": "https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/combinations/hillshade-translucent--raster-translucent": "https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/combinations/hillshade-translucent--symbol-translucent": "https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/combinations/line-translucent--hillshade-translucent": "https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/combinations/raster-translucent--hillshade-translucent": "https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/combinations/symbol-translucent--hillshade-translucent": "https://github.com/mapbox/mapbox-gl-native/pull/10642", + "render-tests/combinations/background-opaque--fill-extrusion-translucent": "needs investigation", + "render-tests/combinations/background-translucent--fill-extrusion-translucent": "needs investigation", + "render-tests/combinations/circle-translucent--fill-extrusion-translucent": "needs investigation", + "render-tests/combinations/fill-extrusion-translucent--background-translucent": "needs investigation", + "render-tests/combinations/fill-extrusion-translucent--circle-translucent": "needs investigation", + "render-tests/combinations/fill-extrusion-translucent--fill-extrusion-translucent": "needs investigation", + "render-tests/combinations/fill-extrusion-translucent--fill-translucent": "needs investigation", + "render-tests/combinations/fill-extrusion-translucent--line-translucent": "needs investigation", + "render-tests/combinations/fill-extrusion-translucent--symbol-translucent": "needs investigation", + "render-tests/combinations/fill-opaque--fill-extrusion-translucent": "needs investigation", + "render-tests/combinations/fill-translucent--fill-extrusion-translucent": "needs investigation", + "render-tests/combinations/line-translucent--fill-extrusion-translucent": "needs investigation", + "render-tests/combinations/raster-translucent--fill-extrusion-translucent": "needs investigation", + "render-tests/combinations/symbol-translucent--fill-extrusion-translucent": "needs investigation" }