Fix invalid tile key issues when swapping rasters when region picker active

[Shane98c]

Adds a couple checks to make sure we're not accessing the tile before it has been initialized. This solves errors when swapping raster properties (like variable) while a region picker is active.

Fixes zoom/center state initialization and makes sure dataset level is correctly calculated before querying.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments

Name	Status	Preview	Comments	Updated (UTC)
maps-demo	⬜️ Ignored (Inspect)	Visit Preview		Feb 15, 2024 3:30pm
maps-docs	⬜️ Ignored (Inspect)	Visit Preview		Feb 15, 2024 3:30pm

[Shane98c]

Ok new direction here. Traced this back to the camera center/zoom properties being undefined. This was due to the way useControls was set up.

Since the map was already loaded, map.on('load', ... was never (or at least not consistently) fired, so those values remained undefined until the map was moved. This change makes the the state initialization ask the map right away for the current values, removes the 'load' event listener, and also cleans up the listener on dismount.

I'm slightly worried that map.getZoom() and map.getCenter() could be undefined if useControls is raced against map initialization, but I don't think that's likely with its current usage.

I'm now seeing key issues (they look totally legit, but don't exist on this.tiles) only in cases where I zoom in somewhat far, which could be an unrelated to maps and instead have something to do with the zarr metadata for OAE not setting maxzoom correctly?

[Shane98c]

Ok I think I have a good solution to the timing thing we were diving into yesterday - we just need to update this.level during initialization since maxZoom is fine in there. We don't actually have to run the entire updateCamera function and worry about timing if we take this route. We also don't need to be in the updateCamera function for access to zoom because this.zoom is correct and accessible in the initialization function.

[katamartin]

This is sooo much cleaner than the callback!!

Question: in the case that triggered the race condition where queryRegion runs before updateCamera (rendering a new Raster in an existing Map with an already active region), will the tiles from the max zoom level always be fetched and queried regardless of what level the tiles actually end up rendering at?

For example, if the map is at zoom ~0 but maxZoom === 1, will the 1-level tiles be fetched by queryRegion while the 0-level tile is fetched for updateCamera? While not a huge deal in this example, this could potentially be quite costly for pyramids with high maxZooms.

[Shane98c]

Hmm if i'm following correctly I think the zoomToLevel function returns the lesser of zoom vs maxzoom, which is what's used by queryRegion, so my understanding is that it should fetch the data for the relevant current zoom of the map, not the maxZoom. This works because this.zoom has been set by updateCamera previously. There might be a possible edge case where if you're swapping out rasters while actively zooming something could get funky - but I don't think this fix makes that situation worse than it would be normally. If zoom is for some reason undefined, I think zoomToLevels will return zero, so I don't see a way that we'd be fetching data for higher levels than needed.

[katamartin]

This works because this.zoom has been set by updateCamera previously.

Ah! I thought that we were hitting this case because updateCamera hadn't finished by the time queryRegion ran but it's actually because updateCamera can run before initialized resolves, right? And because of this we saw the real value of this.zoom but an unexpected value for this.level in queryRegion?

Then this all feels great, especially with the assurance that

If zoom is for some reason undefined, I think zoomToLevels will return zero

[Shane98c]

yes I think that's right! we had a real zoom but maxZoom was undefined because we hadn't finished initializing yet so we were getting a level set based on zoom, with no consideration for maxZoom

So another potential spot issues could arise is if there are places in updateCamera where level not respecting maxZoom could cause issues. It looks like level is used a few times throughout. and some bad tile keys are created, but I think the check for this.loaders[level] prevents anything from going further than that.