Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Prevent pinch move while pinch zooming #1905

Open
translibrius opened this issue Jun 4, 2024 · 3 comments
Open

[FEATURE] Prevent pinch move while pinch zooming #1905

translibrius opened this issue Jun 4, 2024 · 3 comments
Labels
feature This issue requests a new feature

Comments

@translibrius
Copy link

What do you want implemented?

Hey, love the package!

Simillar to #1396 I would love to have a way to disable pinch move while zooming. This is specifically an issue on a laptop's trackpad (windows tested) when a user wants to zoom in via trackpad, but that pans the map somewhere else.

What other alternatives are available?

Simply disabling the pinch move flag, however then the user will be unable to move the map with two fingers.

Can you provide any other information?

I have video footage for refrence. As you can see the map moves when zooming.

example.mp4

I love this package, so I might take a look at potentially doing a PR for this if it's not too complicated.

Severity

Annoying: Currently have to use workarounds

@translibrius translibrius added the feature This issue requests a new feature label Jun 4, 2024
@translibrius
Copy link
Author

Hey @JaffaKetchup, sorry to tag you, it's been quite a while since I posted this. Users basically can't use the pinch zoom feature on laptops with no external mouse, which is really frustrating for 'em.

Do you plan on fixing this anytime soon or should I dip my head into the source and try to do it myself? I see that you are overhauling the gesture system currently so I'm not sure if trying it myself is the best idea since it will be reworked anyway?

If I should, could you give any pointers of how to get started with this? Thanks 😃

@JaffaKetchup
Copy link
Member

JaffaKetchup commented Dec 5, 2024

Hey @translibrius,
Sorry for not responding sooner, we've been letting the gesture issues stack up.

I'm not sure whether this is a bug or a design question. When using the scroll wheel, or double tapping, the zoom occurs at the pointer location (cursor or tap location). However, it looks like the trackpad is trying to do this (at the cursor location), but it feels kinda broken (as your video shows). I guess the question is whether the trackpad should zoom to the cursor location, or to the center of the map. I'm not sure how to detect whether there is a cursor or not, but maybe that's possible and useful? I'm not quite clear on which you'd prefer?

The work on gesture and event overhaul has been dormant for a while now, so feel free to have a dig yourself. Most of the code should be in the MapInteractiveViewer - but be warned, there's some amount of confusing methods going on :D. I'll have a quick look as well.

The code concerned is probably:

void _handleScalePinchZoomAndMove(
ScaleUpdateDetails details,
bool hasPinchZoom,
bool hasPinchMove,
) {
var newCenter = _camera.center;
var newZoom = _camera.zoom;
// Handle pinch zoom.
if (hasPinchZoom && details.scale > 0.0) {
newZoom = _getZoomForScale(
_mapZoomStart,
details.scale + _scaleCorrector,
);
// Handle starting of pinch zoom.
if (!_pinchZoomStarted && newZoom != _mapZoomStart) {
_pinchZoomStarted = true;
if (!_pinchMoveStarted) {
// We want to call moveStart only once for a movement so don't call
// it if a pinch move is already underway.
widget.controller.moveStarted(MapEventSource.onMultiFinger);
}
}
}
// Handle pinch move.
if (hasPinchMove) {
newCenter = _calculatePinchZoomAndMove(details, newZoom);
if (!_pinchMoveStarted && _lastFocalLocal != details.localFocalPoint) {
_pinchMoveStarted = true;
if (!_pinchZoomStarted) {
// We want to call moveStart only once for a movement so don't call
// it if a pinch zoom is already underway.
widget.controller.moveStarted(MapEventSource.onMultiFinger);
}
}
}
if (_pinchZoomStarted || _pinchMoveStarted) {
widget.controller.moveRaw(
newCenter,
newZoom,
hasGesture: true,
source: MapEventSource.onMultiFinger,
);
}
}
LatLng _calculatePinchZoomAndMove(
ScaleUpdateDetails details,
double zoomAfterPinchZoom,
) {
final oldCenterPt = _camera.project(_camera.center, zoomAfterPinchZoom);
final newFocalLatLong =
_camera.offsetToCrs(_focalStartLocal, zoomAfterPinchZoom);
final newFocalPt = _camera.project(newFocalLatLong, zoomAfterPinchZoom);
final oldFocalPt = _camera.project(_focalStartLatLng, zoomAfterPinchZoom);
final zoomDifference = oldFocalPt - newFocalPt;
final moveDifference = _rotateOffset(_focalStartLocal - _lastFocalLocal);
final newCenterPt = oldCenterPt + zoomDifference + moveDifference.toPoint();
return _camera.unproject(newCenterPt, zoomAfterPinchZoom);
}

But I cannot be sure whether it also is used for touchscreen right now - I'll need to check. Indeed, replacing line 640 to oldCenterPt causes zooming into the map center on the trackpad (and maybe other input forms as well). It looks like the calculation here is not quite right for the trackpad (again, maybe its correct for other input methods that use this block).

@translibrius
Copy link
Author

Thank you for the detailed response!

Indeed, I've set the line 640 to _camera.center, and indeed it seems to solve the issue perfectly for me on the trackpad, now it is working as it should. I could very easily just edit this into a fork or something, checking if the platform is windows, apply this workaround, and all my problems are gone, but I would kind of feel bad since this doesn't really solve it at scale.

I will look into the source more on the weekends in my spare time to try and better understand how it all works under the hood to hopefully make a solution. Perhaps an easy fix would be some interaction flag like FLAG_TRACKPAD_CENTER or something and just return the center if such a flag is set. I'm not sure if it's possible to detect if user is specifically using touchpad pinch as well. The flag could be a simple solution since probably not many people use this package with a trackpad like my app.

I will let you know if I find something useful about this :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This issue requests a new feature
Projects
Status: To do
Development

No branches or pull requests

2 participants