vico icon indicating copy to clipboard operation
vico copied to clipboard

Couple crashes with `java.util.ConcurrentModificationException` and `java.util.NoSuchElementException: Key null is missing in the map`

Open L-Andrade opened this issue 1 year ago • 10 comments

How to reproduce

Hey! Thanks again for the library. It's been working great so far, but we've started getting a couple crashes now after pushing it to more users (nothing major yet).

I've tried to reproduce it for the last couple days, but haven't managed to.

I have a LazyColumn with a CartesianChartHost. This is the basic gist of the chart:

ProvideVicoTheme(rememberM2VicoTheme()) {
    val modelProducer = remember { CartesianChartModelProducer.build() }
    LaunchedEffect(entries) {
        modelProducer.tryRunTransaction {
            lineSeries { series(x = entries.map { it.key }, y = entries.map { it.amount }) }
        }
    }
    // ...
    CartesianChartHost(
            modifier = modifier,
            scrollState = rememberVicoScrollState(scrollEnabled = false),
            modelProducer = modelProducer,
            diffAnimationSpec = spring(stiffness = Spring.StiffnessVeryLow),
            chart = rememberCartesianChart(
                rememberLineCartesianLayer(
                    lines = listOf(rememberLineSpec(shader = shader, backgroundShader = null)),
                    axisValueOverrider = remember {
                        object : AxisValueOverrider {
                            override fun getMinY(minY: Float, maxY: Float, extraStore: ExtraStore): Float {
                                return (minY - 0.0005f).coerceAtLeast(0f)
                            }

                            override fun getMaxY(minY: Float, maxY: Float, extraStore: ExtraStore): Float {
                                return (maxY + 0.0005f).coerceAtLeast(0.1f)
                            }
                        }
                    },
                ),
                decorations = listOf(
                    rememberHorizontalLine(
                        y = { state.start },
                        line = rememberLineComponent(
                            color = MaterialTheme.colors.gray80, shape = remember { DashedShape() },
                        ),
                    ),
                ),
                persistentMarkers = // Custom logic to display some markers
            ),
            marker = primaryMarker,
            markerVisibilityListener = // Custom logic for the marker visibility
        )
}

I have removed a few bits for brevity and because I can not share the whole code. Let me know if any of it is important to figuring this issue.

The chart is only shown if there are entries to display (entries is not empty). It seems to happen after the user scrolls to the bottom of the screen (chart is at the top).

Could it be because the CartesianChartModelProducer is recreated when the chart is visible again? Since it's in a LazyColumn, the chart leaves the composition and then returns once the user sees it again. Not sure it should matter though, since the chart is also a "new" one.

I'm not entirely sure if it is an issue on our end or an issue in Vico's end. Do you folks have any idea of how we can try to replicate this issue?

Let me know if you see any issue in our implementation too, please!

Observed behavior

There are a few crashes. I have a couple stack traces, not sure if they're linked to the same issue or not

      Fatal Exception: java.util.ConcurrentModificationException:
       at java.util.LinkedHashMap$LinkedHashIterator.nextNode(LinkedHashMap.java:760)
       at java.util.LinkedHashMap$LinkedValueIterator.next(LinkedHashMap.java:787)
       at kotlin.collections.CollectionsKt___CollectionsKt.mapTo(CollectionsKt___Collections.kt:1620)
       at com.patrykandpatrick.vico.core.cartesian.data.CartesianChartModelProducer.tryUpdate(CartesianChartModelProducer.kt:59)
       at com.patrykandpatrick.vico.core.cartesian.data.CartesianChartModelProducer.access$tryUpdate(CartesianChartModelProducer.kt:38)
       at com.patrykandpatrick.vico.core.cartesian.data.CartesianChartModelProducer$Transaction.tryCommit(CartesianChartModelProducer.kt:206)
       at com.patrykandpatrick.vico.core.cartesian.data.CartesianChartModelProducer.tryRunTransaction(CartesianChartModelProducer.kt:171)
       at com.getbux.android.stocks.ui.chart.ValueChartKt$ValueChart$1$1.invokeSuspend(ValueChart.kt:55)
       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:958)
       at android.os.Handler.dispatchMessage(Handler.java:99)
       at android.os.Looper.loopOnce(Looper.java:224)
       at android.os.Looper.loop(Looper.java:318)
       at android.app.ActivityThread.main(ActivityThread.java:8677)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:561)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1013)

and

      Fatal Exception: java.util.NoSuchElementException: Key null is missing in the map.
       at kotlin.collections.MapsKt__MapWithDefaultKt.getOrImplicitDefaultNullable(MapsKt__MapWithDefault.kt:24)
       at kotlin.collections.MapsKt__MapsKt.getValue(MapsKt__Maps.kt:360)
       at com.patrykandpatrick.vico.core.cartesian.data.MutableChartValuesKt$toImmutable$1.getYRange(MutableChartValues.kt:122)
       at com.patrykandpatrick.vico.core.cartesian.layer.LineCartesianLayer.toDrawingModel(LineCartesianLayer.kt:598)
       at com.patrykandpatrick.vico.core.cartesian.layer.LineCartesianLayer.prepareForTransformation(LineCartesianLayer.kt:583)
       at com.patrykandpatrick.vico.core.cartesian.layer.LineCartesianLayer.prepareForTransformation(LineCartesianLayer.kt:73)
       at com.patrykandpatrick.vico.core.cartesian.CartesianChart$transformationPreparationModelAndLayerConsumer$1.invoke(CartesianChart.java:106)
       at com.patrykandpatrick.vico.core.cartesian.CartesianChart.consume(CartesianChart.kt:328)
       at com.patrykandpatrick.vico.core.cartesian.CartesianChart.forEachWithLayer(CartesianChart.kt:316)
       at com.patrykandpatrick.vico.core.cartesian.CartesianChart.prepareForTransformation(CartesianChart.kt:251)
       at com.patrykandpatrick.vico.compose.cartesian.CartesianChartModelProducerKt$collectAsState$2$1$2.invoke(CartesianChartModelProducer.kt:124)
       at com.patrykandpatrick.vico.compose.cartesian.CartesianChartModelProducerKt$collectAsState$2$1$2.invoke(CartesianChartModelProducer.kt:124)
       at com.patrykandpatrick.vico.core.cartesian.data.CartesianChartModelProducer$UpdateReceiver.handleUpdate(CartesianChartModelProducer.kt:229)
       at com.patrykandpatrick.vico.core.cartesian.data.CartesianChartModelProducer.registerForUpdates(CartesianChartModelProducer.kt:146)
       at com.patrykandpatrick.vico.compose.cartesian.CartesianChartModelProducerKt$collectAsState$2$1.invokeSuspend(CartesianChartModelProducer.kt:114)
       at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
       at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:104)
       at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.java:585)
       at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:802)
       at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:706)
       at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:693)

Expected behavior

The app should not crash

Vico version(s)

2.0.0-alpha.20

Android version(s)

12, 13, 14

Additional information

We're actually on 2.0.0-alpha.19, but there seem to be only small changes in alpha.20.

It's quite hard to figure out where the issue is coming from. Since the stack trace does not point to our app code at all, we don't know if it's an issue with our integration or an internal Vico issue.

And since we're not familiar with Vico's codebase, it's a bit harder to try to replicate it. I've tried scrolling up and down multiple times, adding delays in multiple places, adding more charts to the screen and scrolling, configuration changes, going to the background/foreground, spamming buttons that cause the entries to change, navigating back or to another screen, and more. Let me know if you have any ideas that could replicate it, any help would be great here!

Thanks in advance

L-Andrade avatar May 29 '24 11:05 L-Andrade

Hello. I did some stress testing and I haven't experienced this crash.

Could it be because the CartesianChartModelProducer is recreated when the chart is visible again?

This shouldn't be the reason. Have you tried putting CartesianChartModelProducer in the viewmodel, as suggested here? The examples use LaunchedEffect for simplicity, but especially when the chart leaves and enters the composition, it's more optimal to store the producer in the viewmodel.

Do you folks have any idea of how we can try to replicate this issue?

It's hard to tell without specific information. Is there anything common between the incidents? Like low-spec devices?

Let me know if you see any issue in our implementation too, please!

I can't see any issues that could cause this.

Gowsky avatar May 30 '24 20:05 Gowsky

I also got this crash, spent a few hours looking for the culprit no luck yet

in the below code, yRanges[axisPosition] returns null because yRanges is empty and yRanges.getValue(null) throws the NoSuchElementException with the message "Key null is missing in the map."

in MutableChartValues.kt

public fun MutableChartValues.toImmutable(): ChartValues =
    object : ChartValues {
        private val yRanges = [email protected]
        override val minX: Float = [email protected]
        override val maxX: Float = [email protected]
        override val xStep: Float = [email protected]
        override val model: CartesianChartModel = [email protected]()
        override fun getYRange(axisPosition: AxisPosition.Vertical?): ChartValues.YRange =
            yRanges[axisPosition] ?: yRanges.getValue(null)
    }

UPDATE:

I was using the chart host that takes in a CartesianChartModel instead of the CartesianChartModelProducer. After changing it to match OP's implementation with the model producer and the LaunchedEffect, I no longer see the issue, so I'm not sure why OP has the issue. 🤷

ibcurly avatar May 31 '24 03:05 ibcurly

Hey! Thanks for the reply

Hello. I did some stress testing and I haven't experienced this crash.

Could it be because the CartesianChartModelProducer is recreated when the chart is visible again?

This shouldn't be the reason. Have you tried putting CartesianChartModelProducer in the viewmodel, as suggested here? The examples use LaunchedEffect for simplicity, but especially when the chart leaves and enters the composition, it's more optimal to store the producer in the viewmodel.

Yes, looking further into it, it seems like putting it in the ViewModel would most likely solve it. We were just trying to avoid it since we wanted the chart implementation to be part of a UI-only module, and this way we'll also have to put it a Vico dependency in the module that the ViewModel is in. Or we will have to create a wrapper around it. Reason we wanted to keep it out of the ViewModel is in case we'd want to change the implementation or library, we would have fewer places to change and it would be more contained.

Do you folks have any idea of how we can try to replicate this issue?

It's hard to tell without specific information. Is there anything common between the incidents? Like low-spec devices?

So far only had issues with Android 12, 13, and 14. Devices are not low-spec, they're decent (most common so far is Galaxy A53 5G).

I tried to understand the internal Vico code and had an idea, but might be totally wrong. Maybe still worth checking on your side just in case though. Click here to check the details

I was taking a look at the logic in CartesianChartModelProducer, and I think there are potential race conditions. With debugging breakpoints (non-suspend, log only) we can see that registerForUpdates (suspend function) and unregisterFromUpdates can be called before, during, or after tryUpdate. This can mean that a transaction might be running when a receiver is registered/unregistered, which could cause the ConcurrentModificationException. registerForUpdates is also called inside a scope.launch - which is scheduled to be immediately run, but might there might be a delay (albeit small of course) if

To solve it, registering or unregistering a receiver would need to either:

  1. Check and wait for the Mutex lock before setting/removing
  2. Cancel any ongoing transaction before setting/removing

Otherwise there might be an unfortunate timing.

With that said, I still can't replicate the issue, so I'm probably still missing something! I think I will try to move it to the ViewModel and see if it's fixed, but I believe the race condition will still be possible, just less probable (almost impossible, I guess) since the transaction will most likely be done before the chart is composed, and as such the receiver will always be registered after the transaction is complete (and fewer transactions will also be done, since it will not be done every time the chart is composed), and there's a smaller chance of a transaction being ran when unregistering since there will be fewer transactions too.

L-Andrade avatar May 31 '24 10:05 L-Andrade

@L-Andrade, Vico 2.0.0-alpha.22 fixes the registration problem you described. This should fix the ConcurrentModificationException. As the release notes describe, you must switch to runTransaction to benefit from the fix. tryRunTransaction is now deprecated. NoSuchElementException might be fixed as well. We can't be sure, as we're lacking the reproduction. Please let us know if you still experience it.

@ibcurly, thanks for reporting it. Unfortunately, we weren't able to reproduce it, so we'll need a MRE to fix this.

Gowsky avatar Jul 05 '24 17:07 Gowsky

Hey @Gowsky! Thanks for the update. I took a look and it looks like a nice improvement. In the meantime, we had already shipped the recommended fix of putting the transaction call in the ViewModel (we abstracted it away in a class) which seemed to fix the issue, and it makes sense since it causes much fewer transactions.

However, I saw the recommendation in the Vico docs asking to change the dispatcher to Default on the caller side. I believe this should be something done in the Vico side, following the guideline that suspend functions should be main safe from the caller:

  • https://developer.android.com/kotlin/coroutines/coroutines-best-practices#main-safe

Other big libraries also respect this guideline, such as Room and Retrofit. This would be a further improvement, a bit less error-prone for the library users

Thanks again!

L-Andrade avatar Jul 08 '24 09:07 L-Andrade

Thanks for the update and the suggestion, @L-Andrade! Vico 2.0.0 Alpha 23 makes CartesianChartModelProducer’s suspending functions main-safe. Execution is now moved to Dispatchers.Default internally.

While, as mentioned by @Gowsky, we haven’t been able to reproduce the NoSuchElementException, we believe it stemmed from the same problem as the ConcurrentModificationException. With this problem having been resolved, neither of the two reported exceptions should occur anymore. I’ll thus be adding the “done in alpha” label. Should you face any issues, please let us know. Cheers!

patrickmichalik avatar Jul 19 '24 14:07 patrickmichalik

Thanks both! I also believe the changes fix the issue. We will update to that version when we can.

Feel free to close the issue!

L-Andrade avatar Jul 19 '24 14:07 L-Andrade

Since Vico 1 is still supported, we keep bug reports open until they're addressed in both Vico versions. The "Done in alpha" label basically means that the issue is closed as far as Vico 2 is concerned.

Gowsky avatar Jul 31 '24 19:07 Gowsky

Just tried beta.2 and the crash reappeared:

java.util.NoSuchElementException: Key null is missing in the map.
	at kotlin.collections.MapsKt__MapWithDefaultKt.getOrImplicitDefaultNullable(MapWithDefault.kt:24)
	at kotlin.collections.MapsKt__MapsKt.getValue(Maps.kt:369)
	at com.patrykandpatrick.vico.core.cartesian.data.MutableCartesianChartRangesKt$toImmutable$1.getYRange(MutableCartesianChartRanges.kt:99)
	at com.patrykandpatrick.vico.core.cartesian.axis.DefaultVerticalAxisItemPlacer.getHeightMeasurementLabelValues(DefaultVerticalAxisItemPlacer.kt:56)
	at com.patrykandpatrick.vico.core.cartesian.axis.VerticalAxis.getMaxLabelHeight(VerticalAxis.kt:362)
	at com.patrykandpatrick.vico.core.cartesian.axis.VerticalAxis.updateInsets(VerticalAxis.kt:305)
	at com.patrykandpatrick.vico.core.cartesian.axis.VerticalAxis.updateInsets(VerticalAxis.kt:58)
	at com.patrykandpatrick.vico.core.cartesian.CartesianChart.prepare(CartesianChart.kt:229)
	at com.patrykandpatrick.vico.compose.cartesian.CartesianChartHostKt.CartesianChartHostImpl$lambda$23$lambda$22(CartesianChartHost.kt:195)
	at com.patrykandpatrick.vico.compose.cartesian.CartesianChartHostKt.$r8$lambda$aQQKwZOn9huYyaTRo_AvgGCegn8(Unknown Source:0)
	at com.patrykandpatrick.vico.compose.cartesian.CartesianChartHostKt$$ExternalSyntheticLambda5.invoke(D8$$SyntheticClass:0)
	at androidx.compose.ui.draw.DrawBackgroundModifier.draw(DrawModifier.kt:127)
	at androidx.compose.ui.node.LayoutNodeDrawScope.drawDirect-eZhPAX0$ui_release(LayoutNodeDrawScope.kt:110)
	at androidx.compose.ui.node.LayoutNodeDrawScope.draw-eZhPAX0$ui_release(LayoutNodeDrawScope.kt:89)
	at androidx.compose.ui.node.NodeCoordinator.drawContainedDrawModifiers(NodeCoordinator.kt:450)
	at androidx.compose.ui.node.NodeCoordinator.draw(NodeCoordinator.kt:439)
	at androidx.compose.ui.node.LayoutModifierNodeCoordinator.performDraw(LayoutModifierNodeCoordinator.kt:280)
	at androidx.compose.ui.node.NodeCoordinator.drawContainedDrawModifiers(NodeCoordinator.kt:447)
	at androidx.compose.ui.node.NodeCoordinator.draw(NodeCoordinator.kt:439)
	at androidx.compose.ui.node.LayoutModifierNodeCoordinator.performDraw(LayoutModifierNodeCoordinator.kt:280)
	at androidx.compose.ui.node.NodeCoordinator.drawContainedDrawModifiers(NodeCoordinator.kt:447)
	at androidx.compose.ui.node.NodeCoordinator.draw(NodeCoordinator.kt:439)
	at androidx.compose.ui.node.LayoutModifierNodeCoordinator.performDraw(LayoutModifierNodeCoordinator.kt:280)
	at androidx.compose.ui.node.NodeCoordinator.drawContainedDrawModifiers(NodeCoordinator.kt:447)
	at androidx.compose.ui.node.NodeCoordinator.draw(NodeCoordinator.kt:439)
	at androidx.compose.ui.node.LayoutNode.draw$ui_release(LayoutNode.kt:1000)
	at androidx.compose.ui.node.InnerNodeCoordinator.performDraw(InnerNodeCoordinator.kt:196)
	at androidx.compose.ui.node.NodeCoordinator.drawContainedDrawModifiers(NodeCoordinator.kt:447)
	at androidx.compose.ui.node.NodeCoordinator.draw(NodeCoordinator.kt:439)
	at androidx.compose.ui.node.LayoutModifierNodeCoordinator.performDraw(LayoutModifierNodeCoordinator.kt:280)
	at androidx.compose.ui.node.NodeCoordinator.drawContainedDrawModifiers(NodeCoordinator.kt:447)
	at androidx.compose.ui.node.NodeCoordinator.draw(NodeCoordinator.kt:439)
	at androidx.compose.ui.node.LayoutModifierNodeCoordinator.performDraw(LayoutModifierNodeCoordinator.kt:280)
	at androidx.compose.ui.node.NodeCoordinator.drawContainedDrawModifiers(NodeCoordinator.kt:447)
	at androidx.compose.ui.node.NodeCoordinator.draw(NodeCoordinator.kt:439)
	at androidx.compose.ui.node.LayoutModifierNodeCoordinator.performDraw(LayoutModifierNodeCoordinator.kt:280)
	at androidx.compose.ui.node.NodeCoordinator.drawContainedDrawModifiers(NodeCoordinator.kt:447)
	at androidx.compose.ui.node.NodeCoordinator.draw(NodeCoordinator.kt:439)
	at androidx.compose.ui.node.LayoutNode.draw$ui_release(LayoutNode.kt:1000)
	at androidx.compose.ui.node.InnerNodeCoordinator.performDraw(InnerNodeCoordinator.kt:196)

mak1nt0sh avatar Oct 23 '24 10:10 mak1nt0sh

Hello, @mak1nt0sh. Can you share more details and some context for the issue? Can you provide an MRE?

Gowsky avatar Oct 23 '24 18:10 Gowsky

Experiencing same crash in some instances with beta3 too. I'm trying to reproduce it in a separate sample, but due to the complexity of my chart I cannot reproduce the crash. Everything works greate in beta1

It seems to crash more often when chartGranularity key changes.

@Composable
fun AnalyticsChart(
    chartData: ChartData,
    selectedChartSeries: Set<ChartSeries>,
    modifier: Modifier = Modifier,
    modelProducer: CartesianChartModelProducer = remember { CartesianChartModelProducer() },
    currencyAxisLabelCountKey: ExtraStore.Key<Int> = remember { ExtraStore.Key() },
    chartGranularityKey: ExtraStore.Key<ChartGranularity> = remember { ExtraStore.Key() },
) {
    var isLineChart by remember { mutableStateOf(chartData.income.size > 1) }

    LaunchedEffect(chartData, selectedChartSeries) {
        isLineChart = chartData.income.size > 1
        val ySeries = mutableSetOf<BigDecimal>()

        modelProducer.runTransaction {
            if (isLineChart) {
                lineSeries {
                    if (ChartSeries.TOTAL in selectedChartSeries) {
                        ySeries.addAll(chartData.total.values)
                        series(x = chartData.total.keys, y = chartData.total.values)
                    } else {
                        if (ChartSeries.INCOME in selectedChartSeries) {
                            ySeries.addAll(chartData.income.values)
                            series(x = chartData.income.keys, y = chartData.income.values)
                        } else {
                            series(x = chartData.income.keys, y = listOf(0.00))
                        }

                        if (ChartSeries.EXPENSES in selectedChartSeries) {
                            ySeries.addAll(chartData.expenses.values)
                            series(x = chartData.expenses.keys, y = chartData.expenses.values)
                        } else {
                            series(x = chartData.expenses.keys, y = listOf(0.00))
                        }
                    }
                }
            } else {
                columnSeries {
                    if (ChartSeries.TOTAL in selectedChartSeries) {
                        ySeries.addAll(chartData.total.values)
                        series(x = chartData.total.keys, y = chartData.total.values)
                    } else {
                        if (ChartSeries.INCOME in selectedChartSeries) {
                            ySeries.addAll(chartData.income.values)
                            series(x = chartData.income.keys, y = chartData.income.values)
                        }
                        if (ChartSeries.EXPENSES in selectedChartSeries) {
                            ySeries.addAll(chartData.expenses.values)
                            series(x = chartData.expenses.keys, y = chartData.expenses.values)
                        }
                    }
                }
            }

            extras { mutableExtraStore ->
                mutableExtraStore[currencyAxisLabelCountKey] = when {
                    ySeries.all { it.isZero() } -> 1 // All zero
                    ySeries.distinct().size == 1 -> 3 // All equal
                    else -> 5
                }
                mutableExtraStore[chartGranularityKey] = chartData.chartGranularity
            }
        }
    }

mak1nt0sh avatar Nov 30 '24 15:11 mak1nt0sh

@mak1nt0sh, thanks for the sample code. Unfortunately, it's impossible to investigate the issue based on it, because it is incomplete (no CartesianChartHost) and contains unknown references (e.g. ChartData).

Also:

  • isLineChart shouldn't be a State variable, but a local variable in the LaunchedEffect (I assume that the State variable isn't used later in the composable, as this would be incorrect due to no synchronization).

  • You might be using CartesianChartModelProducer suboptimally. Transactions are always run from within the composable, but modelProducer is a parameter, suggesting that a CartesianChartModelProducer is passed in from outside (perhaps only in some cases, since there's a default value). I'm not sure why that is, but regardless, you should interact with CartesianChartModelProducers at the same level at which they're stored. That is, you should either:

    • Have the producer in the composable (val modelProducer = ... in the function body), pass the composable just the data, and run Transactions from the composable.
    • Have the producer outside, run Transactions there, and pass the composable just the producer. (This is preferable. It's best to store and interact with the producer in a ViewModel.)

    Storing the producer outside but passing the data through Compose and running running Transaction from within the composable makes little architectural sense and introduces unnecessary recompositions.

Gowsky avatar Nov 30 '24 20:11 Gowsky

Great suggestions. There's no crash anymore. I moved the isLineChart to inside the LaunchedEffect as a variable. I used to depend on this state, to hide unused layers, but it seems I can keep them both up as in:

 chart = rememberCartesianChart(
            layers = arrayOf(
                rememberLineChartLayer(isTotalChart = selectedChartSeries.contains(ChartSeries.TOTAL)),
                rememberColumnChartLayer(chartSeries = selectedChartSeries)
            ),

Before it was:

 chart = rememberCartesianChart(
            layers = arrayOf(
                if (isLineChart) {
                    rememberLineChartLayer(isTotalChart = selectedChartSeries.contains(ChartSeries.TOTAL))
                } else {
                    rememberColumnChartLayer(chartSeries = selectedChartSeries)
                }
            ),

Also I depended on this variable to set the correct layerPadding, but it seems the line chart is not affected by scalable padding variables, so I can keep the column specific as default.

Changed:

layerPadding = remember(isLineChart) {
                if (isLineChart) {
                    cartesianLayerPadding()
                } else {
                    cartesianLayerPadding(
                        scalableStart = 12.dp,
                        scalableEnd = 12.dp
                    )
                }
            }

to just cartesianLayerPadding( scalableStart = 12.dp, scalableEnd = 12.dp )

Without the padding, the two columns displayed are not centerred, but drawn at the corners of the chart.

mak1nt0sh avatar Dec 01 '24 12:12 mak1nt0sh

So if there's no way to handle the padding by getting isLineChart from the extra store, how would you suggest switching the padding values based on which type of chart is displayed?

EDIT: Oh, I see there's an active PR for this. Will it get merged? #941

mak1nt0sh avatar Dec 02 '24 08:12 mak1nt0sh

We plan on merging it once the feedback has been addressed.

Gowsky avatar Dec 03 '24 20:12 Gowsky

The fix was ported to Vico 1 in 1.16.0-beta.1. Vico 1.16.0 is now available.

The fix applies to setEntriesSuspending and runTransactionSuspending. It was impossible to apply it to setEntries and runTransaction without potentially breaking some usages, similarly to how there was no reasonable way to add it to tryRunTransaction in Vico 2.

In Vico 2, tryRunTransaction was deprecated. It's also possible to deprecate setEntries and runTransaction in Vico 1, but we'll postpone this until it's confirmed that the crash occurs in Vico 1. This is because:

  • Vico 1 is in maintenance mode. It receives only bug fixes. It's generally intended for those who can't switch to Vico 2 yet due to the evolving API. Therefore, the Vico 1 API is kept as stable as possible, meaning that deprecations are made only when absolutely necessary.
  • This deprecation would be rather significant. It would affect most consumers and extend to some other APIs, like the ChartEntryModelProducer constructor with the entryCollections parameter.

Gowsky avatar Dec 07 '24 12:12 Gowsky