Skip to content
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

Regression when sliding scale min / max values after "[perf] Update/draw only visible line/points" commit #7803

Closed
emmcb opened this issue Sep 16, 2020 · 4 comments · Fixed by #7808
Milestone

Comments

@emmcb
Copy link
Contributor

emmcb commented Sep 16, 2020

I have a time chart which covers a long time range, and a slider which, when slided by the user, sets the time scale min and max properties in order to change the plotted range.

Since #7793, I get very strange animations artefacts during the slide that were not present before.

Fiddle to reproduce: https://jsfiddle.net/am2egz9k

@kurkle
Copy link
Member

kurkle commented Sep 16, 2020

Good, I was almost sure it would introduced some issue. Need to reset those elements that were not updated. I'll take a look tomorrow.

@etimberg etimberg added this to the Version 3.0 milestone Sep 16, 2020
@kurkle
Copy link
Member

kurkle commented Sep 17, 2020

Played around a bit. I see 2 options, either keep updating all the points, or dont animate the partial updates.
Latter can be achieved simply by using 'resize' (or 'none') as updade mode: https://jsfiddle.net/3rkmLyg5/

@kurkle
Copy link
Member

kurkle commented Sep 17, 2020

@emmcbd which way works for your use case?

Is the preformance gain from partial updates desirable or would you rather disable that optimization? The drawing part could still be skipped even if all the points are updated.

Options:

  • add new option for disabling or enabling partial updates.
  • automatically determine if it would be feasible to do partial update (when that update is not going to be animated for any reason)
  • both of above, eg partialUpdates: true/false/'auto', defaulting to 'auto'
  • remove the optimization completely
  • just keep the partial drawing, but update all

@emmcb
Copy link
Contributor Author

emmcb commented Sep 18, 2020

Tried to disable the partial update animation by calling chart.update('none'), I do not think this one can be acceptable from a UX point of view, because it makes the updates really choppy.

For my particular use case, updating all is ok because my number of points is not very high (about 400), so I do not think it is enough to really benefit from these optimisations.

However for a more general use case maybe automatically determining if we must do a full update (because the scale has change for example) is better, I do not know if it is easy to implement or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants