ghybs / Leaflet.TileLayer.Fallback

Replaces missing Tiles by scaled lower zoom Tiles
Apache License 2.0
36 stars 18 forks source link

y value out of tileset range for tms #13

Open m314 opened 5 years ago

m314 commented 5 years ago

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));
    }
ghybs commented 5 years ago

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:

m314 commented 5 years ago

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.

ghybs commented 5 years ago

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.

m314 commented 5 years ago

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.

ghybs commented 5 years ago

Hi,

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

m314 commented 5 years ago

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?

ghybs commented 5 years ago

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!

m314 commented 5 years ago

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!