julie-ng / newtonjs-graph

Cloud Architecture Graphs for Humans
https://julie-ng.github.io/newtonjs-graph/
MIT License
95 stars 20 forks source link

Bug: Graph re-renders on node status update #12

Open julie-ng opened 4 years ago

julie-ng commented 4 years ago

Remove webcola engine because it needs to re-render layout entirely on every change, even just status color. See example below.

This leaves default d3 layout engine - for now.

newton-webcola-re-render

bsvetchine commented 4 years ago

Hello @julie-ng !

When the graph is updated, the links of the graph are re-rendered, and it triggers the Graph._bindLayout method. This method, only if the webcola engine is selected re-adjust the forces. And this new call to this.force.start() for the links of the graph cause the layer to be re-rendered entirely.

I do not think we should specify a InitialUnconstrainedIteration here because webcola will go through these iterations during rendering. However, even when specifying 0, it still re-render the layout entirely, which is super strange. I do not understand why.

Would it make sense, during a graph update, to check if the number of nodes changed, and if not, to avoid the re-rendering of the links by commenting the 2 lines here ?

If that makes sense, I can prepare a PR.

bsvetchine commented 4 years ago

By removing the 2 lines here: https://github.com/julie-ng/newtonjs-graph/blob/master/newton/graph/graph.js#L142 https://github.com/julie-ng/newtonjs-graph/blob/master/newton/graph/graph.js#L143

Everything looks fine (graph init and update, even updates with nodes addition and deletion).

julie-ng commented 4 years ago

Hmm interesting find! You're right, theoretically if we are just updating node states, we do not need to redraw links. I think your suggestion is great as a first step, removing those 2 lines makes sense.

In the feature, I need to distinguish between updating nodes and links. I'm on vacation now, but maybe next week I will get to this :)

bsvetchine commented 4 years ago

Otherwise, but I think you are already aware of this, I believe the problem is located here https://github.com/julie-ng/newtonjs-graph/blob/master/newton/graph/views/links.js#L58. The call to the d3.js selection data method is supposed to identify the new links to create and the old links to remove. However, whatever data that comes in, even if the data is exactly the same than previously, it identifies all the links of the current graph to be deleted and all the new links to be created. However I still have no clue why.

bertrandsvetchine commented 4 years ago

Hello @julie-ng !

I noticed that the data samples provided by the repository newtonjs-demo-data-editor contain a few discrepencies. By fixing them, I cannot reproduce this issue ; i.e. the graph is no more re-rendered entirely. I think that the root cause of this issue might just be malformed incoming data.