Much improved MapControls and edge to edge padding support

[Archdoog]

• Updates maplibre-compose to 0.0.17 to include DPI scaled map control positioning.
• Introduces precise mapControls position calculator.

Unrelated

• Moves Foreground service isStarted toggle to attempt to avoid crash when foreground is stopped immediately after starting navigation. Basically the service connector or binder crashes if you try to stop before it's fully booted/started.

Closes #202 - No longer needed (I think this works well enough automatically in all cases)

[ianthetechie]

What happened to the route polyline? 🤔

[Archdoog]

What happened to the route polyline? 🤔

This is now fixed using a fancier mutableStateOf(mapControls) architecture. It also uses a LaunchedEffect to dynamically update.

[Archdoog]

With the launched effect for map controls, we've actually finally triggered an unrelated problem! I'm speculating this code is making for some pretty bizarre concurrency issues and crashing the demo app all over the place w/ various location component/concurrency fatals. @ianthetechie hopefully you'll see the same thing when you run this on an emulator or device.

There are 3 or so different crashes roughly at launch that I can post in a separate ticket + if it does succeed at running, the location updates a very laggy.

Since the crashes are location component activation, StaticLocationEngine or Concurrency crashes, it led me right to that StaticLocationEngine.

[ianthetechie]

After looking at Rallista/maplibre-compose-playground#44, I don't think it's actually related.

The stack trace doesn't suggest anything more than something is triggering the internal location setup logic, which isn't the fault of the static location manager. Taking the error at face value, it sounds like the style hasn't loaded yet, and (for some reason unbeknownst to me) that's a problem for initializing location in the MapLibre Android SDK.

java.lang.IllegalArgumentException: Style in LocationComponentActivationOptions isn't fully loaded. Wait for the map to fully load before passing the Style object to LocationComponentActivationOptions.
    at com.mapbox.mapboxsdk.location.LocationComponentActivationOptions$Builder.build(LocationComponentActivationOptions.java:298)
    at MapLibreLocationKt.setupLocation(MapLibreLocation.kt:38)
    at com.maplibre.compose.ramani.MapLibreKt$MapLibre$3.invokeSuspend(MapLibre.kt:121)
    at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
    at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:104)
    at androidx.compose.ui.platform.AndroidUiDispatcher.performTrampolineDispatch(AndroidUiDispatcher.android.kt:81)
    at androidx.compose.ui.platform.AndroidUiDispatcher.access$performTrampolineDispatch(AndroidUiDispatcher.android.kt:41)
    at androidx.compose.ui.platform.AndroidUiDispatcher$dispatchCallback$1.run(AndroidUiDispatcher.android.kt:57)
    at android.os.Handler.handleCallback(Handler.java:942)
    at android.os.Handler.dispatchMessage(Handler.java:99)
    at android.os.Looper.loopOnce(Looper.java:201)
    at android.os.Looper.loop(Looper.java:288)
    at android.app.ActivityThread.main(ActivityThread.java:7924)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:936)
    Suppressed: kotlinx.coroutines.internal.DiagnosticCoroutineContextException: [androidx.compose.ui.platform.MotionDurationScaleImpl@4a4ab75, androidx.compose.runtime.BroadcastFrameClock@7d8d70a, StandaloneCoroutine{Cancelling}@494cb7b, AndroidUiDispatcher@57c4d98]

Here's what I've uncovered so far:

The awaitStyle function was being called multiple times, each time resetting the map style. This had some strange effects, as you might imagine, which I have not fully untangled.

You can modify the awaitStyle function to head off the issue of loading the same style URL multiple times in rapid succession like so:

internal suspend fun MapboxMap.awaitStyle(styleUrl: String): Style {
    val loadedStyle = style
    if (loadedStyle != null && loadedStyle.isFullyLoaded && loadedStyle.uri == styleUrl) {
        return loadedStyle
    }
    return suspendCoroutine { continuation ->
        setStyle(styleUrl) { style -> continuation.resume(style) }
    }
}

This removes the crash, but you should be asking at this point why it's calling awaitStyle multiple times!

I haven't yet found the answer, but can confirm the Slack message noting that it is janky whenever it loads without crashing. I don't know why 100%, but it's clearly related to this:

The requestLocationUpdates method is being called multiple times. Wat?

If you look at the call stack, you'll find that these multiple calls appear to be coming from the MapLibre composable's own LaunchEffect recomposing itself multiple times. The result is that you can end up with multiple callbacks registered to separate handlers. I don't know what the internal state of the map is, but it's clearly invoking setupLocation multiple times. I assume that, like much else in the ecosystem, this method is not completely idempotent and has weird effects when called many times back to back.

TL;DR, I don't think it's the fault of anything in the location code that we added (StaticLocationEngine). I suspect it has to do with the interplay of these two LaunchedEffects somehow.

[ianthetechie]

Few questions on this:

1. Shouldn't the function name be something like rememberMapControls or maybe rememberMapControlsForArrivalViewHeight? Kotlin can't really "read" like a Swift API since it doesn't have required argument labels.
2. Are you sure all 3 args should be optional? It seems like the first one should at least require some thought.
3. The other 2 args are never set explicitly, so I'm guessing this was just to make it easier in case we want to change them later? This is an internal API, so it's not something a user could customize; do you anticipate needing to vary it elsewhere or should we just use constants internal to the function?

[Archdoog]

This is a valid discussion and maybe the best move is we push this function to Maplibre-Compose as something more generic. It might take a couple of passes, but I think there's value in rememberMapControls that allows dynamically triggering padding within the window insets (or inside another calculated "window")

Bonus is this would let us debug that crash locally in the maplibre-compose example app.

[ianthetechie]

maybe the best move is we push this function to Maplibre-Compose as something more generic

Agreed. I think that's probably the right place for this.

Bonus is this would let us debug that crash locally in the maplibre-compose example app.

Yeah, actually I think it probably would :D

[ianthetechie]

I'm not entirely convinced that a launched effect is the "correct" way to accomplish this. It sounds like the point of this IS to do side effects. But it's very specifically framed around suspend functions in the docs. The point of this isn't actually to do any sort of suspending operation at all but rather to do something on-off when some state changes. That feels more like "normal" Composable state to me, but I can kinda see the temptation to use this. I suspect this is at least partially to blame though.

[Archdoog]

This discussion https://developer.android.com/develop/ui/compose/side-effects points me toward produceState.

Even though produceState creates a coroutine, it can also be used to observe non-suspending sources of data. To remove the subscription to that source, use the awaitDispose function.

It seems to tidily contain the calculations. That said, since it's still running a coroutine scope, it also causes the crashy/buggy behavior.

I'd say we use it as we continue to debug the crash data you isolated?

[ianthetechie]

Good find; this looks more fit to the task to me!

[Archdoog]

TL;DR, I don't think it's the fault of anything in the location code that we added (StaticLocationEngine). I suspect it has to do with the interplay of these two LaunchedEffects somehow.

Removing the currentMapProperties in MapLibre.kt#L111 fixes the map control update triggering a full await style. Basically that LaunchedEffect will currently kick off any time the value changes re-composing the whole map. In this case it was probably happening in such rapid succession that the style wasn't even loaded before recomposing.

Moving forward I think:

1. We remove anything we can handle dynamic adjustment and replace it with behavior like the camera where MutableState of something can dynamically update itself only. This is a decent standard practice for us to remember on maplibre-compose and may be applicable for pretty much everything (including the location engine if maplibre can simply replace the current)
2. We probably still need to figure out how to cancel the last composition of the map. Since there may still be use cases for a full map rebuild. When it happens, it shouldn't tank an app when they happen to cause the issue like we witnessed on this MR.

I'd say we create a couple of tickets on maplibre-compose and hold this MR until then.

[Archdoog]

Pending Rallista/maplibre-compose-playground#46

[ianthetechie]

Just need to test but looks good code wise!