Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] More LatLngBounds issues in polar CRS #1391

Open
5 tasks done
JosefWN opened this issue Oct 28, 2022 · 12 comments
Open
5 tasks done

[BUG] More LatLngBounds issues in polar CRS #1391

JosefWN opened this issue Oct 28, 2022 · 12 comments
Labels
P: 4 (far future) S: core Scoped to the core flutter_map functionality

Comments

@JosefWN
Copy link
Contributor

JosefWN commented Oct 28, 2022

What is the bug?

In line with #1347 and #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

  • I agree to follow this project's Code of Conduct
  • My Flutter/Dart installation is unaltered, and flutter doctor finds no relevant issues
  • I am using the latest stable version of this package
  • I have checked the FAQs section on the documentation website
  • I have checked for similar issues which may be duplicates
@JosefWN JosefWN added bug This issue reports broken functionality or another error needs triage This new bug report needs reproducing and prioritizing labels Oct 28, 2022
@ibrierley
Copy link
Contributor

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
Copy link
Contributor Author

JosefWN commented Oct 28, 2022

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 #1347, LatLngBounds provides incorrect bounding boxes in polar CRS.

@ibrierley
Copy link
Contributor

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
Copy link
Contributor Author

JosefWN commented Oct 28, 2022

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
Copy link
Contributor

ibrierley commented Oct 28, 2022

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
Copy link
Contributor Author

JosefWN commented Oct 28, 2022

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:

  1. Zero points (do not center, since polyline is empty and Bounds<num>? is null)
  2. One point (do not use fitBounds)
  3. 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
Copy link
Contributor Author

JosefWN commented Nov 1, 2022

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
Copy link
Contributor

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

This comment has been minimized.

@github-actions github-actions bot added the Stale label Dec 2, 2022
@github-actions

This comment has been minimized.

@github-actions github-actions bot closed this as completed Dec 7, 2022
@JaffaKetchup JaffaKetchup reopened this Dec 7, 2022
@JaffaKetchup JaffaKetchup added non-fatal and removed Stale needs triage This new bug report needs reproducing and prioritizing labels Dec 7, 2022
@github-actions

This comment has been minimized.

@github-actions github-actions bot added the Stale label Jan 7, 2023
@github-actions

This comment has been minimized.

@JaffaKetchup JaffaKetchup reopened this Jan 12, 2023
@JaffaKetchup JaffaKetchup added P: 3 (low) (Default priority for feature requests) and removed non-fatal labels Jun 19, 2023
@JaffaKetchup JaffaKetchup added P: 4 (far future) S: core Scoped to the core flutter_map functionality and removed P: 3 (low) (Default priority for feature requests) labels Aug 8, 2024
@JaffaKetchup JaffaKetchup removed the bug This issue reports broken functionality or another error label Jan 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P: 4 (far future) S: core Scoped to the core flutter_map functionality
Projects
None yet
Development

No branches or pull requests

3 participants