[BUG] More LatLngBounds issues in polar CRS

JosefWN commented 2 years ago

What is the bug?

In line with https://github.com/fleaflet/flutter_map/issues/1347 and https://github.com/fleaflet/flutter_map/pull/1295, there are more issues with bounds for polar CRS.

In some cases fitBounds (and my copy flyToBounds) throws an error:

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

                    When the exception was thrown, this was the stack:
                    #0      _Array.[] (dart:core-patch/array.dart:10:36)
                    #1      Proj4Crs.zoom (package:flutter_map/src/geo/crs/crs.dart:312:30)
                    #2      FlutterMapState.getScaleZoom (package:flutter_map/src/map/flutter_map_state.dart:569:16)
                    #3      FlutterMapState.getBoundsZoom (package:flutter_map/src/map/flutter_map_state.dart:541:12)
                    #4      FlutterMapState.getBoundsCenterZoom (package:flutter_map/src/map/flutter_map_state.dart:513:16)
                    #5      AnimatedMapController.flyToBounds (package:iops/layer/core/controller.dart:152:27)

... and when it doesn't throw an error, it gets completely incorrect coordinates. In part, this has to do with the fact that corner coordinates are not necessarily the bounds in a polar CRS. Especially dealing with longitudes is difficult. With either of the poles on the map for example, you have all longitudes possible in the pole, a single point.

There could potentially also be issues in the tile layer, which is using a similar approach for bounds checks, but I haven't encountered those issues personally.

What is the expected behaviour?

That fitBounds would work the same way regardless of CRS.

How can we reproduce this issue?

Try fitBounds in the EPSG:3413 example's initState (add mapController as class variable).

Incorrect position (should center on overlay image):

    mapController = MapController();
    WidgetsBinding.instance.addPostFrameCallback(
      (_) => mapController.fitBounds(
        LatLngBounds(
          LatLng(72.7911372, 162.6196478),
          LatLng(85.2802493, 79.794166),
        ),
      ),
    );

Out of bounds:

    mapController = MapController();
    WidgetsBinding.instance.addPostFrameCallback(
      (_) => mapController.fitBounds(
        LatLngBounds(
          LatLng(74.602998, -170.999769),
          LatLng(74.602998, -170.999769),
        ),
      ),
    );

Do you have a potential solution?

No response

Can you provide any other information?

Using the master version of flutter_map, but also present in older versions.

Platforms Affected

Android, iOS, Web, Windows, MacOS, Linux

Severity

Erroneous: Prevents normal functioning and causes errors in the console

Frequency

Consistently: Always occurs at the same time and location

Requirements

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

ibrierley commented 2 years ago

How many resolutions do you have for the crs ? It sounds a bit like there aren't enough for the available zoom levels, but I may well be talking out of my *** as my crs foo is very weak (I also thought there was a hacky fix to avoid that, so what I'm saying may well be a redherring and meaningless :D)! This is more looking at the exception rather than accuracy issues.

What version is this on ?

JosefWN commented 2 years ago

Using the master version of flutter_map, but also present in older versions.

I think the issue is twofold:

One is the zoom/out of bounds issue, which is not alleviated even when I'm setting maxZoom. It's occurring both in the example and in my code, and I'm not encountering any other issues related to the zoom levels (if I zoom manually for example).

The other is the centering:

final swPoint = project(bounds.southWest!, zoom);
final nePoint = project(bounds.northEast!, zoom);
final center = unproject((swPoint + nePoint) / 2 + paddingOffset, zoom);

As evident in https://github.com/fleaflet/flutter_map/issues/1347, LatLngBounds provides incorrect bounding boxes in polar CRS.

ibrierley commented 2 years ago

Ok, so I guess the exception problem is that fitBounds doesn't have any width/height, as its just a single point, so it hits infinity, and I guess the same issue will still happen if its a very narrow range that falls outside of available the resolutions ? Maybe the Proj4Crs in crs.dart should handle that a bit more elegantly...

JosefWN commented 2 years ago

By default (FitBoundsOptions) there is a 14px or so padding on the point, so it shouldn't effectively be a single pixel, but yes, it could be too small still, if maxZoom is also ignored in at least parts of the code. Using maxZoom didn't seem to change anything when I tried last night.

Larger polylines for example also experience the out of bounds error, but I suppose that could also be because the LatLngBounds are incorrect and could resolve to a single point or so... I can investigate...

EDIT: One use-case for using single-point bounds is to center as closely as possible on a marker. My layers all expose their Bounds<num> so it was the simplest solution to "center on layer" without taking single points into consideration as edge cases. Polylines may also consist of a single point in certain cases.

ibrierley commented 2 years ago

Just thinking out loud. I suspect this may get hit before maxZoom gets checked, as fitbounds needs to calculate the zoom (and hit the error) first I assume before it can test against maxZoom ?

But I'm a little unclear about whats desired here...if people want to focus on a point, shouldn't they be using a center and not fitBounds ? (mm maybe not as you may have a very small poly, but leaving this here anyway).

JosefWN commented 2 years ago

But I'm a little unclear about whats desired here...if people want to focus on a point, shouldn't they be using a center and not fitBounds ?

That would complicate the code a bit, but it's also possible. I have polylines which change dynamically over time. If you have a polyline which can be 0-n points, you have to consider:

Zero points (do not center, since polyline is empty and Bounds<num>? is null)
One point (do not use fitBounds)
More than one point (use fitBounds)

If fitBounds works for the single point as well, it just boils it down to two cases. Like I said in my previous post, I also figured it would be consistent if all of my layers exposed their bounds in a uniform way. I have my own layers wrapping flutter_map layers, and in the base class I have an abstract Bounds<num>? get bounds; which all of my layers implement.

JosefWN commented 2 years ago

You are right, I found I only get out of bounds issues with singular bounds. Perhaps supporting singular bounds would make the API more confusing (although we might benefit from adding an assertion or such for clarity). I don't know, I don't feel very strongly about it.

In my fork I have now removed the LatLngBounds and just use Bounds for flyToBounds and it fits the bounds correctly (when I don't use singular bounds).

ibrierley commented 2 years ago

I have some vague thoughts around it, just not had time really lately. I.e in the crs code we have..

double zoom(double scale) {
    // Find closest number in _scales, down
    final downScale = _closestElement(_scales, scale);
    if (downScale == null) {
      return double.negativeInfinity;
    }
    final downZoom = _scales.indexOf(downScale);
    // Check if scale is downScale => return array index
    if (scale == downScale) {
      return downZoom.toDouble();
    }
    // Interpolate
    final nextZoom = downZoom + 1;
    final nextScale = _scales[nextZoom];

    final scaleDiff = nextScale - downScale;
    return (scale - downScale) / scaleDiff + downZoom;

what about if we had before the interpolate, something like...(code not tested/checked, just as a theory) if(downZoom > _scales.length) { return _scales.last.toDouble(); }

What would happen here...it feels reasonable if we have a set limit of resolutions, not to go over them, but return the closest thing ?

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.

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.

fleaflet / flutter_map