Skip to content
This repository has been archived by the owner on Jul 29, 2019. It is now read-only.

Node drawn on top of circuit when using hierarchical layout. Gives misleading circuit paths. #2311

Closed
ghost opened this issue Nov 14, 2016 · 7 comments

Comments

@ghost
Copy link

ghost commented Nov 14, 2016

When using version 3.12.0, I do not have this problem. However, when using version 4.0.0 or higher, a node will overlap onto a circuit path and give the impression that the circuit is going to that overlapping node. See attached screenshots. This first screenshot is when hierarchical layout is set to false.
newlibhierarchical

This second screenshot is what happens when the hierarchical layout is set to true.
newlibhierarchical_1

As you can see, the second and third nodes have edges pointing to each other. But when in hierarchical mode, the third node is drawn to the left of the second node. Also, the edges pointing between the second and third nodes are overlapping each other, which in this case, means that there are three edges overlapping each other.

The latest version that does not appear to have this problem is 3.12.0. After that, the issue exists.
Here is my setup below:

var nodes = new vis.DataSet([
{id: 1, label: 'First'},
{id: 2, label: 'Second'},
{id: 3, label: 'Third'},
{id: 4, label: 'Fourth'},
{id: 5, label: 'Fifth'}
]);

var edges = new vis.DataSet([
{from: 1, to: 2},
{from: 2, to: 3},
{from: 3, to: 2},
{from: 2, to: 4},
{from: 4, to: 5}
]);

var container = document.getElementById("network-area");
var data = {
	nodes: nodes,
  edges: edges
};

var options = {
autoResize: true,
edges:{
arrows: 'to'
},
interaction:{
dragNodes: false
},
layout:{
improvedLayout: true,
hierarchical:{
enabled: true,
direction: 'LR',
sortMethod: 'directed'
}
}
};

var network = new vis.Network(container, data, options);

Here is a JSFiddle to see it in action - LINK

@mojoaxel
Copy link
Member

mojoaxel commented Dec 6, 2016

@redcameron I see the problem, but how would you expect the graph to look in a hierarchical layout?

@ghost
Copy link
Author

ghost commented Dec 6, 2016

I would expect that the graph would not allow the edges to overlap or for a node to overlap an edge in this small graph because in this situation, a user would only be able to select one edge rather than the other two edges between nodes 'Second' and 'Third'. This would be very inconvenient for the user who would need to select either edge.

This issue gives the false impression that the edge between the 'Second' and 'Third' nodes is a two-way or two-directional connection, which is incorrect. It also gives the false impression that node 'Third' comes before node 'Second', which is also incorrect.

Ideally, I would think that in this situation in a hierarchical layout, the 'Third' node should be displayed above or below the 'Second' node with both edges from 'Second' to 'Third' and 'Third' to 'Second' visible for the user to view and select.

@wimrijnders
Copy link
Contributor

wimrijnders commented Apr 30, 2017

Confirmed. There are two issues here:

For the second issue, I have tracked the problem to closure levelByDirection in LayoutEngine._determineLevelsDirected(). This code has trouble dealing with bidirectional edges.
By forcing the levels assignment to increment only, the layout for the given network can be fixed:

download

However, I'm not convinced yet that this solution is general enough for all hierarchical networks.. I need to test more before submitting a PR.
Update: This solution is inadequate. While it works great for the example in the issue, it fails for other networks, for example network/basicUsage. Back to the drawing board for me.

@wimrijnders wimrijnders marked this as a duplicate of #1957 Jul 22, 2017
wimrijnders added a commit to wimrijnders/vis that referenced this issue Jul 26, 2017
…sDirected()

Fix for almende#2311.

Nodes with bidirectional edges got their levels shifted due to the handling of both edge directions.
This fix adds a check on bidirectionality and blocks any subsequent level adjustment.
Pure tree layouts are unaffected by this change.
yotamberk pushed a commit that referenced this issue Jul 26, 2017
…sDirected() (#3294)

* Levels of direct hierarchical network only incremented

* Cleaned up old code

* Quick fix on presence  globaOptions in mergeOptions()

* Revert fix, doesn't work

* Network: Block recalculation of level in LayoutEngine._determineLevelsDirected()

Fix for #2311.

Nodes with bidirectional edges got their levels shifted due to the handling of both edge directions.
This fix adds a check on bidirectionality and blocks any subsequent level adjustment.
Pure tree layouts are unaffected by this change.
@theredcameron
Copy link

theredcameron commented Sep 19, 2017

OP here. Thank you so much for the fix!!
Any idea what version this fix will be included in and when that version will be released?
@yotamberk @wimrijnders

@theredcameron
Copy link

theredcameron commented Oct 5, 2017

I've applied the changes to a test instance and I can confirm that the real issue has not been fixed according to my original description. I will attempt to resolve this issue. Any assistance is appreciated.

@wimrijnders
Copy link
Contributor

@theredcameron Thanks in advance for your effort! Please regard me as standing by for assistance.

@theredcameron
Copy link

Sorry about the wait. I'll continue work on this soon.

primozs pushed a commit to primozs/vis that referenced this issue Jan 3, 2019
…sDirected() (almende#3294)

* Levels of direct hierarchical network only incremented

* Cleaned up old code

* Quick fix on presence  globaOptions in mergeOptions()

* Revert fix, doesn't work

* Network: Block recalculation of level in LayoutEngine._determineLevelsDirected()

Fix for almende#2311.

Nodes with bidirectional edges got their levels shifted due to the handling of both edge directions.
This fix adds a check on bidirectionality and blocks any subsequent level adjustment.
Pure tree layouts are unaffected by this change.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants