Skip to content

[WIP] Complete the MapLibre Compose migration - #7088

Draft
sargunv wants to merge 109 commits into
streetcomplete:masterfrom
sargunv:codex/maplibre-compose-completion
Draft

[WIP] Complete the MapLibre Compose migration#7088
sargunv wants to merge 109 commits into
streetcomplete:masterfrom
sargunv:codex/maplibre-compose-completion

Conversation

@sargunv

@sargunv sargunv commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

@westnordost

westnordost commented Sep 10, 2026

Copy link
Copy Markdown
Member

Hmm, I am not quite finished with maplibre-compose before handing it off to you. My plan has been to

  • upgrade to maplibre-compose and maplibre-compose:location to 0.16.0

  • have a final look at the *Source classes, see if I can make it so that they don't continue to update data when noone is listening (e.g. the map data for display in overlay doesn't need to be fetched when no overlay is active)

  • leave some implementation notes

From there, you could just continue on the maplibre-compose branch or even preferredly stack it upon it so that it is easier for me to review (I don't need to see the changes that I've done on maplibre-compose branch).

I am not sure where this draft PR stands now. Does it even make sense for me to do the above still or would this just make things more complicated? You write that it is stacked upon #6352, but #6352 has been just the unreviewed probe that migrated everything including what happens after the compose migration, introducing a desktop target, the stuff I split out to separated tickets and maybe more.

Edit: wrong ticket number, my bad!

@westnordost westnordost added the iOS necessary for iOS port label Sep 10, 2026
@sargunv

sargunv commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Ah I guess I mentioned the wrong base pr (that's what I get for deleting and rewriting the ai's description lol)

I started off this draft upgrading to v0.16, then split off the location updates applicable to master into their own PR and merged that branch back into here, which necessitating telling GitHub this branch is based on "master" so it doesn't show unrelated diffs, but instead of now shows your maplibre-compose diffs

I'm happy to reshape this into whatever form is most convenient for you to review; the tedious git cherry pick or rebasing is easily automated now with an agent

@sargunv

sargunv commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Now that I have push access, I can regularly merge main into your maplibre-compose branch, and set this PR's base to that branch, so the diff will be exactly what I changed on top. If you want, I can also split changes into separate PRs, and you can review/merge into maplibre-compose iteratively?

Really whatever process is convenient for you, I can probably work with it

@westnordost

westnordost commented Sep 11, 2026

Copy link
Copy Markdown
Member

Here are my notes regarding the migration to maplibre-compose

  • the current Android map uses SDF icons in some places. Let's not use SDF icons anymore because they don't look good when they overlap. (Note that in the maplibre-compose branch, no icons are displayed at all yet. This is not implemented)

    • the icon of a (quest) pin shall be drawn with pinPainter(questIconPainter)
    • any icon from an overlay (see OverlayStyle) shall be drawn with the WithHaloPainter (replacing usage of SDF icons). Ideally, the color and the width of the halo shall be the same as for text on the map
    • icons in geometry markers are also drawn with the above halo. I am not sure myself whether it would look good to tint them in the Color.GeometryMarker color, in any case, we have the ColorFilterPainter in which the drawable can be wrapped in order to be tinted
  • MapLibreMapTilesDownloader - if possible I'd want to remove the url pointing to https://streetcomplete.app/map-jawg/streetcomplete.json and instead have it point to a local url with a bare-bones style definition. (I want to archive the repo at https://github.com/streetcomplete/maplibre-streetcomplete-style )

  • The MapFragment/MainMapFragment currently does a few things that are not necessary to port:

    1. deleting old offline regions - that's already done by the cleaner
    2. calculating finger size and querying a larger area than normal; this is just a tweak/hack
  • every Overlay can currently define a list of layer ids that should be hidden when the overlay is selected. However, if it is too awkward to implement this functionality, a hack specifically for the AddressOverlay is fine, too, because that's the only overlay which actually uses that feature to hide a layer

  • current structure is:
    MainActivity is parent of MainMapFragment which inherits MapFragment which contains the map. MainActivity also is the parent of the compose MainScreen in which we have various MainScreenControls and the MainBottomSheet that shows the quest forms/overlay forms. MainActivity also owns the MainViewModel, MapViewModel and MainBottomSheetViewModel.
    The map, the controls and the bottom sheet all need to be coordinated with each other. This all bubbles up through the MainActivity which then reaches down again (partly through ViewModels, partly by passing down parameters) to inform the other side of changes. This makes the whole affair look quite messy. Once the map screen is ported to maplibre-compose, much of that can be removed, shrinking the MainActivity by a lot.
    When I last looked a the probe, the AI agent actually did a good job in reducing that complexity, although it did keep quite a few structures that only existed because of the setup described above because it didn't detect these as merely temporary ones to bridge compose code with Android view code.
    Removing all that cruft and rewiring it all will likely be the biggest part of the remaining migration work.

  • because currently the MainActivity needs to communicate between the Android view code and the compose code, for some things it uses view state in the view model, see e.g. the last 8 or so properties in MainViewModel. I'd like to remove or otherwise minimize that view state in view models. There are two main reasons for this:

    • communication through the view model between different levels in the view hierarchy breaks the unidirectional data flow. Some or most of it will not be necessary anymore during/after the migration
    • making app state in ViewModels persist beyond activity/app recreation due to it being cleared from memory is (ironically) quite a lot more awkward to do in ViewModels than doing the same in the composition via rememberSavable or rememberSerializable. We use the latter a lot with quest forms, so that the data input in an open quest form will be restored when the app is recreated into memory.

    So, I'd prefer if ViewModels are are just used to provide access to persisted resources (data from database, from preferences, etc.) rather than keep state themselves

  • note the TODO maplibre-composes in the code. Some TODOs can only be done after the Android map has completely been replaced. Most of these are actually markers what more can be thrown away or at least replaced with less code.

  • the CopyIconsTask task can be deleted after maplibre Android is gone

  • after migration, check for any occurences of R.string. within the project. If none are left (which depends on where exactly you put the cut-off point of the migration) you can also delete the CopyStringsTask task as well as the kotlin.srcDirs(layout.buildDirectory.dir("generated/androidMain/kotlin")) line

  • ShowMapScreen is just for debug and can be deleted before this PR is ready to merge

@westnordost westnordost moved this to In Progress in iOS Port Sep 11, 2026
@westnordost

Copy link
Copy Markdown
Member

Ah I guess I mentioned the wrong base pr (that's what I get for deleting and rewriting the ai's description lol)

Oh, no, actually, looks like you mentioned the correct PR from the start! Apologies! It was me who misread!

@westnordost

Copy link
Copy Markdown
Member

I also noticed that the UpdateMapStyleTask can be removed, too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai generated iOS necessary for iOS port

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

Migrate to Maplibre-Compose

2 participants