-
Notifications
You must be signed in to change notification settings - Fork 659
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add the possibility to addEntries to a chart, without updating the whole dataSets #752
base: master
Are you sure you want to change the base?
Conversation
@angelos3lex Could you please provide an example to show how to use this new feature? |
@wuxudong I updated to add this possibility on BubbleChart, LineChart, BarChart, ScatterChart, HorizontalBarChart and CandleStickChart. I can't add an example to the Examples, as i can't currently build the example proj using the master branch, because of below error:
Also, i can't build the example proj using my repo, cause of another error during loading of the bundler and the react-native-screens, which is probably resolved in your current master. But the error above doesn't let me build that either :P A live repo where you can check the way to append entries with this new way for the moment is here, by checking the App.js file |
Also added a |
Thanks @angelos3lex for sharing. |
Hey @hemgui, On our side we are only using the fork repo in our production app, without any problems for more than 1 year. |
Hello @angelos3lex , thank you for this amazing PR. We tested it on a live data coming in from a bluetooth device at 50Hz, and it provides a great performance boost. We were wondering if there is a way to remove the data points without re-rendering the complete graph? For example, if someone wants to keep the last 1000 samples on graph from incoming data, is there a method to remove the previous data points without re-rendering? Thank you. |
Hey thx for feedback. You can try using the A possible limitation with this will be if you want to keep a lot of values. I guess this may be a bit more problematic because the values are transformed to JSON and back to pass through the bridge to native, so again this needs some time, but with re-render that would also be worse. Another way you can achieve this (depends on your use case) may be by keeping the
|
I like the second approach you suggested but I am concerned about keeping the data in memory (even on the native side) as the BLE device I am using sends data at 50Hz (for 1 hour) and that's a lot of data. While looking at your PR, I realised that a new method similar to addEntries can be added such as removeEntries where on the native side, this can be used. |
@wuxudong
Imagine you have to refresh the chart in ~60 fps. This can be done by updating the datasets and re-render, but after the dataSet reaches some hundreds of values, you can notice the performance drop. When the values go on to thousands, the are constant freezes to the ui, as the rendering of such a big dataSet is too expensive. The JS thread freezes and there is nothing you can do.
By this PR, a new
addEntries
method is introduced, giving the possibility to append just the entries needed to the dataSet. So, one, can just add 1 entry every 17ms, the chart will update natively, thus resulting to a steady ~60fps, no matter of the size of the dataSet. Even if it contains thousands of values, the communication through js to native will contain only the new added entries, so the freeze and expensive re-render is eliminated.I have created a new public repo here demonstrating the new way to add values in real time. (check App.js file) If you wish, change the logic to the previously suggested one, and you will definitely understand and feel the difference from the first second. As the time goes one, and the dataSet gets more and more data, the performance boost is unbelievable.
PR workable on android and ios.
Also, i noticed that current master is not buildable in android at least.react-native-charts-wrapper/android/src/main/java/com/github/wuxudong/rncharts/charts/ChartBaseManager.java:333: error: cannot find symbol chart.setMarker(marker);
So, i didn't merge this with master yet, but there are no conflicts up to now, so you can safely merge.Also, do you think that you'll manage to fix the build errors and release master as a new stable release sometime?