Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Honour coords.z when passed to TileLayer.getTileUrl #9201

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

yohanboniface
Copy link
Member

cf #6004 #6006

Before this change, coords.z would not taken into account, and was always overriden by this._tileZoom.
In the normal flow, coords.z is anyway set from this._tileZoom before getTileUrl is called (in GridLayer._update). But this prevents using getTileUrl out of this _update method. There are some scenarios where this is needed (see #5822), or (which is the case in uMap) when wanting to display a tile as a tilelayer preview.

cf #6004 #6006

Before this change, `coords.z` would not taken into account, and
was always overriden by `this._tileZoom`.
In the normal flow, `coords.z` is anyway set from `this._tileZoom`
before `getTileUrl` is called (in GridLayer._update). But this
prevents using `getTileUrl` out of this `_update` method.
There are some scenarios where this is needed (see #5822), or
(which is the case in uMap) when wanting to display a tile as
a tilelayer preview.
yohanboniface added a commit to umap-project/umap that referenced this pull request Dec 28, 2023
This is not the ideal fix, instead we'd prefer to use
`TileLayer.getTileUrl`, but this is not possible yet.

cf Leaflet/Leaflet#9201

cf https://lists.openstreetmap.org/pipermail/umap/2023-December/000557.html
@Falke-Design
Copy link
Member

Falke-Design commented Dec 28, 2023

It looks like this could be working because of:

coords.z = this._tileZoom;

But I'm not sure if this makes maybe some problems with other plugins.

@IvanSanchez @mourner what do you think?

@yohanboniface
Copy link
Member Author

It looks like this could be working because of:

Yep! We could also fallback to this._tileZoom in _getZoomForUrl, but it seems to me we should not have both, i.e. settings coords.z = this._tileZoom AND the fallback.

But I'm not sure if this makes maybe some problems with other plugins.

That could indeed be the case. Though for this to happen, it seems that a plugin should have rewritten the entire GridLayer._update, which is a huge piece of internal code, so maybe it's a feature if we discover that touching such internal code breaks some external code :p

yohanboniface added a commit to umap-project/umap that referenced this pull request Dec 28, 2023
This is not the ideal fix, instead we'd prefer to use
`TileLayer.getTileUrl`, but this is not possible yet.

cf Leaflet/Leaflet#9201

cf https://lists.openstreetmap.org/pipermail/umap/2023-December/000557.html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants