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.76k stars 860 forks source link

[BUG] Certain CRSs mistreat `minZoom` #1358

Open JosefWN opened 2 years ago

JosefWN commented 2 years ago

What is the bug?

I get the following exception when zooming out as far as possible without explicitly setting minZoom to 0:

                      ══╡ EXCEPTION CAUGHT BY GESTURE ╞═══════════════════════════════════════════════════════════════════
                      The following RangeError was thrown while handling a gesture:
                      RangeError (index): Invalid value: Not in inclusive range 0..14: -1

                      When the exception was thrown, this was the stack:
                      #0      _Array.[] (dart:core-patch/array.dart:10:36)
                      #1      Proj4Crs.scale (package:flutter_map/src/geo/crs/crs.dart:289:32)
                      #2      FlutterMapState.getZoomScale (package:flutter_map/src/map/flutter_map_state.dart:567:16)
                      #3      FlutterMapState.getPixelBounds (package:flutter_map/src/map/flutter_map_state.dart:592:19)
                      #4      FlutterMapState.move (package:flutter_map/src/map/flutter_map_state.dart:457:20)
                      #5      MapGestureMixin.handleScaleUpdate (package:flutter_map/src/gestures/gestures.dart:452:35)
                      #6      ScaleGestureRecognizer._advanceStateMachine.<anonymous closure> (package:flutter/src/gestures/scale.dart:642:18)
                      #7      GestureRecognizer.invokeCallback (package:flutter/src/gestures/recognizer.dart:253:24)
                      #8      ScaleGestureRecognizer._advanceStateMachine (package:flutter/src/gestures/scale.dart:641:7)
                      #9      ScaleGestureRecognizer.handleEvent (package:flutter/src/gestures/scale.dart:497:7)
                      #10     PointerRouter._dispatch (package:flutter/src/gestures/pointer_router.dart:98:12)
                      #11     PointerRouter._dispatchEventToRoutes.<anonymous closure> (package:flutter/src/gestures/pointer_router.dart:143:9)
                      #12     _LinkedHashMapMixin.forEach (dart:collection-patch/compact_hash.dart:617:13)
                      #13     PointerRouter._dispatchEventToRoutes (package:flutter/src/gestures/pointer_router.dart:141:18)
                      #14     PointerRouter.route (package:flutter/src/gestures/pointer_router.dart:127:7)
                      #15     GestureBinding.handleEvent (package:flutter/src/gestures/binding.dart:460:19)
                      #16     GestureBinding.dispatchEvent (package:flutter/src/gestures/binding.dart:440:22)
                      #17     RendererBinding.dispatchEvent (package:flutter/src/rendering/binding.dart:337:11)
                      #18     GestureBinding._handlePointerEventImmediately (package:flutter/src/gestures/binding.dart:395:7)
                      #19     GestureBinding.handlePointerEvent (package:flutter/src/gestures/binding.dart:357:5)
                      #20     GestureBinding._flushPointerEventQueue (package:flutter/src/gestures/binding.dart:314:7)
                      #21     GestureBinding._handlePointerDataPacket (package:flutter/src/gestures/binding.dart:295:7)
                      #22     _invoke1 (dart:ui/hooks.dart:167:13)
                      #23     PlatformDispatcher._dispatchPointerDataPacket (dart:ui/platform_dispatcher.dart:341:7)
                      #24     _dispatchPointerDataPacket (dart:ui/hooks.dart:94:31)

                      Handler: "onUpdate"
                      Recognizer:
                        ScaleGestureRecognizer#9330d

What is the expected behaviour?

I would expect the default for minZoom to be 0 if not set (or make the parameter required, if it is).

How can we reproduce this issue?

Omit `minZoom` in the options, zoom out as far as possible.

Do you have a potential solution?

No response

Can you provide any other information?

This is on flutter_map 3.0.0.

Platforms Affected

MacOS

Severity

Minimum: Allows normal functioning

Frequency

Consistently: Always occurs at the same time and location

Requirements

JaffaKetchup commented 2 years ago

Hi @JosefWN, @TesteurManiak (a maintainer) is having problems reproducing your issue. Can you send a minimal reproducible example please? Many thanks!

TesteurManiak commented 2 years ago

Yeah, just to give a bit more context here's my sample:

Code sample ```dart class Issue1358Page extends StatefulWidget { const Issue1358Page({Key? key}) : super(key: key); @override State createState() => _Issue1358PageState(); } class _Issue1358PageState extends State { final _mapController = MapController(); @override void dispose() { _mapController.dispose(); super.dispose(); } @override Widget build(BuildContext context) { return Scaffold( appBar: AppBar(title: const Text('Issue 1358 - bug when minZoom not set')), floatingActionButton: FloatingActionButton( child: const Icon(Icons.zoom_out), onPressed: () => _mapController.move(_mapController.center, _mapController.zoom - 1), ), body: FlutterMap( mapController: _mapController, options: MapOptions( center: LatLng(51.5, -0.09), zoom: 5, ), children: [ TileLayer( urlTemplate: 'https://tile.openstreetmap.org/{z}/{x}/{y}.png', userAgentPackageName: 'dev.fleaflet.flutter_map.example', ), ], ), ); } } ```

I've tested on MacOS and iOS and was not able to reproduce this exception either by using the MapController or the gestures.

ibrierley commented 2 years ago

My guess would be the initial problem is using a custom crs and not providing the relevant resolution or something ?

JosefWN commented 2 years ago

My guess would be the initial problem is using a custom crs and not providing the relevant resolution or something ?

Yes, exactly. Sorry for the brief description, thought it was a broader problem, although I hadn't seen it until recently. My code actually looks very similar to the EPSG:3413 example.

The problem is with the touch gestures on macOS introduced in Flutter 3.3.0, if I pinch to zoom out the error occurs in that example too.

JosefWN commented 2 years ago

Reinstalled Flutter just in case:

brew update
brew uninstall flutter
brew cleanup
brew install flutter # 3.3.0

In the example/ folder on flutter_map/master:

flutter clean
flutter pub get
flutter create --platforms=macos .
# Add entitlement com.apple.security.network.client
flutter run -d macos

Go to the example "EPSG3413 CRS", then use "pinch to zoom" to zoom out as far as possible. So not just one pinch gesture, it takes several. This is what I get:

Screen Shot 2022-09-08 at 18 49 02
ibrierley commented 2 years ago

Yes, we've kind of been here before. The problem is one should set the minZoom/maxZoom to match the resolutions (we did ponder creating a power of 2 multiplier/divisor for ones that don't exist.

Worth a read of https://github.com/fleaflet/flutter_map/issues/1223

The example should probably have min/maxZooms or relevant resolutions added. Whether there's a "safe" fix for all crs & resolutions that may not be there, I'm not much of an expert in that area.

JosefWN commented 2 years ago

But maxZoom is a little different, isn't minZoom always equal to or greater than zero? At least with a resolutions list we couldn't possibly index resolutions[-1] since negative indices are not not valid in Dart.

Even if we could have a minZoom < 0 I'm just arguing that setting the default to 0 might be preferable, and let users override with negative min values if they are in need of those. That way we avoid surprises with index out of bounds on the negative end.

ibrierley commented 2 years ago

Yes, I don't have an issue with that (but the same issue with zoom greater than resolutions index will still exist I think).

JosefWN commented 2 years ago

Yes, but that's a bit more intuitive to provide. We could perhaps store and expose the resolutions list in the relevant Crs (such as Proj4Crs)? Then the default could be to bound/clamp zoom to the array indices in the list.

ibrierley commented 2 years ago

Yes, I think that makes sense, and was thinking along those lines, just haven't had chance to poke about with any of it, and not as confident with the custom crs stuff (so pull requests as ever welcome!).

JosefWN commented 2 years ago

I might be able to take a look in the coming month, a bit tight on time right now!

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.

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.

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.

github-actions[bot] commented 1 year ago

This issue was closed because it has been stalled for 5 days with no activity.