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

Fix crash when changing styles quickly by wrapping styles in SafeStyle #269

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sargunv
Copy link
Owner

@sargunv sargunv commented Feb 2, 2025

Attempt to fix #252 with an alternative approach to #265.

Copy link
Owner Author

sargunv commented Feb 2, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@sargunv sargunv marked this pull request as ready for review February 2, 2025 11:23
@sargunv sargunv added this to the v0.7.0 milestone Feb 2, 2025
@michalgwo
Copy link
Contributor

It fixes the crashes mentioned in #252 when loading an empty map, but when there is some data with the anchor, for example:

val amtrakStations = rememberGeoJsonSource(id = "amtrak-stations", data = FeatureCollection())

Anchor.Below("background") {
  SymbolLayer(id = "amtrak-stations", source = amtrakStations)
}

There is another crash:

FATAL EXCEPTION: main
Process: dev.sargunv.maplibrecompose.demoapp, PID: 5656
java.lang.IllegalArgumentException: Layer ID 'background' not found in base style
	at dev.sargunv.maplibrecompose.compose.engine.LayerManager.validate(LayerManager.kt:123)
	at dev.sargunv.maplibrecompose.compose.engine.LayerManager.addLayer$maplibre_compose_debug(LayerManager.kt:19)
	at dev.sargunv.maplibrecompose.compose.engine.StyleNode.onChildInserted(StyleNode.kt:21)
	at dev.sargunv.maplibrecompose.compose.engine.MapNodeApplier.insertTopDown(MapNodeApplier.kt:11)
	at dev.sargunv.maplibrecompose.compose.engine.MapNodeApplier.insertTopDown(MapNodeApplier.kt:5)
	at androidx.compose.runtime.changelist.Operation$InsertNodeFixup.execute(Operation.kt:591)
	at androidx.compose.runtime.changelist.Operations.executeAndFlushAllPendingOperations(Operations.kt:310)
	at androidx.compose.runtime.changelist.FixupList.executeAndFlushAllPendingFixups(FixupList.kt:50)
	at androidx.compose.runtime.changelist.Operation$InsertSlotsWithFixups.execute(Operation.kt:552)
	at androidx.compose.runtime.changelist.Operations.executeAndFlushAllPendingOperations(Operations.kt:310)
	at androidx.compose.runtime.changelist.ChangeList.executeAndFlushAllPendingChanges(ChangeList.kt:81)
	at androidx.compose.runtime.CompositionImpl.applyChangesInLocked(Composition.kt:984)
	at androidx.compose.runtime.CompositionImpl.applyChanges(Composition.kt:1013)
	at androidx.compose.runtime.Recomposer.composeInitial$runtime_release(Recomposer.kt:1150)
	at androidx.compose.runtime.ComposerImpl$CompositionContextImpl.composeInitial$runtime_release(Composer.kt:3876)
	at androidx.compose.runtime.ComposerImpl$CompositionContextImpl.composeInitial$runtime_release(Composer.kt:3876)
	at androidx.compose.runtime.CompositionImpl.composeInitial(Composition.kt:649)
	at androidx.compose.runtime.CompositionImpl.setContent(Composition.kt:635)
	at dev.sargunv.maplibrecompose.compose.engine.RememberStyleCompositionKt$rememberStyleComposition$1$1.invokeSuspend(rememberStyleComposition.kt:31)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:100)
	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:7872)
	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@9beac72, androidx.compose.runtime.BroadcastFrameClock@e658c3, StandaloneCoroutine{Cancelling}@af16540, AndroidUiDispatcher@521cc79]

This validation in LayerManager seems to be the last thing that crashes the app (when I comment it out, nothing else crashes), at least I didn't find anything else.

To replicate this I'm using Style Switcher Demo with

LaunchedEffect(selectedIndex) {
      if (selectedIndex == 1) {
        selectedIndex = 2
      }
    }

Then I click on Liberty style. When I'm testing on my Xiaomi Redmi Note 7 it crashes every time, but on Emulator for some reason, I need to click multiple times to crash.

@michalgwo
Copy link
Contributor

I found out that in some cases it also crashes on recreate, gonna investigate futher.

@michalgwo
Copy link
Contributor

michalgwo commented Feb 2, 2025

The crash happens on recreate (for example screen rotation) when there are layers added by the user.

Steps to reproduce on the demo app:

  1. Remove orientation from this line, so the activity can be recreated on screen rotation
    android:configChanges="orientation|screenSize|screenLayout|keyboardHidden|mnc|colorMode|density|fontScale|fontWeightAdjustment|keyboard|layoutDirection|locale|mcc|navigation|smallestScreenSize|touchscreen|uiMode"
  2. Go to Markers Demo and rotate the screen

As for the solution, I think it's safe to unload the style in onDestroy, style will be recreated in onCreate anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash when changing styles quickly on Android
2 participants