Kitware / trame-leaflet

MIT License
2 stars 2 forks source link

Increase zoom level so that tile layer doesn't disappear upon zooming in #7

Closed cardinalgeo closed 1 year ago

cardinalgeo commented 1 year ago

Hello again Trame Team!

I'm attempting to increase the level to which I can zoom, such that I can continue zooming into a tile rather than everything "going grey" (i.e., the tile layer disappearing). This issue is illustrated by the following screen video:

https://user-images.githubusercontent.com/44422832/217973448-8b7decf0-f4b8-4cd1-92f7-6140f1598b39.mov

In the most extreme sense, I'd like to be able to zoom until just one pixel fills the screen. I've tried changing the maxZoom parameter as follows, but without luck:

with leaflet.LMap(zoom=("zoom", 1), maxZoom=15):
    # tiles
    image_viewer = leaflet.LTileLayer(url=("tile_url", "/tile/{z}/{x}/{y}.png"), noWrap=True)

and

with leaflet.LMap(zoom=("zoom", 1)):
    # tiles
    image_viewer = leaflet.LTileLayer(url=("tile_url", "/tile/{z}/{x}/{y}.png"), noWrap=True, maxZoom=15)

Do you have any suggestions?

banesullivan commented 1 year ago

I haven't dug into this too deeply but my initial thought is that vue-leaflet does not expose all of the Leaflet.js options for TileLayer in LTileLayer:

It looks like maxZoom and minZoom are missing in the Vue wrapping (and maybe some other parameters). This makes me think that maxZoom=15, when passed to LTileLayer, is simply ignored.

This is just my initial thought.


I am, however, about to face the same problem and will dig into this a bit more to find a solution. I'm predicting that we will need to set a different, non-geospatial coordinate system for the LMap so that it's in image coordinates and the zoom levels make a bit more sense in the given context.

cardinalgeo commented 1 year ago

Mmm, I see the discrepancy in the documentation between Leaflet.js and vue-leaflet and agree that the parameters are likely getting passed but then ignored — frustrating.

I'd definitely like to be kept in the loop if you find a solution, thanks @banesullivan!

jourdain commented 1 year ago

Ok so the doc is missing the info but the vue code has it... So you can pass it, but the missing part is that trame-leaflet is missing that definition of the zoom which make it not produce what you expect. You can validate that by printing that element print(elem.html).

But you can dynamically add properties so they get pass to the html by providing the _properties=[('max_zoom', 'maxZoom'), ...]

You can see the code here: https://github.com/vue-leaflet/Vue2Leaflet/blob/master/src/components/LMap.vue#L45-L63

cardinalgeo commented 1 year ago

Hi @jourdain, thanks for pointing this out! Indeed adding _properties=[('max_zoom', 'maxZoom'), ...] ensures that those parameters are used. However, it seems that setting maxZoom alone is not sufficient to resolve this issue. As @banesullivan explains in another repo here (almost a year ago today!), maxNativeZoom is an additional parameter that must be set. By setting this value lower than maxZoom, zooming beyond maxNativeZoom will result in rescaling of the tile corresponding with that zoom level up until maxZoom is reached. If the maxNativeZoom parameter is not set, the "greying out" behavior seems to result.

Apparently the maxNativeZoom parameter can be passed in Vue2Leaflet, just to options rather than properties (see discussion here, in which it was decided to not "upgrade" maxNativeZoom from option to property). The usability of this parameter been verified here.

However, I've tried multiple ways to pass the maxNativeZoom parameter in trame-leaflet without success. Examples include the following:

with leaflet.LMap(zoom=("zoom", 2), maxZoom=4, minZoom=2, _properties=[('max_zoom', 'maxZoom'), ('min_zoom', 'minZoom')]):
    # tiles
    image_viewer = leaflet.LTileLayer(url=("tile_url", "/tile/{z}/{x}/{y}.png"), noWrap=True, options="{ maxNativeZoom: 3, maxZoom: 4, minZoom: 2 }")

I suspect there is a simple solution but that my lack of experience with JavaScript is obscuring it. Any thoughts?

jourdain commented 1 year ago
leaflet.LTileLayer(
    url=...,
    options=("{ maxNativeZoom: 3, maxZoom: 4, minZoom: 2 }",),
)

otherwise, options will be set as a string rather than a JS object.

jourdain commented 1 year ago

Also make sure option is passed by printing the html of the element... You need to see :options="{...}" rather than options="{...}".

cardinalgeo commented 1 year ago

Your suggestion resolved it, thanks! As the desired functionality is indeed possible, I'll close this issue.

jourdain commented 1 year ago

If you are up for it, you can add the missing props here using the code as ref instead of the doc in a PR.

Actually while pointing at the code, you already had those props but they are camelCase rather than snake_case.

banesullivan commented 1 year ago

@jourdain, re-open the issue and assign to me -- I'm happy to expose these parameters in trame-leaflet

jourdain commented 1 year ago

The code in trame-leaflet is already good and had everything that was needed. The only thing you could do if you want, is to map those JS names to Python name using the snake case convention.

banesullivan commented 1 year ago

I see, thanks for clarifying

cardinalgeo commented 1 year ago

Hi all, apologies for the delay! I'd love to help if I can. I realize that this would be a fairly trivial change, but this could be a good, simple first PR for a novice contributor. @jourdain, would I just use mapping styled like it is here?

jourdain commented 1 year ago

Yes exactly. On the left the python name and on the right the JS arg. Ideally, you should add Python doc string like here

cardinalgeo commented 1 year ago

Great, I've made the pull request!

I wasn't sure about the doc string – did you mean for all the params in Leaflet.py? Or just the min/max zoom params?