fleaflet / flutter_map

A versatile mapping package for Flutter. Simple and easy to learn, yet completely customizable and configurable, it's the best choice for mapping in your Flutter app.
https://pub.dev/packages/flutter_map
BSD 3-Clause "New" or "Revised" License
2.74k stars 860 forks source link

[FEATURE] Add function animatedMove() to MapController #1263

Closed LostInDarkMath closed 1 year ago

LostInDarkMath commented 2 years ago

Describe The Problem I really like the _animatedMapMove() function from one of your example projects: https://github.com/fleaflet/flutter_map/blob/master/example/lib/pages/animated_map_controller.dart#L42 I find myself copying those in all of my projects taht uses your library.

Describe Your Solution I think it would be nice if this function would became part of your API, maybe of the MapController. But I'm not sure if and how this could be implemented because if depends on the TickerProviderStateMixin.

Applicable Platforms Select platforms that this request applies to.

github-actions[bot] commented 2 years ago

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.

LostInDarkMath commented 2 years ago

staleLabel.remove()

ibrierley commented 2 years ago

My bugbear with this one, is that actually it doesn't work properly.

It looks better than nothing though (especially at similar zoom levels), but I'd probably wince at merging something that I know is broken :D. (Although we do have the same code in the double tap controller...so maybe we could just expose that and then fix it later, so it's at least in once place...or let it be overridden)

If you want to see what I mean, set the AnimationController duration to 5000ms rather than 500ms, click on Paris and then London and vice versa (key being, they have different zooms). You will notice that London actually goes away and off the screen before coming back.

You can't really tween in a simple way a scale and a translation, as the scale affects the translation. I did do a flyTo conversion which corrected that, but I think it was kinda felt maybe it polluted the code a bit, but maybe we should get a plugin working for it (or as I said, expose some method that you can pass your own thing to as well), as it is a good feature.

github-actions[bot] commented 2 years ago

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.

LostInDarkMath commented 2 years ago

staleLabel.remove()

github-actions[bot] commented 2 years ago

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.

LostInDarkMath commented 2 years ago
void onStaleLabelAdded(StaleLabel staleLabel){
  staleLabel.remove();
}
JosefWN commented 2 years ago

I have forked @ibrierley's flyTo and included rotation which I needed. It's relatively straight-forward to extend the MapControllerImpl creating an AnimatedMapController, although it requires https://github.com/fleaflet/flutter_map/pull/1357 to function.

Perhaps we could change the Animated MapController example to utilize this? It's better than what we have and wouldn't require any changes to the flutter_map core. We might need to test the curve a bit though, I have the impression (just visually) that the animation is fairly linear, or perhaps I'm just so used to easeInOut.

EDIT: Also not 100% sure about my weighting on the rotation, using (u(s) / u1).

import 'dart:math' as math;

import 'package:flutter/material.dart';
import 'package:flutter/widgets.dart';
import 'package:flutter_map/plugin_api.dart';
import 'package:latlong2/latlong.dart';

// FlyTo Converted from leaflet.js map.js by ibrierley, modified by JosefWN
/*
  BSD 2-Clause License
  Copyright (c) 2010-2022, Vladimir Agafonkin
  Copyright (c) 2010-2011, CloudMade
  All rights reserved.
  Redistribution and use in source and binary forms, with or without
  modification, are permitted provided that the following conditions are met:
  1. Redistributions of source code must retain the above copyright notice, this
     list of conditions and the following disclaimer.
  2. Redistributions in binary form must reproduce the above copyright notice,
     this list of conditions and the following disclaimer in the documentation
     and/or other materials provided with the distribution.
  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
  AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
  IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
  DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
  FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
  DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
  SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
  CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
  OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/

class AnimatedMapController extends MapControllerImpl {
  bool _flying = false;
  late FlutterMapState _state;

  AnimatedMapController();

  @override
  set state(final FlutterMapState state) {
    _state = state;
    super.state = state;
  }

  void flyTo(
    final LatLng targetCenter, {
    final double? zoom,
    final double? rotation,
    final Curve curve = Curves.easeInOut,
    final Duration? duration,
  }) {
    if (_flying) {
      return;
    }

    _flying = true;
    final from = _state.project(_state.center);
    final to = _state.project(targetCenter);
    final size = _state.size;
    final startZoom = _state.zoom;
    final startRotation = _state.rotation;

    final targetZoom = zoom ?? startZoom;
    final targetRotation = rotation ?? startRotation;

    final w0 = math.max(size.x, size.y);
    final w1 = w0 * _state.getZoomScale(startZoom, targetZoom);
    var u1 = to.distanceTo(from);

    if (u1 == 0) {
      u1 = 1;
    }
    const rho = 1.42;
    const rho2 = rho * rho;

    double r(final int i) {
      final s1 = i == 1 ? -1 : 1;
      final s2 = i == 1 ? w1 : w0;
      final t1 = w1 * w1 - w0 * w0 + s1 * rho2 * rho2 * u1 * u1;
      final b1 = 2 * s2 * rho2 * u1;
      final b = t1 / b1;
      final sq = math.sqrt(b * b + 1) - b;

      // workaround for floating point precision bug when sq = 0,
      // log = -Infinite, thus triggering an infinite loop in flyTo
      return sq < 0.000000001 ? -18 : math.log(sq);
    }

    double sinh(final double n) => (math.exp(n) - math.exp(-n)) / 2;

    double cosh(final double n) => (math.exp(n) + math.exp(-n)) / 2;

    double tanh(final double n) => sinh(n) / cosh(n);

    final r0 = r(0);

    double w(final double s) => w0 * (cosh(r0) / cosh(r0 + rho * s));

    double u(final double s) =>
        w0 * (cosh(r0) * tanh(r0 + rho * s) - sinh(r0)) / rho2;

    final start = DateTime.now();
    final S = (r(1) - r0) / rho;

    final millis = duration != null ? duration.inMilliseconds : 1000 * S * 0.8;

    Future<void> frame() async {
      final t = DateTime.now().difference(start).inMilliseconds / millis;

      if (t <= 1) {
        final progress = curve.transform(t);
        final s = progress * S;
        final newPos = _state.unproject(
          from + ((to - from).multiplyBy(u(s) / u1)),
          startZoom,
        );
        final newZoom = _state.getScaleZoom(w0 / w(s), startZoom);
        final newRotation =
            startRotation + (targetRotation - startRotation) * progress;

        WidgetsBinding.instance.addPostFrameCallback((final _) {
          _state.moveAndRotate(
            newPos,
            newZoom,
            newRotation,
            source: MapEventSource.flingAnimationController,
          );
          frame();
        });
        WidgetsBinding.instance.ensureVisualUpdate();
      } else {
        _state.moveAndRotate(
          targetCenter,
          targetZoom,
          targetRotation,
          source: MapEventSource.flingAnimationController,
        );
        _flying = false;
      }
    }

    frame();
  }

  void flyToBounds(
    final LatLngBounds bounds,
    final FitBoundsOptions options, {
    final Curve curve = Curves.easeInOut,
    final Duration? duration,
  }) {
    final target = _state.getBoundsCenterZoom(bounds, options);
    flyTo(target.center, zoom: target.zoom, curve: curve, duration: duration);
  }
}
JaffaKetchup commented 2 years ago

@JosefWN If you get a spare moment, we'd love to have this as a plugin :)

LostInDarkMath commented 2 years ago

Thank you very much @JosefWN!

I'd like to naively ask again why this can't end up directly in the library?

JaffaKetchup commented 2 years ago

Fair point @LostInDarkMath. @ibrierley What do you think? My default response to feature implementations is now plugins, if they are easy-ish to do that way, but something as simple as this might fit in well.

JosefWN commented 2 years ago

@JosefWN If you get a spare moment, we'd love to have this as a plugin :)

I could do it if @ibrierley doesn't have time, although he wrote 99% of the code, so it would be more fair to attribute it to him, even though it's a port.

JaffaKetchup commented 2 years ago

Maybe someone can make a plugin flutter_map_animations? This could include animated MapControllers, animated Markers, etc. But yeah, for now I think it is better to stick to only the most basic animations (such as tile fade in) in this repo.

In the meantime until we make a decision, I'm happy to add this snippet to the docs as a better manual method of implementing an animated MapController.

ibrierley commented 2 years ago

I'm 50/50 on this, which is partly why I never did either :D. I don't really have time to work on that really atm (but would try and help out if there were bugs), so happy for @JosefWN or anyone else to do a plugin or integrate.

My main issue was I didn't want chunks of code in the core that only one person knew for maintenance reasons. If someone else adapts and tests, then at least there's 2 :D, and it's fairly standalone (atm). I'm also a bit wary of too many plugins for people, and major version changes meaning every plugin needing to update.

It also would partly remove one of my minor gripes that the current animation is kind of buggy, as you can't properly animate rotation/scale/translation together like we do, as scale affects the translation transformation (which is why if you look closely, currently in some cases an animation briefly translates in the wrong direction before looking more correct..someone who is good at maths may be able to sort that).

Initially I also wanted to replace FMs double tap type animation with it, however I'm not sure if there is an easy way to integrate that without getting a bit too entangled with all the gesture code and again adding to maintenance complexity. That would sway me to think including in FM was the way to go, but again, only if the way it's integrated looks intuitive. (could double tap just call flyTo? Ive lost a bit of track with it)

ibrierley commented 2 years ago

Actually, it would probably need to be tested for double tap before bothering to integrate anyway, as it may not be the correct movement feel for a very quick movement like double taps needs.

JosefWN commented 2 years ago

My main issue was I didn't want chunks of code in the core that only one person knew for maintenance reasons.

Yeah, it's also not idiomatic/intuitive to "manually" animate from a Dart/Flutter perspective, and the code is pretty complex/mathematical. Then again I suspect that over time this piece of code will remain fairly static once thoroughly tested.

Maybe starting out as a plugin and then potentially graduating to inclusion in FM could be a good approach? Looking at other libraries like Mapbox it's an integrated feature which users might come to expect of us too in the long run.

ibrierley commented 2 years ago

A plugin maybe a good way to 1) get it out there and tested for bugs :), 2) get some others familiar with it, so that if ever needed it's not so alien.

Just out of interest, looking at the original leaflet code, there is also flyToBounds...

    flyToBounds: function (bounds, options) {
        var target = this._getBoundsCenterZoom(bounds, options);
        return this.flyTo(target.center, target.zoom, options);
    },

Maybe that could be easy to incorporate as well with the code above, using _state.getBoundsCenterZoom(bounds, options) calling flyTo as an extra feature?

aytunch commented 2 years ago

I totally agree with @ibrierley on this statement.

I'm also a bit wary of too many plugins for people, and major version changes meaning every plugin needing to update.

Every time flutter_map is upgraded to use newer dependencies, we need to open new issues in flutter map plugin repos and wait for them to catch up. For niche plugins this is the only way, but "flying" is more of a functionality than a plugin imho.

aytunch commented 2 years ago

@JosefWN about your concern on the animation curve

that the animation is fairly linear, or perhaps I'm just so used to easeInOut

Why don't we expose the curve parameter and let the devs choose it according to their like?

JaffaKetchup commented 2 years ago

Yeah, that's a good point. For something as small as this, the extra work needed to keep it updated with flutter_map as a plugin probably will be unsustainable. I think if we do merge this into the project, it needs to have very good in-code comments so that it's clear what it all does for future maintainers that may not be familiar.

ibrierley commented 2 years ago

Who needs in code comments when you have https://www.win.tue.nl/~vanwijk/zoompan.pdf#page=5

/me runs off

JosefWN commented 2 years ago

Why don't we expose the curve parameter and let the devs choose it according to their like?

Good point! Swapped the easeOut function for a Curve in my example above. Still feels like something is slightly off with the animation, but I'll have to test a bit, maybe it's just me.

EDIT:

Just out of interest, looking at the original leaflet code, there is also flyToBounds...

Added an (untested) flyToBounds in the example above.

JaffaKetchup commented 2 years ago

Hi there, bit of a while since this one was in conversation, and I'm trying to clean up :D. Is anything happening here?

github-actions[bot] commented 1 year ago

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.

LostInDarkMath commented 1 year ago

Is anything happening here?

JaffaKetchup commented 1 year ago

@JosefWN Can you make a PR for your animation functions if you've got a moment?

rorystephenson commented 1 year ago

Big ➕ from a plugin author's perspective for adding this to flutter_map. I've implemented it in all of the clustering/popup plugins I've written and it's non-trivial to get right when you consider (optionally) stopping the animation when other gestures occur and avoiding multiple animations running at once. I haven't looked in to other implementations but if it can be useful here is mine which you are welcome to use however you want: https://github.com/rorystephenson/flutter_map_supercluster/blob/ded3b290b240e7568066299f29a156ee04aee1f6/lib/src/layer/center_zoom_controller.dart

JaffaKetchup commented 1 year ago

(Hey @rorystephenson, don't think your plugin is listed yet on the docs. Would you like me to add it?)

rorystephenson commented 1 year ago

@JaffaKetchup Sure, thanks :)

There is also https://github.com/rorystephenson/flutter_map_radius_cluster which is probably not listed.

I also just updated my center zoom controller after retesting the behaviour when a center zoom is fired before the last one finished and finding it was broken. I also modified it to emit map events for when the center zoom starts/finishes so that this can be listened to.

https://github.com/rorystephenson/flutter_map_supercluster/blob/c018de4daf534c52bda22e77a4e962445ce3d569/lib/src/layer/center_zoom_controller.dart

aytunch commented 1 year ago

If I understood the math correctly from @rorystephenson 's center zoom code, instead of using a fixed duration for the animation, he uses a fixed velocity for the flying animation. So flying from London to Paris and from Madrid to New York won't have such a big difference in animation speed. This feels better in theory but I guess we need to do some bench-marking from Apple Maps or Google Maps or Leaflet.

Edit: I just realized that this is for the Zoom animation. We need this logic especially on the lat/long animation for a smooth transitioning.

In the example animated_move function we are using a 500ms fixed duration for both the lat/long and zoom animations


    // Create a animation controller that has a duration and a TickerProvider.
    final controller = AnimationController(
        duration: const Duration(milliseconds: 500), vsync: this);

Maybe We can use Rory's velocity logic instead of Duration or a combination of both? I dunno :D

aytunch commented 1 year ago

A small benchmark from Apple Maps:

https://user-images.githubusercontent.com/6442915/206270754-91ca430c-31ea-4376-b0a2-3f0c5b74e796.mov

A small benchmark from Google Maps:

https://user-images.githubusercontent.com/6442915/206271484-f2416db1-3f61-49fd-a5f5-65a201f12743.mov

JaffaKetchup commented 1 year ago

Interesting to see that Apple Maps doesn't really do the whole zoom thing, but Google does. Maybe this should be an option?

@rorystephenson Both added to the docs site: https://docs.fleaflet.dev/plugins/list. Let me know if this works for you!

aytunch commented 1 year ago

@JaffaKetchup Apple Maps do the zoom animation only when we are going to a lat/long which is already in the current bounds. (New York to Manhattan) But it doesn't do zoom animation when the distance is too much or when the destination is not already on the viewport. Google does it even in big distances but the flying animation is not pleasant. It is janky.

rorystephenson commented 1 year ago

If I understood the math correctly from @rorystephenson 's center zoom code, instead of using a fixed duration for the animation, he uses a fixed velocity for the flying animation. So flying from London to Paris and from Madrid to New York won't have such a big difference in animation speed. This feels better in theory but I guess we need to do some bench-marking from Apple Maps or Google Maps or Leaflet.

Edit: I just realized that this is for the Zoom animation. We need this logic especially on the lat/long animation for a smooth transitioning.

In the example animated_move function we are using a 500ms fixed duration for both the lat/long and zoom animations


    // Create a animation controller that has a duration and a TickerProvider.
    final controller = AnimationController(
        duration: const Duration(milliseconds: 500), vsync: this);

Maybe We can use Rory's velocity logic instead of Duration or a combination of both? I dunno :D

Yeah I avoided a fixed duration as I would end up with values that were too fast for big distances and too slow for small distances. That said my end result uses a few 'magic numbers' which is not ideal.

JosefWN commented 1 year ago

The code written by @ibrierley and modified by me suffers from the same issue to some extent:

final millis = duration != null ? duration.inMilliseconds : 1000 * S * 0.8;

It's good at predicting a reasonable animation duration for longer distances, but with my addition of rotation (for example if you are in navigation mode), the duration gets very close to zero for shorter flyTo's although the rotation is significant. This will cause the viewport to snap rather than rotate with animation, so there should probably be a minimum animation duration...

github-actions[bot] commented 1 year ago

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.

LostInDarkMath commented 1 year ago

staleLabel.remove();

JaffaKetchup commented 1 year ago

Hi all. This is now solved by @TesteurManiak's plugin: https://github.com/TesteurManiak/flutter_map_animations. Apologies for the delay!