perliedman / leaflet-routing-machine

Control for routing in Leaflet
https://www.liedman.net/leaflet-routing-machine/
Other
1.08k stars 350 forks source link

Change broken url to OSM tiles #587

Closed ThomasG77 closed 4 years ago

ThomasG77 commented 4 years ago

The url in the example will never work as there is no resolution in the official OpenStreetMap tiles urls.

perliedman commented 4 years ago

Hm, a bit unsure what you mean that they will "never work"? They seem to be working for me?

ThomasG77 commented 4 years ago

Try https://gist.githack.com/ThomasG77/57de3f1dc94817df153780661df5909d/raw/6797566f979b4a843456e96eaad517ca82dba8f8/osrm.html (url for the tiles proposed in the PR) with no 404

vs

https://gist.githack.com/ThomasG77/57de3f1dc94817df153780661df5909d/raw/6797566f979b4a843456e96eaad517ca82dba8f8/osrm-ko.html with 404 error in network panel.

The "they will "never work" means that official tiles does not manage resolution so {r} is not available if using retina screen whereas the current url to tiles consider it can.

perliedman commented 4 years ago

Ok, I do not get 404s on the second link, but I guess that is because my system is not loading retina tiles for some reason.

I misunderstood the change as being from tile.osm.org to tile.openstreetmap.org, but the change is really to remove {r} from the URL?

ThomasG77 commented 4 years ago

If you look at https://github.com/Leaflet/Leaflet/commit/de1a62fa529b392da76225d38aa49518ae58c23c, you will see {r} can be empty string or @2x You can't see the issue because you don't have a retina screen IMHO but as already stated {r} has to be removed or people with retina display like me will get 404s.

I've forced for the demonstration the resolution:

perliedman commented 4 years ago

Ok, thanks for explaining the issue! Merged now.