From 36a265006592601c13e2421253f5dc3a18db9496 Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Sat, 7 Dec 2019 09:29:40 -0500 Subject: [PATCH 1/4] Don't update map view while dragging/wheeling --- src/plots/mapbox/mapbox.js | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/plots/mapbox/mapbox.js b/src/plots/mapbox/mapbox.js index b93cfcb87cd..7c051357ba2 100644 --- a/src/plots/mapbox/mapbox.js +++ b/src/plots/mapbox/mapbox.js @@ -49,6 +49,8 @@ function Mapbox(gd, id) { this.traceHash = {}; this.layerList = []; this.belowLookup = {}; + this.dragging = false; + this.wheeling = false; } var proto = Mapbox.prototype; @@ -351,10 +353,12 @@ proto.updateLayout = function(fullLayout) { var map = this.map; var opts = fullLayout[this.id]; - map.setCenter(convertCenter(opts.center)); - map.setZoom(opts.zoom); - map.setBearing(opts.bearing); - map.setPitch(opts.pitch); + if(!this.dragging && !this.wheeling) { + map.setCenter(convertCenter(opts.center)); + map.setZoom(opts.zoom); + map.setBearing(opts.bearing); + map.setPitch(opts.pitch); + } this.updateLayers(fullLayout); this.updateFramework(fullLayout); @@ -435,8 +439,6 @@ proto.initFx = function(calcData, fullLayout) { var gd = self.gd; var map = self.map; - var wheeling = false; - // keep track of pan / zoom in user layout and emit relayout event map.on('moveend', function(evt) { if(!self.map) return; @@ -451,7 +453,7 @@ proto.initFx = function(calcData, fullLayout) { // mouse target (filtering out API calls) to not // duplicate 'plotly_relayout' events. - if(evt.originalEvent || wheeling) { + if(evt.originalEvent || self.wheeling) { var optsNow = fullLayoutNow[self.id]; Registry.call('_storeDirectGUIEdit', gd.layout, fullLayoutNow._preGUI, self.getViewEdits(optsNow)); @@ -460,10 +462,13 @@ proto.initFx = function(calcData, fullLayout) { optsNow._input.zoom = optsNow.zoom = viewNow.zoom; optsNow._input.bearing = optsNow.bearing = viewNow.bearing; optsNow._input.pitch = optsNow.pitch = viewNow.pitch; - gd.emit('plotly_relayout', self.getViewEditsWithDerived(viewNow)); } - wheeling = false; + if(evt.originalEvent && evt.originalEvent.type === 'mouseup') { + self.dragging = false; + } else if(self.wheeling) { + self.wheeling = false; + } if(fullLayoutNow._rehover) { fullLayoutNow._rehover(); @@ -471,7 +476,7 @@ proto.initFx = function(calcData, fullLayout) { }); map.on('wheel', function() { - wheeling = true; + self.wheeling = true; }); map.on('mousemove', function(evt) { @@ -500,7 +505,10 @@ proto.initFx = function(calcData, fullLayout) { Fx.loneUnhover(fullLayout._hoverlayer); } - map.on('dragstart', unhover); + map.on('dragstart', function() { + self.dragging = true; + unhover(); + }); map.on('zoomstart', unhover); map.on('mouseout', function() { From 7e355d7ec4fac17f9e98c434e86dc88fb9d765cc Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Sat, 7 Dec 2019 15:44:12 -0500 Subject: [PATCH 2/4] optimize update of mapbox image source --- src/plots/mapbox/layers.js | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/plots/mapbox/layers.js b/src/plots/mapbox/layers.js index dd47dcd188f..b8892e51edf 100644 --- a/src/plots/mapbox/layers.js +++ b/src/plots/mapbox/layers.js @@ -38,6 +38,8 @@ proto.update = function update(opts) { // IMPORTANT: must create source before layer to not cause errors this.updateSource(opts); this.updateLayer(opts); + } else if(this.needsNewImage(opts)) { + this.updateImage(opts); } else if(this.needsNewSource(opts)) { // IMPORTANT: must delete layer before source to not cause errors this.removeLayer(); @@ -52,6 +54,18 @@ proto.update = function update(opts) { this.visible = isVisible(opts); }; +proto.needsNewImage = function(opts) { + var map = this.subplot.map; + return ( + map.getSource(this.idSource) && + this.sourceType === 'image' && + opts.sourcetype === 'image' && + (this.source !== opts.source || + JSON.stringify(this.coordinates) !== + JSON.stringify(opts.coordinates)) + ); +}; + proto.needsNewSource = function(opts) { // for some reason changing layer to 'fill' or 'symbol' // w/o changing the source throws an exception in mapbox-gl 0.18 ; @@ -70,6 +84,13 @@ proto.needsNewLayer = function(opts) { ); }; +proto.updateImage = function(opts) { + var map = this.subplot.map; + map.getSource(this.idSource).updateImage({ + url: opts.source, coordinates: opts.coordinates + }); +}; + proto.updateSource = function(opts) { var map = this.subplot.map; @@ -223,6 +244,11 @@ function convertOpts(opts) { 'text-opacity': opts.opacity }); break; + case 'raster': + Lib.extendFlat(paint, { + 'raster-fade-duration': 0 + }); + break; } return { From 4f0c5e7a526b3c9c4b800b4d301f2377b3203e9b Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Tue, 10 Dec 2019 12:35:42 -0500 Subject: [PATCH 3/4] Add tests that center/zoom relayout updates are not made while panning/zooming --- test/jasmine/tests/mapbox_test.js | 47 +++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/test/jasmine/tests/mapbox_test.js b/test/jasmine/tests/mapbox_test.js index 35bd9b09b4e..bf3aceffb13 100644 --- a/test/jasmine/tests/mapbox_test.js +++ b/test/jasmine/tests/mapbox_test.js @@ -584,6 +584,53 @@ describe('@noCI, mapbox plots', function() { .then(done); }, LONG_TIMEOUT_INTERVAL); + it('@gl should not update center while dragging', function(done) { + var map = gd._fullLayout.mapbox._subplot.map; + spyOn(map, 'setCenter').and.callThrough(); + + var p1 = [pointPos[0] + 50, pointPos[1] - 20]; + + _mouseEvent('mousemove', pointPos, noop).then(function() { + return Plotly.relayout(gd, {'mapbox.center': {lon: 13.5, lat: -19.5}}); + }).then(function() { + // First relayout on mapbox.center results in setCenter call + expect(map.setCenter).toHaveBeenCalledWith([13.5, -19.5]); + expect(map.setCenter).toHaveBeenCalledTimes(1); + }).then(function() { + return _mouseEvent('mousedown', pointPos, noop); + }).then(function() { + return _mouseEvent('mousemove', p1, noop); + }).then(function() { + return Plotly.relayout(gd, {'mapbox.center': {lat: 0, lon: 0}}); + }).then(function() { + return _mouseEvent('mouseup', p1, noop); + }).then(function() { + // Second relayout on mapbox.center does not result in a setCenter + // call since map drag is underway + expect(map.setCenter).toHaveBeenCalledTimes(1); + }).then(done); + }, LONG_TIMEOUT_INTERVAL); + + it('@gl should not update zoom while scroll wheeling', function(done) { + var map = gd._fullLayout.mapbox._subplot.map; + spyOn(map, 'setZoom').and.callThrough(); + + _mouseEvent('mousemove', pointPos, noop).then(function() { + return Plotly.relayout(gd, {'mapbox.zoom': 5}); + }).then(function() { + // First relayout on mapbox.zoom results in setZoom call + expect(map.setZoom).toHaveBeenCalledWith(5); + expect(map.setZoom).toHaveBeenCalledTimes(1); + }).then(function() { + mouseEvent('scroll', pointPos[0], pointPos[1], {deltaY: -400}); + return Plotly.relayout(gd, {'mapbox.zoom': 2}).then(function() { + // Second relayout on mapbox.zoom does not result in setZoom + // call since a scroll wheel zoom is underway + expect(map.setZoom).toHaveBeenCalledTimes(1); + }); + }).then(done); + }, LONG_TIMEOUT_INTERVAL); + it('@gl should be able to restyle', function(done) { var restyleCnt = 0; var relayoutCnt = 0; From 4158916421deb440690dd3379a2783419a95112c Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Tue, 10 Dec 2019 13:21:57 -0500 Subject: [PATCH 4/4] Add test case to ensure the layer source image is updated and not removed and recreated. --- test/jasmine/tests/mapbox_test.js | 51 +++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/test/jasmine/tests/mapbox_test.js b/test/jasmine/tests/mapbox_test.js index bf3aceffb13..9dcb3e82304 100644 --- a/test/jasmine/tests/mapbox_test.js +++ b/test/jasmine/tests/mapbox_test.js @@ -935,6 +935,57 @@ describe('@noCI, mapbox plots', function() { .then(done); }, LONG_TIMEOUT_INTERVAL); + it('@gl should be able to update layer image', function(done) { + var coords = [ + [-80.425, 46.437], + [-71.516, 46.437], + [-71.516, 37.936], + [-80.425, 37.936] + ]; + function makeFigure(source) { + return { + data: [{type: 'scattermapbox'}], + layout: { + mapbox: { + layers: [{ + 'sourcetype': 'image', + 'coordinates': coords, + 'source': source + }] + } + } + }; + } + + var map = null; + var layerSource = null; + + // Single pixel PNGs generated with http://png-pixel.com/ + var prefix = 'data:image/png;base64,'; + var redImage = prefix + 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42m' + + 'P8z8DwHwAFBQIAX8jx0gAAAABJRU5ErkJggg=='; + var greenImage = prefix + 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42m' + + 'Nk+M/wHwAEBgIApD5fRAAAAABJRU5ErkJggg=='; + + Plotly.react(gd, makeFigure(redImage)).then(function() { + var mapbox = gd._fullLayout.mapbox._subplot; + map = mapbox.map; + layerSource = map.getSource(mapbox.layerList[0].idSource); + + spyOn(layerSource, 'updateImage').and.callThrough(); + spyOn(map, 'removeSource').and.callThrough(); + return Plotly.react(gd, makeFigure(greenImage)); + }) + .then(function() { + expect(layerSource.updateImage).toHaveBeenCalledWith( + {url: greenImage, coordinates: coords} + ); + expect(map.removeSource).not.toHaveBeenCalled(); + }) + .catch(failTest) + .then(done); + }, LONG_TIMEOUT_INTERVAL); + it('@gl should be able to react to layer changes', function(done) { function makeFigure(color) { return {