-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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
skip clipped points with zero opacity #5363
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does the performance vary with the value used to check the ratio? ie, for a large chart what's the difference in rendering time for ratio >= 0.01
vs ratio >= 0.1
?
I haven't tested that, but in most of my observations, the majority of points outside the chart area are assigned a ratio of 0. The reason I've set the ratio cutoff to 0.01 is because the existing ratio calculation uses |
@etimberg I'm (re?)-discovering this Right now, it's a huge performance issue in graph containing many points outside the visible area as reported by this PR. @veggiesaurus I understand why you fixed it like that (backward compatibility?) but I think we should get rid of it completely and only draw points that are inside the chart area. |
@simonbrunel I agree, it is a really weird piece of code. I couldn't understand why clipped points weren't just ignored in the first place. |
@simonbrunel I agree. We should just skip everything that does not need to be drawn |
@etimberg what happens if I draw a line, with one point inside the clipping area, and one point outside. Does the line not draw at all? What is the desired behaviour? |
Nothing changes for the line, the data point is still valid, we simply don't draw the point. |
updated PR to just skip points outside clipping area. I see in |
I don't see any clipping in |
yes, sorry you're right, it's done there. My proposal (which I could look into in a separate PR) would be the following:
Beziers would need some extra thought though. An alternative would be to just keep the context clipping, but still optimise things by not drawing lines that are completely out of the chart area |
I doubt that doing clipping ourselves will be more efficient than simply clip the context since we will always have to consider Bézier lines. It also doesn't work in case you want to draw a gradient line because you will have to interrupt the line drawing when it alternates between the inside and outside of the chart area, breaking the gradient continuity. I'm not sure either that computing the bounding box of lines + intersecting with the chart area, at every animation frame, to prevent drawing it when completely outside, will be a huge speed improvement: most of the time the number of line to draw is pretty low compared to the number of points. So I don't think we should "try" to optimize something that currently doesn't need to be optimized at the risk of introducing more code and probably new bugs. |
I agree with @simonbrunel. Bézier curves are the first things that jumped to mind as to why we implemented canvas clipping instead of our own. |
sorry, accidentally some weird merging that I have now reverted, so PR is updated, but no code changed |
I really don't have any context here but are you sure that "(chartArea === undefined " is correct. is it not meant to be "if(chartArea !== undefined " i probably very wrong just seems off to render when something is not defined. looking at the changes, not that i understand them. |
@Seabizkit if no chart area parameter is passed in, then it's assumed that there is no clipping area, so all points are rendered regardless of their position. This is the same behaviour as the previous implementation |
the main if block was inverted i.e. the opposite.... again i have no context here, just seems off to me. |
Yes, the if block was inverted, and rendering was moved inside the if block, so that only points meeting that requirement are rendered. Previously, points meeting the opposite requirement were still rendered, but were set to zero opacity. So the same logical checks are applied, but now points that would have zero opacity are not rendered at all. I'm not entirely sure what's "off" in this case. |
I need this change for my project, I'm drawing signals in real time and sometimes this signal are saturated (out of range). I have test it pulling the master branch and works fine. I'd like to know when Version 2.8 will be released. Thanks!! |
Hello, is there a way to optimize the charts more, any new solutions? i have tried down sampling my data (down to 100 to 300 points from 10k-100k), it does show improvement but when doing zoom&pan it is still significantly slower |
Are you using 2.7.3 or 2.8.0-rc1? This improvement was merged into |
It's 2.7.3 actually, I thought my version was up to date honestly (up to date with this improvement) |
Any update to optimise rendering on large data scale ? I have a line chart time type with an horizontal scroll. I would like to skip points that are not in the view ( but they are in the same data level ) |
v3 should be much better. also see https://github.com/chartjs/Chart.js/blob/master/docs/general/performance.md shameless plug: https://github.com/leeoniya/uPlot |
Any resources to follow v3 news ? |
All the work on Chart.js is currently working towards v3. If you want to watch the code you can follow the commits on I think the code is probably about 3x faster on |
Currently, there is an issue with rendering performance when setting the axis range to a subrange of the data range (i.e. some data points lie outside of the chart area, and should therefore not be rendered). This is a common situation when zooming in on a graph, and has been referenced in the chartjs-plugin-zoom repo: chartjs/chartjs-plugin-zoom#75
The problem arises from the approach taken when drawing points outside the chart area. Data points outside the chart area are assigned an opacity based on how far they are outside the chart. The opacity is rounded to the nearest percent. This is an expensive process, as, for each data point outside of the chart area, the following must occur:
This PR improves performance in the common situation where the assigned opacity is zero. In this case, it skips the last two steps above.
This is a codepen demonstrating the current behaviour. The CPU usage increases significantly when selecting a higher minimum range (by pressing the buttons below the graph). From my tests, CPU usage increased from 50% (and an average rendering time of 6 ms) to 95% (and an average rendering time of 29 ms) when selecting the "90%" minimum.
Here is another codepen with the issue fixed. In this case CPU usage decreases when fewer points are rendered, which is what one would intuitively expect.