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

[BUG] MacOS: Multitouch introduction causes issues (Flutter 3.3.0) #1354

Closed
9 tasks done
JosefWN opened this issue Sep 1, 2022 · 34 comments · Fixed by #1543
Closed
9 tasks done

[BUG] MacOS: Multitouch introduction causes issues (Flutter 3.3.0) #1354

JosefWN opened this issue Sep 1, 2022 · 34 comments · Fixed by #1543
Labels
bug This issue reports broken functionality or another error

Comments

@JosefWN
Copy link
Contributor

JosefWN commented Sep 1, 2022

What is the bug?

With Flutter's new multitouch support extending to desktop, the default behavior in flutter_map on Flutter 3.3.0 is two finger pan/rotate and pinch to zoom. This is completely in line with the Apple Maps app on Mac for example.

A couple of issues:

  • When I rotate clockwise with my fingers, the map rotates counter-clockwise. EDIT: Fixed in Flutter engine in a coming release.
  • The panning threshold is a bit tight, so you need some speed when you two-finger swipe to get it to continue the panning as an animation. With slight movements there is no animation and the pan stops abruptly, which is not the case in the native Maps application on macOS for example.
  • Pinch to zoom does not center zoom on mouse pointer.
  • Pinch to rotate does not center rotation on mouse pointer.

It seems like #1358 is also related to this umbrella issue (where a resolutions list is provided, there seems to be bound checks for other gestures like scroll to zoom, but not for pinch to zoom). Tracked in a separate ticket.

Combined rotation and zoom in a single gesture (which works just fine on iPad) doesn't seem to work on Mac at all, neither with flutter_map nor Apple Maps, unless it's some setting I need to change on my end. Tried disabling some more complex multitouch gestures in the macOS settings to see if they could be interfering but that was not the case.

As things stand right now I almost liked the old behavior (with scroll to zoom and click & drag to pan) a bit more, although that prevented me from rotating the map intuitively.

What is the expected behaviour?

Rotating clockwise with a touch gesture in Apple Maps or rotating images in Preview (macOS) rotates the canvas clockwise. The same goes for flutter_map built for iPad. It would make sense if it worked the same way on macOS.

How can we reproduce this issue?

Run on macOS with a touchpad, try to rotate the canvas with a touch gesture.

Do you have a potential solution?

No response

Can you provide any other information?

Running flutter_map 3.0.0-beta3 on M1 Macbook Air.

Platforms Affected

MacOS

Severity

Minimum: Allows normal functioning

Frequency

Consistently: Always occurs at the same time and location

Requirements

  • I agree to follow this project's Code of Conduct
  • My Flutter/Dart installation is unaltered, and flutter doctor finds no relevant issues
  • I am using the latest stable version of this package
  • I have checked the FAQs section on the documentation website
  • I have checked for similar issues which may be duplicates

EDIT by @JaffaKetchup: The second bullet point above is a low priority enhancement unrelated to the main issue, so that will be ignored for now. Please create a separate feature request for this, if still wanted.

@JosefWN JosefWN added bug This issue reports broken functionality or another error needs triage This new bug report needs reproducing and prioritizing labels Sep 1, 2022
@JaffaKetchup
Copy link
Member

Hi @JosefWN,
I can't verify this as I don't have a Mac: can you post a screen recording?
In terms of the new image provider mechanisms, I've just seen that after updating. We'll be releasing v3 with 4 counts of deprecated APIs, as we may be rewriting quite a bit of the surrounding code anyways. If you can help migrate to the new APIs, that would be greatly appriciated!
Many thanks!

@JaffaKetchup
Copy link
Member

Just added Flutter 3.3.0 support (in terms of the DecoderBufferCallback and loadBuffer inside the image providers) in #1355. Hopefully this doesn't break support for <3.3.0!

@TesteurManiak
Copy link
Contributor

I've tested the gestures with the sample application on my MacBook M1 Pro:

  • Didn't have any issue with the combined rotation & zoom (can send a video for proof), as you said it might be an issue on your end.
  • Still cannot reproduce the exception you've mentionned in [BUG] Certain CRSs mistreat minZoom #1358

I'm going to take a look at the clockwise rotation, I think you might already be able to customize the behavior with the provided properties but I guess it would be nice to have it implemented by default on MacOS.

For the pinch to zoom on the mouse pointer I wonder if this behavior should be specific to MacOS or generalized to all desktop platforms (maybe web too).

@JosefWN
Copy link
Contributor Author

JosefWN commented Sep 8, 2022

Didn't have any issue with the combined rotation & zoom (can send a video for proof), as you said it might be an issue on your end.

I didn't know that was possible on macOS? I've never been able to do it in any program on my Macbook Air M1, that is, in a single touch gesture (not two gestures), both zoom and rotate similarly to what you can do on iOS. New feature on the Macbook Pro's or some hidden setting somewhere? Yeah can you please send the video, I'm curious if we are talking about the same thing. I will dig out my old Macbook Pro as well just to see if something is wrong with my Macbook Air.

I'm going to take a look at the clockwise rotation, I think you might already be able to customize the behavior with the provided properties but I guess it would be nice to have it implemented by default on MacOS.

In my case I target macOS, Linux, Windows, iOS and Android... can become a lot of code duplicated over every library user's code base.

I will try to make videos as well, showing the difference between how the same app acts on iPad (everything touch-related works since pre-Flutter 3.3.0) and macOS (multiple issues since Flutter 3.3.0). I could try re-installing Flutter before that, just for the heck of it, to make sure it's not my own install that's borked.

For the pinch to zoom on the mouse pointer I wonder if this behavior should be specific to MacOS or generalized to all desktop platforms (maybe web too).

It would be nice (but for web I don't think you have any multitouch gestures?), and in line with how scroll to zoom works in flutter_map as of late.

@JosefWN
Copy link
Contributor Author

JosefWN commented Sep 8, 2022

I will dig out my old Macbook Pro as well just to see if something is wrong with my Macbook Air.

Same on Macbook Pro 2017, just tried rotating and zooming in a combined gesture ("pinch and rotate") on a picture in Preview and it starts flickering, competing for either zoom or rotate. Could be good to be aware of this limitation on the touchpads if you've come to expect the functionality from iOS to carry over to macOS. Perhaps it's only the latest high-end laptops that support this, then.

@TesteurManiak
Copy link
Contributor

@JosefWN
Copy link
Contributor Author

JosefWN commented Sep 8, 2022

Here's the video:
https://user-images.githubusercontent.com/14369698/189156750-8d4abb4e-60f6-46b3-80f3-ebaf2bf0cde4.mp4

Not sure we are talking about the same thing, I can't clearly see the combined gesture in your video.

This is what I'm talking about (example on iOS where it works):
https://user-images.githubusercontent.com/12534742/189218486-9892f53e-82c8-42db-af48-6c3350feb4b5.mov

Pardon the low resolution, apparently Quick Time is not compressing too much so I had to go with a lower resolution. Coming videos will look better.

But as I said, I think this is a touchpad limitation rather than an issue with Flutter. No other app seems to support this behavior on macOS either. Tested with Preview and Affinity Designer and neither seem to support "pinch and rotate" to both zoom and rotate at the same time.

Still cannot reproduce the exception you've mentionned in #1358

Clarified the ticket with steps to reproduce and a screenshot. Running a fresh install of Flutter 3.3.0 and an untouched flutter_map (master).

@JosefWN
Copy link
Contributor Author

JosefWN commented Sep 8, 2022

Example in this video:

  • Clockwise rotation (showing as counterclockwise on the map)
  • Pinch to zoom, mouse in the upper right corner of the window as I'm zooming in, and upper left corner as I'm zooming out, but flutter_map seems to use the center of the window.
test.mp4

@ibrierley
Copy link
Contributor

Out of interest, does it all work correct in Google Maps ?

@TesteurManiak
Copy link
Contributor

TesteurManiak commented Sep 8, 2022

Out of interest, does it all work correct in Google Maps ?

I've tested Google Maps on MacOS with Google Chrome and the pinch to zoom is influenced by the mouse position.
And the map is rotated clockwise.

@JosefWN
Copy link
Contributor Author

JosefWN commented Sep 8, 2022

Out of interest, does it all work correct in Google Maps ?

I've tested Google Maps on MacOS with Google Chrome and the pinch to zoom is influenced by the mouse position.
And the map is rotated clockwise.

Pinch works for me but I've never gotten rotation touch gestures to work in the browser. On macOS flutter_map pre-Flutter 3.3.0 I used to use the scroll, there was a PR fixing support for zooming on cursor position when scrolling: #1191

The touchpad works correctly on Apple Maps (the macOS app), I haven't noticed any issues with flutter_map on iOS either. Couldn't find Google Maps or anything but an ancient Google Earth Pro for installation on macOS to try the native experience.

@JosefWN
Copy link
Contributor Author

JosefWN commented Sep 23, 2022

I have tested a bit more throughly and updated the list in my initial post.

I have also looked around online and found that although there is a JavaScript touch API supporting multitouch, it seems that more complex gestures have scarce support on the web, especially the twist gesture for rotations. Support also seems to be lacking in Safari, so multitouch in the browser on iPad for example is also limited (it seems pinch to zoom works in the browser but not pinch to rotate).

For testing we might need to use native applications like Apple Maps on macOS, if we want to align with the native multitouch behavior and leverage multitouch fully, as opposed to comparing to Google Maps. Hopefully this is somewhat cross-platform too, so we only need one implementation to support multitouch trackpads on laptops.

The touch issues are not major, but enough to be quite annoying to the user experience. If I manage I'll have a look at the touch code, but I have no experience working with touch, so I'd be happy if someone else beat me to it 😅

@ianthetechie
Copy link
Contributor

ianthetechie commented Sep 27, 2022

Just came here to confirm that the pinch rotate (and to a much lesser extent zoom as well) behavior feels pretty unnatural on Mac (esp the rotation bug). If anyone can point me to the relevant code, I'd be happy to take a look. I'm actually a bit new to Flutter dev, but have over 10 years of experience building macOS and iOS apps so I'd be happy to be a sounding board and contribute code or otherwise.

@JaffaKetchup
Copy link
Member

Hi @ianthetechie. I can't point you to the code right now, but all help is appreciated! If you go into the FlutterMap state, I think there's a gesture arena inside, and it's quite a complicated bit of code.

@ianthetechie
Copy link
Contributor

@JaffaKetchup I fired up a good ol debug session and have a vague idea of what's up now... It looks like you're essentially using the Flutter stdlib scale gesture recognizer (small miracle this exists in essentially pure dart... that's on MASSIVE hunk of code!) and just taking the gesture rotation value from the events it generates, so this actually might be an upstream issue. I have a feeling nobody at Google cares about Mac or something. I'll go poke around there a bit more and maybe raise an issue upstream, but an hour or 2 of poking seems to indicate it might not be your problem.

@JaffaKetchup
Copy link
Member

Bug thanks @ianthetechie :). Yeah, I'm not familiar with the gesture code, and you seem to be much more comfortable with it.

I know we use positioned_tap_detector (maybe v2), maybe that's part of the problem? Not sure to be honest. But if you found for rotation that we're just using values from somewhere else and not modifying them in any way, that would certainly indicate an issue on their part.
I guess part of the problem is, we have a very niche set of gesture detections and rules, so nobody probably realised before - if it is indeed their issue.

@ianthetechie
Copy link
Contributor

No worries. I've come up with an MRE demonstrating the issue using nothing but a super vanilla Flutter app and standard components, so the positioned tap detector isn't likely an issue here.

I opened a discussion on the Flutter official discord. Unless someone tells me I'm stupid / "holding it wrong" I'll update you guys here once it's tracked there via a GitHub issue ;)

@ianthetechie
Copy link
Contributor

Acknowledged by a Flutter dev, who just opened a PR: flutter/engine#36444. What a quick turnaround :D

@JaffaKetchup
Copy link
Member

Hooray! Indeed a quick turnaround :)

@brentlintner
Copy link

brentlintner commented Sep 28, 2022

Side note here:

We ran into this regression ourselves (lack of momentum unlike Apple Maps). To get the previous behaviour back (for anyone reading this) just revert flutter_map to v.2.2.0 and Flutter v3.0.5, which seems to work.

Thanks for looking into this btw!! 👏 ❤️

@JaffaKetchup
Copy link
Member

Ok, the flutter/engine#36444 PR seems to have been merged, so it is either already available, or will be available soon. It seems like this is the root cause (please correct me if I am misunderstanding), so I will close this issue for now. Please ping me if you need it reopened!

@JaffaKetchup JaffaKetchup added non-fatal and removed needs triage This new bug report needs reproducing and prioritizing labels Oct 10, 2022
@ianthetechie
Copy link
Contributor

IMO there are still some follow-up tasks here. The upstream issue that was fixed (looks slated for release in 3.4) just fixes the problem with polarity being wrong, which was huge 😂 The "unnatural-ness" described (points 2-4) are still open bugs and I haven't yet had time to determine if they are issues with flutter_map or upstream. I would suspect several of them are in this codebase though so I'm not sure we can close the issue yet.

Once the upstream fix drops in stable, think (someone who knows the ecosystem better correct if I'm wrong) the resolution from the flutter_map side will be to depend on Flutter 3.4.0 (or whatever), no? Coming from Apple dev land or Rust, it's pretty reasonable to only support the latest SDK as a bulid target but idk if that's the norm here.

@JaffaKetchup
Copy link
Member

Hi @ianthetechie, in this case I will reopen.

In terms of depending on the latest Flutter SDK, it should be ok? That is quite confusing in Flutter, as you've got the local Flutter versions that your machine can actually run, then every project has its own version bounds, but sometimes those are ignored. It should be ok, as 3.4 will be non breaking.

@JaffaKetchup JaffaKetchup reopened this Oct 11, 2022
@JosefWN
Copy link
Contributor Author

JosefWN commented Oct 11, 2022

Updated the issue with the current status :)

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Nov 11, 2022
@JosefWN
Copy link
Contributor Author

JosefWN commented Nov 11, 2022

Can we disable the stale bot perhaps? Seems like more trouble than help...

@JaffaKetchup
Copy link
Member

@JosefWN We can I think, but it helps to keep me on top of things when waiting for responses etc.

@JaffaKetchup
Copy link
Member

It would probably be better to add an exception tag so the bot skips certain issues. I'll think about implementing that.

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@JaffaKetchup
Copy link
Member

@JosefWN I think the bottom two points are now fixed in 'master' for v4. Please confirm.

In terms of the remaining point, I think that's more of an enhancement/feature addition than a bug. The 'animation' is known as the fling - and you're right that there is probably a threshold somewhere. But I think this is really low priority.

If you can confirm that the bottom two points are resolved, I'll close this issue.

@JosefWN
Copy link
Contributor Author

JosefWN commented Apr 12, 2023

Pinch to zoom does not center zoom on mouse pointer.

This is still an issue I think, but the other point you mention seems to be solved!

@JaffaKetchup
Copy link
Member

Hey @JosefWN,
When you get a moment, can you re-evaluate whether this issue is still applicable in the 'master' branch with the latest Flutter 3.10 version?
Thanks!

@JaffaKetchup JaffaKetchup changed the title [BUG] Multitouch on macOS (Flutter 3.3.0) [BUG] MacOS: Multitouch introduction causes issues (Flutter 3.3.0) May 20, 2023
@JosefWN
Copy link
Contributor Author

JosefWN commented May 21, 2023

Hi, yes, this is still valid on latest Flutter/Dart/flutter_map/macOS. My guess is that we need to do something analogous to what we did for rotations? #1437

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue reports broken functionality or another error
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants