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

[BUG] TMS support seems to be partially broken #1394

Open gpsim opened 1 year ago

gpsim commented 1 year ago

What do you want implemented?

Hello, I tried to achieve Baidu layer tiles, but always render out of order.

http://online1.map.bdimg.com/onlinelabel/?qt=tile&x={x}&y={y}&z={z}&styles=pl&scaler=1&p=1

What other alternatives are available?

I tried to modify the proj4 property with the following code.


import 'package:flutter/material.dart';
import 'package:flutter_map/plugin_api.dart';
import 'package:flutter_map_example/widgets/drawer.dart';
import 'package:latlong2/latlong.dart';
import 'package:proj4dart/proj4dart.dart' as proj4;

class EPSG3413Page extends StatefulWidget {
  static const String route = 'EPSG3413 Page';

  const EPSG3413Page({Key? key}) : super(key: key);

  @override
  _EPSG3413PageState createState() => _EPSG3413PageState();
}

class _EPSG3413PageState extends State<EPSG3413Page> {
  late final Proj4Crs epsg3413CRS;

  double? maxZoom;

  @override
  void initState() {
    super.initState();

    // 9 example zoom level resolutions
    final resolutions = <double>[
      262144,
      131072,
      65536,
      32768,
      16384,
      8192,
      4096,
      2048,
      1024,
      512,
      256,
      128,
      64,
      32,
      16,
      8,
      4,
      2,
      1
    ];

    final epsg3413Bounds = Bounds<double>(
      const CustomPoint<double>(0.0, 20037508.342789244),
      const CustomPoint<double>(20037508.342789244, 0.0),
    );

    maxZoom = (resolutions.length - 1).toDouble();

    // EPSG:3413 is a user-defined projection from a valid Proj4 definition string
    // From: http://epsg.io/3413, proj definition: http://epsg.io/3413.proj4
    // Find Projection by name or define it if not exists
    final proj4.Projection epsg3413 = proj4.Projection.add('EPSG:3857',
        '+proj=merc +a=6378206 +b=6356584.314245179 +lat_ts=0.0 +lon_0=0.0 +x_0=0 +y_0=0 +k=1.0 +units=m +nadgrids=@null +wktext  +no_defs');

    epsg3413CRS = Proj4Crs.fromFactory(
      code: 'EPSG:3857',
      proj4Projection: epsg3413,
      resolutions: resolutions,
      //bounds: epsg3413Bounds,
      origins: [const CustomPoint(0, 0)],
      scales: null,
      transformation: null,
    );
  }

  @override
  Widget build(BuildContext context) {
    // These circles should have the same pixel radius on the map
    final circleMarkers = <CircleMarker>[
      CircleMarker(
          point: LatLng(21, 110),
          color: Colors.blue.withOpacity(0.7),
          borderStrokeWidth: 2,
          useRadiusInMeter: true,
          radius: 1112000 // 2000 meters | 2 km
          ),
    ];

    return Scaffold(
      appBar: AppBar(title: const Text('EPSG:3857 CRS')),
      drawer: buildDrawer(context, EPSG3413Page.route),
      body: Padding(
        padding: const EdgeInsets.all(8),
        child: Column(
          children: [
            Flexible(
              child: FlutterMap(
                options: MapOptions(
                  crs: epsg3413CRS,
                  center: LatLng(22.63785, 114.0121),
                  zoom: 4,
                  maxZoom: 4,
                ),
                children: [
                  TileLayer(
                      opacity: 1,
                      backgroundColor: Colors.transparent,
                      urlTemplate:
                          'http://online1.map.bdimg.com/onlinelabel/?qt=tile&x={x}&y={y}&z={z}&styles=pl&scaler=1&p=1'),
                  CircleLayer(circles: circleMarkers),
                ],
              ),
            ),
          ],
        ),
      ),
    );
  }
}

Can you provide any other information?

image

Platforms Affected

Android, iOS

Severity

Obtrusive: No workarounds are available, and this is essential to me

Requirements

ibrierley commented 1 year ago

What happens if you try -y and not y in your template url.. ?

gpsim commented 1 year ago

In this way, an error will be reported directly, and the layer resource cannot be requested

gpsim commented 1 year ago

I see that the web side uses the proj4 and proj4leaflet.js plug-ins to call this layer. I don't know the flutter_ Does the map support proj4leaflet.js

JaffaKetchup commented 1 year ago

Hmm, maybe this is an issue in proj4dart then? Have no idea, but there is not JavaScript dependency here!

JosefWN commented 1 year ago

I think our templateFunction needs to be a little smarter for this to work in the "standard" way, like @ibrierley suggested:

What happens if you try -y and not y in your template url.. ?

https://wiki.openstreetmap.org/wiki/TMS

In JOSM, iD, Potlatch 2, and Merkaartor, you can use TMS tile sources (''e.g.'', for imagery background) which do the Y coordinates the TMS way, by simply inserting a minus in the tile URL specifier.

Right now, from what I can see, we don't take this into account when we build the tile URL from the template. On the other hand, if Baidu is using TMS tiles, there seems to be some sort of inversion of y going on in _getTileUrl. So simply setting the TileLayer property tms: true might help?

Would be prettier to handle this in the URL template though, since that seems to be the way others do it.

ibrierley commented 1 year ago

Typically all things being equal I think it makes sense to match up with what Leaflet.js does as much as possible.

JosefWN commented 1 year ago

Fair enough, they seem to be using the tms flag for their tile layers.

@gpsim if you can clean up your example so it's easier to use it, it would also be easier for us to test. The formatting is all over the place and it looks like it contains some dead code as well.

ibrierley commented 1 year ago

(have tidied it up a bit, see if that helps)

JosefWN commented 1 year ago

The definition of TileProvider.invertY seems a bit unintuitive:

int invertY(int y, int z) {
  return ((1 << z) - 1) - y;
}

... or maybe this is not the intended purpose. With tms: true and the following:

int invertY(int y, int z) {
  return -y;
}

... the code seems to work.

JosefWN commented 1 year ago

It seems Leaflet does support -y as well, which we don't: https://github.com/Leaflet/Leaflet/blob/main/src/layer/tile/TileLayer.js#L193

The confusing invertY seems to come from: https://github.com/Leaflet/Leaflet/commit/d5e5f33d79d419988637097c7aea697ff3e0e469

This has since been fixed, to: https://github.com/Leaflet/Leaflet/blob/main/src/layer/tile/TileLayer.js#L189

EDIT: It also seems like we are unconditionally requesting retina/large tiles when r is provided in the template? https://github.com/fleaflet/flutter_map/blob/master/lib/src/layer/tile_layer/tile_provider/base_tile_provider.dart#L33

JosefWN commented 1 year ago

I tinkered a bit trying to align our code with Leaflet's but the tile layer code is so hairy... might be better to address this as part of a larger refactor.

This is as far as I got without making larger modifications. See my comments inline.

String _getTileUrl(String urlTemplate, Coords coords, TileLayer options) {
  final z = _getZoomForUrl(coords, options);
  // No easy access to _pxBoundsToTileRange in TileLayerState, also no access to projected tile bounds.
  // Needless to call this for every tile if we have a globalTileRage which is always up-to-date
  // ... but important to keep it up-to-date: https://github.com/ghybs/Leaflet.TileLayer.Fallback/pull/14
  final globalTileRange = _pxBoundsToTileRange(options.tileSize, bounds);
  final invertedY = '${(globalTileRange.max.y - coords.y).round()}';

  final opts = <String, String>{
    'x': '${coords.x.round()}',
    // No check for !crs.infinite
    'y': options.tms ? invertedY : '${coords.y.round()}',
    // No check for !crs.infinite
    '-y': invertedY,
    'z': '${z.round()}',
    's': getSubdomain(coords, options),
    // Added retinaMode check
    'r': options.retinaMode ? '@2x' : '',
  }..addAll(options.additionalOptions);

  return options.templateFunction(urlTemplate, opts);
}

// Duplicated code from TileLayerState
Bounds _pxBoundsToTileRange(num tileSizePx, Bounds bounds) {
  final tileSize = CustomPoint(tileSizePx, tileSizePx);
  return Bounds(
    bounds.min.unscaleBy(tileSize).floor(),
    bounds.max.unscaleBy(tileSize).ceil() - const CustomPoint(1, 1),
  );
}
JaffaKetchup commented 1 year ago

Yeah @JosefWN, we've come to a similar conclusion internally. There's so many features to add, fix and remove from TileLayer, that it's worth rewriting it. Unfortunatley, we've got quite limited time, so we're not sure when we're going to get it done!

ibrierley commented 1 year ago

It's kind of difficult to visualise a rewrite of the tile layer I think without something a bit more concrete. I'm not sure what the specific aims and criteria would be. Maybe it needs to be part of a separate discussion about what's needed and how it would be interfaced to (especially with the notion of when we have multiple tile layers).

I certainly can understand the issues around access, as I've had similar myself and done my own version in the past here, so more than happy to ponder somewhere.

JosefWN commented 1 year ago

Yeah, I think several aspects need to change. In general: less is more, there are some features I doubt anyone is using (like controlling where fade-in animations start/stop). Everything comes with a cost, if not in performance then in maintenance. I think a sane way of looking at things in the long run is not "Is this a nice to have?" but rather "Is this something more people than the author of the PR will actually benefit from?". Forking is not that hard anyway, if you have really exotic needs.

Then there is the architectural issues; the current solution feels both complex and rigid. I can just intuitively tell that "something is not right" when I'm working with that piece of code, but I haven't thought further in terms of how to make it so.

JaffaKetchup commented 1 year ago

Should we be making a Project to track things that might be added to a larger refactor/rewrite? Unfortunatley we can't add a project to the new Projects version on GitHub (lack of permissions), but we can create a 'classic' Project.

JaffaKetchup commented 1 year ago

(We might want to do something similar to group issues about MacOS gestures (similar to something like #1395))

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.

JosefWN commented 1 year ago

I think this could be relabelled a bug, with TMS support being at least partially broken in flutter_map.

JaffaKetchup commented 3 months ago

The only (good) way to re-add TMS support is to get a MapCamera through to the tile provider. At the moment, this will require a breaking change if I'm not mistaken.