Correct mouse/touch position for elements scaled via CSS transform

[kkaefer]

Inspired by #9014, I looked at correcting mouse/touch interaction when the Map is transformed via the CSS transform: scale() property. It scales the screen pixel coordinates to logical map coordinates.

This is distinct from pixelRatio.

I tested in Safari, Chrome and Firefox on macOS.

[jingsam]

How css transform affects el.clientLeft ? Does it return the original size or tranformed size?

[kkaefer]

Good catch, @jingsam. clientLeft does not have the transform applied, so I moved it to outside the multiplication. However, this only matters if a user applies a border or scrollbar to the <div class="mapboxgl-canvas-container"></div> that we use for tracking events, and I'm having a hard time seeing why anyone would do that.

[kkaefer]

Fixes #6079

[kkaefer]

Previous attempt to solve this was discarded in #8017. I tested this PR on iOS Safari and it works as expected.

[kkaefer]

Unfortunately, JSDom doesn't have a layout engine, so our unit tests fail due to division by zero.

[jingsam]

@kkaefer great work. Does it affect performance?

[ryanhamley]

This looks great. I tested it with transform: scale(), padding and margins all set and it worked as expected. Do you have an idea on how to fix the tests?

[kkaefer]

Do you have an idea on how to fix the tests?

We could either mock the getters, but that kind of means we're only testing our mock values. I think a better approach would be to switch unit tests to run in a browser and e.g. use puppeteer on CI to run them.

[asheemmamoowala]

Now that we have selenium tests (#9245), what are the next steps needed to land this PR?

[kkaefer]

what are the next steps needed to land this PR?

Write a selenium test that double clicks/taps, or sends mouse wheel events at a particular location and check that the resulting zoom correctly reflects the location when the container is zoomed via CSS.

[karimnaaji]

@ryanhamley still good for release-d?

[ryanhamley]

@kkaefer I tried to add a Selenium test and it works locally but fails on CI. Do you have any insight into what I might be doing wrong?

[kkaefer]

@ryanhamley it could be related to pixel ratio. I think our Selenium tests on CI are running at 1×, while your tests locally are running at 2×.

[sgolbabaei]

Is this good for release-e? @ryanhamley

[ryanhamley]

@sgolbabaei I need to take a look at this tomorrow or Friday. The Selenium test I wrote fails on CI and I'm not sure how to get it to pass locally and on CI if the problem is the pixel ratio. Looks like it needs to be rebased against master too

[ryanhamley]

After playing around with this some more, I'm not convinced that this fixes the issue. Or at least, I'm running into some confusing behavior. Compare this demo showing the bug and the behavior in this forked version which uses this branch. The click is correct, but zooming behaves differently. Similarly, in this demo which uses this branch, zooming appears to not work as expected. When I have basically the same set up in a local debug page though, things work as expected.

[ryanhamley]

After consulting with the team, it looks like @karimnaaji is working on implementing dynamic scaling in a way that would make this change unnecessary. I'm going to close this PR in anticipation of that work being a more robust solution. If we find that the need for this still exists in the future, we can revisit it.

[adamcbrewer]

@ryanhamley @karimnaaji is there any update with regards to dynamic scaling?