From 85d706e644d48317488e5d7a2891aad5c697df64 Mon Sep 17 00:00:00 2001 From: Patrick Michalik <120058021+patrickmichalik@users.noreply.github.com> Date: Wed, 27 Sep 2023 15:44:07 +0200 Subject: [PATCH] Resolve caching and concurrency issues affecting `ChartEntryModelProducer` and `ComposedChartEntryModelProducer` (WIP) Co-authored-by: Patryk Goworowski --- .../chart/entry/ChartEntryModelExtensions.kt | 81 ++++-- vico/core/build.gradle | 1 + .../patrykandpatrick/vico/core/chart/Chart.kt | 2 +- .../vico/core/chart/column/ColumnChart.kt | 2 +- .../vico/core/chart/composed/ComposedChart.kt | 2 +- .../vico/core/chart/line/LineChart.kt | 2 +- .../core/entry/ChartEntryModelProducer.kt | 204 ++++++++----- .../vico/core/entry/ChartModelProducer.kt | 4 +- .../ComposedChartEntryModelProducer.kt | 267 +++++++++++------- .../diff/DefaultDrawingModelInterpolator.kt | 34 ++- .../entry/diff/DrawingModelInterpolator.kt | 2 +- .../core/extension/CollectionExtensions.kt | 6 + .../vico/views/chart/BaseChartView.kt | 85 ++++-- 13 files changed, 450 insertions(+), 242 deletions(-) diff --git a/vico/compose/src/main/java/com/patrykandpatrick/vico/compose/chart/entry/ChartEntryModelExtensions.kt b/vico/compose/src/main/java/com/patrykandpatrick/vico/compose/chart/entry/ChartEntryModelExtensions.kt index 88a34e292..c22e2f472 100644 --- a/vico/compose/src/main/java/com/patrykandpatrick/vico/compose/chart/entry/ChartEntryModelExtensions.kt +++ b/vico/compose/src/main/java/com/patrykandpatrick/vico/compose/chart/entry/ChartEntryModelExtensions.kt @@ -32,6 +32,8 @@ import com.patrykandpatrick.vico.core.chart.values.ChartValuesManager import com.patrykandpatrick.vico.core.entry.ChartEntryModel import com.patrykandpatrick.vico.core.entry.ChartModelProducer import com.patrykandpatrick.vico.core.entry.diff.MutableDrawingModelStore +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.launch @@ -57,6 +59,7 @@ public fun ChartModelProducer.collectAsState( runInitialAnimation: Boolean = true, chartValuesManager: ChartValuesManager, getXStep: ((Model) -> Float)?, + dispatcher: CoroutineDispatcher = Dispatchers.Default, ): State> { val chartEntryModelState = remember(chart, producerKey) { ChartEntryModelState() } @@ -65,52 +68,70 @@ public fun ChartModelProducer.collectAsState( val scope = rememberCoroutineScope() val isInPreview = LocalInspectionMode.current - var animationJob: Job? = null - var isAnimationRunning: Boolean? + var outerAnimationJob: Job? = null + var innerAnimationJob: Job? = null + var finalAnimationFrameJob: Job? = null + var isAnimationRunning: Boolean + var isAnimationFrameGenerationRunning = false DisposableEffect(chart, producerKey, runInitialAnimation, isInPreview) { - val afterUpdate = { progressModel: (chartKey: Any, progress: Float) -> Unit -> + val afterUpdate: (progressModel: suspend (chartKey: Any, progress: Float) -> Unit) -> Unit = { progressModel -> if (animationSpec != null && !isInPreview && (chartEntryModelState.value.first != null || runInitialAnimation) ) { - isAnimationRunning = false - animationJob = scope.launch { + isAnimationRunning = true + outerAnimationJob = scope.launch(dispatcher) { animate( initialValue = Animation.range.start, targetValue = Animation.range.endInclusive, animationSpec = animationSpec, ) { value, _ -> - if (isAnimationRunning == false) { - progressModel(chart, value) + when { + !isAnimationRunning -> return@animate + !isAnimationFrameGenerationRunning -> { + isAnimationFrameGenerationRunning = true + innerAnimationJob = scope.launch(dispatcher) { + progressModel(chart, value) + isAnimationFrameGenerationRunning = false + } + } + value == 1f -> { + finalAnimationFrameJob = scope.launch(dispatcher) { + innerAnimationJob?.cancelAndJoin() + progressModel(chart, value) + isAnimationFrameGenerationRunning = false + } + } } } } } else { - progressModel(chart, Animation.range.endInclusive) + scope.launch(dispatcher) { progressModel(chart, Animation.range.endInclusive) } } } - registerForUpdates( - key = chart, - cancelAnimation = { - runBlocking { animationJob?.cancelAndJoin() } - isAnimationRunning = true - }, - startAnimation = afterUpdate, - getOldModel = { chartEntryModelState.value.first }, - modelTransformerProvider = modelTransformerProvider, - drawingModelStore = drawingModelStore, - updateChartValues = { model -> - chartValuesManager.resetChartValues() - chart.updateChartValues(chartValuesManager, model, getXStep?.invoke(model)) - chartValuesManager - }, - ) { updatedModel -> - chartEntryModelState.set(updatedModel) - } - onDispose { - unregisterFromUpdates(chart) - animationJob = null - isAnimationRunning = null + scope.launch(dispatcher) { + registerForUpdates( + key = chart, + cancelAnimation = { + runBlocking { + outerAnimationJob?.cancelAndJoin() + innerAnimationJob?.cancelAndJoin() + finalAnimationFrameJob?.cancelAndJoin() + } + isAnimationRunning = false + }, + startAnimation = afterUpdate, + getOldModel = { chartEntryModelState.value.first }, + modelTransformerProvider = modelTransformerProvider, + drawingModelStore = drawingModelStore, + updateChartValues = { model -> + chartValuesManager.resetChartValues() + chart.updateChartValues(chartValuesManager, model, getXStep?.invoke(model)) + chartValuesManager + }, + onModelCreated = chartEntryModelState::set, + ) } + onDispose { unregisterFromUpdates(chart) } } return chartEntryModelState } diff --git a/vico/core/build.gradle b/vico/core/build.gradle index ec3f2feb0..8d01976c9 100644 --- a/vico/core/build.gradle +++ b/vico/core/build.gradle @@ -61,6 +61,7 @@ afterEvaluate { dependencies { implementation libs.androidXAnnotation + implementation libs.coroutinesCore implementation libs.kotlinStdLib testImplementation libs.JUnit testImplementation libs.JUnitExt diff --git a/vico/core/src/main/java/com/patrykandpatrick/vico/core/chart/Chart.kt b/vico/core/src/main/java/com/patrykandpatrick/vico/core/chart/Chart.kt index c4832b01d..18a01ee6b 100644 --- a/vico/core/src/main/java/com/patrykandpatrick/vico/core/chart/Chart.kt +++ b/vico/core/src/main/java/com/patrykandpatrick/vico/core/chart/Chart.kt @@ -235,7 +235,7 @@ public interface Chart : BoundsAware, ChartInsetter { /** * Carries out the pending difference animation. [fraction] is the animation progress. */ - public abstract fun transform(drawingModelStore: MutableDrawingModelStore, fraction: Float) + public abstract suspend fun transform(drawingModelStore: MutableDrawingModelStore, fraction: Float) } } diff --git a/vico/core/src/main/java/com/patrykandpatrick/vico/core/chart/column/ColumnChart.kt b/vico/core/src/main/java/com/patrykandpatrick/vico/core/chart/column/ColumnChart.kt index b5f3c1db0..c343535f4 100644 --- a/vico/core/src/main/java/com/patrykandpatrick/vico/core/chart/column/ColumnChart.kt +++ b/vico/core/src/main/java/com/patrykandpatrick/vico/core/chart/column/ColumnChart.kt @@ -454,7 +454,7 @@ public open class ColumnChart( ) } - override fun transform(drawingModelStore: MutableDrawingModelStore, fraction: Float) { + override suspend fun transform(drawingModelStore: MutableDrawingModelStore, fraction: Float) { drawingModelStore[key] = drawingModelInterpolator.transform(fraction) } diff --git a/vico/core/src/main/java/com/patrykandpatrick/vico/core/chart/composed/ComposedChart.kt b/vico/core/src/main/java/com/patrykandpatrick/vico/core/chart/composed/ComposedChart.kt index f734bb382..a1a4c4c33 100644 --- a/vico/core/src/main/java/com/patrykandpatrick/vico/core/chart/composed/ComposedChart.kt +++ b/vico/core/src/main/java/com/patrykandpatrick/vico/core/chart/composed/ComposedChart.kt @@ -188,7 +188,7 @@ public class ComposedChart( } } - override fun transform(drawingModelStore: MutableDrawingModelStore, fraction: Float) { + override suspend fun transform(drawingModelStore: MutableDrawingModelStore, fraction: Float) { getModelTransformers().forEach { transformer -> transformer.transform(drawingModelStore, fraction) } diff --git a/vico/core/src/main/java/com/patrykandpatrick/vico/core/chart/line/LineChart.kt b/vico/core/src/main/java/com/patrykandpatrick/vico/core/chart/line/LineChart.kt index 70352846e..d96f0eefd 100644 --- a/vico/core/src/main/java/com/patrykandpatrick/vico/core/chart/line/LineChart.kt +++ b/vico/core/src/main/java/com/patrykandpatrick/vico/core/chart/line/LineChart.kt @@ -613,7 +613,7 @@ public open class LineChart( ) } - override fun transform(drawingModelStore: MutableDrawingModelStore, fraction: Float) { + override suspend fun transform(drawingModelStore: MutableDrawingModelStore, fraction: Float) { drawingModelStore[key] = drawingModelInterpolator.transform(fraction) } diff --git a/vico/core/src/main/java/com/patrykandpatrick/vico/core/entry/ChartEntryModelProducer.kt b/vico/core/src/main/java/com/patrykandpatrick/vico/core/entry/ChartEntryModelProducer.kt index 272ae4b2b..d9a160601 100644 --- a/vico/core/src/main/java/com/patrykandpatrick/vico/core/entry/ChartEntryModelProducer.kt +++ b/vico/core/src/main/java/com/patrykandpatrick/vico/core/entry/ChartEntryModelProducer.kt @@ -16,119 +16,166 @@ package com.patrykandpatrick.vico.core.entry -import com.patrykandpatrick.vico.core.DEF_THREAD_POOL_SIZE +import androidx.annotation.WorkerThread import com.patrykandpatrick.vico.core.chart.Chart import com.patrykandpatrick.vico.core.chart.values.ChartValuesManager import com.patrykandpatrick.vico.core.entry.diff.DrawingModelStore import com.patrykandpatrick.vico.core.entry.diff.MutableDrawingModelStore import com.patrykandpatrick.vico.core.extension.copy -import com.patrykandpatrick.vico.core.extension.setToAllChildren -import java.util.concurrent.Executor -import java.util.concurrent.Executors +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Deferred +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.currentCoroutineContext +import kotlinx.coroutines.ensureActive +import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex /** * A [ChartModelProducer] implementation that generates [ChartEntryModel] instances. * - * @param entryCollections a two-dimensional list of [ChartEntry] instances used to generate the [ChartEntryModel]. - * @param backgroundExecutor an [Executor] used to generate instances of the [ChartEntryModel] off the main thread. + * @param entryCollections the initial list of series (where each series is a list of [ChartEntry] instances). + * @param dispatcher the [CoroutineDispatcher] to use to handle updates. * * @see ChartModelProducer */ public class ChartEntryModelProducer( entryCollections: List>, - backgroundExecutor: Executor = Executors.newFixedThreadPool(DEF_THREAD_POOL_SIZE), + dispatcher: CoroutineDispatcher = Dispatchers.Default, ) : ChartModelProducer { + private var series = emptyList>() private var cachedInternalModel: InternalModel? = null - - private var entriesHashCode: Int? = null - + private val mutex = Mutex() + private val coroutineScope = CoroutineScope(dispatcher) private val updateReceivers: HashMap = HashMap() - private val executor: Executor = backgroundExecutor - - /** - * A mutable two-dimensional list of the [ChartEntry] instances used to generate the [ChartEntryModel]. - */ - private val entries: ArrayList> = ArrayList() - public constructor( vararg entryCollections: List, - backgroundExecutor: Executor = Executors.newFixedThreadPool(DEF_THREAD_POOL_SIZE), - ) : this(entryCollections.toList(), backgroundExecutor) + dispatcher: CoroutineDispatcher = Dispatchers.Default, + ) : this(entryCollections.toList(), dispatcher) init { setEntries(entryCollections) } /** - * Updates the two-dimensional list of [ChartEntry] instances and notifies listeners about the update. + * Requests a data update. If the update is accepted, `true` is returned. If the update is rejected, which occurs + * when there’s already an update in progress, `false` is returned. * * @see entries * @see registerForUpdates */ - public fun setEntries(entries: List>) { - this.entries.setToAllChildren(entries) - entriesHashCode = entries.hashCode() + public fun setEntries(entries: List>): Boolean { + if (!mutex.tryLock()) return false + series = entries.copy() cachedInternalModel = null - updateReceivers.values.forEach { updateReceiver -> - executor.execute { - updateReceiver.cancelAnimation() - updateReceiver.modelTransformer?.prepareForTransformation( - oldModel = updateReceiver.getOldModel(), - newModel = getModel(), - drawingModelStore = updateReceiver.drawingModelStore, - chartValuesManager = updateReceiver.updateChartValues(getModel()), - ) - updateReceiver.startAnimation(::progressModel) + var runningUpdateCount = 0 + updateReceivers + .values + .takeIf { it.isNotEmpty() } + ?.forEach { updateReceiver -> + runningUpdateCount++ + coroutineScope.launch { + updateReceiver.handleUpdate() + if (--runningUpdateCount == 0) mutex.unlock() + } } - } + ?: mutex.unlock() + return true } /** - * Updates the two-dimensional list of [ChartEntry] instances and notifies listeners about the update. + * Runs a data update. Unlike [setEntries], this function suspends the current coroutine and waits until an update + * can be run, meaning the update cannot be rejected. The returned [Deferred] implementation is marked as completed + * once the update has been processed. * * @see entries * @see registerForUpdates */ - public fun setEntries(vararg entries: List) { - setEntries(entries.toList()) + public suspend fun setEntriesSuspending(entries: List>): Deferred { + mutex.lock() + series = entries.copy() + cachedInternalModel = null + val completableDeferred = CompletableDeferred() + var runningUpdateCount = 0 + updateReceivers + .values + .takeIf { it.isNotEmpty() } + ?.map { updateReceiver -> + runningUpdateCount++ + coroutineScope.launch { + updateReceiver.handleUpdate() + if (--runningUpdateCount == 0) { + mutex.unlock() + completableDeferred.complete(Unit) + } + } + } + ?: run { + mutex.unlock() + completableDeferred.complete(Unit) + } + return completableDeferred } - private fun getInternalModel(drawingModelStore: DrawingModelStore = DrawingModelStore.empty): InternalModel { - cachedInternalModel.let { if (it != null) return it } - val xRange = entries.xRange - val yRange = entries.yRange - val aggregateYRange = entries.calculateStackedYRange() - return InternalModel( - entries = entries.copy(), - minX = xRange.start, - maxX = xRange.endInclusive, - minY = yRange.start, - maxY = yRange.endInclusive, - stackedPositiveY = aggregateYRange.endInclusive, - stackedNegativeY = aggregateYRange.start, - xGcd = entries.calculateXGcd(), - id = entriesHashCode ?: entries.hashCode().also { entriesHashCode = it }, - drawingModelStore = drawingModelStore, - ) - } + /** + * Requests a data update. If the update is accepted, `true` is returned. If the update is rejected, which occurs + * when there’s already an update in progress, `false` is returned. + * + * @see entries + * @see registerForUpdates + */ + public fun setEntries(vararg entries: List): Boolean = setEntries(entries.toList()) + + /** + * Runs a data update. Unlike [setEntries], this function suspends the current coroutine and waits until an update + * can be run, meaning the update cannot be rejected. The returned [Deferred] implementation is marked as completed + * once the update has been processed. + * + * @see entries + * @see registerForUpdates + */ + public suspend fun setEntriesSuspending(vararg entries: List): Deferred = + setEntriesSuspending(entries.toList()) + + private fun getInternalModel(drawingModelStore: DrawingModelStore = DrawingModelStore.empty) = + cachedInternalModel?.copy(drawingModelStore = drawingModelStore) + ?: run { + val xRange = series.xRange + val yRange = series.yRange + val aggregateYRange = series.calculateStackedYRange() + InternalModel( + entries = series, + minX = xRange.start, + maxX = xRange.endInclusive, + minY = yRange.start, + maxY = yRange.endInclusive, + stackedPositiveY = aggregateYRange.endInclusive, + stackedNegativeY = aggregateYRange.start, + xGcd = series.calculateXGcd(), + id = series.hashCode(), + drawingModelStore = drawingModelStore, + ).also { cachedInternalModel = it } + } override fun getModel(): ChartEntryModel = getInternalModel() - override fun progressModel(key: Any, progress: Float) { + override suspend fun progressModel(key: Any, progress: Float) { with(updateReceivers[key] ?: return) { - executor.execute { - modelTransformer?.transform(drawingModelStore, progress) - onModelCreated(getInternalModel(drawingModelStore.copy())) - } + modelTransformer?.transform(drawingModelStore, progress) + val internalModel = getInternalModel(drawingModelStore.copy()) + currentCoroutineContext().ensureActive() + onModelCreated(internalModel) } } + @WorkerThread override fun registerForUpdates( key: Any, cancelAnimation: () -> Unit, - startAnimation: (progressModel: (chartKey: Any, progress: Float) -> Unit) -> Unit, + startAnimation: (progressModel: suspend (chartKey: Any, progress: Float) -> Unit) -> Unit, getOldModel: () -> ChartEntryModel?, modelTransformerProvider: Chart.ModelTransformerProvider?, drawingModelStore: MutableDrawingModelStore, @@ -145,16 +192,14 @@ public class ChartEntryModelProducer( getOldModel, updateChartValues, ) - executor.execute { - cancelAnimation() - modelTransformer?.prepareForTransformation( - oldModel = getOldModel(), - newModel = getModel(), - drawingModelStore = drawingModelStore, - chartValuesManager = updateChartValues(getModel()), - ) - startAnimation(::progressModel) - } + cancelAnimation() + modelTransformer?.prepareForTransformation( + oldModel = getOldModel(), + newModel = getModel(), + drawingModelStore = drawingModelStore, + chartValuesManager = updateChartValues(getModel()), + ) + startAnimation(::progressModel) } override fun unregisterFromUpdates(key: Any) { @@ -163,15 +208,26 @@ public class ChartEntryModelProducer( override fun isRegistered(key: Any): Boolean = updateReceivers.containsKey(key = key) - private data class UpdateReceiver( + private inner class UpdateReceiver( val cancelAnimation: () -> Unit, - val startAnimation: (progressModel: (chartKey: Any, progress: Float) -> Unit) -> Unit, + val startAnimation: (progressModel: suspend (chartKey: Any, progress: Float) -> Unit) -> Unit, val onModelCreated: (ChartEntryModel) -> Unit, val drawingModelStore: MutableDrawingModelStore, val modelTransformer: Chart.ModelTransformer?, val getOldModel: () -> ChartEntryModel?, val updateChartValues: (ChartEntryModel) -> ChartValuesManager, - ) + ) { + fun handleUpdate() { + cancelAnimation() + modelTransformer?.prepareForTransformation( + oldModel = getOldModel(), + newModel = getModel(), + drawingModelStore = drawingModelStore, + chartValuesManager = updateChartValues(getModel()), + ) + startAnimation(::progressModel) + } + } private data class InternalModel( override val entries: List>, diff --git a/vico/core/src/main/java/com/patrykandpatrick/vico/core/entry/ChartModelProducer.kt b/vico/core/src/main/java/com/patrykandpatrick/vico/core/entry/ChartModelProducer.kt index 65476139b..72c601125 100644 --- a/vico/core/src/main/java/com/patrykandpatrick/vico/core/entry/ChartModelProducer.kt +++ b/vico/core/src/main/java/com/patrykandpatrick/vico/core/entry/ChartModelProducer.kt @@ -37,7 +37,7 @@ public interface ChartModelProducer { * Calculates an intermediate list of entries for difference animations for the associated [key], where [progress] * is the balance between the previous and current lists of entries. */ - public fun progressModel(key: Any, progress: Float) + public suspend fun progressModel(key: Any, progress: Float) /** * Registers an update listener associated with a [key]. [cancelAnimation] and [startAnimation] are @@ -50,7 +50,7 @@ public interface ChartModelProducer { public fun registerForUpdates( key: Any, cancelAnimation: () -> Unit, - startAnimation: (progressModel: (chartKey: Any, progress: Float) -> Unit) -> Unit, + startAnimation: (progressModel: suspend (chartKey: Any, progress: Float) -> Unit) -> Unit, getOldModel: () -> Model?, modelTransformerProvider: Chart.ModelTransformerProvider?, drawingModelStore: MutableDrawingModelStore, diff --git a/vico/core/src/main/java/com/patrykandpatrick/vico/core/entry/composed/ComposedChartEntryModelProducer.kt b/vico/core/src/main/java/com/patrykandpatrick/vico/core/entry/composed/ComposedChartEntryModelProducer.kt index afe516f3d..f78842e97 100644 --- a/vico/core/src/main/java/com/patrykandpatrick/vico/core/entry/composed/ComposedChartEntryModelProducer.kt +++ b/vico/core/src/main/java/com/patrykandpatrick/vico/core/entry/composed/ComposedChartEntryModelProducer.kt @@ -16,7 +16,7 @@ package com.patrykandpatrick.vico.core.entry.composed -import com.patrykandpatrick.vico.core.DEF_THREAD_POOL_SIZE +import androidx.annotation.WorkerThread import com.patrykandpatrick.vico.core.chart.Chart import com.patrykandpatrick.vico.core.chart.composed.ComposedChartEntryModel import com.patrykandpatrick.vico.core.chart.values.ChartValuesManager @@ -29,10 +29,18 @@ import com.patrykandpatrick.vico.core.entry.diff.DrawingModelStore import com.patrykandpatrick.vico.core.entry.diff.MutableDrawingModelStore import com.patrykandpatrick.vico.core.entry.xRange import com.patrykandpatrick.vico.core.entry.yRange +import com.patrykandpatrick.vico.core.extension.copy import com.patrykandpatrick.vico.core.extension.gcdWith import com.patrykandpatrick.vico.core.extension.setAll -import java.util.concurrent.Executor -import java.util.concurrent.Executors +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Deferred +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.currentCoroutineContext +import kotlinx.coroutines.ensureActive +import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex /** * A [ChartModelProducer] implementation that generates [ComposedChartEntryModel] instances. @@ -40,81 +48,119 @@ import java.util.concurrent.Executors * @see ComposedChartEntryModel * @see ChartModelProducer */ -public class ComposedChartEntryModelProducer private constructor( - private val backgroundExecutor: Executor = Executors.newFixedThreadPool(DEF_THREAD_POOL_SIZE), -) : ChartModelProducer> { +public class ComposedChartEntryModelProducer private constructor(dispatcher: CoroutineDispatcher) : + ChartModelProducer> { - private var dataSets = mutableListOf>>() - private var cachedInternalModel: InternalModel? = null + private var dataSets = emptyList>>() + private var cachedInternalComposedModel: InternalComposedModel? = null + private val mutex = Mutex() + private val coroutineScope = CoroutineScope(dispatcher) private val updateReceivers = mutableMapOf() - private fun setDataSets(entries: List>>) { - this.dataSets.setAll(entries) - cachedInternalModel = null - updateReceivers.values.forEach { updateReceiver -> - backgroundExecutor.execute { - updateReceiver.cancelAnimation() - updateReceiver.modelTransformer?.prepareForTransformation( - oldModel = updateReceiver.getOldModel(), - newModel = getModel(), - drawingModelStore = updateReceiver.drawingModelStore, - chartValuesManager = updateReceiver.getChartValuesManager(getModel()), - ) - updateReceiver.startAnimation(::progressModel) + private fun setDataSets(dataSets: List>>): Boolean { + if (!mutex.tryLock()) return false + this.dataSets = dataSets.copy() + cachedInternalComposedModel = null + var runningUpdateCount = 0 + updateReceivers + .values + .takeIf { it.isNotEmpty() } + ?.forEach { updateReceiver -> + runningUpdateCount++ + coroutineScope.launch { + updateReceiver.handleUpdate() + if (--runningUpdateCount == 0) mutex.unlock() + } } - } + ?: mutex.unlock() + return true } - private fun getInternalModel(drawingModelStore: DrawingModelStore = DrawingModelStore.empty): InternalModel { - cachedInternalModel.let { if (it != null) return it } - val models = dataSets.map { dataSet -> - val xRange = dataSet.xRange - val yRange = dataSet.yRange - val aggregateYRange = dataSet.calculateStackedYRange() - object : ChartEntryModel { - override val entries: List> = dataSet - override val minX: Float = xRange.start - override val maxX: Float = xRange.endInclusive - override val minY: Float = yRange.start - override val maxY: Float = yRange.endInclusive - override val stackedPositiveY: Float = aggregateYRange.endInclusive - override val stackedNegativeY: Float = aggregateYRange.start - override val xGcd: Float = dataSet.calculateXGcd() - override val drawingModelStore: DrawingModelStore = drawingModelStore + private suspend fun setDataSetsSuspending(dataSets: List>>): Deferred { + mutex.lock() + this.dataSets = dataSets.copy() + cachedInternalComposedModel = null + val completableDeferred = CompletableDeferred() + var runningUpdateCount = 0 + updateReceivers + .values + .takeIf { it.isNotEmpty() } + ?.map { updateReceiver -> + runningUpdateCount++ + coroutineScope.launch { + updateReceiver.handleUpdate() + if (--runningUpdateCount == 0) { + mutex.unlock() + completableDeferred.complete(Unit) + } + } } - } - return InternalModel( - composedEntryCollections = models, - entries = models.map { it.entries }.flatten(), - minX = models.minOf { it.minX }, - maxX = models.maxOf { it.maxX }, - minY = models.minOf { it.minY }, - maxY = models.maxOf { it.maxY }, - stackedPositiveY = models.maxOf { it.stackedPositiveY }, - stackedNegativeY = models.minOf { it.stackedNegativeY }, - xGcd = models.fold(null) { gcd, model -> - gcd?.gcdWith(model.xGcd) ?: model.xGcd - } ?: 1f, - id = models.map { it.id }.hashCode(), - drawingModelStore = drawingModelStore, - ) + ?: run { + mutex.unlock() + completableDeferred.complete(Unit) + } + return completableDeferred } + private fun getInternalModel(drawingModelStore: DrawingModelStore = DrawingModelStore.empty) = + cachedInternalComposedModel + ?.let { composedModel -> + composedModel.copy( + composedEntryCollections = composedModel.composedEntryCollections + .map { model -> model.copy(drawingModelStore = drawingModelStore) }, + drawingModelStore = drawingModelStore, + ) + } + ?: run { + val models = dataSets.map { dataSet -> + val xRange = dataSet.xRange + val yRange = dataSet.yRange + val aggregateYRange = dataSet.calculateStackedYRange() + InternalModel( + entries = dataSet, + minX = xRange.start, + maxX = xRange.endInclusive, + minY = yRange.start, + maxY = yRange.endInclusive, + stackedPositiveY = aggregateYRange.endInclusive, + stackedNegativeY = aggregateYRange.start, + xGcd = dataSet.calculateXGcd(), + drawingModelStore = drawingModelStore, + ) + } + InternalComposedModel( + composedEntryCollections = models, + entries = models.map { it.entries }.flatten(), + minX = models.minOf { it.minX }, + maxX = models.maxOf { it.maxX }, + minY = models.minOf { it.minY }, + maxY = models.maxOf { it.maxY }, + stackedPositiveY = models.maxOf { it.stackedPositiveY }, + stackedNegativeY = models.minOf { it.stackedNegativeY }, + xGcd = models.fold(null) { gcd, model -> + gcd?.gcdWith(model.xGcd) ?: model.xGcd + } ?: 1f, + id = models.map { it.id }.hashCode(), + drawingModelStore = drawingModelStore, + ).also { cachedInternalComposedModel = it } + } + override fun getModel(): ComposedChartEntryModel = getInternalModel() - override fun progressModel(key: Any, progress: Float) { + override suspend fun progressModel(key: Any, progress: Float) { with(updateReceivers[key] ?: return) { - backgroundExecutor.execute { - modelTransformer?.transform(drawingModelStore, progress) - onModelCreated(getInternalModel(drawingModelStore.copy())) - } + modelTransformer?.transform(drawingModelStore, progress) + val internalModel = getInternalModel(drawingModelStore.copy()) + currentCoroutineContext().ensureActive() + onModelCreated(internalModel) } } + @WorkerThread override fun registerForUpdates( key: Any, cancelAnimation: () -> Unit, - startAnimation: (progressModel: (chartKey: Any, progress: Float) -> Unit) -> Unit, + startAnimation: (progressModel: suspend (chartKey: Any, progress: Float) -> Unit) -> Unit, getOldModel: () -> ComposedChartEntryModel?, modelTransformerProvider: Chart.ModelTransformerProvider?, drawingModelStore: MutableDrawingModelStore, @@ -131,16 +177,14 @@ public class ComposedChartEntryModelProducer private constructor( getOldModel, updateChartValues, ) - backgroundExecutor.execute { - cancelAnimation() - modelTransformer?.prepareForTransformation( - oldModel = getOldModel(), - newModel = getModel(), - drawingModelStore = drawingModelStore, - chartValuesManager = updateChartValues(getModel()), - ) - startAnimation(::progressModel) - } + cancelAnimation() + modelTransformer?.prepareForTransformation( + oldModel = getOldModel(), + newModel = getModel(), + drawingModelStore = drawingModelStore, + chartValuesManager = updateChartValues(getModel()), + ) + startAnimation(::progressModel) } override fun isRegistered(key: Any): Boolean = updateReceivers.containsKey(key) @@ -155,24 +199,28 @@ public class ComposedChartEntryModelProducer private constructor( public fun createTransaction(): Transaction = Transaction() /** - * Creates a [Transaction], runs [block], and calls [Transaction.commit]. + * Creates a [Transaction], runs [block], and calls [Transaction.commit], returning its output. */ - public fun runTransaction(block: Transaction.() -> Unit) { - createTransaction().also(block).commit() - } + public fun runTransaction(block: Transaction.() -> Unit): Boolean = createTransaction().also(block).commit() + + /** + * Creates a [Transaction], runs [block], and calls [Transaction.commit], returning its output. + */ + public suspend fun runTransactionSuspending(block: Transaction.() -> Unit): Deferred = + createTransaction().also(block).commitSuspending() /** * Handles data updates. An initially empty list of data sets is created and can be updated via the class’s * functions. Each data set corresponds to a single nested [Chart]. */ public inner class Transaction internal constructor() { - private val newEntries = mutableListOf>>() + private val newDataSets = mutableListOf>>() /** * Populates the new list of data sets with the current data sets. */ public fun populate() { - newEntries.setAll(dataSets) + newDataSets.setAll(dataSets) } /** @@ -186,28 +234,28 @@ public class ComposedChartEntryModelProducer private constructor( * Removes the data set at the specified index. */ public fun removeAt(index: Int) { - newEntries.removeAt(index) + newDataSets.removeAt(index) } /** * Replaces the data set at the specified index with the provided data set. */ public fun set(index: Int, dataSet: List>) { - newEntries[index] = dataSet + newDataSets[index] = dataSet } /** * Adds a data set. */ public fun add(dataSet: List>) { - newEntries.add(dataSet) + newDataSets.add(dataSet) } /** * Adds a data set. */ public fun add(index: Int, dataSet: List>) { - newEntries.add(index, dataSet) + newDataSets.add(index, dataSet) } /** @@ -221,29 +269,58 @@ public class ComposedChartEntryModelProducer private constructor( * Clears the new list of data sets. */ public fun clear() { - newEntries.clear() + newDataSets.clear() } /** - * Finalizes the data update. + * Requests a data update. If the update is accepted, `true` is returned. If the update is rejected, which + * occurs when there’s already an update in progress, `false` is returned. */ - public fun commit() { - setDataSets(newEntries) - } + public fun commit(): Boolean = setDataSets(newDataSets) + + /** + * Runs a data update. Unlike [commit], this function suspends the current coroutine and waits until an update + * can be run, meaning the update cannot be rejected. The returned [Deferred] implementation is marked as + * completed once the update has been processed. + */ + public suspend fun commitSuspending(): Deferred = setDataSetsSuspending(newDataSets) } - private data class UpdateReceiver( + private inner class UpdateReceiver( val cancelAnimation: () -> Unit, - val startAnimation: (progressModel: (chartKey: Any, progress: Float) -> Unit) -> Unit, + val startAnimation: (progressModel: suspend (chartKey: Any, progress: Float) -> Unit) -> Unit, val onModelCreated: (ComposedChartEntryModel) -> Unit, val drawingModelStore: MutableDrawingModelStore, val modelTransformer: Chart.ModelTransformer>?, val getOldModel: () -> ComposedChartEntryModel?, - val getChartValuesManager: (ComposedChartEntryModel) -> ChartValuesManager, - ) + val updateChartValues: (ComposedChartEntryModel) -> ChartValuesManager, + ) { + fun handleUpdate() { + cancelAnimation() + modelTransformer?.prepareForTransformation( + oldModel = getOldModel(), + newModel = getModel(), + drawingModelStore = drawingModelStore, + chartValuesManager = updateChartValues(getModel()), + ) + startAnimation(::progressModel) + } + } private data class InternalModel( - override val composedEntryCollections: List, + override val entries: List>, + override val minX: Float, + override val maxX: Float, + override val minY: Float, + override val maxY: Float, + override val stackedPositiveY: Float, + override val stackedNegativeY: Float, + override val xGcd: Float, + override val drawingModelStore: DrawingModelStore, + ) : ChartEntryModel + + private data class InternalComposedModel( + override val composedEntryCollections: List, override val entries: List>, override val minX: Float, override val maxX: Float, @@ -253,18 +330,18 @@ public class ComposedChartEntryModelProducer private constructor( override val stackedNegativeY: Float, override val xGcd: Float, override val id: Int, - override val drawingModelStore: DrawingModelStore = DrawingModelStore.empty, + override val drawingModelStore: DrawingModelStore, ) : ComposedChartEntryModel public companion object { /** - * Creates a [ComposedChartEntryModelProducer], running an initial [Transaction]. [backgroundExecutor] is used - * to run calculations off the main thread. + * Creates a [ComposedChartEntryModelProducer], running an initial [Transaction]. [dispatcher] is the + * [CoroutineDispatcher] to use to handle updates. */ public fun build( - backgroundExecutor: Executor = Executors.newFixedThreadPool(DEF_THREAD_POOL_SIZE), + dispatcher: CoroutineDispatcher = Dispatchers.Default, transaction: Transaction.() -> Unit = {}, ): ComposedChartEntryModelProducer = - ComposedChartEntryModelProducer(backgroundExecutor).also { it.runTransaction(transaction) } + ComposedChartEntryModelProducer(dispatcher).also { it.runTransaction(transaction) } } } diff --git a/vico/core/src/main/java/com/patrykandpatrick/vico/core/entry/diff/DefaultDrawingModelInterpolator.kt b/vico/core/src/main/java/com/patrykandpatrick/vico/core/entry/diff/DefaultDrawingModelInterpolator.kt index bf4067195..a1579741f 100644 --- a/vico/core/src/main/java/com/patrykandpatrick/vico/core/entry/diff/DefaultDrawingModelInterpolator.kt +++ b/vico/core/src/main/java/com/patrykandpatrick/vico/core/entry/diff/DefaultDrawingModelInterpolator.kt @@ -17,6 +17,8 @@ package com.patrykandpatrick.vico.core.entry.diff import com.patrykandpatrick.vico.core.extension.orZero +import kotlinx.coroutines.currentCoroutineContext +import kotlinx.coroutines.ensureActive import kotlin.math.max /** @@ -26,7 +28,7 @@ import kotlin.math.max public class DefaultDrawingModelInterpolator> : DrawingModelInterpolator { - private val transformationMaps = mutableListOf>>() + private var transformationMaps = emptyList>>() private var oldDrawingModel: R? = null private var newDrawingModel: R? = null @@ -38,13 +40,16 @@ public class DefaultDrawingModelInterpolator map - .mapNotNull { (x, model) -> model.transform(fraction)?.let { drawingInfo -> x to drawingInfo } } + .mapNotNull { (x, model) -> + currentCoroutineContext().ensureActive() + model.transform(fraction)?.let { drawingInfo -> x to drawingInfo } + } .takeIf { list -> list.isNotEmpty() } ?.toMap() }, @@ -54,16 +59,17 @@ public class DefaultDrawingModelInterpolator - val map = mutableMapOf>() - oldDrawingModel - ?.getOrNull(index) - ?.forEach { (x, drawingInfo) -> map[x] = TransformationModel(drawingInfo) } - newDrawingModel - ?.getOrNull(index) - ?.forEach { (x, drawingInfo) -> map[x] = TransformationModel(map[x]?.old, drawingInfo) } - transformationMaps.add(map) + transformationMaps = buildList { + repeat(max(oldDrawingModel?.size.orZero, newDrawingModel?.size.orZero)) { index -> + val map = mutableMapOf>() + oldDrawingModel + ?.getOrNull(index) + ?.forEach { (x, drawingInfo) -> map[x] = TransformationModel(drawingInfo) } + newDrawingModel + ?.getOrNull(index) + ?.forEach { (x, drawingInfo) -> map[x] = TransformationModel(map[x]?.old, drawingInfo) } + add(map) + } } } diff --git a/vico/core/src/main/java/com/patrykandpatrick/vico/core/entry/diff/DrawingModelInterpolator.kt b/vico/core/src/main/java/com/patrykandpatrick/vico/core/entry/diff/DrawingModelInterpolator.kt index 3909680dd..7d1f9af4a 100644 --- a/vico/core/src/main/java/com/patrykandpatrick/vico/core/entry/diff/DrawingModelInterpolator.kt +++ b/vico/core/src/main/java/com/patrykandpatrick/vico/core/entry/diff/DrawingModelInterpolator.kt @@ -29,5 +29,5 @@ public interface DrawingModelInterpolator List.getRepeating(index: Int): T { return get(index % size.coerceAtLeast(1)) } +@JvmName("copyDouble") +internal fun List>.copy() = map { it.toList() } + +@JvmName("copyTriple") +internal fun List>>.copy() = map { it.copy() } + /** * Replaces all of the elements of this [MutableList] with the elements of the provided collection. */ diff --git a/vico/views/src/main/java/com/patrykandpatrick/vico/views/chart/BaseChartView.kt b/vico/views/src/main/java/com/patrykandpatrick/vico/views/chart/BaseChartView.kt index 860a89170..20b609be5 100644 --- a/vico/views/src/main/java/com/patrykandpatrick/vico/views/chart/BaseChartView.kt +++ b/vico/views/src/main/java/com/patrykandpatrick/vico/views/chart/BaseChartView.kt @@ -74,6 +74,14 @@ import com.patrykandpatrick.vico.views.gestures.movedYDistance import com.patrykandpatrick.vico.views.scroll.ChartScrollSpec import com.patrykandpatrick.vico.views.scroll.copy import com.patrykandpatrick.vico.views.theme.ThemeHandler +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.cancel +import kotlinx.coroutines.cancelAndJoin +import kotlinx.coroutines.launch +import kotlin.coroutines.EmptyCoroutineContext import kotlin.properties.Delegates.observable import kotlin.properties.ReadWriteProperty @@ -139,6 +147,14 @@ public abstract class BaseChartView internal constructo private val drawingModelStore = MutableDrawingModelStore() + private var coroutineScope: CoroutineScope? = null + + private var animationJob: Job? = null + + private var finalAnimationFrameJob: Job? = null + + private var isAnimationFrameGenerationRunning: Boolean = false + private var markerTouchPoint: Point? = null private var wasMarkerVisible: Boolean = false @@ -217,6 +233,11 @@ public abstract class BaseChartView internal constructo */ public var runInitialAnimation: Boolean = true + /** + * The [CoroutineDispatcher] to use to handle data updates. + */ + public var dispatcher: CoroutineDispatcher = Dispatchers.Default + /** * The [Chart] displayed by this [View]. */ @@ -243,39 +264,47 @@ public abstract class BaseChartView internal constructo } private fun registerForUpdates() { - entryProducer?.registerForUpdates( - key = this, - cancelAnimation = { handler.post(animator::cancel) }, - startAnimation = { - if (model != null || runInitialAnimation) { - handler.post(animator::start) - } else { - progressModelOnAnimationProgress(progress = Animation.range.endInclusive) + coroutineScope?.launch(dispatcher) { + entryProducer?.registerForUpdates( + key = this@BaseChartView, + cancelAnimation = { + handler.post(animator::cancel) + animationJob?.cancel() + }, + startAnimation = { + if (model != null || runInitialAnimation) { + handler.post(animator::start) + } else { + progressModelOnAnimationProgress(progress = Animation.range.endInclusive) + } + }, + getOldModel = { model }, + modelTransformerProvider = chart?.modelTransformerProvider, + drawingModelStore = drawingModelStore, + updateChartValues = { model -> + chartValuesManager.resetChartValues() + chart?.updateChartValues(chartValuesManager, model, getXStep?.invoke(model)) + chartValuesManager + }, + ) { model -> + post { + setModel(model = model, updateChartValues = false) + postInvalidateOnAnimation() } - }, - getOldModel = { model }, - modelTransformerProvider = chart?.modelTransformerProvider, - drawingModelStore = drawingModelStore, - updateChartValues = { model -> - chartValuesManager.resetChartValues() - chart?.updateChartValues(chartValuesManager, model, getXStep?.invoke(model)) - chartValuesManager - }, - ) { model -> - post { - setModel(model = model, updateChartValues = false) - postInvalidateOnAnimation() } } } override fun onAttachedToWindow() { super.onAttachedToWindow() + coroutineScope = CoroutineScope(EmptyCoroutineContext) if (entryProducer?.isRegistered(key = this) != true) registerForUpdates() } override fun onDetachedFromWindow() { super.onDetachedFromWindow() + coroutineScope?.cancel() + coroutineScope = null entryProducer?.unregisterFromUpdates(key = this) } @@ -467,7 +496,19 @@ public abstract class BaseChartView internal constructo } private fun progressModelOnAnimationProgress(progress: Float) { - entryProducer?.progressModel(this, progress) + if (!isAnimationFrameGenerationRunning) { + isAnimationFrameGenerationRunning = true + animationJob = coroutineScope?.launch(dispatcher) { + entryProducer?.progressModel(this@BaseChartView, progress) + isAnimationFrameGenerationRunning = false + } + } else if (progress == 1f) { + finalAnimationFrameJob = coroutineScope?.launch(dispatcher) { + animationJob?.cancelAndJoin() + entryProducer?.progressModel(this@BaseChartView, progress) + isAnimationFrameGenerationRunning = false + } + } } override fun onMeasure(widthMeasureSpec: Int, heightMeasureSpec: Int) {