search

halfhp / androidplot

Charts and plots for Android
http://androidplot.com
Apache License 2.0
505 stars 159 forks source link

Fix app freeze when gridsteps are much larger than the actual plot range #76

Closed Jonathan727 closed 6 years ago

Jonathan727 commented 6 years ago

Hello, firstly, sorry that this pull set has so many whitespace/formatting change. If you want me to submit a simplified pull request I can, just let me know. Android studio did that. The real important change is in XYGraphWidget.java#drawGrid(Canvas canvas).

I had been struggling for a while with an empty plot would freeze my app. I figured out that when the plot was empty, the domain grid loop was iterating one loop over a hundred million times.

I've rewritten the for loop and 3 others that are similar. I was very verbose in my commits so anyone can verify that the changes I made leave a loop that functions the same but in some cases skips a great many iterations that were no-op anyway. My reductions allowed me to remove the if condition inside the for loop.

Jonathan727 commented 6 years ago

Sorry, I should have described how the long running loop actually froze my app.

Basically, the render thread would be calculating grids, iterating over 100 million times. Meanwhile, the main thread would be blocked at synchronize(pingPong)

Jonathan727 commented 6 years ago

I made the grid loops much more compact and the logic easier to follow in commit 02cdc94

halfhp commented 6 years ago

Thanks for the contribution @Jonathan727 ! I'll do my best to do a code review this evening.

Jonathan727 commented 6 years ago

Thanks for catching those issues!!!

halfhp commented 6 years ago

@Jonathan727 thanks for fixing! I think this looks good. For fixes and features I squash the changeset into a single commit so its easy to reference in the future. If you prefer to squash the commit log with a message of your own choosing, feel free otherwise I'm more than happy to do it with the following message: (you'll still be listed as the author of the changeset)

Fix app freeze when gridsteps are much larger than the actual plot range

Jonathan727 commented 6 years ago

Hi @halfhp , I wondered if you might be willing to create a formal release on gradle with these changes. I think 1.5.4 is the last release and it doesn't include this fix. Not a huge priority since I know you are donating your time.

halfhp commented 6 years ago

Hey @Jonathan727 apologies for the delay. I have one other issue to resolve and then should be ready to push a new release. I'll do my best to get this done by Wednesday.

halfhp commented 6 years ago

@Jonathan727 it's now live in 1.5.5

Jonathan727 commented 6 years ago

Thanks a Million!

-Jonathan (via mobile)

On Wed, May 23, 2018, 11:27 Nick Fellows notifications@github.com wrote:

@Jonathan727 https://github.com/Jonathan727 it's now live in 1.5.5

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/halfhp/androidplot/pull/76#issuecomment-391411782, or mute the thread https://github.com/notifications/unsubscribe-auth/AFXumDT5ww_kU-OgP1ppDwe5k4JE1xUGks5t1Y35gaJpZM4SjqgP .