rotate about gesture center

[Zzerr0r]

Currently, when rotating via pinch gesture, the map would rotate about the map center. The purpose of this PR is to change this behavior to rotate about the center of the gesture instead.
This has only been tested with a previous version of flutter_map, as it is quite the hassle to get a fork to work locally because you have to change all the imports (at least I didn't find another way) ,which I didn't want to do again for version 3.0.0. So please test before merging.
I didn't add an option for the behavior, as I think it is generally expected to work like this. I can add it though, if required.

[ibrierley]

I think this was fixed some time ago via #1081 ?

[Zzerr0r]

The PR you mentioned was only to zoom into the center of the pinch gesture, not for rotation

[ibrierley]

Not sure if there's some misunderstanding here....currently with a pinchzoom, it will rotate around the gesture center (just tested it again for my sanity)...maybe it's because you are using an old version ?

[Zzerr0r]

could you check again with enableMultiFingerGestureRace: true, ? I just checked with enableMultiFingerGestureRace: false, and this way it actually works, but not with enableMultiFingerGestureRace: true, , at least for me

[ibrierley]

Sure, that will stop it, but then don't do that :D. Jk!

Firstly, it would be good to know what problem you are getting, to set that to true (I'm not very familiar with the race issues).

Secondly, there is some very similar code above it, which "looks" like it does something quite similar.

I think it would be better here to either a) move them (or combine them both) into a separate function if they do very similar things, or b) if they don't, we should document what each is doing specifically, and why they are different.

Just my initial thoughts.

[Zzerr0r]

hahaha, okay, I will try to describe it in depth.
When GestureRace is enabled, you cannot simultaneously zoom and rotate via pinch gestures, you can only do either or.
This is something I want, since I don't want my users to accidentally rotate the map, when they just wanna zoom or vice versa.
So I want to have GestureRace enabled.
The Code from the PR you mentioned (which as you might have noticed is also from me) is only designed to zoom into the center of the pinch gesture. It has nothing to do with rotation per se. The fact that it also rotates about the gesture-center is only a byproduct and it will only act this way if you have simultaneous zooming and rotating enabled via having GestureRace set to false.
If you have GestureRace set to true, the function that was changed in the mentioned PR is not called at all, in the case that you are only rotating. Thus, the desired behavior, of rotating about the gesture-center is not produced in that case.
The code to achieve zooming into the gesture-center is also different from the one that achieves rotating about the gesture-center.
From my current perspective, I don't see a way to sensibly combine them.
Note that including the changes proposed in this PR wont affect the behavior of having GestureRace disabled at all. It only enables rotating about the gesture-center when GestureRace is enabled.
So fundamentally, the difference is: one is for rotating, one is for zooming. the code is also in the expected place for zooming and rotating respectively.
Maybe I misunderstood what you meant with "there is some very similar code above it" ? But I think you are referring to the code that was included from my previous PR. indeed the code is not to many lines above the one I am proposing now, and I agree it looks similar, but I think they are too different or I am just unable to see a way to combine them, because these things were quite complicated to write in the first place and I am just glad it works now.

[JaffaKetchup]

ping @ibrierley

[ibrierley]

I think my problem is the above when it comes to maintaining this, unless I (or someone else!) gets time to have a proper dig...

Ultimately as is, it isn't clear from the code when someone stumbles upon these in a years time (and the above description isn't very succinct either), (also the whole gesture code needs a rethink, but thats for another day/year/century).

As a minimum, I think there should be more clarity in the code, whether that's from documentation/comments within the code, I don't mind.

[Zzerr0r]

Okay, I understand (somewhat).
I've gone back to maintaining my own version of your library anyways now, so this isn't an issue for me at least anymore.
I still think it is weird behavior to have the map rotate about the map-center when using gestures and I think a lot of users (that would like to have gestureRace enabled) could profit from this being changed. If you at any time change your mind about merging, feel free to change or comment the code in a way that makes it clearer what is happening. At the moment, I don't see how I would achieve that to your satisfaction, as I don't fully understand the extent or nature of the unclarity.

[ibrierley]

Personally I don't have any issue with the intention of the change, its purely in the clarity of code when it comes to maintenance.

Things I would probably do if I was writing this (but this is just personal taste here, anyone else feel free to give thoughts)...

1. Separate the two chunks of rotation code into separate functions, with a name that gives clarity to the intention behind it.
2. Add some comments above it.
3. Include some info in there about GestureRace.
4. Make sure with the above, it's clear if someone in the future wants to change that code at all, it's clear for someone to know how to test and that they haven't broken it.

If you look lower down in the code at _getNewEventCenterZoomPosition for example, it may give an idea of what I mean. There's nothing major there, but it does give clarity to the intention of the whole code.

[JaffaKetchup]

@ibrierley This is a very small change, I'm fine with merging this without proper documentation. What's 6 more spaghetti in a bolognese :)? If you're still not convinced, let's try to get this merged for v4.

(cc. @Zzerr0r, @mootw, @TesteurManiak)

[ibrierley]

I guess I'm easy if others are fine!

[JaffaKetchup]

@Zzerr0r Will test tomorrow. If OK, will be merged after #1475 & #1482.

[JaffaKetchup]

LGTM