patrykandpatrick / vico

A light and extensible chart library for Android.
https://patrykandpatrick.com/vico/wiki
Apache License 2.0
2.07k stars 125 forks source link

Chart does not recompose when AxisValuesOverrider maxY is changed from 0 to greater than 0 #791

Closed rpaynetoast closed 2 months ago

rpaynetoast commented 2 months ago

How to reproduce

On columnChart, set the axisValuesOverrider parameter to AxisValuesOverrider.fixed(maxY = 0). Then toggle the maxY to a number greater than 0.

val initialEntries = listOf(
    listOf(FloatEntry(0.0f, 0.0f), FloatEntry(1.0f, 0.0f), FloatEntry(2.0f, 0.0f)),
)

val updatedEntries = listOf(
    listOf(FloatEntry(0.0f, 10.15f), FloatEntry(1.0f, 30.24f), FloatEntry(2.0f, 37.0f))
)

@Composable
fun FullSizedChart(
    modifier: Modifier = Modifier,
) {
    var entries by remember { mutableStateOf(initialEntries) }
    val chartEntryModelProducer = remember { ChartEntryModelProducer() }
    LaunchedEffect(entries) {
        chartEntryModelProducer.setEntries(entries)
    }
    Column(
        horizontalAlignment = Alignment.CenterHorizontally
    ) {
        val maxY = entries.flatten().maxOf { it.y }
        val axisValuesOverrider = AxisValuesOverrider.fixed(maxY = maxY)
        Chart(
            chart = columnChart(axisValuesOverrider = axisValuesOverrider),
            chartModelProducer = chartEntryModelProducer,
            endAxis = rememberEndAxis(),
            bottomAxis = rememberBottomAxis(),
            modifier = modifier
        )
        Button(onClick = {
            entries = if (entries == initialEntries) {
                updatedEntries
            } else {
                initialEntries
            }
        }) {
            Text(text = "Toggle chart")
        }
    }
}

Observed behavior

Chart does not recompose:

https://github.com/patrykandpatrick/vico/assets/166438828/baae0154-6073-43af-b8d4-385fae77042f

Expected behavior

Chart should recompose

Vico version(s)

1.15.0

Android version(s)

14

Additional information

Workaround

If maxY is zero, set axisValuesOverrider to null. Set an itemPlacer on bottomAxis that toggles between 0 and a number greater than 0.

...

@Composable
fun FullSizedChart(
    modifier: Modifier = Modifier,
) {
    ...
        val axisValuesOverrider = if (maxY == 0f) null else AxisValuesOverrider.fixed(maxY = maxY)
        val maxItemCount = if (maxY == 0f) 0 else 4
        Chart(
            chart = columnChart(axisValuesOverrider = axisValuesOverrider),
            chartModelProducer = chartEntryModelProducer,
            endAxis = rememberEndAxis(
                itemPlacer = AxisItemPlacer.Vertical.default(maxItemCount = maxItemCount),
            ),
            ...
}
Gowsky commented 2 months ago

I'm reposting the comment as I forgot to add something important.

First, with ChartEntryModelProducer, it's incorrect to swap a Chart's AxisValuesOverrider this way. Instead, you should use a single implementation with dynamic behavior. This relates to the asynchronous behavior of ChartEntryModelProducer and the fact that AxisValuesOverrider is used during data updates. These topics are described in detail in the Vico 2 documentation. The API is a bit different, but the same principles apply. See "AxisValueOverrider" (particularly the warning box) and "Asynchrony and extras." The correct approach in your case is to create an AxisValuesOverrider implementation like this:

object : AxisValuesOverrider<ChartEntryModel> {
    override fun getMaxY(model: ChartEntryModel) = // Use `model` here, not `entries`. See `model.maxY`. 
}

Second, you can't set the maximum y value to zero when the minimum y value is also zero. Such a y range would imply that the chart is zoomed in infinitely, which is impossible. This is the reason for the columns not being drawn.

patrickmichalik commented 2 months ago

Hello, @rpaynetoast! I wanted to share some further information.

The reason why the chart remains blank when updatedEntries and a nonzero maximum y value are applied is that the initial, incorrect y range causes persistent corruption. Since DrawingModels are interpolated, one broken DrawingModel can cause subsequently generated ones to be invalid too. We’ll look into adding a check that throws an exception when the y range is overridden incorrectly.

AxisItemPlacer.Vertical is the appropriate means of making a VerticalAxis blank. Overriding the y range isn’t necessary. However, the AxisItemPlacer.Vertical-switching mechanism that you’ve shared may cause synchronization issues. Since ChartEntryModelProducer handles updates asynchronously, entries may not always match what’s displayed on the chart. No critical problems should arise, but you may observe minor glitches. For proper synchronization, such data-dependent changes should be handled via extras. Unfortunately, in Vico 1, switching to extras here would prove quite challenging. Even with a custom AxisItemPlacer.Vertical implementation, you’d run into the problem of the ExtraStore being unavailable in a few places. An exceeding number of overrides would be required. Thus, this use case requires either manual ChartEntryModel creation or Vico 2. Here’s a quick Vico 2 example:

val y1 = listOf(0, 0, 0)
val y2 = listOf<Number>(10.15, 30.24, 37)
val isEndAxisBlankKey = ExtraStore.Key<Boolean>()
val endAxisItemPlacer = AxisItemPlacer.Vertical.count({ if (it[isEndAxisBlankKey]) 0 else null })

@Preview
@Composable
fun Example() {
  var y by remember { mutableStateOf<List<Number>>(y1) }
  val modelProducer = remember { CartesianChartModelProducer() }
  LaunchedEffect(y) {
    withContext(Dispatchers.Default) {
      modelProducer.runTransaction {
        columnSeries { series(y) }
        extras { it[isEndAxisBlankKey] = y == y1 }
      }
    }
  }
  Column(modifier = Modifier.padding(16.dp), verticalArrangement = Arrangement.spacedBy(16.dp)) {
    CartesianChartHost(
      rememberCartesianChart(
        rememberColumnCartesianLayer(),
        endAxis = rememberEndAxis(itemPlacer = endAxisItemPlacer),
        bottomAxis = rememberBottomAxis(),
      ),
      modelProducer,
    )
    Button({ y = if (y == y1) y2 else y1 }) { Text("Update data") }
  }
}