Leaflet.TileLayer.Fallback icon indicating copy to clipboard operation
Leaflet.TileLayer.Fallback copied to clipboard

y value out of tileset range for tms

Open m314 opened this issue 6 years ago • 8 comments

Incorrect tile search for option tms=true.

this._globalTileRange is not updated with subsequent calls to getTileUrl, so it looks for y values that are out of range.

Following Leaflet's _resetGrid function, getTileUrl probably needs to get the bounds for the new zoom layer. So something like this:

 getTileUrl: function (coords) {
        var z = coords.z = coords.fallback ? coords.z : this._getZoomForUrl();

        var data = {
            r: L.Browser.retina ? '@2x' : '',
            s: this._getSubdomain(coords),
            x: coords.x,
            y: coords.y,
            z: z
        };

        if (this._map && !this._map.options.crs.infinite) {
            // these lines are new
            var bounds = this._map.getPixelWorldBounds(z); // get bounds
            if (bounds) {
                var _globalTileRange = this._pxBoundsToTileRange(bounds); // update tile range
                var invertedY = _globalTileRange.max.y - coords.y;
                if (this.options.tms) {
                    data['y'] = invertedY;
                }
                data['-y'] = invertedY;
            }
        }

        return L.Util.template(this._url, L.extend(data, this.options));
    }

m314 avatar Oct 29 '18 15:10 m314

Hi,

Thank you for your message.

Please can you provide a live example (e.g. using Plunker) that reproduces the issue? E.g. using a public TMS source and tampering with some tiles URL to prevent them from loading, to force the plugin to kick in. See the example in the code of the demo page: https://github.com/ghybs/Leaflet.TileLayer.Fallback/blob/master/examples/tileLayerFallback-demo.html#L56

Then please feel free to submit your code proposition as a Pull Request (even if is just some indications), the code formatting in a PR is much better (directly displays code difference). Thanks! :smiley:

ghybs avatar Nov 01 '18 03:11 ghybs

Hello,

Here's a Plunker with a tampered tiles url to force reloading and set to tms=true. (Setting tms=true scrambles the tiles for OSM, but it gives the idea of what's going wrong). In the console, you can see that it eventually looks for tile at zoom 0, x 0, y 7. This is out of range.

m314 avatar Nov 08 '18 14:11 m314

Hi,

Thank you for the Plunker. While I can see indeed the coordinates in the requested tiles URL, it would be much clearer if you could provide an actual TMS source. That way we could definitely check that the result is appropriate.

ghybs avatar Nov 09 '18 20:11 ghybs

Okay, that took a lot of Googling! The map I use is behind a firewall. I found an alternate TMS source, but it only goes up to zoom level 8. That may actually be good for our purposes, because if you zoom out, you can see the tiles going to the incorrect, out-of-range indices. Here's the updated Plunker, complete with tampered URLs similar to the demo you provided. I tampered with all of zoom level 9.

m314 avatar Nov 12 '18 15:11 m314

Hi,

Thank you for your research! I have not seen the difference with the previous Plunker, did I miss something?

ghybs avatar Nov 13 '18 19:11 ghybs

Hmmm. I'm not super familiar with Plunker, but it looks like they have a freeze button to set the currently viewable version, which I had not updated. Could you please try the link again and see if you can see the updated version now?

m314 avatar Nov 14 '18 20:11 m314

Nice! :+1: Indeed I did not think about looking at available newer versions. What I usually do in such cases is to fork my Plunk (instead of save) to get a new URL.

This clearly confirms that updating getTileUrl as you propose solves the issue for TMS Tile Layer, while maintaining the functionality for non TMS source: https://plnkr.co/edit/uPfxCZMhi9dAQCecIpg5?p=preview

Please would you care sending your PR targeting the master branch of this repo, instead of your fork? Thank you!

ghybs avatar Nov 15 '18 00:11 ghybs

Forking the Plunker is a good idea. Thank you for the feedback, and also for the better Plunker including the test code. :+1:

Okay, I opened a new PR. I appreciate your help!

m314 avatar Nov 15 '18 14:11 m314