benjeffery / react-plotlyjs

ReactJS / PlotlyJS integration. Draw plotly.js graphs in your react app.
MIT License
111 stars 29 forks source link

Stop performing a full redraw on every update #6

Open otisg opened 8 years ago

otisg commented 8 years ago

I saw "Performs a full redraw on every update, I intend to make this more performant soon." listed as a problem in the README, so I thought I'd add it here to make it easier to track.

Not sure if you're aware of https://github.com/plotly/plotly.js/issues/204, but it looks like you project is the primary hope React+Plotly :)

benjeffery commented 8 years ago

Thanks. I'll get round to this when other priorities are out of the way, but currently that is looking like months. Pull requests welcome!

rreusser commented 8 years ago

FWIW, I've been working for quite a while now on plotly animations (just released!), and avoiding a full redraw was one of the major obstacles since it's obviously impractical to fully redraw the plot every time a point moves a tiny bit.

Long story short, Plotly is designed from the ground up to redraw so although it's not hopeless, there's only so much that can be done without handling the details manually. My solution for animations was to redraw at the tail end of any tweening. This ensures anything that couldn't be nicely tweened is at least just redrawn (so that if everything can be tweened, the redraw is invisible).

The full redraw approach is good for consistency (and not that slow in most cases), except when you're trying to do something like a simulation where the data changes quickly and a full redraw slows things dramatically and unnecessarily. I'm working on the documentation for animations. Here is an example that explicitly passes {redraw: false} to the animation command so that the data is simply tweened (a.k.a. eased; it's d3) without a redraw.

At this point, I'm making a short story long. So basically, here's a thought on how this might be accomplished: Perhaps it could make optional use of the Plotly.animate feature (which would be nice anyway! Smooth updates when changes occur (currently just for scatter traces)!) and allow passing redraw: true/false in order to optimize the case where a full redraw is unnecessary. I'm not 100% sure whether you'd want to perform a diff and pass the result to Plotly.update or whether it would perform just a well to pass the whole state and let Plotly.update sort it out.

All of this requires more manual massaging than I'd like, but to my knowledge, it's maybe the best that can be done.

Sorry that stops short of a PR, but it seems relevant and it's the best I have for now!

PaulMest commented 7 years ago

If anybody is looking to work on this issue, here are some high-level guidelines from Plotly on how to work with React. http://academy.plot.ly/react/3-with-plotly/

benjeffery commented 7 years ago

@PaulMest Unless I'm missing something in that tutorial it advises calling newPlot on every state change, which is the current behaviour of this library. Ideally we would do something like @rreusser suggests, where we detect if the state change can be easily tweened, and only resort to newPlot when it is not.

PaulMest commented 7 years ago

Oh, I see.

When I looked in the repo, I saw shouldComponentUpdate show a TODO to look for changes to the props... but those are actually implemented in componentDidUpdate so I had assumed even some basic logic to prevent redraws wasn't implemented... but it just appears to be in a different place.

Is there any reason why if (prevProps.data !== this.props.data || prevProps.layout !== this.props.layout) { is in componentDidUpdate and not in shouldComponentUpdate?

benjeffery commented 7 years ago

I put the check in componentDidUpdate as you still need a render without a newPlot if ...other props (for style etc.) have changed. A check could be added to shouldComponentUpdate but it will have very little performance benefit as the component only renders a single div.

benjeffery commented 7 years ago

@rreusser Sadly I'm getting a 403 for your URL http://plotly.rickyreusser.com/animation.html But I've found https://github.com/rreusser/animation-experiments so I'll check those out.

rreusser commented 7 years ago

@benjeffery Is there anything I can help with? To be honest, I've tried to be a bit more careful about what I put out into the open web just a bit since it's better if there's not a lot of unofficial documentation and misleading development examples floating around. I'm glad to help solve issues or bette understand the pain points to help improve the documentation though!

jackparmer commented 7 years ago

@benjeffery there's also this documentation: https://plot.ly/javascript/animations/

TinyTheBrontosaurus commented 6 years ago

Question: is this as simple as having componentDidUpdate call relayout or update instead of newPlot? Maybe i'm oversimplifying this issue. The specific problem i'm having is that if i'm zoomed in, then add a trace, the trace is added but the zoom is lost. If i'm not missing anything, then I'm probably going to make this change.

TinyTheBrontosaurus commented 6 years ago

Bah. Nevermind that comment was based upon a faulty test. My update() call was doing nothing. Once I started implementing it got more complicated (as you're aware) :-/

sharq1 commented 6 years ago

I couldn't get update to work as expected so I went with animate - which unfortunately works only for scatter type charts (see https://github.com/plotly/plotly.js/issues/1019 ). But it's better than nothing :) Here's my implementation:


  componentDidUpdate(prevProps) {
    const { data, layout, config } = this.props;

    if (!_.isEqual(prevProps.layout, layout) || !_.isEqual(prevProps.config, config) || data.some((d) => d.type !== 'scatter')) {
      plotlyInstance.newPlot(this.container, _.cloneDeep(data), _.cloneDeep(layout), config);
      this.attachListeners();
    } else if (!_.isEqual(prevProps.data, data)) {
      plotlyInstance.animate(this.container, { data: _.cloneDeep(data) }, { transition: { duration: 500, easing: 'cubic-in-out' } });
    }
  }

By the way notice that I'm performing cloneDeep on data array - not sure why it wasn't implemented this way, as plotly mutates data as well (it adds uid to each array element) which was messing up with my Redux store.

jackparmer commented 6 years ago

This issue has been solved by using the new Plotly.react method in the official plotly.js react component. Please see https://github.com/plotly/react-plotly.js/issues/2#issuecomment-365058285

AyoCodess commented 2 years ago

is there a solution for this?. As the plot automatically re-draws when any the data object changes. i.e an x-axis data point.

benjeffery commented 2 years ago

is there a solution for this?. As the plot automatically re-draws when any the data object changes. i.e an x-axis data point.

Not without significant work. I'm no longer working in webdev, so won't be getting to this, happy to check and merge any contributions though.