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

Explore migration from tangram to MapLibre #3123

Closed
smichel17 opened this issue Aug 5, 2021 · 57 comments · Fixed by #5693
Closed

Explore migration from tangram to MapLibre #3123

smichel17 opened this issue Aug 5, 2021 · 57 comments · Fixed by #5693
Assignees
Labels
enhancement iOS necessary for iOS port

Comments

@smichel17
Copy link
Member

Originally posted by @westnordost in #1892 (comment)

MapLibre (ex Mapbox SDK) now supports Metal, so an iOS port would probably (need to) use MapLibre instead of tangram-es. This requres to

  • create a mapstyle for mapbox / maplibre (for jawg.io map tiles) that matches the map style for tangram
  • check it out, see if it has all features required by StreetComplete (probably yes)

As tangram is not really developed further, it might be a good choice to change to using the maplibre SDK for Android too. Or - bluesky - use/create a map view that directly renders the OSM data.

Additionally, there's issues like #3101 which are complicated to fix due to working around tangram limitations.

From the parts I am familiar with, I think StreetComplete already has a decent wrapper around Tangram (e.g. KtMapController), so in many places it would "just" be a matter of swapping out the implementation of those interfaces, and then plugging any leaks in the abstraction. There's also the scene files; I don't yet know how they map to MapLibre features. And any other tangram features we use that I'm not familiar with.

@westnordost
Copy link
Member

  • the tangram wrapper is very tangram specific. There is no sense in keeping if if moving to MapLibre
  • scene files are incompatible. A custom style would have to be written anew.

@smichel17
Copy link
Member Author

smichel17 commented Aug 5, 2021

My first impression was that MapLibre isn't quite ready for use on Android— I wasn't able to find any docs besides this README, which still contains many links back to the mapbox docs. Based on the commit history it also seems like the Android version is not under particularly more active development than tangram (equivalent history). (That could be misleading, though, if most of the work is happening in code that's shared between platforms.)

@smichel17
Copy link
Member Author

smichel17 commented Aug 5, 2021

Reading the MapBox camera guide, it seems similar enough to tangram's api, and perhaps the center the camera within a map area feature could be used to fix #3101. So if these are still accurate, perhaps it will not be as much effort as I thought.

@westnordost
Copy link
Member

I consider MapLibre / MapBox SDK somewhat more mature than tangram-es though. (Without having had a closer look though.): It is older, i.e. had some years headstart, and now for 3 years, tangram-es wasn't really much developed on.

@smichel17
Copy link
Member Author

smichel17 commented Aug 5, 2021

I'm reading through the MapBox docs now, and I agree it seems that way. Particularly interesting to me: it looks like dynamic display of quests may be possible without re-rendering the entire map. The main problem is that I'm not sure what features have changed since the MapLibre split; I didn't find any MapLibre documentation, and I don't see any way to reference the MapBox guides at the version before the split (the javadocs are versioned, so that's fine).

edit: Here's a good overview: https://docs.mapbox.com/help/getting-started/add-markers/. Unfortunately,

  • Mapbox Maps SDKs v10 includes wrappers supporting this approach.
  • Mapbox Maps SDKs less than v10 offer plugins supporting a limited version of this approach.

The MapLibre fork happened just before version 9.3, so we might be limited to the latter.

Even still, combined with data driven styling, I think we could easily enough do things like make the quest dots the same color as the background of the pin that will appear when they're expanded.

@smichel17
Copy link
Member Author

smichel17 commented Aug 5, 2021

They have a few different render & camera modes, which I think could replace our custom "compass mode" https://docs.mapbox.com/android/maps/guides/location-component/#rendermode (although with this approach, we give up some control / ability to tweak later)

@westnordost
Copy link
Member

westnordost commented Oct 5, 2021

So the following steps need to be taken here:

Phase 1

  1. create a requirements catalogue, i.e. what features must the library provide, what would be good but is not required, what is extra
  2. create a minimal Android app and implement/demo all the features mentioned above
  3. finally, evaluate. Are all necessary features covered? Estimate how long it would take to migrate and evaluate what we gain from that. (Do we need less code because there are less workarounds? Do we need to add additional workarounds? etc.)

After the evaluation, if we decide it is worth it, there may be a phase 2 - to actually do the migration.
I can help with the requirements catalogue, i.e. when it is almost complete, I'll review it and also add anything you may not have thought of (because features that require this don't exist yet).

@matkoniecz
Copy link
Member

create a requirements catalogue, i.e. what features must the library provide, what would be good but is not required, what is extra

generic mandatory features:

  • decent performance (hundreds of SVG icons should not harm performance)
  • decent battery performance (at least not even worse than Tangram)
  • actively maintained
  • usable as far as documentation and bugginess goes
  • on a compatible license

specific mandatory features:

  • show map from a vector source
    • compatible with data schema used by Jawg
    • compatible with data schemas used by alternative providers (see branches of map style)
  • custom styling of a vector map
  • display of interface above map (zoom/locate/undo/hamburger icons, star count)
  • show many SVG based icons at specified positions (quest icons)
    • raise interface on clicking them
  • display of a custom node geometry (quest object, hidden dots, current location)
  • display of a custom way geometry (quest object, GPS tracks)
  • display of a custom area geometry (quest object)
  • display of a custom labels and icons (display of nearby objects for some quests)
  • display of rotated image at specific point (to display direction of view, pointer to the current location when offscreen)
  • fetching map data into cache
  • smooth zooming (on quest selection, map panning, following GPS location)
  • 3D view (is it a critical feature or nice to have?)

nice to have:

What is missing in this listing?

pinging @westnordost as it is relatively important

pinging @smichel17 as help with finding missed issues is likely here

@westnordost
Copy link
Member

Hmm, the following more, otherwise I think it is complete

further mandatory:

  • possibility to display a line left and a line right of the "real" line, each with a different color. (For sidewalk/cycleway layer). Very likely possible, see e.g. zlant parking lanes viewer
  • fetching map data into cache: This means, it should be possible to fill the cache for MapLibre using a process that has nothing to do with MapLibre (or MapLibre has specifically the functionality to pre-download an area). That's what we do currently - download an area into the cache that tangram uses.
  • zooming into and/or calculating which camera position is necessary to fully display a given geometry (with offsets, because the bottom sheet obscures a part of the map)
  • regarding quest pins: 1. it should be performant to display thousands of them simultaneously. 2. it should be performant to add/remove single quest pins.
  • iOS support. But that is a given.

nice to have:

  • simultaneous camera animations. E.g. the view is animating the rotation with the movement direction but then also an animation for zoom-in (after user presses button) starts

@westnordost
Copy link
Member

@matkoniecz did not start working on it yet, so for the time being, this is free for the taking / can be done by other people interested in this. If you do, I can assign you to "reserve" this.

@adrianclay
Copy link
Contributor

I've made a start exploring this.

I've hacked together a version of SC with a side-by-side of Tangram / MapLibre, as I believe it will be (for me) an easier way of demoing desired requirements.

image

For now I'll be focusing on exploring the list of requirements above.

@westnordost
Copy link
Member

Whoa, cool!

Note that for some time, I have been playing with the thought whether it would not be less effort / more flexible / faster to skip vector map libraries and directly render the OSM data (etc.) onto the map like Vespucci or iD does. I.e. facilitate some (hardware accelerated) canvas-drawing of sorts.

Advantages:

  • less download necessary. No map cache necessary. I.e. no vector map service provider needs to be used at all or at least only for low zoom levels
  • absolute flexibility
  • no need to bend the code to the API of the map rendering library (which has been a strain sometimes for tangram-es)

Not sure:

  • Effort. Drawing some lines will be easy, and all the additional stuff the app is drawing and generally doing with the map has been a strain to implement with tangram-es. It could turn out to be equally as difficult (or not possible) with MapLibre. On the other hand, for doing the drawing all by oneself, the devil is probably in the details. E.g. rendering road names on such lines would probably be something that falls onto ones feet.

Disadvantages:

  • Coastlines will be a problem. Maybe to circumvent this, always show the satellite view and ditch the light/dark mode
  • tilting the map and possibly rotating the map will probably be much harder to implement (when thinking about simply drawing all the stuff on a simple 2D canvas)

Anyway, these were my thoughts so far for the unicorny outlook of maybe rendering the map by oneself, using the Canvas or somesuch. I felt I needed to record this here before looking into MapLibre starts in earnest, just to mention another alternative.

That considered, I am of course very happy that someone looks into this, as tangram-es issues unfortunately have been piling up and piling up, to the point where I do not even bother to open bug reports in their repo anymore as it looks like it has finally been abandoned. Moving to the widely used MapLibre looks very much like the step in the right direction here.

@smichel17
Copy link
Member Author

tilting the map and possibly rotating the map will probably be much harder to implement (when thinking about simply drawing all the stuff on a simple 2D canvas)

I wonder if we could borrow significant amounts of code from MapLibre/Tangram for this.

@westnordost
Copy link
Member

If you have any updates or news, I'd be glad to hear about it! This is one remaining abandonware thorn in the sides of StreetComplete it would be very nice to get rid of (or explore how it can be done at least). If you have the code somewhere public, it can also help for future attempts even if you do not continue to look into it.

@adrianclay
Copy link
Contributor

Hey - I'm afraid to say I've been unable to prioritise this issue and give it the effort it needs (which is a lot).

I've pushed up the changes I'd made here to 8f2d596

I've documented some of my thinking within the commit message. If anybody is interested in taking this over, I'd be happy to spend some time talking through the code and answering questions over a video call.

My super summarised view is that I can't see any major reason not to migrate, other than the effort required in doing so 😄. Any time spent on this is time not spent on other changes, conversely the sooner this migration happens the less painful it will be.

Bonne chance.

@Helium314
Copy link
Collaborator

Helium314 commented Mar 4, 2023

I played a bit with the mapLibre work of @adrianclay, and added some more stuff: https://github.com/Helium314/SCEE/tree/maplibre
After updating mapLibre to 10.0.2 it was necessary to update OkHttp to 4.10, leading to some more small changes.

As for the feature list

generic mandatory features:

  • decent performance (hundreds of SVG icons should not harm performance)
    • small but noticeable preformance impact, icons need to be converted to bitmap before passing to MapLibre, so actually not SVG icons
  • decent battery performance (at least not even worse than Tangram)
    • Actually there is definitely higher CPU usage when scrolling and zooming, even without any added icons, so effect on battery is quite likely. Though there is a chance this is caused by the jawg default style used for testing.
  • actively maintained
  • usable as far as documentation and bugginess goes
  • on a compatible license
  • iOS support. But that is a given.

specific mandatory features:

  • show map from a vector source
    • compatible with data schema used by Jawg
    • compatible with data schemas used by alternative providers (see branches of map style)
      • raster tiles not tested, but should be possible
  • custom styling of a vector map
  • display of interface above map (zoom/locate/undo/hamburger icons, star count)
  • show many SVG based icons at specified positions (quest icons)
    • not actually SVG based, as all drawables are converted to bitmaps by MapLibre
    • raise interface on clicking them
  • display of a custom node geometry (quest object, hidden dots, current location)
  • display of a custom way geometry (quest object, GPS tracks)
    • tracks not tested, but way geometry works
  • display of a custom area geometry (quest object)
  • display of a custom labels and icons (display of nearby objects for some quests)
  • display of rotated image at specific point (to display direction of view, pointer to the current location when offscreen)
    • not tested, but rotated direction of view is displayed
  • fetching map data into cache
  • smooth zooming (on quest selection, map panning, following GPS location)
    • manual zooming is smooth, but zoom animations on quest selection are choppy
  • 3D view (is it a critical feature or nice to have?)
  • possibility to display a line left and a line right of the "real" line, each with a different color. (For sidewalk/cycleway layer). Very likely possible, see e.g. zlant parking lanes viewer
  • fetching map data into cache: This means, it should be possible to fill the cache for MapLibre using a process that has nothing to do with MapLibre (or MapLibre has specifically the functionality to pre-download an area). That's what we do currently - download an area into the cache that tangram uses.
  • zooming into and/or calculating which camera position is necessary to fully display a given geometry (with offsets, because the bottom sheet obscures a part of the map)
  • regarding quest pins: 1. it should be performant to display thousands of them simultaneously. 2. it should be performant to add/remove single quest pins.
      1. there is a small, but noticeble slowdown compared to no pins
      1. performance comparison is difficult but changing single pins require re-setting all data for the source. Rhough Removing (or rather hiding) might be possible using filters.

nice to have:

not in the list, but relevant and not tested:

  • anything related to changing the map style

I think the next steps should be:

  • try the untested mandatory features (especially offline tile management, including a defined tile cache size and raster tiles for satellite view)
  • actually make things show the same as tangram (overlays, quest dots, nearby element highlighting, hiding pins when highlighting a quest)
  • remove tangram and test performance (to make sure side-by-side view doesn't do bad things here)
  • cosmetic stuff (map style, actually showing pins instead of just the round icons,...)

@westnordost westnordost moved this from Todo to In Progress in iOS Port Dec 20, 2023
@Helium314
Copy link
Collaborator

@Helium314, would you consider the work you did to advance this in the maplibre branch on SCEE to be a good base to finish the implementation, or would you say that it was rather a prototype to have a stab at how (and if) things could be done with MapLibre? Regarding your post from April, have you lost motivation to work on it for good, or is this something you may carry on with later? (Maybe spurred by the creation of that MapLibre style? 😛)

I'd definitely call it a prototype. Some parts are probably fine, while others are more experimental.
E.g. all the stuff in MainMapFragment is mostly for testing how to do things, contains a lot of comment on what works well and what doesn't, contains overlay styles that should better be in some style json, contains stuff that should not be in there,...

@Helium314
Copy link
Collaborator

I think I would work on it again, but can't really say whether I'll have the necessary time

@westnordost
Copy link
Member

westnordost commented Dec 21, 2023

Cool! It would also be the most efficient way to go about this, because you already familiarized yourself somewhat with the framework. I'll consider this ticket reserved for now (there is enough other things that potential contributors could do).

@Helium314
Copy link
Collaborator

I started checking the current state again and worked on some stuff that was still missing.

Anyway, I remembered the main issue with the current state: interaction with the map (sources and annotation managers) must be done on UI thread, and for testing I simply exposed the MainActivity in its companion object for runOnUiThread. This works, but is not the way to go.
How should it be handled properly?

@westnordost
Copy link
Member

Hm well, any interaction with the UI has to be done on the UI thread in Android. So I guess, the answer is, to

  • (optional:) annotate every method that controls the Maplibre map with @UIThread to make the IDE understand and highlight that it is an error to not call it from a UI thread
  • wrap any data updates the same way as it is done for UI updates (viewLifecycleScope.launch { ... })

@westnordost
Copy link
Member

FYI work in progress here: Helium314#516

@riQQ
Copy link
Collaborator

riQQ commented Apr 14, 2024

All the issues related to Tangram ES that I could find.

Might help with

Could be reopened, no more investment in old renderer

Should resolve

Caused by zoom limitation

already worked around, but migration fixes performance hit

already worked around, workaround can probably be removed

@westnordost
Copy link
Member

I went through the list and took the liberty to add strikethrough for issues that will not be touched by migration to maplibre.

@westnordost
Copy link
Member

I searched in the issue tracker for "maplibre" and further found the following issues:

Could be revisited (that it can be solved with maplibre is not guaranteed)

Resolved

@westnordost
Copy link
Member

I searched for "tangram" and looked at issues down to # 3000. You missed some, I added them to your post.

@westnordost
Copy link
Member

westnordost commented Apr 15, 2024

Thank you, I looked through the list and added them to the changelog here.

The Caused by zoom limitation-stuff plus #179, #869, #1264 I did not add yet, because we did not implement quest clustering yet, which may or may not solve the issue of several quests at the same position but (ofc) only one being selectable.

@westnordost westnordost moved this from In Progress to Blocked in iOS Port May 23, 2024
@westnordost
Copy link
Member

We are basically done with this now, see the todo list to the top of the PR.

Final integration is blocked by two critical bugs in MapLibre and (maybe) by a to-be-reported issue - at least we'd need to work around that if it is not fixed.

Helium314#516

@westnordost westnordost added blocked blocked by another issue and removed help wanted help by contributors is appreciated; might be a good first contribution for first-timers labels May 23, 2024
@westnordost
Copy link
Member

Anyone who would like to see this going forward and knows C++: MapLibre is always on the lookout for contributors.

@FloEdelmann FloEdelmann linked a pull request Jul 20, 2024 that will close this issue
@github-project-automation github-project-automation bot moved this from Blocked to Done in iOS Port Aug 14, 2024
@riQQ riQQ removed the blocked blocked by another issue label Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement iOS necessary for iOS port
Projects
Status: Released
Development

Successfully merging a pull request may close this issue.

10 participants