jung
jung copied to clipboard
TreeLayout neither implements reset(), nor calls it when setting a new graph
TreeLayout requires a non-null graph as an argument to its constructor. Thus the setGraph()
method can only be intended for changing the graph post initialization. This clearly entails cleaning out the positions of the nodes from the old graph.
I propose adding the following implementation of reset()
, and then calling reset()
instead of buildTree()
in the setGraph()
method:
public void reset() {
basePositions.clear();
locations.invalidateAll();
alreadyDone.clear();
buildTree();
}
Note: I think that we probably still have some issues with this in the post-visualization-system refactoring world (which, to be fair, is still in a transitional state): while changing the graph for a LayoutModel
does cause the change support to fire off an event, I don't think that the locations are invalidated as a result. The way that LoadingCacheLayoutModel
works, I believe that any nodes that might be in common between the two Graph
objects will retain the same positions, which is not necessarily what the user would want.
Stepping back a moment, we should give some thought to whether we even want setGraph()
at all.