Update CartesianChartModelProducer to prevent unexpected `Cartesian…

…ChartModel` changes

Co-authored-by: Patryk Goworowski <[email protected]>

vico/compose/src/main/java/com/patrykandpatrick/vico/compose/model/ChartEntryModelExtensions.kt

@@ -72,7 +72,6 @@ public fun CartesianChartModelProducer.collectAsState(
         var finalAnimationFrameJob: Job? = null
         var isAnimationRunning: Boolean
         var isAnimationFrameGenerationRunning = false
-        var chartValues: ChartValues = ChartValues.Empty
         val startAnimation: (transformModel: suspend (key: Any, fraction: Float) -> Unit) -> Unit = { transformModel ->
             if (animationSpec != null && !isInPreview &&
                 (modelWrapperState.value.model != null || runInitialAnimation)
@@ -136,9 +135,9 @@ public fun CartesianChartModelProducer.collectAsState(
                         mutableChartValues.toImmutable()
                     } else {
                         ChartValues.Empty
-                    }.also { values -> chartValues = values }
+                    }
                 },
-            ) { model ->
+            ) { model, chartValues ->
                 modelWrapperState.set(model, chartValues)
             }
         }

vico/core/src/main/java/com/patrykandpatrick/vico/core/model/CartesianChartModel.kt

@@ -87,12 +87,11 @@ public class CartesianChartModel {
     }
 
     /**
-     * Creates a copy of this [CartesianLayerModel] with the given [CartesianLayerModel]s and [ExtraStore].
+     * Creates a copy of this [CartesianChartModel] with the given [ExtraStore], which is also applied to the
+     * [CartesianLayerModel]s.
      */
-    public fun copy(
-        models: List<CartesianLayerModel>,
-        extraStore: ExtraStore,
-    ): CartesianChartModel = CartesianChartModel(models, id, width, xDeltaGcd, extraStore)
+    public fun copy(extraStore: ExtraStore): CartesianChartModel =
+        CartesianChartModel(models.map { it.copy(extraStore) }, id, width, xDeltaGcd, extraStore)
 
     /**
      * Creates an immutable copy of this [CartesianChartModel].

vico/core/src/main/java/com/patrykandpatrick/vico/core/model/CartesianChartModelProducer.kt

@@ -69,36 +69,29 @@ public class CartesianChartModelProducer private constructor(dispatcher: Corouti
         return completableDeferred
     }
 
-    private fun getModel(extraStore: ExtraStore? = null) =
+    private fun getModel(extraStore: ExtraStore) =
         if (partials.isEmpty()) {
             null
         } else {
-            val mergedExtraStore = this.extraStore.let { if (extraStore != null) it + extraStore else it }
+            val mergedExtraStore = this.extraStore + extraStore
             cachedModel
-                ?.let { chartModel ->
-                    chartModel.copy(
-                        chartModel.models.map { layerModel -> layerModel.copy(mergedExtraStore) },
-                        mergedExtraStore,
-                    )
+                ?.copy(mergedExtraStore)
+                ?: CartesianChartModel(partials.map { it.complete(mergedExtraStore) }, mergedExtraStore).also {
+                    cachedModel = it
                 }
-                ?: CartesianChartModel(partials.map { it.complete(mergedExtraStore) }, mergedExtraStore)
-                    .also { cachedModel = it }
         }
 
-    /**
-     * Creates an intermediate [CartesianChartModel] for difference animations. [fraction] is the balance between the
-     * initial and target [CartesianChartModel]s.
-     */
-    public suspend fun transformModel(
+    private suspend fun transformModel(
         key: Any,
         fraction: Float,
+        model: CartesianChartModel?,
+        chartValues: ChartValues,
     ) {
-        with(updateReceivers[key] ?: return) {
-            transform(extraStore, fraction)
-            val internalModel = getModel(extraStore.copy())
-            currentCoroutineContext().ensureActive()
-            onModelCreated(internalModel)
-        }
+        val updateReceiver = updateReceivers[key] ?: return
+        updateReceiver.transform(updateReceiver.extraStore, fraction)
+        val transformedModel = model?.copy(extraStore + updateReceiver.extraStore.copy())
+        currentCoroutineContext().ensureActive()
+        updateReceiver.onModelCreated(transformedModel, chartValues)
     }
 
     /**
@@ -113,12 +106,12 @@ public class CartesianChartModelProducer private constructor(dispatcher: Corouti
     public fun registerForUpdates(
         key: Any,
         cancelAnimation: () -> Unit,
-        startAnimation: (transformModel: suspend (chartKey: Any, fraction: Float) -> Unit) -> Unit,
+        startAnimation: (transformModel: suspend (key: Any, fraction: Float) -> Unit) -> Unit,
         prepareForTransformation: (CartesianChartModel?, MutableExtraStore, ChartValues) -> Unit,
         transform: suspend (MutableExtraStore, Float) -> Unit,
         extraStore: MutableExtraStore,
         updateChartValues: (CartesianChartModel?) -> ChartValues,
-        onModelCreated: (CartesianChartModel?) -> Unit,
+        onModelCreated: (CartesianChartModel?, ChartValues) -> Unit,
     ) {
         UpdateReceiver(
             cancelAnimation,
@@ -201,17 +194,19 @@ public class CartesianChartModelProducer private constructor(dispatcher: Corouti
 
     private inner class UpdateReceiver(
         val cancelAnimation: () -> Unit,
-        val startAnimation: (transformModel: suspend (chartKey: Any, fraction: Float) -> Unit) -> Unit,
-        val onModelCreated: (CartesianChartModel?) -> Unit,
+        val startAnimation: (transformModel: suspend (key: Any, fraction: Float) -> Unit) -> Unit,
+        val onModelCreated: (CartesianChartModel?, ChartValues) -> Unit,
         val extraStore: MutableExtraStore,
         val prepareForTransformation: (CartesianChartModel?, MutableExtraStore, ChartValues) -> Unit,
         val transform: suspend (MutableExtraStore, Float) -> Unit,
         val updateChartValues: (CartesianChartModel?) -> ChartValues,
     ) {
         fun handleUpdate() {
             cancelAnimation()
-            prepareForTransformation(getModel(), extraStore, updateChartValues(getModel()))
-            startAnimation(::transformModel)
+            val model = getModel(extraStore)
+            val chartValues = updateChartValues(model)
+            prepareForTransformation(model, extraStore, chartValues)
+            startAnimation { key, fraction -> transformModel(key, fraction, model, chartValues) }
         }
     }
 

vico/views/src/main/java/com/patrykandpatrick/vico/views/chart/CartesianChartView.kt

@@ -67,6 +67,7 @@ import com.patrykandpatrick.vico.views.extension.dpInt
 import com.patrykandpatrick.vico.views.extension.isLtr
 import com.patrykandpatrick.vico.views.extension.specMode
 import com.patrykandpatrick.vico.views.extension.specSize
+import com.patrykandpatrick.vico.views.extension.start
 import com.patrykandpatrick.vico.views.extension.verticalPadding
 import com.patrykandpatrick.vico.views.gestures.ChartScaleGestureListener
 import com.patrykandpatrick.vico.views.gestures.MotionEventHandler
@@ -136,7 +137,6 @@ public open class CartesianChartView
             ValueAnimator.ofFloat(Animation.range.start, Animation.range.endInclusive).apply {
                 duration = Animation.DIFF_DURATION.toLong()
                 interpolator = FastOutSlowInInterpolator()
-                addUpdateListener { transformModelForAnimation(it.animatedFraction) }
             }
 
         private val scrollValueAnimator: ValueAnimator =
@@ -169,8 +169,6 @@ public open class CartesianChartView
 
         private var wasZoomOverridden = false
 
-        private var chartValues: ChartValues = ChartValues.Empty
-
         private var horizontalDimensions = MutableHorizontalDimensions()
 
         private val themeHandler: ThemeHandler = ThemeHandler(context, attrs)
@@ -251,19 +249,7 @@ public open class CartesianChartView
                         isAnimationRunning = false
                         isAnimationFrameGenerationRunning = false
                     },
-                    startAnimation = { transformModel ->
-                        if (model != null || runInitialAnimation) {
-                            handler?.post {
-                                isAnimationRunning = true
-                                animator.start()
-                            }
-                        } else {
-                            finalAnimationFrameJob =
-                                coroutineScope?.launch(dispatcher) {
-                                    transformModel(this@CartesianChartView, Animation.range.endInclusive)
-                                }
-                        }
-                    },
+                    startAnimation = ::startAnimation,
                     prepareForTransformation = { model, extraStore, chartValues ->
                         chart?.prepareForTransformation(model, extraStore, chartValues)
                     },
@@ -276,10 +262,9 @@ public open class CartesianChartView
                             mutableChartValues.toImmutable()
                         } else {
                             ChartValues.Empty
-                        }.also { values -> chartValues = values }
+                        }
                     },
-                ) { model ->
-                    val chartValues = chartValues
+                ) { model, chartValues ->
                     post {
                         setModel(model = model, updateChartValues = false)
                         measureContext.chartValues = chartValues
@@ -518,25 +503,37 @@ public open class CartesianChartView
             }
         }
 
-        private fun transformModelForAnimation(fraction: Float) {
-            when {
-                !isAnimationRunning -> return
-                !isAnimationFrameGenerationRunning -> {
-                    isAnimationFrameGenerationRunning = true
-                    animationFrameJob =
-                        coroutineScope?.launch(dispatcher) {
-                            modelProducer?.transformModel(this@CartesianChartView, fraction)
-                            isAnimationFrameGenerationRunning = false
-                        }
-                }
-                fraction == 1f -> {
-                    finalAnimationFrameJob =
-                        coroutineScope?.launch(dispatcher) {
-                            animationFrameJob?.cancelAndJoin()
-                            modelProducer?.transformModel(this@CartesianChartView, fraction)
-                            isAnimationFrameGenerationRunning = false
+        private fun startAnimation(transformModel: suspend (key: Any, fraction: Float) -> Unit) {
+            if (model != null || runInitialAnimation) {
+                handler?.post {
+                    isAnimationRunning = true
+                    animator.start { fraction ->
+                        when {
+                            !isAnimationRunning -> return@start
+                            !isAnimationFrameGenerationRunning -> {
+                                isAnimationFrameGenerationRunning = true
+                                animationFrameJob =
+                                    coroutineScope?.launch(dispatcher) {
+                                        transformModel(this@CartesianChartView, fraction)
+                                        isAnimationFrameGenerationRunning = false
+                                    }
+                            }
+                            fraction == 1f -> {
+                                finalAnimationFrameJob =
+                                    coroutineScope?.launch(dispatcher) {
+                                        animationFrameJob?.cancelAndJoin()
+                                        transformModel(this@CartesianChartView, fraction)
+                                        isAnimationFrameGenerationRunning = false
+                                    }
+                            }
                         }
+                    }
                 }
+            } else {
+                finalAnimationFrameJob =
+                    coroutineScope?.launch(dispatcher) {
+                        transformModel(this@CartesianChartView, Animation.range.endInclusive)
+                    }
             }
         }
 

vico/views/src/main/java/com/patrykandpatrick/vico/views/extension/ValueAnimatorExtensions.kt

@@ -0,0 +1,39 @@
+/*
+ * Copyright 2023 by Patryk Goworowski and Patrick Michalik.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.patrykandpatrick.vico.views.extension
+
+import android.animation.Animator
+import android.animation.AnimatorListenerAdapter
+import android.animation.ValueAnimator
+
+internal fun ValueAnimator.start(block: (Float) -> Unit) {
+    val updateListener = ValueAnimator.AnimatorUpdateListener { block(it.animatedFraction) }
+    addUpdateListener(updateListener)
+    addListener(
+        object : AnimatorListenerAdapter() {
+            override fun onAnimationEnd(animation: Animator) {
+                removeUpdateListener(updateListener)
+                removeListener(this)
+            }
+
+            override fun onAnimationCancel(animation: Animator) {
+                onAnimationEnd(animation)
+            }
+        },
+    )
+    start()
+}