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

[BUG] unbounded horizontal scroll breaks most tile grids #1976

Open arneke opened 1 week ago

arneke commented 1 week ago

What is the bug?

My EPSG:3006 WMTS layer works in 7.0.2, but stopped working after I switched to the master branch. On closer inspection, the reason was that flutter_map is requesting the wrong x,y,z tiles for a given lat,long position.

I've not completely isolated #1948 as the cause, but I think it's the most likely thing in the commit log, and there is an assumption about the modulus that is not generally true.

How can we reproduce it?

Most non-WebMercator grids should trigger this, but here is one.

// OSM based, uses somewhat strange bounds. No access restrictions or fees in GetCapabilities as of 2024-10-15
FlutterMap(
    options: MapOptions(
        initialCameraFit: CameraFit.bounds(
          bounds: LatLngBounds(
              LatLng(61.285991891313344, 17.006922572652666),
              LatLng(61.279648385340494, 17.018309853620366)),
        ),
        crs: Proj4Crs.fromFactory(
          code: 'EPSG:3006',
          proj4Projection: proj4.Projection.add(
            'EPSG:3006',
            '+proj=utm +zone=33 +ellps=GRS80 +towgs84=0,0,0,0,0,0,0 +units=m +no_defs +type=crs',
          ),
          origins: [
            Point(-58122915.354077406, 19995929.885879364),
          ],
          resolutions: const <double>[
            4096,
            2048,
            1024,
            512,
            256,
            128,
            64,
            32,
            16,
            8,
            4,
            2,
            1,
            0.5,
            0.25,
            0.125
          ],
        )),
    children: [
      TileLayer(
          tileBuilder: coordinateDebugTileBuilder,
          urlTemplate:
              'https://maps.trafikinfo.trafikverket.se/MapService/wmts.axd/BakgrundskartaNorden.gpkg?layer=Background&style=default&tilematrixset=Default.256.3006&Service=WMTS&Request=GetTile&Version=1.0.0&Format=image%2Fpng&TileMatrix={z}&TileCol={x}&TileRow={y}')
    ],
  )

Do you have a potential solution?

I don't uderstand all the code in #1948 , but I believe the problem originates in

final modulo = 1 << coordinates.z;

https://github.com/fleaflet/flutter_map/blob/d73d2f1a230957b79a12f82d96de420b68715006/lib/src/layer/tile_layer/tile_coordinates.dart#L28

I don't think the code currently handles cases where

In general, I think you need a flag on the CRS to know whether unbounded scroll applies. Or only enable it for WebMercator for now.

Platforms

All

Severity

Obtrusive: Prevents normal functioning but causes no errors in the console

arneke commented 1 week ago

( @monsieurtanuki )

monsieurtanuki commented 1 week ago

I don't think the code currently handles cases where

  • the grid is not square
  • has more than one tile for z = 0
  • or the projection does not cover the world

@arneke Indeed, what I know about GIS is limited to

Which of my assumptions is wrong in your case? With your help, I'd be able to make both systems work - webmercator and more exotic ones. Possibly with a flag.

I'll have a look at your example and see what happens.

arneke commented 1 week ago

Thanks for responding 👍

Unfortunately it gets a bit more complicated. Resolutions where you divide by two are common, but there are also grids where each matrix corresponds to 1:1000, 1:2000, 1:2500 , etc, to match old requirements.

And to be fair, there is quite a bit of code in flutter_map that makes this confusing. (The projection has to be the same for all layers, but each layer should actually have a separate set of resolutions and origins, so that tile boundaries from different sources do not have to align perfectly)

You can have a look at https://www.trafikverket.se/trafikinformation/vag/ , the web client illustrates that you cannot zoom out to a single tile, z = 0 is actually 6 x 10 tiles (approximately).

The tilematrix is described in And https://maps.trafikinfo.trafikverket.se/MapService/wmts.axd/BakgrundskartaNorden.gpkg?Service=WMTS&Request=getcapabilities

To be honest, I think you should only do this for webmercator, revert to the old behavior for all others. Will cover 90% of the use cases and not break anything.

monsieurtanuki commented 1 week ago

@arneke Looking back at the code, hopefully it won't be hard to fix (?) because I only added a couple of new classes. Those classes make sense anyway, it's just that in "your" case they have a slightly different behavior - e.g. "no it makes no sense to add power(2, zoom) to an X coordinate".

monsieurtanuki commented 1 week ago
@arneke Good news - in a first approach - I've managed to roll back my changes in only 5 files. before after
Screenshot_1728567156 Screenshot_1728567262

That doesn't mean that the implementation for both systems is done yet ;)

arneke commented 1 week ago

nice 👏

monsieurtanuki commented 5 days ago

@arneke I think I'll PR tomorrow.

monsieurtanuki commented 4 days ago

@arneke I'm about to PR. Would that be OK for me to include your ESPG:3006 code snippet as an example, so we can check from version to version if everything is alright? Actually, any "no -180+180" projection would be good as far as I'm concerned, but I'm not able to invent one.

arneke commented 4 days ago

No, as written in the comment, that one is a bit encumbered, would not make it "official".

https://demo.fleaflet.dev/crs_epsg4326

appears to be broken on master, but it's not a very general case.

I can look tonight for a better example

monsieurtanuki commented 4 days ago

I can look tonight for a better example

Thank you, that would be perfect: just find the easiest / most stupid example you can, that doesn't work with the master but works with 7.0.2.

arneke commented 4 days ago

I have updated the example above with one I believe is ok to add to the repo. Comment above, origin and urlTemplate have been replaced.

arneke commented 2 days ago

If you make the branch you are working on public somewhere, I'd be happy to test it. (Waiting for this issue to be resolved before I can deploy a new version of my own app.)

monsieurtanuki commented 2 days ago

Waiting for this issue to be resolved before I can deploy a new version of my own app.

Oh, I wasn't aware of that. First test with new example looks ok. I'll try to PR this morning. Screenshot_1729062141

monsieurtanuki commented 2 days ago

@arneke cf. #1978