diff --git a/CHANGELOG.md b/CHANGELOG.md index b3f71289377..c1a5a1ffdd4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## 0.53.1 + +## Bug fixes +* Turn off telemetry for Mapbox Atlas ([#7945](https://github.com/mapbox/mapbox-gl-js/pull/7945)) +* Fix order of 3D features in query results (fix #7883) ([#7953](https://github.com/mapbox/mapbox-gl-js/pull/7953)) +* Fix RemovePaintState benchmarks ([#7930](https://github.com/mapbox/mapbox-gl-js/pull/7930)) + ## 0.53.0 ## Features and improvements diff --git a/bench/benchmarks/remove_paint_state.js b/bench/benchmarks/remove_paint_state.js index d4a04394f1c..a67f9b32f6a 100644 --- a/bench/benchmarks/remove_paint_state.js +++ b/bench/benchmarks/remove_paint_state.js @@ -68,7 +68,7 @@ class RemovePaintState extends Benchmark { } } -class propertyLevelRemove extends RemovePaintState { +class PropertyLevelRemove extends RemovePaintState { bench() { for (let i = 0; i < this.numFeatures; i += 50) { @@ -82,7 +82,7 @@ class propertyLevelRemove extends RemovePaintState { } } -class featureLevelRemove extends RemovePaintState { +class FeatureLevelRemove extends RemovePaintState { bench() { for (let i = 0; i < this.numFeatures; i += 50) { @@ -96,7 +96,7 @@ class featureLevelRemove extends RemovePaintState { } } -class sourceLevelRemove extends RemovePaintState { +class SourceLevelRemove extends RemovePaintState { bench() { for (let i = 0; i < this.numFeatures; i += 50) { @@ -111,7 +111,7 @@ class sourceLevelRemove extends RemovePaintState { } export default [ - propertyLevelRemove, - featureLevelRemove, - sourceLevelRemove + PropertyLevelRemove, + FeatureLevelRemove, + SourceLevelRemove ]; diff --git a/bench/versions/benchmarks.js b/bench/versions/benchmarks.js index 5729ee2eae3..f431bafd0ba 100644 --- a/bench/versions/benchmarks.js +++ b/bench/versions/benchmarks.js @@ -22,7 +22,7 @@ import SymbolLayout from '../benchmarks/symbol_layout'; import WorkerTransfer from '../benchmarks/worker_transfer'; import Paint from '../benchmarks/paint'; import PaintStates from '../benchmarks/paint_states'; -import RemovePaintState from '../benchmarks/remove_paint_state'; +import RemovePaintStateBenchmarks from '../benchmarks/remove_paint_state'; import LayerBenchmarks from '../benchmarks/layers'; import Load from '../benchmarks/map_load'; import Validate from '../benchmarks/style_validate'; @@ -47,7 +47,7 @@ register(new StyleLayerCreate(style)); ExpressionBenchmarks.forEach((Bench) => register(new Bench(style))); register(new WorkerTransfer(style)); register(new PaintStates(center)); -register(new RemovePaintState(center)); +RemovePaintStateBenchmarks.forEach((Bench) => register(new Bench(center))); LayerBenchmarks.forEach((Bench) => register(new Bench())); register(new Load()); register(new LayoutDDS()); diff --git a/package.json b/package.json index 31c2ee3e967..c2b064cc50f 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "mapbox-gl", "description": "A WebGL interactive maps library", - "version": "0.53.0", + "version": "0.53.1", "main": "dist/mapbox-gl.js", "style": "dist/mapbox-gl.css", "license": "SEE LICENSE IN LICENSE.txt", diff --git a/src/style/style.js b/src/style/style.js index 5673f87a3e5..a71f4e9c9f0 100644 --- a/src/style/style.js +++ b/src/style/style.js @@ -942,32 +942,68 @@ class Style extends Evented { } _flattenAndSortRenderedFeatures(sourceResults: Array) { - const features = []; + // Feature order is complicated. + // The order between features in two 2D layers is always determined by layer order. + // The order between features in two 3D layers is always determined by depth. + // The order between a feature in a 2D layer and a 3D layer is tricky: + // Most often layer order determines the feature order in this case. If + // a line layer is above a extrusion layer the line feature will be rendered + // above the extrusion. If the line layer is below the extrusion layer, + // it will be rendered below it. + // + // There is a weird case though. + // You have layers in this order: extrusion_layer_a, line_layer, extrusion_layer_b + // Each layer has a feature that overlaps the other features. + // The feature in extrusion_layer_a is closer than the feature in extrusion_layer_b so it is rendered above. + // The feature in line_layer is rendered above extrusion_layer_a. + // This means that that the line_layer feature is above the extrusion_layer_b feature despite + // it being in an earlier layer. + + const isLayer3D = layerId => this._layers[layerId].type === 'fill-extrusion'; + + const layerIndex = {}; const features3D = []; for (let l = this._order.length - 1; l >= 0; l--) { const layerId = this._order[l]; - for (const sourceResult of sourceResults) { - const layerFeatures = sourceResult[layerId]; - if (layerFeatures) { - if (this._layers[layerId].type === 'fill-extrusion') { + if (isLayer3D(layerId)) { + layerIndex[layerId] = l; + for (const sourceResult of sourceResults) { + const layerFeatures = sourceResult[layerId]; + if (layerFeatures) { for (const featureWrapper of layerFeatures) { features3D.push(featureWrapper); } - } else { - for (const featureWrapper of layerFeatures) { - features.push(featureWrapper.feature); - } } } } } features3D.sort((a, b) => { - return a.intersectionZ - b.intersectionZ; + return b.intersectionZ - a.intersectionZ; }); - for (const featureWrapper of features3D) { - features.push(featureWrapper.feature); + const features = []; + for (let l = this._order.length - 1; l >= 0; l--) { + const layerId = this._order[l]; + + if (isLayer3D(layerId)) { + // add all 3D features that are in or above the current layer + for (let i = features3D.length - 1; i >= 0; i--) { + const topmost3D = features3D[i].feature; + if (layerIndex[topmost3D.layer.id] < l) break; + features.push(topmost3D); + features3D.pop(); + } + } else { + for (const sourceResult of sourceResults) { + const layerFeatures = sourceResult[layerId]; + if (layerFeatures) { + for (const featureWrapper of layerFeatures) { + features.push(featureWrapper.feature); + } + } + } + } } return features; diff --git a/src/util/config.js b/src/util/config.js index 0d8ad3017f8..58a55f04359 100644 --- a/src/util/config.js +++ b/src/util/config.js @@ -2,7 +2,7 @@ type Config = {| API_URL: string, - EVENTS_URL: string, + EVENTS_URL: ?string, FEEDBACK_URL: string, REQUIRE_ACCESS_TOKEN: boolean, ACCESS_TOKEN: ?string, @@ -12,10 +12,13 @@ type Config = {| const config: Config = { API_URL: 'https://api.mapbox.com', get EVENTS_URL() { + if (!this.API_URL) { return null; } if (this.API_URL.indexOf('https://api.mapbox.cn') === 0) { return 'https://events.mapbox.cn/events/v2'; - } else { + } else if (this.API_URL.indexOf('https://api.mapbox.com') === 0) { return 'https://events.mapbox.com/events/v2'; + } else { + return null; } }, FEEDBACK_URL: 'https://apps.mapbox.com/feedback', diff --git a/src/util/mapbox.js b/src/util/mapbox.js index a0b9c89bcd9..9752b2a4aec 100644 --- a/src/util/mapbox.js +++ b/src/util/mapbox.js @@ -253,6 +253,7 @@ class TelemetryEvent { * to TelemetryEvent#saveData */ postEvent(timestamp: number, additionalPayload: {[string]: any}, callback: (err: ?Error) => void) { + if (!config.EVENTS_URL) return; const eventsUrlObject: UrlObject = parseUrl(config.EVENTS_URL); eventsUrlObject.params.push(`access_token=${config.ACCESS_TOKEN || ''}`); const payload: Object = { @@ -297,7 +298,8 @@ export class MapLoadEvent extends TelemetryEvent { postMapLoadEvent(tileUrls: Array, mapId: number) { //Enabled only when Mapbox Access Token is set and a source uses // mapbox tiles. - if (config.ACCESS_TOKEN && + if (config.EVENTS_URL && + config.ACCESS_TOKEN && Array.isArray(tileUrls) && tileUrls.some(url => isMapboxURL(url) || isMapboxHTTPURL(url))) { this.queueRequest({id: mapId, timestamp: Date.now()}); @@ -336,7 +338,8 @@ export class TurnstileEvent extends TelemetryEvent { postTurnstileEvent(tileUrls: Array) { //Enabled only when Mapbox Access Token is set and a source uses // mapbox tiles. - if (config.ACCESS_TOKEN && + if (config.EVENTS_URL && + config.ACCESS_TOKEN && Array.isArray(tileUrls) && tileUrls.some(url => isMapboxURL(url) || isMapboxHTTPURL(url))) { this.queueRequest(Date.now()); diff --git a/test/integration/query-tests/regressions/mapbox-gl-js#7883/expected.json b/test/integration/query-tests/regressions/mapbox-gl-js#7883/expected.json new file mode 100644 index 00000000000..15920911781 --- /dev/null +++ b/test/integration/query-tests/regressions/mapbox-gl-js#7883/expected.json @@ -0,0 +1,142 @@ +[ + { + "geometry": { + "type": "Polygon", + "coordinates": [ + [ + [ + -30.0146484375, + 0 + ], + [ + -30.0146484375, + 50.00773901463688 + ], + [ + 30.0146484375, + 50.00773901463688 + ], + [ + 30.0146484375, + 0 + ], + [ + -30.0146484375, + 0 + ] + ] + ] + }, + "type": "Feature", + "properties": { + "layer": "upper" + }, + "source": "fill-upper", + "state": {} + }, + { + "geometry": { + "type": "Polygon", + "coordinates": [ + [ + [ + -10.01953125, + -20.01464544534136 + ], + [ + -10.01953125, + 40.01078714046551 + ], + [ + 39.990234375, + 40.01078714046551 + ], + [ + 39.990234375, + -20.01464544534136 + ], + [ + -10.01953125, + -20.01464544534136 + ] + ] + ] + }, + "type": "Feature", + "properties": { + "layer": "extrusion_closer" + }, + "source": "extrusion_closer", + "state": {} + }, + { + "geometry": { + "type": "Polygon", + "coordinates": [ + [ + [ + -39.990234375, + -40.01078714046552 + ], + [ + -39.990234375, + 40.01078714046551 + ], + [ + 10.01953125, + 40.01078714046551 + ], + [ + 10.01953125, + -40.01078714046552 + ], + [ + -39.990234375, + -40.01078714046552 + ] + ] + ] + }, + "type": "Feature", + "properties": { + "layer": "extrusion_further" + }, + "source": "extrusion", + "state": {} + }, + { + "geometry": { + "type": "Polygon", + "coordinates": [ + [ + [ + -50.009765625, + -50.00773901463686 + ], + [ + -50.009765625, + 50.00773901463688 + ], + [ + 50.009765625, + 50.00773901463688 + ], + [ + 50.009765625, + -50.00773901463686 + ], + [ + -50.009765625, + -50.00773901463686 + ] + ] + ] + }, + "type": "Feature", + "properties": { + "layer": "fill-lower" + }, + "source": "fill-lower", + "state": {} + } +] \ No newline at end of file diff --git a/test/integration/query-tests/regressions/mapbox-gl-js#7883/style.json b/test/integration/query-tests/regressions/mapbox-gl-js#7883/style.json new file mode 100644 index 00000000000..d9f6901e79c --- /dev/null +++ b/test/integration/query-tests/regressions/mapbox-gl-js#7883/style.json @@ -0,0 +1,202 @@ +{ + "version": 8, + "metadata": { + "test": { + "debug": true, + "width": 200, + "height": 200, + "queryGeometry": [ + 100, + 40 + ] + } + }, + "pitch": 25, + "center": [ + 0, + 0 + ], + "zoom": 0, + "sources": { + "fill-lower": { + "type": "geojson", + "data": { + "type": "Feature", + "properties": { + "layer": "fill-lower" + }, + "geometry": { + "type": "Polygon", + "coordinates": [ + [ + [ + -50, + -50 + ], + [ + -50, + 50 + ], + [ + 50, + 50 + ], + [ + 50, + -50 + ], + [ + -50, + -50 + ] + ] + ] + } + } + }, + "extrusion_closer": { + "type": "geojson", + "data": { + "type": "Feature", + "properties": { + "layer": "extrusion_closer" + }, + "geometry": { + "type": "Polygon", + "coordinates": [ + [ + [ + -10, + -20 + ], + [ + -10, + 40 + ], + [ + 40, + 40 + ], + [ + 40, + -20 + ], + [ + -10, + -20 + ] + ] + ] + } + } + }, + "extrusion": { + "type": "geojson", + "data": { + "type": "Feature", + "properties": { + "layer": "extrusion_further" + }, + "geometry": { + "type": "Polygon", + "coordinates": [ + [ + [ + -40, + -40 + ], + [ + -40, + 40 + ], + [ + 10, + 40 + ], + [ + 10, + -40 + ], + [ + -40, + -40 + ] + ] + ] + } + } + }, + "fill-upper": { + "type": "geojson", + "data": { + "type": "Feature", + "properties": { + "layer": "upper" + }, + "geometry": { + "type": "Polygon", + "coordinates": [ + [ + [ + -30, + -0 + ], + [ + -30, + 50 + ], + [ + 30, + 50 + ], + [ + 30, + -0 + ], + [ + -30, + -0 + ] + ] + ] + } + } + } + }, + "layers": [ + { + "id": "fill-lower", + "type": "fill", + "source": "fill-lower", + "paint": { + "fill-color": "#0f0" + } + }, + { + "id": "extrusion_closer", + "type": "fill-extrusion", + "source": "extrusion_closer", + "paint": { + "fill-extrusion-color": "#ecc", + "fill-extrusion-height": 12000000 + } + }, + { + "id": "fill-upper", + "type": "fill", + "source": "fill-upper", + "paint": { + "fill-color": "#00f" + } + }, + { + "id": "extrusion", + "type": "fill-extrusion", + "source": "extrusion", + "paint": { + "fill-extrusion-color": "#ccc", + "fill-extrusion-height": 10000000 + } + } + ] +} diff --git a/test/unit/util/mapbox.test.js b/test/unit/util/mapbox.test.js index 5fa3f3a45b2..9e37fdfa4ea 100644 --- a/test/unit/util/mapbox.test.js +++ b/test/unit/util/mapbox.test.js @@ -29,6 +29,7 @@ test("mapbox", (t) => { t.beforeEach((callback) => { config.ACCESS_TOKEN = 'key'; config.REQUIRE_ACCESS_TOKEN = true; + config.API_URL = 'https://api.mapbox.com'; callback(); }); @@ -64,13 +65,11 @@ test("mapbox", (t) => { }); t.test('handles custom API_URLs with paths', (t) => { - const previousUrl = config.API_URL; config.API_URL = 'https://test.example.com/api.mapbox.com'; t.equal( mapbox.normalizeStyleURL('mapbox://styles/foo/bar'), 'https://test.example.com/api.mapbox.com/styles/v1/foo/bar?access_token=key' ); - config.API_URL = previousUrl; t.end(); }); @@ -118,13 +117,11 @@ test("mapbox", (t) => { }); t.test('handles custom API_URLs with paths', (t) => { - const previousUrl = config.API_URL; config.API_URL = 'https://test.example.com/api.mapbox.com'; t.equal( mapbox.normalizeSourceURL('mapbox://one.a'), 'https://test.example.com/api.mapbox.com/v4/one.a.json?secure&access_token=key' ); - config.API_URL = previousUrl; t.end(); }); @@ -148,13 +145,11 @@ test("mapbox", (t) => { }); t.test('handles custom API_URLs with paths', (t) => { - const previousUrl = config.API_URL; config.API_URL = 'https://test.example.com/api.mapbox.com'; t.equal( mapbox.normalizeGlyphsURL('mapbox://fonts/boxmap/{fontstack}/{range}.pbf'), 'https://test.example.com/api.mapbox.com/fonts/v1/boxmap/{fontstack}/{range}.pbf?access_token=key' ); - config.API_URL = previousUrl; t.end(); }); @@ -216,13 +211,11 @@ test("mapbox", (t) => { }); t.test('handles custom API_URLs with paths', (t) => { - const previousUrl = config.API_URL; config.API_URL = 'https://test.example.com/api.mapbox.com'; t.equal( mapbox.normalizeSpriteURL('mapbox://sprites/mapbox/streets-v8', '', '.json'), 'https://test.example.com/api.mapbox.com/styles/v1/mapbox/streets-v8/sprite.json?access_token=key' ); - config.API_URL = previousUrl; t.end(); }); @@ -412,7 +405,6 @@ test("mapbox", (t) => { }); t.test('POSTs cn event when API_URL change to cn endpoint', (t) => { - const previousUrl = config.API_URL; config.API_URL = 'https://api.mapbox.cn'; event.postTurnstileEvent(mapboxTileURLs); @@ -421,7 +413,20 @@ test("mapbox", (t) => { req.respond(200); t.true(req.url.indexOf('https://events.mapbox.cn') > -1); - config.API_URL = previousUrl; + t.end(); + }); + + t.test('POSTs no event when API_URL unavailable', (t) => { + config.API_URL = null; + event.postTurnstileEvent(mapboxTileURLs); + t.equal(window.server.requests.length, 0, 'no events posted'); + t.end(); + }); + + t.test('POSTs no event when API_URL non-standard', (t) => { + config.API_URL = 'https://api.example.com'; + event.postTurnstileEvent(mapboxTileURLs); + t.equal(window.server.requests.length, 0, 'no events posted'); t.end(); }); @@ -465,7 +470,6 @@ test("mapbox", (t) => { t.test('POSTs event when previously stored anonId is not a valid uuid', (t) => { const now = +Date.now(); - window.localStorage.setItem(`mapbox.eventData.uuid:${config.ACCESS_TOKEN}`, 'anonymous'); window.localStorage.setItem(`mapbox.eventData:${config.ACCESS_TOKEN}`, JSON.stringify({ lastSuccess: now @@ -720,7 +724,6 @@ test("mapbox", (t) => { }); t.test('POSTs cn event when API_URL changes to cn endpoint', (t) => { - const previousUrl = config.API_URL; config.API_URL = 'https://api.mapbox.cn'; event.postMapLoadEvent(mapboxTileURLs, 1); @@ -729,7 +732,20 @@ test("mapbox", (t) => { req.respond(200); t.true(req.url.indexOf('https://events.mapbox.cn') > -1); - config.API_URL = previousUrl; + t.end(); + }); + + t.test('POSTs no event when API_URL unavailable', (t) => { + config.API_URL = null; + event.postMapLoadEvent(mapboxTileURLs, 1); + t.equal(window.server.requests.length, 0, 'no events posted'); + t.end(); + }); + + t.test('POSTs no event when API_URL is non-standard', (t) => { + config.API_URL = "https://api.example.com"; + event.postMapLoadEvent(mapboxTileURLs, 1); + t.equal(window.server.requests.length, 0, 'no events posted'); t.end(); });