search

Esri / esri-leaflet

A lightweight set of tools for working with ArcGIS services in Leaflet. :rocket:
https://developers.arcgis.com/esri-leaflet/
Apache License 2.0
1.61k stars 799 forks source link

DynamicMapLayer clears old image when you move map #1124

Open ogix opened 6 years ago

ogix commented 6 years ago

All

1.3.4

2.2.3

Steps to reproduce the error:

  1. Create a dynamicMapLayer
  2. Wait until layer is loaded
  3. Move map outside of layer bounds
  4. Return map to same position again

What happens is the loaded layer image gone. You have to wait again until image is loaded.

I was expecting the layer image to stay in the same way as with WMS layer.

jgravois commented 6 years ago

it's near impossible to pan the map to exactly the same location. that said, I guess technically we could wait to remove the old image until after the next one is retrieved.

the relevant code is here if anyone is interested: https://github.com/Esri/esri-leaflet/blob/master/src/Layers/RasterLayer.js#L48

ogix commented 6 years ago

Maybe it is possible to make this layer extend from leaflet tileLayer and make it work in the same way as wms layer does?

tomwayson commented 6 years ago

I don't think it would make sense to extend from tileLayer. DynamicMapLayer is not based on tiles. I fetches one image created dynamically on the server (as opposed to many cached tiles) for each pan/zoom.

Do you see the behavior you expect when working with any other type of non-tiled layer?

jgravois commented 6 years ago

WMS is a dynamic service type too. Leaflet likely just uses a grid to cache requests.

https://github.com/Leaflet/Leaflet/blob/master/src/layer/tile/TileLayer.WMS.js

I'd be interested in seeing a PR and evaluating the result for dynamicMapLayer if anyone is interested in tackling it. it doesn't look like it'd take much code.

jwasilgeo commented 5 years ago

@jgravois @ogix was this fixed already, following the first non-tiling approach mentioned above?

wait to remove the old image until after the next one is retrieved

The order of operations seems good to me now that I'm going back and stepping through RasterLayer.js and what it extends from.

jgravois commented 5 years ago

was this fixed already, following the first non-tiling approach mentioned above?

nope. no work has been done here.

jwasilgeo commented 5 years ago

Alright, we've confirmed that the DynamicMapLayer does function as expected by design now, and will leave this open as a potential enhancement with the idea suggested here: https://github.com/Esri/esri-leaflet/issues/1124#issuecomment-418622194.