V3 indexation #1378
V3 indexation #1378
Conversation
See issue #1290 and its comments. If there is no draggedEvents, we don't need to call the refresh. Tested with it `hideEdgesOnMove`, and the behavior is good. Moreover it avoid a render on a simple click.
- reducers are only call when the node/Edge data changed (not at every hard render) - listeners on graphology event do the cache data indexation
@Yomguithereal We would like your feedback with @sim51 on those suggestions, especially on everything related to public APIs. |
Function refresh can updates all dataCache, update the programs data and call a render if a partial graph is provided. It bypass all actions related to nodes coordinates (like quadtree, labelGrid) if layoutUnchanged is at true.. Moreover, you can also specify if it should do a render or schedule it.
sim51
force-pushed
the
v3-indexation
branch
from
60ccc42 to
e9a35fe
Compare
| // Compute the partial that we need to re-render | ||
| // to optimized the refresh | ||
| const partialGraph = { nodes: [], edges: [] }; | ||
| partialGraph.nodes = graph.nodes().filter((n) => n !== state.hoveredNode && !state.hoveredNeighbors.has(n)); |
There was a problem hiding this comment.
You should use graph.filterNodes and graph.filterEdges for better performance.
There was a problem hiding this comment.
The next part uses a O(n) lookup in the nodes array but I guess the likelihood of it being the problem at hover level is quite low since the number of relevant nodes will be low? But why not asking the question to the graph itself? Like performing the union of edges incident to the hovered nodes or something of the kind?
| "@babel/core": "^7.16.5", | ||
| "@types/chai": "^4.3.0", | ||
| "@types/mocha": "^9.0.0", | ||
| "@babel/core": "^7.22.17", |
There was a problem hiding this comment.
Should we merge the package updates separately?
| array.length += l2; | ||
|
|
||
| let i = 0; | ||
| for (const value of values) { |
There was a problem hiding this comment.
I have not benchmarked this kind of stuff lately but usually using Set.forEach is faster than for of.
| const updatedFields = e.hints?.attributes; | ||
| // we process all nodes | ||
| const nodes = this.graph.nodes(); | ||
| nodes.forEach((node) => this.updateNode(node)); |
There was a problem hiding this comment.
graph.forEachNode will be faster.
| this.activeListeners.graphUpdate = () => { | ||
| this.scheduleRefresh(); | ||
| this.activeListeners.eachNodeAttributesUpdatedGraphUpdate = (e: { hints?: { attributes?: string[] } }) => { | ||
| const updatedFields = e.hints?.attributes; |
There was a problem hiding this comment.
Those hints are completely optional. I guess we need to consider everything might have changed if the hint was not provided to avoid frustration? It might be what the current code does.
|
I did not read everything thoroughly but can this enable us getting rid of the weird internal |
|
(I manually merged this PR) |
See #1372