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

ANR called for basic line graph #824

Closed joey101 closed 1 month ago

joey101 commented 1 month ago

How to reproduce

https://github.com/patrykandpatrick/vico/discussions/821#discussioncomment-10204599

Observed behavior

Causes ANR

Expected behavior

Graph is meant to be there

Vico version(s)

2.0.0-alpha.25

Android version(s)

14

Additional information

No response

Gowsky commented 1 month ago

Hi @joey101, the ANR is related to the timestamps being incorrectly calculated. From the code you've provided:

val staticDataX = listOf(
        LocalDateTime.now().minusMinutes(10).atZone(ZoneId.systemDefault()).toInstant()
            .toEpochMilli(),
        LocalDateTime.now().minusMinutes(8).atZone(ZoneId.systemDefault()).toInstant()
            .toEpochMilli(),
        LocalDateTime.now().minusMinutes(6).atZone(ZoneId.systemDefault()).toInstant()
            .toEpochMilli(),
        LocalDateTime.now().minusMinutes(4).atZone(ZoneId.systemDefault()).toInstant()
            .toEpochMilli(),
        LocalDateTime.now().minusMinutes(2).atZone(ZoneId.systemDefault()).toInstant()
            .toEpochMilli(),
        LocalDateTime.now().atZone(ZoneId.systemDefault()).toInstant().toEpochMilli()
    )

LocalDateTime.now() may be different between invocations. When this happens, the x values aren't evenly spaced. For example, some pairs are spaced by 120K ms (2 min), and others are spaced by 199,999 ms. The x step then becomes 1, which makes the chart content extremely wide (millions of pixels). Realistically, such a chart would be unusable.

Please correct the logic so that the reference timestamp doesn’t change:

val now = LocalDateTime.now()
val staticDataX = listOf(
    now.minusMinutes(10).atZone(ZoneId.systemDefault()).toInstant().toEpochMilli(),
    now.minusMinutes(8).atZone(ZoneId.systemDefault()).toInstant().toEpochMilli(),
    …
)

The chart will then behave properly.

If you had intermediate values, you could instead override the x step and set it to 120K ms, but here, it seems like the aim is to have timestamps spaced by two minutes.

We’ll look into fixing the ANR regardless, but this is low-priority, since it occurs during incorrect usage that would generate an effectively infinite chart.

Gowsky commented 1 month ago

Vico 2.0.0-alpha.27 fixes the ANR, but my comment about the data still applies. Cheers!

joey101 commented 1 month ago

thank you works now for me, keep up the hard work this library is fun to use. @Gowsky

Gowsky commented 1 month ago

@joey101 You're welcome! I'm glad you enjoy it!