search

acalcutt / tileserver-gl

Vector and raster maps with GL styles. Server side rendering by Mapbox GL Native. Map tile server for Mapbox GL JS, Android, iOS, Leaflet, OpenLayers, GIS via WMTS, etc.
https://tileserver-gl.readthedocs.io/en/latest/
Other
5 stars 3 forks source link

Add marker to static images #4

Closed boldtrn closed 2 years ago

boldtrn commented 2 years ago

Thanks a lot for maintaining this fork! I just gave this a try and it seems to work well :tada:. One thing that is currently missing from tileserver-gl is the ability to add markers to static images.

There is an open PR upstream for this: https://github.com/maptiler/tileserver-gl/pull/427

I am wondering if you would be interested in adding this feature to your fork?

acalcutt commented 2 years ago

I'd be interested in adding in markers. What do you think is better, https://github.com/maptiler/tileserver-gl/pull/427 or the solution bu @mnutt in that thread.

boldtrn commented 2 years ago

I think it would be great if the marker image/symbol would be customisable. I think @mnutt did this in his solution. But I think the implementation is more complex, so I am not sure if this would be too much regarding implementation effort?

I would be happy with either solution.

acalcutt commented 2 years ago

I merged in the simpler markers from https://github.com/maptiler/tileserver-gl/pull/427 in v4.0.24 . Before that I also did a lot of work trying to merge in @mnutts changes, but he also changed the structure which made it hard to merge.

Examples https://tiles.wifidb.net/styles/WDB_OSM/static/13.389587,52.515359,12/500x500.png?marker=center

https://tiles.wifidb.net/styles/WDB_OSM/static/13.389587,52.515359,12/500x500.png?marker=13.399587,52.491359

https://tiles.wifidb.net/styles/WDB_OSM/static/13.389587,52.515359,12/500x500.png?marker=13.399587,52.491359|13.381187,52.515359

note this also has the static image fix I mentioned in your other thread

boldtrn commented 2 years ago

Awesome, thank you very much 🎉. I tested this on my server as well and it seems to work great.

Just one minor note, tileserver-gl also offers the auto option, which doesn't seem to work with the marker. So this url is not working:

https://tiles.wifidb.net/styles/WDB_OSM/static/auto/500x500.png?marker=13.399587,52.491359

I think it makes sense that it only works with paths. As we have no way to get the zoom level right anyway. But probably the marker is not included in the auto calculation. I guess this might be something for a follow-up issue?

Do you think it would be possible to make the marker configurable? For example in the config.json? My idea would be to do this similarly to how we define styles/fonts, we could define a resources folder and in the request, we could add a parameter like markerName and the name would be the file name in the resources folder. That way we could serve context based markers.

Before that I also did a lot of work trying to merge in @mnutts changes, but he also changed the structure which made it hard to merge.

I already thought that this might be tricky 👍

acalcutt commented 2 years ago

what does auto do? not sure why the code added would break that

I think it would be great to make it more configurable, but I'm not sure I have the time to do a lot with it. I think at minimum color, text, and maybe size options would be nice. I think It could be simple to add those into the url. I'd welcome a PR...

Also, if anyone is feeling ambitious and has a bit more knowledge of javascript than me, It would be great to finish merging @mnutts's fork I was working on here https://github.com/acalcutt/tileserver-gl/tree/markers2 . I feel it was getting close but I was getting a bit lost in it.

boldtrn commented 2 years ago

what does auto do?

It automatically sets the map area and zoom to fit the path, the same would make sense for markers as well. Not for a single marker, but if you have multiple markers or mix markers and path, it might be nice.

I'd welcome a PR...

I can take a shot at it, but it might take a bit for me to come around :)

acalcutt commented 2 years ago

I saw this in the insights/network. someone improving the markers to be more flexible https://github.com/bikemap/tileserver-gl/commit/b7a41327e190eac477f5510ca685b3b08a6ad84e

Is this you or someone else working on something similar

boldtrn commented 2 years ago

Thanks for digging deeper here @acalcutt. I haven't seen this repo earlier. For me this looks like what I am looking for (at least from looking at the code). I haven't tried running it though.

acalcutt commented 2 years ago

I haven't run it either, I just happened to stumble across it and saw it was similar to what we were working on. could be good to bring over some of those changes.

acalcutt commented 2 years ago

one thing i notice that differs is it looks like it only supports a single marker

boldtrn commented 2 years ago

Ah you are right, they seem to be using the path variable and check if there are is only one coordinate, then they thread it as a marker instead of a polyline. I guess mixing both approach would be they way to go then :)?

boldtrn commented 2 years ago

I gave the changes by @benedikt-brandtner-bikemap a try and it seems to work. Also with multiple markers.

So for example the following works: http://localhost:8080/styles/basic-preview/static/8.536,47.379,12/500x500.png?marker=8.536,47.378|marker.svg|scale:0.05&marker=8.536,47.4|marker.svg|scale:0.05&path=8.536,47.4|8.536,47.378

Screenshot 2022-09-20 at 18 15 10

I tried the changes by @benedikt-brandtner-bikemap on top of https://github.com/acalcutt/tileserver-gl/commit/c7a6f753fce26804fb1c69d11ff145e3818ee5f1. The commit history here is a bit messy (I should have cherry picked but merged instead 🤷). https://github.com/acalcutt/tileserver-gl/compare/main...boldtrn:bikemap_markers?expand=1

As a first experiment this seems to work and it might really be interesting to use this instead of our current approach. I added a bit of input sanitation, I think before using this on a user facing service more safety checks would be required and maybe using any remote hosted images might not be a good idea for user input, so this should probably be configurable.

Since the changes were made by @benedikt-brandtner-bikemap, maybe he would be interested in contributing these changes back upstream?

benedikt-brandtner-bikemap commented 2 years ago

Hey,

happy to see that my approach could help you :+1:

yeah i always planned to push the changes back to the upstream openmaptiles repository, just never got the time to create a proper PR...

But I have recently rebased the changes ontop of the current master and will hopefully get to it while working on my current project.

acalcutt commented 2 years ago

@benedikt-brandtner-bikemap , would you mind if we merged those changes here for now?

I am also working on getting my changes to main and getting a bug in static images with maplibre-native fixed, but it would be a nice addition here since my custom maplibre fixes the static image bug.

acalcutt commented 2 years ago

well I kind of figured out why 'auto' doesn't work. It needs a 'path' like http://[server]:8080/styles/basic-preview/static/auto/800x600.png?path=5.9,45.8|5.9,47.8|10.5,47.8|10.5,45.8|5.9,45.8 (from page 21 here https://www.slideshare.net/klokan/tileservergl-hosting-vector-tile-maps-on-your-own-server-foss4g-2016-bonn )

the marker I added doesn't seem to work with it though

benedikt-brandtner-bikemap commented 2 years ago

Hey, sorry was a bit busy the last week..

Thanks for your fixes concerning the static-image rendering and implementing multiple render pools :)

I have rebased my changes on top of the current master (4.1.1) and opened a PR at the upstream repository! https://github.com/maptiler/tileserver-gl/pull/619

Greets

acalcutt commented 2 years ago

merged in static markers from maptiler/master. closed as completed