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

remove redundant lineTo commands by rendering data as columns #15

Closed
1wheel opened this issue Oct 10, 2019 · 5 comments
Closed

remove redundant lineTo commands by rendering data as columns #15

1wheel opened this issue Oct 10, 2019 · 5 comments
Labels
feature New feature or request perf

Comments

@1wheel
Copy link

1wheel commented Oct 10, 2019

example: https://bl.ocks.org/1wheel/0e7f71ac0325b9be92c9dd19fe4bcc44

@leeoniya
Copy link
Owner

leeoniya commented Oct 10, 2019

hey @1wheel , this looks pretty sweet. i actually saw this technique buried in some chart lib's docs with a bunch of caveats just a few days ago. i'm trying to find where i saw it and it's driving me nuts!

something similar here: https://www.amcharts.com/docs/v4/concepts/performance/#Simplifying_paths

thanks for the demo! unless you want to take a stab at this impl in uPlot, i'm going to focus on some of the other todos first and revisit this later. you know what they say about premature optimization 🤣

EDIT: ah, here we go [1], but it's not really talking about line charts, just column and bar chart rects all being 1px wide. tangentially related.

[1] https://www.highcharts.com/docs/advanced-chart-features/boost-module

@leeoniya leeoniya added feature New feature or request perf labels Oct 10, 2019
@leeoniya
Copy link
Owner

leeoniya commented Oct 12, 2019

@1wheel i did a basic test of this strategy in another branch [1] and it does indeed shave about 50% off the render time. toggling a series on/off (which is a full rescale/redraw) went from ~23ms -> ~12ms. i'm using fillRect() calls at the pixel boundaries rather than lineTo() at +0.5 offsets. fillRect handles 1x1 sizes more naturally, too.

i don't have any gap-filling logic there yet, so at higher zoom levels it's just dots.

[1] https://github.com/leeoniya/uPlot/tree/reduce-lineTo

@leeoniya
Copy link
Owner

leeoniya commented Oct 12, 2019

a simplified variant of this has been implemented. thanks for the suggestion & PoC! 💯

interestingly, this optimization does not save the same 12ms off the cold start render time - that only drops by ~2ms (out of ~45ms).

leeoniya added a commit that referenced this issue Oct 13, 2019
@leeoniya
Copy link
Owner

leeoniya commented Oct 21, 2019

btw, i think i nabbed the cold-start issue in 9d71ac9. i'm guessing it was a V8 de-opt due to some stuff ending up as NaNs as a result of declared but undefined vars at the start of the drawing loop.

@1wheel
Copy link
Author

1wheel commented Oct 21, 2019

oh interesting!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request perf
Projects
None yet
Development

No branches or pull requests

2 participants