Complete the MapLibre Compose migration by sargunv · Pull Request #7088 · streetcomplete/StreetComplete

sargunv · 2026-09-10T03:49:08Z

Finishes the MapLibre Compose migration on top of the maplibre-compose branch. The main screen is Compose on all platforms, and the legacy Android map is gone.

Stacked on #6352, resolves #6072.

Deviations from @westnordost's implementation notes

Preset icons are SDF with native halos, not WithHaloPainter. MapLibre Compose's imperative image registration api requires the app to handle rasterization (will fix in next release).
Quest pins and overlay data stay subscribed while a form hides them so that closing the form does not reload them. Edit history pins are only loaded while the sidebar is open.

Validation

Verified manually on Galaxy Z Fold7 and Moto G 5G (2024), compared side by side with a master build on each device
Verified manually on iPhone 16 Pro with a temporary local launcher that launches the shared map
Verified manually with an Android API 25 emulator after a small patch in MapLibre Compose (will fix in next release)

…ping changes

…activity

westnordost · 2026-09-11T01:27:22Z

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

Key the inspection effect on the inspected object, zoom to clusters through the map state so the camera state needs no late init, and rely on the overlay effect alone to close an overlay form.

sargunv · 2026-09-18T05:12:07Z

These last six commits, 585ee76 to f24ef8a, I factored some logic out of MainScreen into other files to try to make it easier to digest.

westnordost

Ah, finally a complete review!

I am not entirely sure about (all those) states introduced. I have the vague feeling that they kind of make it harder to see/realize that certain state doesn't actually need to exist and instead make it less malleable to clean up. Sorry for being so vague... err... it's maybe related to that I haven't used the *State pattern much so far and also that my brain is working at 80% or so right now. But I do think that at least in a few instances, see below, I have a point.

Finally, not part of your changes, but mine in maplibre-compose and before, also driven by necessity before everything was compose. I have introduced quite a few ViewModels. Question is, since I read somewhere that ideally, ViewModels should be used on a per-screen basis, whether to merge the MapViewModel, BottomSheetViewModel etc into the MainViewModel? What's your thought on this?

westnordost · 2026-09-18T13:42:10Z

+                GeographicLayout(
+                    Modifier
+                        .windowInsetsPadding(WindowInsets.safeDrawing)
+                        .padding(if (sheet.isOpen) sheetPadding else PaddingValues(0.dp))
+                ) {
+                    if (!showIntroTutorial) {
+                        location.location?.position?.let { position ->
+                            PointerPinButton(targetPosition = position, onClick = ::followLocation) {
+                                Image(painterResource(Res.drawable.location_dot_small), null)
+                            }
+                        }
                    }


Man, this GeographicLayout stuff is pretty cool. You might have noticed that we place quite a few things on a certain map position. Such as:

on moving a node, the start of the arrow

the scissors when splitting a way (which snaps to the way)

in the address overlay, when adding a new address, the pin, as it snaps to building outlines; and for a few overlays, too

Right now, this is done in the quest / overlay form itself, so not at all in the overlay = {...} part of the MapLibre Map. Instead, we use the LocalGetOffsetCallback composition local ((LatLon) -> Offset?) to talk with the MapLibre map from deep within the composition hierarchy in order to get the current screen offset from any given geographic position.

A side effect of the "drawing on map from a different part of the composition hierarchy than one might expect" is that on the slide-in appear-animation of forms, the content that ought to be "on" the map, slides in from the bottom too, which doesn't look so good. Otherwise, it works fine, although since the maplibre-compose migration, it feels a little bit janky. (But that on 1. an emulator and 2. maybe it is due to other unrelated performance issues.)

The current solution was necessary with the Android map. Now that it is compose, we have at least the opportunity to think about whether it would be more straightforward to do it another way, e.g. with the GeographicLayout. Maybe... allow forms to specify a composable function to inject into the overlay part of the MapLibre map (if that's even possible)? What are your thoughts on this?

done in 3c4db1d, though I'm unsure about how I wired up this OnMap thing, it feels like a bit of a hack

I think doing it more neatly requires some deeper surgery in form state

took another pass at it in 1842041. Now each form has an overlay slot, more like the maplibre-compose demo app.

claude review flagged this, lmk if you want the entrance flag saved like before:

The entrance flag used to be rememberSaveable inside the form. As a field on the form object it no longer survives process death, since the object is recreated. The sub-form choice does survive because its state comes from rememberSerializable at the screen. If that matters, the flag can be constructed the same way.

Hmm, I read through it, and I find it looks very very complicated. This affects authors of new (overlay) forms, too. I want to keep this as simple as possible.

If there is no more straightforward way to do it, then I think I'd rather use the previous method. As written, the only (visible) downside of the previous method is that the content shown on the overlay is also animated in the enter and exit transition.

I initially was thinking that maybe a composition local might be used for this. Something like

val LocalMapOverlayContentCallback = compositionLocalOf<(@Composable (MapOverlayScope.() -> Unit)?> { null }

, provided by the main screen map and ready to be filled by whoever "answers the call".

But whether a composable function can be a composition local... unlikely!

…ose-completion # Conflicts: # androidApp/src/main/kotlin/de/westnordost/streetcomplete/screens/main/MainActivity.kt # app/src/androidMain/kotlin/de/westnordost/streetcomplete/AndroidModule.kt # app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/quest/AutoSyncer.kt # app/src/commonMain/kotlin/de/westnordost/streetcomplete/screens/main/MainViewModelImpl.kt # app/src/iosMain/kotlin/de/westnordost/streetcomplete/IosModule.kt

…n' into codex/maplibre-compose-completion

sargunv · 2026-09-18T19:57:17Z

All in all, MainLocationState class looks a bit unclear to me. Other than that the interface is very large and some properties seem to somewhat overlap each other or maybe don't even need to be public, I can't really say what I find wrong with it, if there is even anything wrong with it. And it somewhat makes sense to not have everything in MainScreen. I have so far not used this pattern a lot, it is probably the correct way to extract logic and most of all state from a composable and I am just not used to it.

I am not entirely sure about (all those) states introduced. I have the vague feeling that they kind of make it harder to see/realize that certain state doesn't actually need to exist and instead make it less malleable to clean up. Sorry for being so vague... err... it's maybe related to that I haven't used the *State pattern much so far and also that my brain is working at 80% or so right now. But I do think that at least in a few instances, see below, I have a point.

It's the usual way in Compose (and React, and similar frameworks) to extract and encapsulate tightly coupled bundles of logic. But exactly what is or should be bundled like that, where the boundaries are drawn, is pretty subjective.

for example in this app there's a lot of complexity in managing the camera, as it tracks the user, or focuses on quests, transitions between different modes. So extracting all that into a single state machine MainMapCameraState helps (me) understand and manipulate the camera more easily.

The other bundles, I'm less sure about; we can reshape them to boundaries that make more sense to you, or inline them if they're not serving their purpose well.

…ose-completion

…n' into codex/maplibre-compose-completion

sargunv · 2026-09-18T20:14:13Z

Question is, since I read somewhere that ideally, ViewModels should be used on a per-screen basis, whether to merge the MapViewModel, BottomSheetViewModel etc into the MainViewModel? What's your thought on this?

Generally I think a single ViewModel per screen is good, but I wouldn't just merge these three into one as-is. Some of this complexity is because the data layer exposes callbacks, and the "ViewModel" layer adapts them to StateFlows for Compose. We could simplify by making the data layer itself Flow based, then the ViewModels have less work to do, and can focus on simply adapting those services to UI, merged into one. I'd do that in separate PRs, perhaps one per data source.

For now, I think these MainBottomSheetViewModel, MainMapViewModel, EditHistoryViewModel classes can be kept but tweaked to not be ViewModel, but owned by the main ViewModel. Perhaps "adapters" or the existing naming "sources" and "controllers". Done in c6ca7d7, lmk what you think.

westnordost · 2026-09-18T21:50:56Z

for example in this app there's a lot of complexity in managing the camera, as it tracks the user, or focuses on quests, transitions between different modes. So extracting all that into a single state machine MainMapCameraState helps (me) understand and manipulate the camera more easily.

The other bundles, I'm less sure about; we can reshape them to boundaries that make more sense to you, or inline them if they're not serving their purpose well.

I see, thank you! This also helps me to see where you are coming from. In the next review I will focus on the changes you made / on the states. (By the way, feel free to ask me questions, too! Usually I remember pretty well why this or that has been implemented (before) in this or that way and whether it is important to keep it that way or if this could be done differently or simpler etc.)

westnordost · 2026-09-18T21:52:44Z

We could simplify by making the data layer itself Flow based, then the ViewModels have less work to do, and can focus on simply adapting those services to UI, merged into one. I'd do that in separate PRs, perhaps one per data source.

Yeah, that would be a whole new endeavour I'd like to not touch right now 😅.

it wasn't possible before because the attribution button was popup-based, i.e. it would display in front of everything else

…tting, eliminate MainLocationState Regarding the latter: Done because it was a leaky abstraction - locationProvider was still used in MainScreen. I found it was a detriment to readability and only saved up to 20 lines of code

westnordost · 2026-09-21T00:44:53Z

(I started reviewing - a bit more hands-on. Will continue tomorrow at the todo review markers I left. Did neither look at the "bottom sheet includes edit history sheet" nor the "forms may inject stuff into the map" yet but the former will be next.

Notes:

May need to split up the ShownBottomSheet stuff into a hierarchy so that there is no hacky and awkward bottomSheet == EditHistory ? -> oh, don't actually do anything because it is not a bottom sheet handling
that MainSheetSelection is public to MainScreen is sus. It should be an internal state that just triggers the necessary data to be loaded to in the end the ShownBottomSheet state being updated (which is exposed to Mainscreen
*Controller class names are already used in a somewhat different context. Investigate if they just should be named differently, whether to return to ViewModels, or something else

)

docs: plan remaining MapLibre Compose migration

eae253e

sargunv changed the title ~~Complete the MapLibre Compose migration~~ [WIP] Complete the MapLibre Compose migration Sep 10, 2026

sargunv added 3 commits September 9, 2026 21:18

Upgrade MapLibre Compose to 0.16.0

30fde51

Fix iOS framework linker arguments and record device validation gaps

ae78e8e

Remove unrelated Xcode build tracking from migration scope

2bc70b0

sargunv force-pushed the codex/maplibre-compose-completion branch from 09fc25b to 2bc70b0 Compare September 10, 2026 05:52

sargunv added 4 commits September 9, 2026 23:48

Upgrade MapLibre Compose location to 0.16.0

f37154f

Scope Android location provider to the activity and simplify tests

1c7e559

Merge PR streetcomplete#7090 location upgrade and consolidate overlap…

76814c9

…ping changes

Restart location updates on permission changes and scope provider to …

90ea458

…activity

sargunv changed the base branch from maplibre-compose to master September 10, 2026 08:48

sargunv added 3 commits September 10, 2026 02:14

Move shared map state and actions into the view model

39a595c

Refresh map geometry when inputs change

c9b852d

Document migration branch working preferences

57be4f0

This comment was marked as outdated.

Sign in to view

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

This comment was marked as outdated.

Sign in to view

sargunv added 2 commits September 10, 2026 11:56

Complete shared map data and interaction wiring

d9e3786

Keep auto sync ownership common and simplify location tests

aa30f18

westnordost added this to iOS Port Sep 11, 2026

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

This comment was marked as outdated.

Sign in to view

sargunv added 2 commits September 10, 2026 19:35

Track upstream permission recovery fix

6151945

Test converted location ordering in RecentLocationsTest

e582178

sargunv added the ai generated label Sep 11, 2026

sargunv added 3 commits September 10, 2026 19:49

Merge PR streetcomplete#7090 review fixes and simplify shared map code

9bec9f5

Align migration plan with maintainer implementation notes

d42faf1

Use painter halos for shared map icons

869a620

sargunv added 8 commits September 17, 2026 20:46

Never show the edit history sidebar together with a bottom sheet

c490052

Simplify the main screen's camera wiring

c075a0f

Key the inspection effect on the inspected object, zoom to clusters through the map state so the camera state needs no late init, and rely on the overlay effect alone to close an overlay form.

Move location and heading handling into a state holder

585ee76

Move camera inspection coordination next to the camera state

50a24e5

Let the map content choose and collect its own pins

3450a58

Decide the download area in the view model

ca836a7

Move crosshair and window offset math onto the map state

a7526dd

Order the main screen into state, map, actions, effects and content

f24ef8a

westnordost reviewed Sep 18, 2026

View reviewed changes

westnordost and others added 6 commits September 18, 2026 17:01

Treat the edit history sidebar as a bottom sheet selection

e2232d6

Show the location dialogs from the main screen

d1555f6

Describe map clicks and long clicks with the same structure

19d0672

Mark the main screen's sections as regions

98b636b

Merge remote-tracking branch 'origin/codex/maplibre-compose-completio…

a097c74

…n' into codex/maplibre-compose-completion

westnordost and others added 3 commits September 18, 2026 22:12

Merge branch 'maplibre-compose' into fork/sargunv/codex/maplibre-comp…

0a60200

…ose-completion

Run the main screen on one view model

c6ca7d7

Merge remote-tracking branch 'origin/codex/maplibre-compose-completio…

587a72f

…n' into codex/maplibre-compose-completion

sargunv mentioned this pull request Sep 18, 2026

[WIP] Move the application shell to shared Compose navigation sargunv/StreetComplete#1

Draft

Place form content on the map through the map overlay

3c4db1d

sargunv and others added 4 commits September 18, 2026 15:01

Give forms a map overlay slot that shares their state

1842041

Show each location dialog from its own flag

6d9aa32

may show main screen controls when tutorial is in front now

db60c68

it wasn't possible before because the attribution button was popup-based, i.e. it would display in front of everything else

Uh oh!

Conversation

sargunv commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deviations from @westnordost's implementation notes

Validation

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

westnordost commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

sargunv commented Sep 18, 2026

Uh oh!

westnordost left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

westnordost Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sargunv Sep 18, 2026

Choose a reason for hiding this comment

Uh oh!

sargunv Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

westnordost Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sargunv commented Sep 18, 2026

Uh oh!

sargunv commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

westnordost commented Sep 18, 2026

Uh oh!

westnordost commented Sep 18, 2026

Uh oh!

westnordost commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sargunv commented Sep 10, 2026 •

edited

Loading

westnordost commented Sep 11, 2026 •

edited

Loading

westnordost left a comment •

edited

Loading

westnordost Sep 18, 2026 •

edited

Loading

sargunv Sep 18, 2026 •

edited

Loading

westnordost Sep 21, 2026 •

edited

Loading

sargunv commented Sep 18, 2026 •

edited

Loading

westnordost commented Sep 21, 2026 •

edited

Loading