search

margelo / react-native-graph

📈 Beautiful, high-performance Graphs and Charts for React Native built with Skia
https://margelo.io
MIT License
2.04k stars 115 forks source link

chore: move graph animations to UI thread #74

Closed Nodonisko closed 1 year ago

Nodonisko commented 1 year ago

https://github.com/Shopify/react-native-skia/pull/1341 made possible to use Reanimated Values directly, is now possible to run animations fully on UI thread (Skia runs them only partially on UI thread).

TODO:

chrispader commented 1 year ago

LGTM! 🚀 Thanks for a great PR. This is a really good performance improvement

Needed to bump react-native version, but this PR was a good opportunity, as we were way behind the current version.

Nodonisko commented 1 year ago

Thanks for help with example app! Looks good to me.

chrispader commented 1 year ago

I still need to fix the android build of the example app, but the rest works fine!

Nodonisko commented 1 year ago

@chrispader please also bump Skia to latest version because there was bug that randomly crashed app when it was used in combination with Reanimated https://github.com/Shopify/react-native-skia/issues/1517

ArtKullashi commented 1 year ago

I hope this also fixes a weird bug with the graph not animating between data changes. I was going create an issue but will wait for this update first.

chrispader commented 1 year ago

Fixed the build for android, will merge this now.

Thanks to everybody who contributed on this change! 🚀🥳 @Nodonisko @lewiscasewell

wcandillon commented 1 year ago

very nice. We use similar code on our apps and it seems to work great!

Nodonisko commented 1 year ago

@wcandillon The animation of graph line it's still done using Skia animations because I am not sure how to animate SkPath using reanimated. Can you hint me how to do that?

wcandillon commented 1 year ago

yes this would be the source of truth at the moment: https://github.com/Shopify/react-native-skia/blob/main/example/src/Examples/Wallet/Wallet.tsx#L34