Skip to content

Commit

Permalink
Disable snapping to pixel on animated markers (#11167)
Browse files Browse the repository at this point in the history
* debounce rounding position on Marker
  • Loading branch information
malekeym committed Dec 17, 2021
1 parent cc4db4e commit 5058720
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 20 deletions.
65 changes: 45 additions & 20 deletions src/ui/marker.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ export default class Marker extends Evented {
_rotationAlignment: string;
_originalTabIndex: ?string; // original tabindex of _element
_fadeTimer: ?TimeoutID;
_updateFrameId: number;
_updateMoving: () => void;

constructor(options?: Options, legacyOptions?: Options) {
super();
Expand Down Expand Up @@ -110,6 +112,7 @@ export default class Marker extends Evented {
this._rotation = options && options.rotation || 0;
this._rotationAlignment = options && options.rotationAlignment || 'auto';
this._pitchAlignment = options && options.pitchAlignment && options.pitchAlignment !== 'auto' ? options.pitchAlignment : this._rotationAlignment;
this._updateMoving = () => this._update(true);

if (!options || !options.element) {
this._defaultMarker = true;
Expand Down Expand Up @@ -184,10 +187,13 @@ export default class Marker extends Evented {
* .addTo(map); // add the marker to the map
*/
addTo(map: Map) {
if (map === this._map) {
return this;
}
this.remove();
this._map = map;
map.getCanvasContainer().appendChild(this._element);
map.on('move', this._update);
map.on('move', this._updateMoving);
map.on('moveend', this._update);
map.on('remove', this._clearFadeTimer);
map._addMarker(this);
Expand All @@ -213,7 +219,7 @@ export default class Marker extends Evented {
remove() {
if (this._map) {
this._map.off('click', this._onMapClick);
this._map.off('move', this._update);
this._map.off('move', this._updateMoving);
this._map.off('moveend', this._update);
this._map.off('mousedown', this._addDragHandler);
this._map.off('touchstart', this._addDragHandler);
Expand Down Expand Up @@ -268,7 +274,7 @@ export default class Marker extends Evented {
this._lngLat = LngLat.convert(lnglat);
this._pos = null;
if (this._popup) this._popup.setLngLat(this._lngLat);
this._update();
this._update(true);
return this;
}

Expand Down Expand Up @@ -446,41 +452,60 @@ export default class Marker extends Evented {
position.y >= 0 && position.y < tr.height;
}

_update(e?: {type: 'move' | 'moveend'}) {
if (!this._map) return;
_updateDOM() {
const pos = this._pos || new Point(0, 0);
const pitch = this._calculatePitch();
const rotation = this._calculateRotation();
this._element.style.transform = `${anchorTranslate[this._anchor]} translate(${pos.x}px, ${pos.y}px) rotateX(${pitch}deg) rotateZ(${rotation}deg)`;
}

if (this._map.transform.renderWorldCopies) {
this._lngLat = smartWrap(this._lngLat, this._pos, this._map.transform);
_calculatePitch() {
if (this._pitchAlignment === "viewport" || this._pitchAlignment === "auto") {
return 0;
} if (this._pitchAlignment === "map") {
return this._map.getPitch();
}
return 0;
}

this._pos = this._map.project(this._lngLat)._add(this._transformedOffset());

let rotation = "";
_calculateRotation() {
if (this._rotationAlignment === "viewport" || this._rotationAlignment === "auto") {
rotation = `rotateZ(${this._rotation}deg)`;
} else if (this._rotationAlignment === "map") {
rotation = `rotateZ(${this._rotation - this._map.getBearing()}deg)`;
return this._rotation;
} if (this._rotationAlignment === "map") {
return this._rotation - this._map.getBearing();
}
return 0;
}

let pitch = "";
if (this._pitchAlignment === "viewport" || this._pitchAlignment === "auto") {
pitch = "rotateX(0deg)";
} else if (this._pitchAlignment === "map") {
pitch = `rotateX(${this._map.getPitch()}deg)`;
_update(delaySnap?: boolean) {
window.cancelAnimationFrame(this._updateFrameId);
if (!this._map) return;

if (this._map.transform.renderWorldCopies) {
this._lngLat = smartWrap(this._lngLat, this._pos, this._map.transform);
}

this._pos = this._map.project(this._lngLat)._add(this._transformedOffset());

// because rounding the coordinates at every `move` event causes stuttered zooming
// we only round them when _update is called with `moveend` or when its called with
// no arguments (when the Marker is initialized or Marker#setLngLat is invoked).
if (!e || e.type === "moveend") {
if (delaySnap === true) {
this._updateFrameId = window.requestAnimationFrame(() => {
if (this._element && this._pos && this._anchor) {
this._pos = this._pos.round();
this._updateDOM();
}
});
} else {
this._pos = this._pos.round();
}

this._map._requestDomTask(() => {
if (!this._map) return;

if (this._element && this._pos && this._anchor) {
this._element.style.transform = `${anchorTranslate[this._anchor]} translate(${this._pos.x}px, ${this._pos.y}px) ${pitch} ${rotation}`;
this._updateDOM();
}

if ((this._map.getTerrain() || this._map.getFog()) && !this._fadeTimer) {
Expand Down
45 changes: 45 additions & 0 deletions test/unit/ui/marker.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {createMap as globalCreateMap} from '../../util/index.js';
import Marker from '../../../src/ui/marker.js';
import Popup from '../../../src/ui/popup.js';
import LngLat from '../../../src/geo/lng_lat.js';
import {Event} from '../../../src/util/evented.js';
import Point from '@mapbox/point-geometry';
import simulate from '../../util/simulate_interaction.js';

Expand Down Expand Up @@ -921,3 +922,47 @@ test('Marker and fog', (t) => {
});
});
});

test('Snap To Pixel', (t) => {
const map = createMap(t);
const marker = new Marker({draggable: true})
.setLngLat([1, 2])
.addTo(map);
t.test("Snap To Pixel immediately after initializing marker", (t) => {
t.same(marker._pos, marker._pos.round());
t.end();
});
t.test("Not Immediately Snap To Pixel After setLngLat", (t) => {
marker.setLngLat([2, 1]);
const pos = marker._pos;
setTimeout(() => {
t.notSame(marker._pos, pos);
t.same(marker._pos, pos.round());
t.end();
}, 100);
});
t.test("Immediately Snap To Pixel on moveend", (t) => {
map.fire(new Event("moveend"));
t.same(marker._pos, marker._pos.round());
t.end();
});
t.test("Not Immediately Snap To Pixel when Map move", (t) => {
map.fire(new Event("move"));
t.notSame(marker._pos, marker._pos.round());
window.requestAnimationFrame(() => {
t.same(marker._pos, marker._pos.round());
t.end();
});
});
t.test("Not Immediately Snap To Pixel when Map move and setLngLat", (t) => {
marker.setLngLat([1, 2]);
map.fire(new Event("move"));
t.notSame(marker._pos, marker._pos.round());
setTimeout(() => {
t.same(marker._pos, marker._pos.round());
t.end();
}, 100);
});
map.remove();
t.end();
});

0 comments on commit 5058720

Please sign in to comment.