search

SpatialServer / Leaflet.MapboxVectorTile

A Leaflet Plugin that renders Mapbox Vector Tiles on HTML5 Canvas.
Other
299 stars 95 forks source link

layer redraw() function doesn't seem to follow expected behavior #25

Open jaketangosierra opened 9 years ago

jaketangosierra commented 9 years ago

According to the Leaflet docs, the TileLayer.redraw() function is supposed to re-fetch the tiles, and re-draw them.

It appears that the MVTSource.redraw() function isn't actually re-fetching, but instead just passing through to the TileLayer.Canvas.redraw() call.

It could be that I'm not familiar enough with the code and missed something, but it looks like this means that the ajax call to re-request the tile gets stepped around, by passing off the TileLayer.Canvas too early? Is this an easy fix?

apollolm commented 9 years ago

Thanks for the question. This is something I was looking at yesterday - redraw does basically just call Tilelayer.Canvas.redraw - this eventually triggers a call to _draw, which does re-fetch tiles.

I suspect that your tiles will be cached by the browser, though, so your tiles will be re-requested, but will probably come from the cache, not the server.

I'm actually thinking of an optional way to skip the re-fetching and just get straight to the drawing so we can be more efficient processing the tiles.

Thanks, Ryan

On Jun 3, 2015, at 4:36 PM, Jacob Thebault-Spieker notifications@github.com wrote:

According to the Leaflet docs, the TileLayer.redraw() function is supposed to re-fetch the tiles, and re-draw them.

It appears that the MVTSource.redraw() function isn't actually re-fetching, but instead just passing through to the TileLayer.Canvas.redraw() call.

It could be that I'm not familiar enough with the code and missed something, but it looks like this means that the ajax call to re-request the tile gets stepped around, by passing off the TileLayer.Canvas too early? Is this an easy fix?

— Reply to this email directly or view it on GitHub.

jaketangosierra commented 9 years ago

Do you know of the right way then, if tiles are being cached, to trigger an actual re-request to the server? That's how I read the comment on TileLayer.Canvas.redraw(), but it's logical that leaflet might go to the cache first.

My vector tiles are changing properties on occasion, which is why this is important.

apollolm commented 9 years ago

I’m not sure how the browser/cached tile/leaflet relationship works. I’m guessing that in order to circumvent browser caching from a Javascript library, you’d have to employ a workaround that makes uniquely named calls to the tile endpoint - like adding a dummy querystring parameter that changes to the end of the request (which would be ignored). Like z/x/y.pbf?ts=12345679, where the number is some timestamp that is constantly changing. I’ll have to think about how to make an option where this is built in to the library so you wouldn’t have to do this. Perhaps something like options: { bustCache: true }, or similar?

Thanks, Ryan

On Jun 3, 2015, at 2:13 PM, Jacob Thebault-Spieker notifications@github.com wrote:

Do you know of the right way then, if tiles are being cached, to trigger an actual re-request to the server? That's how I read the comment on TileLayer.Canvas.redraw(), but it's logical that leaflet might go to the cache first.

My vector tiles are changing properties on occasion, which is why this is important.

— Reply to this email directly or view it on GitHub https://github.com/SpatialServer/Leaflet.MapboxVectorTile/issues/25#issuecomment-108617539.

jaketangosierra commented 9 years ago

Something like that would be reasonable. It seems surprising to me that this isn't built into Leaflet proper. I'll maybe do some digging to ensure that the call chain is doing what we'd expect.

apollolm commented 9 years ago

Just came across this when looking up something else:

http://stackoverflow.com/questions/15048096/how-do-i-force-a-leaflet-map-to-reload-all-tiles-including-visible-ones http://stackoverflow.com/questions/15048096/how-do-i-force-a-leaflet-map-to-reload-all-tiles-including-visible-ones

On Jun 4, 2015, at 11:56 AM, Jacob Thebault-Spieker notifications@github.com wrote:

Something like that would be reasonable. It seems surprising to me that this isn't built into Leaflet proper. I'll maybe do some digging to ensure that the call chain is doing what we'd expect.

— Reply to this email directly or view it on GitHub https://github.com/SpatialServer/Leaflet.MapboxVectorTile/issues/25#issuecomment-109009401.

apollolm commented 9 years ago

Added a bustCache config option to the master-interrupt branch. See here: https://github.com/SpatialServer/Leaflet.MapboxVectorTile/blob/master-interrupt/docs/configuration.md#config

Let me know if that solves the problem.

jaketangosierra commented 9 years ago

I'll try to get to this this week, apologies for the delay.