flutter_map icon indicating copy to clipboard operation
flutter_map copied to clipboard

[FEATURE] Add function animatedMove() to MapController

Open LostInDarkMath opened this issue 3 years ago • 22 comments

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.

  • [x] All/any

LostInDarkMath avatar Jun 09 '22 11:06 LostInDarkMath

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[bot] avatar Jul 10 '22 02:07 github-actions[bot]

staleLabel.remove()

LostInDarkMath avatar Jul 10 '22 08:07 LostInDarkMath

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.

ibrierley avatar Jul 10 '22 09:07 ibrierley

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[bot] avatar Aug 10 '22 02:08 github-actions[bot]

staleLabel.remove()

LostInDarkMath avatar Aug 10 '22 06:08 LostInDarkMath

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[bot] avatar Sep 10 '22 02:09 github-actions[bot]

void onStaleLabelAdded(StaleLabel staleLabel){
  staleLabel.remove();
}

LostInDarkMath avatar Sep 10 '22 06:09 LostInDarkMath

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);
  }
}

JosefWN avatar Sep 23 '22 13:09 JosefWN

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

JaffaKetchup avatar Sep 23 '22 13:09 JaffaKetchup

Thank you very much @JosefWN!

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

LostInDarkMath avatar Sep 23 '22 13:09 LostInDarkMath

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.

JaffaKetchup avatar Sep 23 '22 13:09 JaffaKetchup

@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.

JosefWN avatar Sep 23 '22 13:09 JosefWN

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.

JaffaKetchup avatar Sep 23 '22 13:09 JaffaKetchup

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 avatar Sep 23 '22 14:09 ibrierley

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.

ibrierley avatar Sep 23 '22 14:09 ibrierley

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.

JosefWN avatar Sep 23 '22 14:09 JosefWN

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?

ibrierley avatar Sep 23 '22 14:09 ibrierley

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 avatar Sep 23 '22 16:09 aytunch

@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?

aytunch avatar Sep 23 '22 16:09 aytunch

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.

JaffaKetchup avatar Sep 23 '22 17:09 JaffaKetchup

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

/me runs off

ibrierley avatar Sep 23 '22 17:09 ibrierley

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.

JosefWN avatar Sep 23 '22 17:09 JosefWN

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

JaffaKetchup avatar Oct 10 '22 20:10 JaffaKetchup

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[bot] avatar Nov 10 '22 02:11 github-actions[bot]

Is anything happening here?

LostInDarkMath avatar Nov 10 '22 05:11 LostInDarkMath

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

JaffaKetchup avatar Nov 10 '22 07:11 JaffaKetchup

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

rorystephenson avatar Dec 07 '22 08:12 rorystephenson

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

JaffaKetchup avatar Dec 07 '22 10:12 JaffaKetchup

@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

rorystephenson avatar Dec 07 '22 10:12 rorystephenson

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 avatar Dec 07 '22 18:12 aytunch