Disambiguate incoming/outgoing arrows in the model

[GoogleCodeExporter]

Currently, if a node in the model has an incoming and an outgoing edge, then at 
certain node positions the two edges are merged ambiguously. That is, it 
becomes impossible to tell which of the edges is incoming, and which of the 
edges is outgoing.

The proposed solution is to shift incoming and outgoing arrow locations away 
from the node center. For example, incoming edges can be shifted to the left of 
center, and outgoing edges can be shifted to the right of center (so that 
overall flow mimics the flow from INITIAL node on the left to the TERMINAL node 
on the right).

Original issue reported on code.google.com by bestchai on 16 Nov 2011 at 12:59

[GoogleCodeExporter]

This will also help with two headed arrows -- cases when two nodes form a 
cycle. The shift amount might need to correspond to the space necessary for the 
probability labels to fit on the edges without obscuring the adjacent 
(incoming/outgoing) edge.

Original comment by bestchai on 16 Nov 2011 at 1:01

[GoogleCodeExporter]

The natural thing to do here is to split every edge into thirds.  Incoming 
arrows can land 1/3 of the way from the leading edge (leading when going around 
the node clockwise), and outgoing arrows 1/3 from the trailing edge.  This 
allows edge points to be equidistant from the closest node corner and the other 
edge point.  

One problem with this solution is that while it removes (I believe) all 
ambiguity from edge direction, it may not be immediately obvious to a user.  
Imagine a case with 4 incoming arrows.  To make sure all of these are incoming 
(without knowing out convention), the user would have to follow each of the 
arrows backwards and make sure none of them have arrow heads on the other end.  
Good news is that none of them will have arrow heads there and none of them 
will appear to have arrow heads there from other arrows.  

Original comment by yuriy.b...@gmail.com on 16 Nov 2011 at 2:03

[GoogleCodeExporter]

Original comment by kevin.a....@gmail.com on 27 Nov 2011 at 7:50

[GoogleCodeExporter]

The solution discussed in the meeting today is the following:

1. For each node in the model, create one array of (x,y) coordinates for 
"input" points on the node, and another array of (x,y) coordinates for "output" 
points on the node. Input points represent locations where an arrow that 
connects this node to some other node may start. Output points, on the other 
hand, represent the locations where an arrow that is starting at some other 
node and ending at this node may terminate.

An important feature of solution in (1) is that it allows us to experiment with 
different schemes, without radically changing the underlying code. That is, it 
allows us to define a variable number of input/output locations on a node, and 
it disassociates input points from output points.  

2. Re-create the current model layout by building on step (1). First, set the 
input points array to the output points array, and set the output points array 
to the four points currently used for each node (i.e., the four edge 
mid-points).

3. Create a new layout behavior with an array of two input points at opposite 
corners (e.g., top left and bottom right), and an array of two output points at 
the other two corners (e.g., bottom left and top right).

4. Create a new layout behavior with an array of 4 input points and 4 output 
points. Each input point is displaced 1/6 of edge length clockwise from the 
mid-point of each edge. And each output points is displaced 1/6 of edge length 
counter-clockwise from the mid-point of each edge.

5. Compare 3 and 4, and determine which one looks best.

Original comment by bestchai on 29 Nov 2011 at 8:32

• Changed state: Started

[GoogleCodeExporter]

Here's my initial thinking of how to solve this:
1) In ModelTab.java, put these "input" and "output" arrays for every node into 
a map or array.
2) Pass this map or array to createGraph() in ModelGraphic.java
3) Then in createGraph(), when I'm adding each node into the graph, I can 
associate an "input" and "output" array   with that node.
4) I'll have access to these two arrays for dracula_graph.js to do all the 
layout behavior.

Could I have your feedback on this approach?

Original comment by kevin.a....@gmail.com on 4 Dec 2011 at 3:49

[GoogleCodeExporter]

The problem with this approach is that input/output coordinates are variables 
that depend on (1) the node, and (2) the current position of the node. If you 
maintain these coords in separate arrays at the Java/JSNI layer then there 
isn't a sensible default value for them (since the layout algorithm hasn't been 
run yet), and caching these doesn't make much sense as their values will change 
whenever you move nodes around or resize the window.

I think a good solution would have a small footprint on the current code, and 
only modify dracula_graffle.js (if possible). That is, a JS-only solution, that 
only modifies how edges are drawn. (A good feature of this solution is that 
you've already dealt with this code for solving self-loops!).

If you look in dracula_graffle, the else branch of the isSelfLoop condition 
defines an array p, which represents "/* coordinates for potential connection 
coordinates from/to the objects */". Note that this array defines "inputs" and 
"outputs" simultaneously. Therefore, step (1) in the solution above basically 
says that you would like to create a p_inputs and p_outputs -- two versions of 
the p array. While step (2) above says that p_inputs = p_outputs = p, and that 
you will need to update the code below (in the same else block) to work with 
p_inputs and p_outputs. This code is non-trivial, so be careful and make sure 
to understand what it does before making any significant modifications.

I think that this approach is easiest, and the most sensible.

Original comment by bestchai on 4 Dec 2011 at 6:47

[GoogleCodeExporter]

I have implemented the second model layout behavior (the 1/6 edge 
displacements), but had troubles with implementing the first model layout. I 
have a fix to the second layout in revision 2164f4b075bb. Please code review. 

I think it works well and implementing the first layout might be unnecessary. 
Please let me know if you want me continue trying to implement the first layout.

Original comment by kevin.a....@gmail.com on 5 Dec 2011 at 8:07

[GoogleCodeExporter]

I left comments on revision 2164f4b075bb.

The solution looks great. But, I think it is important to try both layouts, for 
two reasons:
1. It will test that choosing between the two solutions does not require 
changes to either of the two if branches (it should not).

2. It will test that the modifications you've made to both branches generalize 
beyond the one layout it works for (this is a key property of the two arrays 
solution).

You can encapsulate the two solution in two functions, and the code can then 
choose a layout by calling the right function.

Original comment by bestchai on 5 Dec 2011 at 10:35

[GoogleCodeExporter]

Addressed this initial code review in revision 18f3ce9e4df8. Will work on 
implementing the other layout now.

Original comment by kevin.a....@gmail.com on 6 Dec 2011 at 6:56

[GoogleCodeExporter]

Solution in revision ab1efeb03acc. The layout with the corners don't look as 
good the edge displacement layout. The "INITIAL" and "TERMINAL" nodes have 
edges that don't directly connect because these nodes are oval and the corners 
are significantly different than the coordinates returned by getBBox().

Original comment by kevin.a....@gmail.com on 21 Dec 2011 at 3:10

[GoogleCodeExporter]

Fixed in revision e01c09ff1edd

Original comment by bestchai on 23 Dec 2011 at 9:14

• Changed state: Fixed