[519044] Fix bendpoints coordinates for edge creation.

Now, during edge creation, source and target connection bendpoints are
computed according to the intersection of the line relying source and
target anchor and source and target figure bound.
If edge is rectilinear, some intermediate bendpoints are added on the
edge. If the edge has three segments, two intermediate bendpoints are
added else if the edge has two segments, only one intermediate bendpoint
is added.

Bug: 519044 
Change-Id: I3dc52baf0ceea6dca76ed7bb9aa2152b45e4a6eb
Signed-off-by: jmallet <jessy.mallet@obeo.fr>

5 files changed, 119 insertions, 66 deletions

diff --git a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/graphical/edit/policies/SiriusGraphicalNodeEditPolicy.java b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/graphical/edit/policies/SiriusGraphicalNodeEditPolicy.java
index 55b7824578..49796b02d7 100644
--- a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/graphical/edit/policies/SiriusGraphicalNodeEditPolicy.java
+++ b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/graphical/edit/policies/SiriusGraphicalNodeEditPolicy.java
@@ -921,9 +921,9 @@ public class SiriusGraphicalNodeEditPolicy extends TreeGraphicalNodeEditPolicy {
         // Applied the zoom of the current diagram to set the pointList, the
         // source reference point and the target reference point.
         PrecisionPoint absoluteSourceLocationSnap = new PrecisionPoint(absoluteSourceLocationSnapIn100Percent);
-        GraphicalHelper.applyInverseZoomOnPoint(srcEditPart, absoluteSourceLocationSnap);
+        GraphicalHelper.logical2screen(absoluteSourceLocationSnap, srcEditPart);
         PrecisionPoint absoluteTargteLoactionSnap = new PrecisionPoint(absoluteTargetLocationSnapIn100Percent);
-        GraphicalHelper.applyInverseZoomOnPoint(tgtEditPart, absoluteTargteLoactionSnap);
+        GraphicalHelper.logical2screen(absoluteTargteLoactionSnap, tgtEditPart);
 
         edgeLayoutData.setSourceRefPoint(absoluteSourceLocationSnap);
         edgeLayoutData.setTargetRefPoint(absoluteTargteLoactionSnap);
diff --git a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/operation/RemoveBendpointsOperation.java b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/operation/RemoveBendpointsOperation.java
index e9d2ad162e..045bc38560 100644
--- a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/operation/RemoveBendpointsOperation.java
+++ b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/operation/RemoveBendpointsOperation.java
@@ -126,7 +126,9 @@ public class RemoveBendpointsOperation extends AbstractModelChangeOperation<Void
                 Point tgtPoint = tgtConnectionBendpoint.get();
                 PointList pointList = null;
                 if (Routing.RECTILINEAR_LITERAL.equals(routingStyle)) {
-                    pointList = RectilinearEdgeUtil.computeRectilinearCenteredBendpoints(srcAbsoluteBounds, tgtAbsoluteBounds, srcPoint, tgtPoint);
+                    RectilinearEdgeUtil.alignBoundPointTowardAnchor(srcAbsoluteBounds, srcPoint, absoluteSrcAnchorCoordinates);
+                    RectilinearEdgeUtil.alignBoundPointTowardAnchor(tgtAbsoluteBounds, tgtPoint, absoluteTgtAnchorCoordinates);
+                    pointList = RectilinearEdgeUtil.computeRectilinearBendpoints(srcAbsoluteBounds, tgtAbsoluteBounds, srcPoint, tgtPoint);
                 } else {
                     pointList = new PointList();
                     pointList.addPoint(srcPoint);
diff --git a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/diagram/ConnectionsFactory.java b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/diagram/ConnectionsFactory.java
index 639628c99c..f00062422e 100644
--- a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/diagram/ConnectionsFactory.java
+++ b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/diagram/ConnectionsFactory.java
@@ -22,6 +22,7 @@ import org.eclipse.draw2d.geometry.PointList;
 import org.eclipse.draw2d.geometry.PrecisionPoint;
 import org.eclipse.draw2d.geometry.Rectangle;
 import org.eclipse.emf.ecore.EObject;
+import org.eclipse.gef.GraphicalEditPart;
 import org.eclipse.gmf.runtime.diagram.core.preferences.PreferencesHint;
 import org.eclipse.gmf.runtime.diagram.core.providers.IViewProvider;
 import org.eclipse.gmf.runtime.diagram.core.util.ViewUtil;
@@ -66,6 +67,11 @@ import org.eclipse.sirius.ext.gmf.runtime.editparts.GraphicalHelper;
 public class ConnectionsFactory {
 
     /**
+     * Value of null Terminal.
+     */
+    private static final String EMPTY_TERMINAL = ""; //$NON-NLS-1$
+
+    /**
      * The {@link Diagram} corresponding to the DSemanticDiagram to synchronize.
      */
     private Diagram gmfDiagram;
@@ -260,26 +266,25 @@ public class ConnectionsFactory {
                     SlidableAnchor targetAnchor = new SlidableAnchor(target, targetRelativeReference);
                     targetRefPoint = targetAnchor.getLocation(targetAnchor.getReferencePoint());
 
-
                     Option<Point> srcConnectionBendpoint = GraphicalHelper.getIntersection(sourceRefPoint, targetRefPoint, optionalSourceBounds.get(), true);
                     Option<Point> tgtConnectionBendpoint = GraphicalHelper.getIntersection(sourceRefPoint, targetRefPoint, optionaltargetBounds.get(), false);
 
                     if (srcConnectionBendpoint.some() && tgtConnectionBendpoint.some()) {
-                            pointList.addPoint(srcConnectionBendpoint.get());
-                            pointList.addPoint(tgtConnectionBendpoint.get());
-                            EdgeQuery edgeQuery = new EdgeQuery(edge);
-                            if (edgeQuery.isEdgeWithRectilinearRoutingStyle()) {
-                                RectilinearEdgeUtil.centerEdgeEnds(pointList, sourceRefPoint, targetRefPoint, CenteringStyle.BOTH);
-                            }
+                        pointList.addPoint(srcConnectionBendpoint.get());
+                        pointList.addPoint(tgtConnectionBendpoint.get());
+                        EdgeQuery edgeQuery = new EdgeQuery(edge);
+                        if (edgeQuery.isEdgeWithRectilinearRoutingStyle()) {
+                            RectilinearEdgeUtil.centerEdgeEnds(pointList, sourceRefPoint, targetRefPoint, CenteringStyle.BOTH);
+                        }
                     } else {
                         // no intersection found, case when source and target are overlapped
                         pointList.addPoint(sourceRefPoint);
                         pointList.addPoint(targetRefPoint);
                     }
                 }
+                pointList = RectilinearEdgeUtil.normalizeToStraightLineTolerance(pointList, 2);
             }
         }
-        pointList = RectilinearEdgeUtil.normalizeToStraightLineTolerance(pointList, 2);
     }
 
     /**
@@ -298,19 +303,54 @@ public class ConnectionsFactory {
      */
     protected void getAttributesForSourceOrTargetMove(EdgeLayoutData egdeLayoutData, Edge edge, View source, View target) {
         sourceTerminal = egdeLayoutData.getSourceTerminal();
+        if (EMPTY_TERMINAL.equals(sourceTerminal)) {
+            sourceTerminal = GMFNotationUtilities.getTerminalString(0.5d, 0.5d);
+        }
         targetTerminal = egdeLayoutData.getTargetTerminal();
+        if (EMPTY_TERMINAL.equals(targetTerminal)) {
+            targetTerminal = GMFNotationUtilities.getTerminalString(0.5d, 0.5d);
+        }
         // sourceRefPoint, targetRefPoint and pointList of
         // the edgeLayoutData can be null if the edge is
         // hide when the layout data is stored
         if (egdeLayoutData.getSourceRefPoint() != null) {
-            sourceRefPoint = egdeLayoutData.getSourceRefPoint();
+            sourceRefPoint = egdeLayoutData.getSourceRefPoint().getCopy();
         }
         if (egdeLayoutData.getTargetRefPoint() != null) {
-            targetRefPoint = egdeLayoutData.getTargetRefPoint();
+            targetRefPoint = egdeLayoutData.getTargetRefPoint().getCopy();
         }
 
         if (egdeLayoutData.getPointList() != null) {
-            pointList = egdeLayoutData.getPointList();
+            GraphicalEditPart srceEditPart = GMFHelper.getGraphicalEditPart(source).get();
+            GraphicalEditPart tgtEditPart = GMFHelper.getGraphicalEditPart(target).get();
+
+            if (srceEditPart != null && tgtEditPart != null) {
+                GraphicalHelper.screen2logical(sourceRefPoint, srceEditPart);
+                GraphicalHelper.screen2logical(targetRefPoint, tgtEditPart);
+
+                // Compute connection bendpoints
+                Rectangle optionalSourceBounds = GraphicalHelper.getAbsoluteBoundsIn100Percent(srceEditPart);
+                Option<Point> srcConnectionBendpoint = GraphicalHelper.getIntersection(sourceRefPoint, targetRefPoint, optionalSourceBounds, true);
+                Rectangle optionaltargetBounds = GraphicalHelper.getAbsoluteBoundsIn100Percent(tgtEditPart);
+                Option<Point> tgtConnectionBendpoint = GraphicalHelper.getIntersection(sourceRefPoint, targetRefPoint, optionaltargetBounds, false);
+
+                if (srcConnectionBendpoint.some() && tgtConnectionBendpoint.some()) {
+                    EdgeQuery edgeQuery = new EdgeQuery(edge);
+                    Routing routingStyle = edgeQuery.getRoutingStyle();
+                    // Compute anchor logical coordinates
+                    if (Routing.RECTILINEAR_LITERAL.equals(routingStyle)) {
+                        pointList = RectilinearEdgeUtil.computeRectilinearBendpoints(optionalSourceBounds, optionaltargetBounds, srcConnectionBendpoint.get(), tgtConnectionBendpoint.get());
+                    } else {
+                        pointList.addPoint(srcConnectionBendpoint.get());
+                        pointList.addPoint(tgtConnectionBendpoint.get());
+                    }
+                }
+            }
+            if (pointList.size() == 0) {
+                // no intersection found, case when source and target are overlapped
+                pointList.addPoint(sourceRefPoint);
+                pointList.addPoint(targetRefPoint);
+            }
         }
     }
 
diff --git a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/tools/internal/routers/RectilinearEdgeUtil.java b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/tools/internal/routers/RectilinearEdgeUtil.java
index 9edeea21dd..f52301d8dd 100644
--- a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/tools/internal/routers/RectilinearEdgeUtil.java
+++ b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/tools/internal/routers/RectilinearEdgeUtil.java
@@ -15,7 +15,6 @@ package org.eclipse.sirius.diagram.ui.tools.internal.routers;
 import org.eclipse.draw2d.PositionConstants;
 import org.eclipse.draw2d.geometry.Point;
 import org.eclipse.draw2d.geometry.PointList;
-import org.eclipse.draw2d.geometry.PrecisionPoint;
 import org.eclipse.draw2d.geometry.Rectangle;
 import org.eclipse.gmf.runtime.draw2d.ui.geometry.LineSeg;
 import org.eclipse.sirius.diagram.description.CenteringStyle;
@@ -279,56 +278,65 @@ public final class RectilinearEdgeUtil {
      *            target point of the rectilinear edge
      * @return the point list (in absolute coordinates) composing rectilinear edge.
      */
-    public static PointList computeRectilinearCenteredBendpoints(Rectangle srcAbsoluteBounds, Rectangle tgtAbsoluteBounds, Point srcPoint, Point tgtPoint) {
+    public static PointList computeRectilinearBendpoints(Rectangle srcAbsoluteBounds, Rectangle tgtAbsoluteBounds, Point srcPoint, Point tgtPoint) {
         PointList pointList = new PointList();
+        pointList.addPoint(srcPoint);
         // check if source and target side of connection is horizontal
         LineSeg srcSeg = getBoundSideIntersection(srcPoint, srcAbsoluteBounds);
         LineSeg tgtSeg = getBoundSideIntersection(tgtPoint, tgtAbsoluteBounds);
         if (srcSeg != null && tgtSeg != null) {
-            Point middleSrcSegment = getMiddleOfSegment(srcSeg);
-            Point middleTgtSegment = getMiddleOfSegment(tgtSeg);
             if (srcSeg.isHorizontal()) {
-                srcPoint.setX(middleSrcSegment.x);
-                pointList.addPoint(srcPoint);
                 if (srcPoint.x == tgtPoint.x) {
-                    // edge is vertical
-                    tgtPoint.setX(middleTgtSegment.x);
+                    // no intermediate point to add
                 } else if (tgtSeg.isHorizontal()) {
                     // edge is composed of 3 segments (4 points)
-                    tgtPoint.setX(middleTgtSegment.x);
                     int middleY = (srcPoint.y + tgtPoint.y) / 2;
                     pointList.addPoint(new Point(srcPoint.x, middleY));
                     pointList.addPoint(new Point(tgtPoint.x, middleY));
                 } else {
                     // edge is composed of 2 segments (3 points)
-                    tgtPoint.setY(middleTgtSegment.y);
                     pointList.addPoint(new Point(srcPoint.x, tgtPoint.y));
                 }
-                pointList.addPoint(tgtPoint);
             } else {
-                srcPoint.setY(middleSrcSegment.y);
-                pointList.addPoint(srcPoint);
                 if (srcPoint.y == tgtPoint.y) {
-                    // edge is horizontal
-                    tgtPoint.setY(middleTgtSegment.y);
+                    // no intermediate point to add
                 } else if (tgtSeg.isVertical()) {
                     // edge is composed of 3 segments (4 points)
-                    tgtPoint.setY(middleTgtSegment.y);
                     int middleX = (srcPoint.x + tgtPoint.x) / 2;
                     pointList.addPoint(new Point(middleX, srcPoint.y));
                     pointList.addPoint(new Point(middleX, tgtPoint.y));
                 } else {
                     // edge is composed of 2 segments (3 points)
-                    tgtPoint.setX(middleTgtSegment.x);
                     pointList.addPoint(new Point(tgtPoint.x, srcPoint.y));
                 }
-                pointList.addPoint(tgtPoint);
             }
         }
+        pointList.addPoint(tgtPoint);
         return pointList;
     }
 
     /**
+     * Align point of figure's bound toward anchor.
+     * 
+     * @param figureBounds
+     *            bounds of figure which contains point to align
+     * @param point
+     *            the point to align with anchor
+     * @param absoluteAnchorCoordinates
+     *            the anchor coordinates.
+     */
+    public static void alignBoundPointTowardAnchor(Rectangle figureBounds, Point point, Point absoluteAnchorCoordinates) {
+        LineSeg segment = getBoundSideIntersection(point, figureBounds);
+        if (segment != null) {
+            if (segment.isHorizontal()) {
+                point.setX(absoluteAnchorCoordinates.x);
+            } else {
+                point.setY(absoluteAnchorCoordinates.y);
+            }
+        }
+    }
+
+    /**
      * Get the segment of bounds figure which include a given point.
      * 
      * @param boundPoint
@@ -358,23 +366,8 @@ public final class RectilinearEdgeUtil {
     }
 
     /**
-     * Get the middle point of a given segment.
-     * 
-     * @param segment
-     *            segment to determine the middle
-     * @return the middle of the segment.
-     */
-    private static Point getMiddleOfSegment(LineSeg segment) {
-        if (segment != null) {
-            return new PrecisionPoint((segment.getOrigin().x + segment.getTerminus().x) * 0.5d, (segment.getOrigin().y + segment.getTerminus().y) * 0.5d);
-        } else {
-            return null;
-        }
-    }
-
-    /**
-     * Goes through line segments of a polyline and makes strict straight segments from nearly straight segments.
-     * Then remove also unnecessary points.
+     * Goes through line segments of a polyline and makes strict straight segments from nearly straight segments. Then
+     * remove also unnecessary points.
      * 
      * @param line
      *            polyline to straight
@@ -383,6 +376,10 @@ public final class RectilinearEdgeUtil {
      * @return the line made straight with only necessary points
      */
     public static PointList normalizeToStraightLineTolerance(PointList line, int tolerance) {
+        if (line.size() < 3) {
+            // line is too short to be straight
+            return line;
+        }
         // straight edge
         for (int i = 0; i < line.size() - 1; i++) {
             Point pt1 = line.getPoint(i);
@@ -396,17 +393,15 @@ public final class RectilinearEdgeUtil {
         // delete unnecessary points
         PointList newLine = new PointList();
         newLine.addPoint(line.getPoint(0));
-        if (line.size() > 2) {
-            for (int i = 1; i < line.size() - 1; i++) {
-                Point pt1 = line.getPoint(i - 1);
-                Point pt2 = line.getPoint(i);
-                Point pt3 = line.getPoint(i + 1);
-                if ((pt1.x == pt2.x && pt3.x == pt2.x) || (pt1.y == pt2.y && pt3.y == pt2.y)) {
-                    // three point are aligned horizontally or vertically
-                    // point is not necessary
-                } else {
-                    newLine.addPoint(pt2);
-                }
+        for (int i = 1; i < line.size() - 1; i++) {
+            Point pt1 = line.getPoint(i - 1);
+            Point pt2 = line.getPoint(i);
+            Point pt3 = line.getPoint(i + 1);
+            if ((pt1.x == pt2.x && pt3.x == pt2.x) || (pt1.y == pt2.y && pt3.y == pt2.y)) {
+                // three point are aligned horizontally or vertically
+                // point is not necessary
+            } else {
+                newLine.addPoint(pt2);
             }
         }
         newLine.addPoint(line.getPoint(line.size() - 1));
diff --git a/plugins/org.eclipse.sirius.tests.swtbot/src/org/eclipse/sirius/tests/swtbot/EdgeCreationPositionTest.java b/plugins/org.eclipse.sirius.tests.swtbot/src/org/eclipse/sirius/tests/swtbot/EdgeCreationPositionTest.java
index 91df4969e9..f011035097 100644
--- a/plugins/org.eclipse.sirius.tests.swtbot/src/org/eclipse/sirius/tests/swtbot/EdgeCreationPositionTest.java
+++ b/plugins/org.eclipse.sirius.tests.swtbot/src/org/eclipse/sirius/tests/swtbot/EdgeCreationPositionTest.java
@@ -92,9 +92,7 @@ public class EdgeCreationPositionTest extends AbstractSiriusSwtBotGefTestCase {
      *            The name of the diagram to open.
      */
     protected void openDiagram(String name) {
-        editor = (SWTBotSiriusDiagramEditor) openRepresentation(localSession.getOpenedSession(), VIEWPOINT_NAME + " " + name, name, DDiagram.class);
-        editor.setSnapToGrid(false);
-        editor.zoom(ZoomLevel.ZOOM_100);
+        openDiagram(name, ZoomLevel.ZOOM_100);
     }
 
     /**
@@ -117,6 +115,14 @@ public class EdgeCreationPositionTest extends AbstractSiriusSwtBotGefTestCase {
     }
 
     /** */
+    public void test_Node_WithRectilinearEdge() {
+        changeDiagramPreference(SiriusDiagramCorePreferences.PREF_ENABLE_OVERRIDE, true);
+        changeDiagramPreference(SiriusDiagramCorePreferences.PREF_LINE_STYLE, EdgeRouting.MANHATTAN);
+        createEdgeAndValidateAnchors("Node", "A", AbstractDiagramNodeEditPart.class, new PrecisionPoint(0.5, 0.5), "B",
+                AbstractDiagramNodeEditPart.class, TOP_LEFT_CORNER);
+    }
+
+    /** */
     public void test_Node_Aligned() {
         createEdgeAndValidateAnchors("Node", "A", AbstractDiagramNodeEditPart.class, new PrecisionPoint(0.98, 0.2737), "B", AbstractDiagramNodeEditPart.class, new PrecisionPoint(0.02, 0.35));
     }
@@ -138,7 +144,7 @@ public class EdgeCreationPositionTest extends AbstractSiriusSwtBotGefTestCase {
 
     /**
      * Same as test_Container() but with specific location that reveals a bug
-     * for snapToGrid (see bugzilla xxxxxx).
+     * for snapToGrid (see bugzilla 519305).
      */
     public void test_Container_Aligned() {
         createEdgeAndValidateAnchors("Container", "A", AbstractDiagramContainerEditPart.class, new PrecisionPoint(0.98, 0.2737), "B", AbstractDiagramContainerEditPart.class,
@@ -386,12 +392,22 @@ public class EdgeCreationPositionTest extends AbstractSiriusSwtBotGefTestCase {
         // Draw 2D bendpoints
         PointList figurePoints = connectionFigure.getPoints();
 
-        assertEquals("Bad number of bendpoints after edge creation", routingConstraint.size(), figurePoints.size());
-
+        // check draw2D points coordinates
         assertEquals("Wrong x coordinate for source.", sourceIntersection.get().x, figurePoints.getFirstPoint().x, 1);
         assertEquals("Wrong y coordinate for source.", sourceIntersection.get().y, figurePoints.getFirstPoint().y, 1);
         assertEquals("Wrong x coordinate for target.", targetIntersection.get().x, figurePoints.getLastPoint().x, 1);
         assertEquals("Wrong y coordinate for target.", targetIntersection.get().y, figurePoints.getLastPoint().y, 1);
+
+        // check GMF bendpoints coordinates
+        assertEquals("Bad number of bendpoints after edge creation", routingConstraint.size(), figurePoints.size());
+
+        for (int i = 0; i < routingConstraint.size(); i++) {
+            Point gmfPoint = routingConstraint.get(i).getLocation();
+            assertEquals("Wrong x gmf coordinate for point number " + i + ".", gmfPoint.x, figurePoints.getPoint(i).x,
+                    1);
+            assertEquals("Wrong y gmf coordinate for point number " + i + ".", gmfPoint.y, figurePoints.getPoint(i).y,
+                    1);
+        }
     }
 
     /**