mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
3.82k stars 1.14k forks source link

[charts] smoother path animation #12918

Closed romgrk closed 2 days ago

romgrk commented 3 weeks ago

I was checking the charts and I noticed the path is animated via a clipping mask rather than a path animation. You could use this technique to animate it more smoothly. It uses SVGPath.getPointAtLength(n: number) to create an animated path from a set of linear segments that sample the original path every N pixels.

before after
Screencast from 2024-04-26 08-31-31.webm Screencast from 2024-04-26 08-33-48.webm
mui-bot commented 3 weeks ago

Deploy preview: https://deploy-preview-12918--material-ui-x.netlify.app/

Generated by :no_entry_sign: dangerJS against 77a48773e640cea14f8fab4be5a12d31d825a001

alexfauquette commented 3 weeks ago

The initial reason why using the mask was to share it with the marks, without having to do any computation to know if a mark is at a place for which the line is already visible or not. Trying to avoid this intermediate state where marks are visible but not the line

image

romgrk commented 3 weeks ago

Didn't notice. With the points data, adding the mask back would be quite simple: maskRect.width = points[current].x - points[0].x. I also see in argos that disjointed paths will need some special handling. Lmk what you think of the approach, if you're fine with it I'll do those changes.

alexfauquette commented 4 days ago

Lmk what you think of the approach, if you're fine with it I'll do those changes.

I'm not in favor for two reasons: maintenance and performances

Maintenance

I'm not sure about this PR, because it seems more complex than the dummy clipping and I fear it adds more edge cases to handle. Maybe it's because I don't have a strong design sensibility, but the ratio UI improvement vs the risk of adding something complex to maintain does not feel good.

Perf

It seems to deteriorate performances when rendering the line chart for the first time:

I checked in the docs preview performances when resetting a demo and it shows some blocking time that I don't see in production image

I tried in codesandbox with a state to toggle with/without data and this PR adds a delay to show the data

alexfauquette commented 2 days ago

I spotted a usecase where it would make more sense: plotting a line in a scatter plot

image

In that case, the clip does not work and we can accept to pay the extra computation cost to have the animation

romgrk commented 2 days ago

I have added a commit to compute the points lazily as the animation runs, and the performance is terrible. It's at least 2ms for each point, very ugly, doesn't run smoothly. Not sure why it's so slow. The geometry browser APIs are all terrible.

The best solution to get a smooth animation would be to use a JS math library (like 2d-geometry) but that's too big to include here.

I'll close it, feel free to pick it up for scatter plot if you think it's worth it.

alexfauquette commented 2 days ago

Thanks for the additional investigation :pray: