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] unbounded horizontal scroll breaks most tile grids #1976

Open
arneke opened this issue Oct 10, 2024 · 14 comments · May be fixed by #1978
Open

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

arneke opened this issue Oct 10, 2024 · 14 comments · May be fixed by #1978
Labels
bug This issue reports broken functionality or another error P: 2 (soon™?) S: core Scoped to the core flutter_map functionality

Comments

@arneke
Copy link

arneke commented Oct 10, 2024

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;

final modulo = 1 << coordinates.z;

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

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 arneke added bug This issue reports broken functionality or another error needs triage This new bug report needs reproducing and prioritizing labels Oct 10, 2024
@arneke
Copy link
Author

arneke commented Oct 10, 2024

( @monsieurtanuki )

@monsieurtanuki
Copy link
Contributor

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

  • zoom is a positive integer
  • there are power(2, zoom) x and y coordinates (therefore 1*1 tile for zoom = 0)
  • tiles are identical squares - I guess identical rectangles would be fine too
  • projections cover all longitudes

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

arneke commented Oct 10, 2024

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

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

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

arneke commented Oct 10, 2024

nice 👏

@monsieurtanuki
Copy link
Contributor

@arneke I think I'll PR tomorrow.

@monsieurtanuki
Copy link
Contributor

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

arneke commented Oct 14, 2024

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

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

arneke commented Oct 14, 2024

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

arneke commented Oct 16, 2024

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

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 monsieurtanuki linked a pull request Oct 16, 2024 that will close this issue
@monsieurtanuki
Copy link
Contributor

@arneke cf. #1978

@JaffaKetchup JaffaKetchup linked a pull request Oct 21, 2024 that will close this issue
@JaffaKetchup JaffaKetchup added P: 2 (soon™?) S: core Scoped to the core flutter_map functionality and removed needs triage This new bug report needs reproducing and prioritizing labels Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue reports broken functionality or another error P: 2 (soon™?) S: core Scoped to the core flutter_map functionality
Projects
Status: To do
Development

Successfully merging a pull request may close this issue.

3 participants