patrykandpatrick / vico

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

Decoration is not drawn above chart in ComposedChart #500

Closed Lonexera closed 7 months ago

Lonexera commented 9 months ago

How to reproduce

Add Decoration that draws itself through onDrawAboveChart(..) function into decorations list within any chart in composed chart. In the following sample code I've added ThreholdLine to the columnChart:

@Preview
@Composable
fun SamplePreview() {
    val columnEntries = listOf(
        FloatEntry(y = 5f, x = 0f),
        FloatEntry(y = 6f, x = 1f),
        FloatEntry(y = 9f, x = 2f),
    )
    val lineEntries = listOf(
        FloatEntry(y = 4f, x = 0f),
        FloatEntry(y = 5f, x = 1f),
        FloatEntry(y = 3f, x = 2f),
        FloatEntry(y = 7f, x = 3f),
    )

    val columnChart = columnChart(
        columns = currentChartStyle.columnChart.columns,
        decorations = listOf(ThresholdLine(3f)),
    )
    val lineChart = lineChart(
        lines = currentChartStyle.lineChart.lines,
    )

    ProvideChartStyle {
        Chart(
            modifier = Modifier.fillMaxSize(),
            chart = remember(columnChart, lineChart) { columnChart + lineChart },
            chartModelProducer = ComposedChartEntryModelProducer.build {
                add(columnEntries)
                add(lineEntries)
            },
            startAxis = rememberStartAxis(guideline = null),
            bottomAxis = rememberBottomAxis(guideline = null),
        )
    }
}

Observed behavior

The decoration is not drawn. The onDrawAboveChart(..) method of the ThresholdLine class is not even called.

Also noticed that decorations will be drawn if decorations list is set to the ComposedChart directly

chart = remember(columnChart, lineChart) { 
    (columnChart + lineChart).also {
        it.setDecorations(listOf(ThresholdLine(3f)))
    }
},

During debug process I learned that drawDecorationAboveChart(..)method of theBaseChartis only called once for theComposedChartitself (which decorations list is empty and doesn't contain any decorations from composed charts), opposed to thedrawDecorationBehindChart(..)method that is called forComposedChart` and for all composed charts within it.

Expected behavior

The ThresholdLine is drawn above the column chart.

Any Decoration that uses onDrawAboveChart(..) function should be drawn after the chart it was set in is drawn.

Vico version(s)

Latest stable version

Android version(s)

api 34

Additional information

This bug also applies to persistent markers list

patrickmichalik commented 9 months ago

Hello! Thanks for the report. This is no longer an issue in Vico 2.0.0 Alpha 1 and later. (Regarding persistent Markers, linking them to specific CartesianLayers isn’t possible yet, but this restriction manifests itself as the lack of an API that enables such functionality, not as the ignoring of input, as in Vico 1.13.1.) While we’re now focusing on Vico 2, and no new features will be added to Vico 1, we plan on releasing version 1.13.2 to address this bug. I’ll keep you updated.

Lonexera commented 9 months ago

Hi! Unfortunately we can't use alpha version of the library, so 1.13.2 release with the bug fix sounds great, thank you!

patrickmichalik commented 8 months ago

There’s been a small change of plans: the next stable version will be 1.14.0, not 1.13.2. This doesn’t change anything as far as the bug fix is concerned—the Decoration problem will, of course, be resolved. Vico 1.14.0 Alpha 1 includes the fix, and Vico 1.14.0 (stable) should be out soon, as no large changes are planned.

patrickmichalik commented 7 months ago

Vico 1.14.0 is rolling out. Cheers!