The maxNativeZoom option of the layer is not being set when using L.esri.basemapLayer

[Diegodlt]

Bug

The maxNativeZoom option of the layer is not being set when using the method L.esri.basemapLayer('Streets') along with the options object. This means that some parts of the map display the "Map data not yet available" when the zoom level is greater than 19.

I've tried two different ways of setting the maxZoomLevel, both unsuccessful:

var map = L.map('map');
L.esri.basemapLayer('Imagery', { maxNativeZoom: 19}).addTo(map);

var map = L.map('map');
var layer = L.esri.basemapLayer('Imagery');
layer.options.maxNativeZoom = 19;
layer.addTo(map)

Expected Behavior

The map should be auto-scaled when zooming over the set maxNativeZoom preventing the "Map data not yet available" tiles from being displayed. Using the L.tileLayer Leaflet method with the appropriate Url for Imagery and setting the maxNativeZoom option does result in the expected behavior.

Screenshots

Environment Information

• Version of Leaflet (L.version): 1.6.0
• Version of Esri Leaflet (L.esri.VERSION): 2.3.2
• Your OS: Windows 10
• Browser and Version: Chrome 83.0.4103.116

[gavinr]

min and max zoom are set here (example here) for each basemap. Are you saying you want to set that to something different?

[jgravois]

Are you saying you want to set that to something different?

yup. it sounds like he's trying to override the resampling i landed in #1011.

min and max zoom are set here (example here) for each basemap.

user supplied constructor options should definitely supersede any defaults set internally.

[Diegodlt]

@jgravois Could Object.assign work to fix the issue in this part of the code
https://github.com/Esri/esri-leaflet/blob/master/src/Layers/BasemapLayer.js#L224, like so
var tileOptions = Object.assign(config.options, options);

[jgravois]

Could Object.assign work to fix the issue

L.Util.extend is actually equivalent to Object.assign and friendly for old browsers.

something else is afoot. 🦶

[jwasilgeo]

I think you are right that it has something to do with the changes introduced in PR #1011. I tested and verified that the maxNativeZoom option does work for all basemaps and lets a user "over zoom" without requesting new tiles at map zoom levels beyond said option.

But, for the Imagery* basemaps at map zoom levels 14+ it attempts to fetch new tiles when this condition is met:

https://github.com/Esri/esri-leaflet/pull/1011/files#diff-04398c1818006d730a158ca4d0ad9d3eR257

Then there are 2 spots in these changes where this.options.maxNativeZoom is being potentially altered, if I am interpreting it correctly:

https://github.com/Esri/esri-leaflet/pull/1011/files#diff-04398c1818006d730a158ca4d0ad9d3eR278
https://github.com/Esri/esri-leaflet/pull/1011/files#diff-04398c1818006d730a158ca4d0ad9d3eR283

@jgravois can we return early from this running if there is a maxNativeZoom 13 or smaller? And then add some other conditions to handle when it is 14+ without interfering with the improvements that landed in 1101?

[jgravois]

can we return early from this running if there is a maxNativeZoom 13 or smaller?

afaik, World Imagery is cached worldwide to zoom level 19 just like Streets, Topographic etc. fetching the tilemap is a technique to sniff out specific areas where tiles are cooked all the way down to 22.

this means you can probably just skip wiring up the zoomanim handler altogether when maxNativeZoom <= 19.

[jwasilgeo]

Thanks for the additional insight and info, @jgravois! Here's how I was hoping to approach this so far:

1. Assuming someone wants to set a maxNativeZoom on their layer, allow the zoomanim handler to continue to be wired up as it is now in all cases, to cover when someone wants to do either of the following:

   1. provide a maxNativeZoom as a layer constructor option right away,
   2. or change the default maxNativeZoom later, e.g. myLayer.options.maxNativeZoom = 6;

2. From there, because the handler is wired up at all times and these lines attempt to internally manipulate the maxNativeZoom:

   https://github.com/Esri/esri-leaflet/pull/1011/files#diff-04398c1818006d730a158ca4d0ad9d3eR278
   https://github.com/Esri/esri-leaflet/pull/1011/files#diff-04398c1818006d730a158ca4d0ad9d3eR283

   I find myself going in circles on how to stop early from letting the bulk of this run when a user has set a maxNativeZoom on their layer: https://github.com/Esri/esri-leaflet/pull/1011/files#diff-04398c1818006d730a158ca4d0ad9d3eR257

(Work started in branch 1210-maxNativeZoom-fix.)

[jgravois]

i. provide a maxNativeZoom as a layer constructor option right away
ii. or change the default maxNativeZoom later, e.g. myLayer.options.maxNativeZoom = 6;

call me lazy, but i don't think its important to support either of the ☝️ use cases above.

maxNativeZoom is a constructor option, its typically only set once. i know we're tweaking the value internally to support the tilemap, but i'd call that an edge case. if a developer sets maxNativeZoom, they'd likely expect it to be left alone.

[patrickarlt]

@Diegodlt I'm Jumping in here to ask why do you want to set your own maxNativeZoom? Currently I cannot reproduce a case where setting maxNativeZoom creates empty tiles. https://codepen.io/patrickarlt/pen/RwrvYPR

Internally maxNativeZoom is used to define the maximum zoom level of available tiles on the https://leafletjs.com/reference-1.6.0.html#gridlayer-maxnativezoom we set this for you when you use L.esri.BasemapLayer because we know what the max zoom of the tiles are. There shouldn't be a need to ever set maxNativeZoom.

If your intent is to allow the user to zoom in PAST the maximum zoom of the tiles you need to use maxZoom.

@jgravois @jwasilgeo I think we can just close this unless @Diegodlt can help us with a reproduction.

[Diegodlt]

@patrickarlt I have noticed that sometimes it shows the correct tiles, but on other occasions it does not. I want to set the maxNativeZoom to 19 so that the "Map data not yet available" tiles do not appear on the map. Here's a screenshot of the issue happening along with the coordinates of that point. This happened after I zoomed in past level 19

[patrickarlt]

@jgravois @jwasilgeo I can sort of reproduce this with https://codepen.io/patrickarlt/pen/RwrvYPR?editors=1000 but only is I zoom in REALLY fast. It doesn't actually seem to matter if maxNativeZoom is set or not. To reproduce you can usually:

1. Wait for the demo to load.
2. Mash zoom in as fast as possible until you hit the max zoom (22)
3. You should probably see missing tiles.

It doesn't seem to matter what part of the world this is in. I'm not sure if we can rely on the world imageeru server to serve tiles at all zoom levels.

@jgravois I'm pretty sure we can't actually resolve #1011. We have to resample different tiles in different parts of the world at different sizes which is just too difficult with Leaflet without rewriting our own tile layer from scratch. My guy says to revert #1011 and cap this at zoom 19 again.

[jgravois]

Mash zoom in as fast as possible

it makes sense that 'mash zooming' is problematic if there's no time to inspect the tilemap response and learn that there's no content in the current location.

it would be nice to at least let folks opt into rendering the sweet high resolution imagery @Esri has in some locations, but i don't have any plans to work on this further myself so if y'all prefer to just revert #1011 and call it a day instead, you won't hear any complaints out of me.

[Diegodlt]

@jgravois @patrickarlt Allowing the maxNativeZoom option to be overwritten would solve the issue no? Nothing would have to be reverted, and users would still get the ability to zoom in past 19. Manually setting the tile layer with L.tileLayer and setting the maxZoom = 19 solves the issue of missing tiles.

[jwasilgeo]

I like that approach: opt in to (or opt out from) the tilemap enhancements that came in #1011 but also allow a maxNativeZoom to be set from the outside.

Otherwise, yes, we could revert #1011 which would also allow the maxNativeZoom to be set.

Thoughts?

[jgravois]

@patrickarlt made a good point that folks shouldn't need to be messing with maxNativeZoom at all.

i think the nicest thing you could do would be to introduce a special constructor option (for Imagery only) called useTilemap and default to false.

[jwasilgeo]

Now I want to backtrack a little on what I just said above. I've often found myself going back & forth on these different ideas, all of which have merit. 😄

I'm leaning towards reverting #1011 to give the Imagery basemap layer full control over its maxZoom option (and maxNativeZoom option) to allow for those special cases where developers really want to change them. As @patrickarlt mentioned we would then default and cap maxZoom at 19.

I think your enhancements in #1011 are very interesting, useful, and clever, @jgravois. What if we were to put that in the issue backlog and revisit once we understand how to deal with the missing tiles behavior described in #1210 (comment), #1210 (comment), and #1210 (comment)?

I'm (clearly) open to being talked into something else if anyone has other thoughts or feelings about this, and then when we settle on our approach I am happy to work on what we agree to. Thanks y'all!

[patrickarlt]

@jwasilgeo I think I agree reverting #1011 is the best course of action until we can figure out other bugs with tile map. We need to get with the JS API team or another team internally who can actually explain the tilemap format and how to use it.

[jwasilgeo]

@Diegodlt thanks for opening this issue in the first place. You should be able to control the maxZoom and maxNativeZoom as you wish the next time we publish a release of this library. As for the other items we all discussed and identified, the conversation can continue in #1222.

[Diegodlt]

@jwasilgeo Thanks to the Esri team for responding so quickly and looking into the issue!