From 0f73f16cfaa87f3dee4af0dac14db53e8fac39ae Mon Sep 17 00:00:00 2001 From: Olivier Guyot Date: Fri, 5 Apr 2019 10:26:23 +0200 Subject: [PATCH 1/3] View / apply constraints when an interaction starts Previously, an interaction could begin while target values (center/resolution) were out of the allowed range, causing a glitch where the view zoom/position would jump suddenly. --- src/ol/View.js | 4 +++ src/ol/control/Zoom.js | 1 + src/ol/interaction/Interaction.js | 1 + test/spec/ol/view.test.js | 46 +++++++++++++++++++++++++++++++ 4 files changed, 52 insertions(+) diff --git a/src/ol/View.js b/src/ol/View.js index 3dee0cefbd..8bcc32c21b 100644 --- a/src/ol/View.js +++ b/src/ol/View.js @@ -1325,10 +1325,14 @@ class View extends BaseObject { /** * Notify the View that an interaction has started. + * The view state will be resolved to a stable one if needed + * (depending on its constraints). * @api */ beginInteraction() { this.setHint(ViewHint.INTERACTING, 1); + + this.resolveConstraints(0); } /** diff --git a/src/ol/control/Zoom.js b/src/ol/control/Zoom.js index 379e703fc4..851c557c2e 100644 --- a/src/ol/control/Zoom.js +++ b/src/ol/control/Zoom.js @@ -114,6 +114,7 @@ class Zoom extends Control { // upon it return; } + view.resolveConstraints(0); const currentZoom = view.getZoom(); if (currentZoom !== undefined) { const newZoom = view.getConstrainedZoom(currentZoom + delta); diff --git a/src/ol/interaction/Interaction.js b/src/ol/interaction/Interaction.js index 80a9644758..13c2c3758b 100644 --- a/src/ol/interaction/Interaction.js +++ b/src/ol/interaction/Interaction.js @@ -135,6 +135,7 @@ export function zoomByDelta(view, delta, opt_anchor, opt_duration) { const newZoom = view.getConstrainedZoom(currentZoom + delta); const newResolution = view.getResolutionForZoom(newZoom); + view.resolveConstraints(0); if (view.getAnimating()) { view.cancelAnimations(); } diff --git a/test/spec/ol/view.test.js b/test/spec/ol/view.test.js index 8512e3339e..054fc0e4bc 100644 --- a/test/spec/ol/view.test.js +++ b/test/spec/ol/view.test.js @@ -1801,6 +1801,52 @@ describe('ol.View', function() { }); }); +describe('does not start unexpected animations during interaction', function() { + let map; + beforeEach(function() { + map = new Map({ + target: createMapDiv(512, 256) + }); + }); + afterEach(function() { + disposeMap(map); + }); + + it('works when initialized with #setCenter() and #setZoom()', function(done) { + const view = map.getView(); + let callCount = 0; + view.on('change:resolution', function() { + ++callCount; + }); + view.setCenter([0, 0]); + view.setZoom(0); + view.beginInteraction(); + view.endInteraction(); + setTimeout(function() { + expect(callCount).to.be(1); + done(); + }, 500); + }); + + it('works when initialized with #animate()', function(done) { + const view = map.getView(); + let callCount = 0; + view.on('change:resolution', function() { + ++callCount; + }); + view.animate({ + center: [0, 0], + zoom: 0 + }); + view.beginInteraction(); + view.endInteraction(); + setTimeout(function() { + expect(callCount).to.be(1); + done(); + }, 500); + }); +}); + describe('ol.View.isNoopAnimation()', function() { const cases = [{ From 070c1ec029b4e7392954ab39241b8a5d656a0b06 Mon Sep 17 00:00:00 2001 From: ahocevar Date: Fri, 5 Apr 2019 17:13:55 +0200 Subject: [PATCH 2/3] Resolve constraints for View#animate() API calls --- src/ol/View.js | 26 +++++++++++--------- src/ol/control/Zoom.js | 1 - src/ol/interaction/Interaction.js | 3 +-- test/spec/ol/interaction/keyboardpan.test.js | 4 +-- 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/ol/View.js b/src/ol/View.js index 8bcc32c21b..e913348690 100644 --- a/src/ol/View.js +++ b/src/ol/View.js @@ -284,8 +284,6 @@ class View extends BaseObject { */ this.updateAnimationKey_; - this.updateAnimations_ = this.updateAnimations_.bind(this); - /** * @private * @const @@ -452,6 +450,16 @@ class View extends BaseObject { * @api */ animate(var_args) { + if (this.isDef() && !this.getAnimating()) { + this.resolveConstraints(0); + } + this.animate_.apply(this, arguments); + } + + /** + * @param {...(AnimationOptions|function(boolean): void)} var_args Animation options. + */ + animate_(var_args) { let animationCount = arguments.length; let callback; if (animationCount > 1 && typeof arguments[animationCount - 1] === 'function') { @@ -641,11 +649,7 @@ class View extends BaseObject { // prune completed series this.animations_ = this.animations_.filter(Boolean); if (more && this.updateAnimationKey_ === undefined) { - this.updateAnimationKey_ = requestAnimationFrame(this.updateAnimations_); - } - - if (!this.getAnimating()) { - setTimeout(this.resolveConstraints.bind(this), 0); + this.updateAnimationKey_ = requestAnimationFrame(this.updateAnimations_.bind(this)); } } @@ -1085,7 +1089,7 @@ class View extends BaseObject { const callback = options.callback ? options.callback : VOID; if (options.duration !== undefined) { - this.animate({ + this.animate_({ resolution: resolution, center: this.getConstrainedCenter(center, resolution), duration: options.duration, @@ -1312,7 +1316,7 @@ class View extends BaseObject { this.cancelAnimations(); } - this.animate({ + this.animate_({ rotation: newRotation, center: newCenter, resolution: newResolution, @@ -1330,9 +1334,9 @@ class View extends BaseObject { * @api */ beginInteraction() { - this.setHint(ViewHint.INTERACTING, 1); - this.resolveConstraints(0); + + this.setHint(ViewHint.INTERACTING, 1); } /** diff --git a/src/ol/control/Zoom.js b/src/ol/control/Zoom.js index 851c557c2e..379e703fc4 100644 --- a/src/ol/control/Zoom.js +++ b/src/ol/control/Zoom.js @@ -114,7 +114,6 @@ class Zoom extends Control { // upon it return; } - view.resolveConstraints(0); const currentZoom = view.getZoom(); if (currentZoom !== undefined) { const newZoom = view.getConstrainedZoom(currentZoom + delta); diff --git a/src/ol/interaction/Interaction.js b/src/ol/interaction/Interaction.js index 13c2c3758b..7a3cc810a2 100644 --- a/src/ol/interaction/Interaction.js +++ b/src/ol/interaction/Interaction.js @@ -111,7 +111,7 @@ export function pan(view, delta, opt_duration) { const currentCenter = view.getCenter(); if (currentCenter) { const center = [currentCenter[0] + delta[0], currentCenter[1] + delta[1]]; - view.animate({ + view.animate_({ duration: opt_duration !== undefined ? opt_duration : 250, easing: linear, center: view.getConstrainedCenter(center) @@ -135,7 +135,6 @@ export function zoomByDelta(view, delta, opt_anchor, opt_duration) { const newZoom = view.getConstrainedZoom(currentZoom + delta); const newResolution = view.getResolutionForZoom(newZoom); - view.resolveConstraints(0); if (view.getAnimating()) { view.cancelAnimations(); } diff --git a/test/spec/ol/interaction/keyboardpan.test.js b/test/spec/ol/interaction/keyboardpan.test.js index 59f3b7db49..8a6c59da16 100644 --- a/test/spec/ol/interaction/keyboardpan.test.js +++ b/test/spec/ol/interaction/keyboardpan.test.js @@ -24,7 +24,7 @@ describe('ol.interaction.KeyboardPan', function() { describe('handleEvent()', function() { it('pans on arrow keys', function() { const view = map.getView(); - const spy = sinon.spy(view, 'animate'); + const spy = sinon.spy(view, 'animate_'); const event = new MapBrowserEvent('keydown', map, { type: 'keydown', target: map.getTargetElement(), @@ -51,7 +51,7 @@ describe('ol.interaction.KeyboardPan', function() { expect(spy.getCall(3).args[0].center).to.eql([128, 0]); view.setCenter([0, 0]); - view.animate.restore(); + view.animate_.restore(); }); }); From 16e132caeae8eb3f651527723bc086119281e3d3 Mon Sep 17 00:00:00 2001 From: ahocevar Date: Fri, 5 Apr 2019 17:21:23 +0200 Subject: [PATCH 3/3] Mark new animate_ method private --- src/ol/View.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ol/View.js b/src/ol/View.js index e913348690..1628fc83b9 100644 --- a/src/ol/View.js +++ b/src/ol/View.js @@ -457,6 +457,7 @@ class View extends BaseObject { } /** + * @private * @param {...(AnimationOptions|function(boolean): void)} var_args Animation options. */ animate_(var_args) {