Mapbox-gl-js: CSS transform issue

[jingsam]

When emit click, zoom, touch event on map, we calculate the mouse position in map as follows:

https://github.com/mapbox/mapbox-gl-js/blob/master/src/util/dom.js#L104-L111

DOM.mousePos = function (el: HTMLElement, e: any) {
    const rect = el.getBoundingClientRect();
    e = e.touches ? e.touches[0] : e;
    return new Point(
        e.clientX - rect.left - el.clientLeft,
        e.clientY - rect.top - el.clientTop
    );
};

This process is simple like this:

The calculation is correct on most cases, except the case that we add css transform property on .mapboxgl-canvas-container. For example, we add rotate(10deg)

If we scale or skew or even transform3d the canvas container, we also can not get the right position from mousePos(). The zoom, pan, touch, click behaviour will be affected:

https://jsfiddle.net/jingsam/3wh5grko/25/

Try to click somewhere on the map.

This issue is also related to the same problem.

Solution

What we want from mousePos is the position is map space, not the viewport space or other space. Luckily, we have offsetX offsetY. CSSOM says:

The offsetX attribute must follow these steps:

1. If the event’s dispatch flag is set, return the x-coordinate of the position where the event occurred relative to the origin of the padding edge of the target node, ignoring the transforms that apply to the element and its ancestors, and terminate these steps.
2. Return the value of the event’s pageX attribute.

offsetX and offsetY are just we want. so mousePos should changed as follows:

DOM.mousePos = function (el: HTMLElement, e: any) {
    e = e.touches ? e.touches[0] : e;
    return new Point(
        e.offsetX,
        e.offsetY
    );
};

[jingsam]

@mourner @ansis any idea on this issue?

[peterqliu]

Per the Mozilla documentation, looks like offsetX is still experimental -- would be worth revisiting when it gets broader browser support

[jingsam]

@peterqliu The browsers compatibility table shows that offsetX/offsetY have been widely supported even with IE6. The only exception is Safari on iOS, but we could fallback to thee.clientX - rect.left - el.clientLeft to not break on iOS. How do you think about it?

[ryanhamley]

MDN doesn't list it as experimental because of browser support but because the spec for offsetX is a working draft. One of the criteria MDN uses to define experimental features is:

Its defining spec is not stable and likely to change significantly. Its syntax and behavior is therefore subject to change in future versions of browsers as the specification changes.

That being said, offsetX and offsetY have been in browsers for some time (e.g. Firefox 39 was released in 2015) and have fairly simple behavior. I'd say we definitely need a fallback or polyfill in case the properties aren't present, but I'm not sure if there's a big risk in using them.

[adamcbrewer]

@jingsam when using offsetX & offsetY there's an issue while still panning and the mouse leaves the map container and enters DOM elements which don't have transforms applied to them. The original issue then returns and actually causes the map to jump between states.

Using your suggestion, this is also related to related to #6079 where using scale has the same effect and jumps all over the place depending on the element the mouse if over.

I think this solution needs to consider how the panning interactions work on the map. However, since this only affects CSS transforms I'm wondering if the rest of the codebase and methods should altered. Perhaps there's a way to pass in some transform arguments to factor in the transforms in the mouse even

[adamcbrewer]

So I've been trying to get my head around figuring out the best way to approach this. As an overview, DOM.mousePos is something that could have unwanted side effects if we modify it. e.offsetX might work here, but since the position data is calculated on current element the offsetX value is going to change as soon as the mouse enters another node, such as drag-panning, since all element nodes have their own offset values.

I think we can stay out of DOM.mousePos, but ultimately we need to know the scale and rotation of the initial event called during _start. When we have this we can modify the Point coordinates accordingly. I have a small example below showing how this works for transform:scale():

// drag_pan.js

_start(e: MouseEvent | TouchEvent) {
    ...
    const pos = DOM.mousePos(this._el, e);
    const rect = this._el.getBoundingClientRect();
    this._initialScale = (e.target.offsetWidth / rect.width);
    this._applyScaleToPos(pos);

    ...
}

_applyScaleToPos(pos) {
    if (this._initialScale && this._initialScale !== 1) {
        // @mapbox/point-geomerty has built-in transformations methods
        // including the ability to rotate
        pos.mult(this._initialScale);
    }
}

_onMove(e: MouseEvent | TouchEvent) {
    e.preventDefault();

    const pos = DOM.mousePos(this._el, e);
    this._applyScaleToPos(pos);
    if (this._lastPos.equals(pos) || (this._state === 'pending' && pos.dist(this._mouseDownPos) < this._clickTolerance)) {
        return;
    }

    ...
}

This is just an example, but just to illustrate where the fix can be applied.

This also works with zooming when also applied in scroll_zoom.js (which fixes #6079). As noted in the comments we can also apply some rotation to the original Point which would actually fix the this main issue.

[adamcbrewer]

Been thinking a little more about this. There are two ways we can offset a css transform: either as an option or calculating the relevant transforms within _start in all handler scripts.

I'd normally avoid adding another option, but even if we calculate the scale within _start I'm not so sure it's as easy to do when the container has been rotated. Besides, if we calculate everything within Mapbox it has more of a likelihood of introducing bugs and cross device/browser headaches, whereas adding an option(s) for transforms puts the responsibility in the hands of the developer.

If adding another option seems better then perhaps something like this:

new Map({
  cssTransformScale: 0.58,
  cssTransformRotate: 45 // degrees
});

// or 

new Map({
  cssTransform: {
    scale: 0.58,
    rotate: 45
  }
});

If someone could guide me in the right direction I'm happy to work on a PR

[adamcbrewer]

So I've just submitted a proposed PR ☝️ for this issue. The functionality works, but once I've had feedback I'll continue with the implementation and writing tests. Any eyes-on would be appreciated!

[jhchance]

DOM.mousePos = function (el , e )

var rect = el.getBoundingClientRect();

let scaleX = window.innerWidth / el.clientWidth;
// let scaleY = window.innerHeight / canvas.height;

e = e.touches ? e.touches[0] : e;
return new __chunk_1.Point(
    (e.clientX - rect.left - el.clientLeft)/scaleX,
    (e.clientY - rect.top - el.clientTop)/scaleX
);

};

[ryanhamley]

A partial fix for this was introduced with https://github.com/mapbox/mapbox-gl-js/pull/10096