Ticket #3771: relation-dialog_loops.patch

src/org/openstreetmap/josm/gui/dialogs/relation/MemberTableModel.java

@@ -14 +14 @@
 
 import javax.swing.DefaultListSelectionModel;
 import javax.swing.ListSelectionModel;
-import javax.swing.table.AbstractTableModel;
-
+import javax.swing.table.AbstractTableModel;
+import javax.swing.event.TableModelListener;
+import javax.swing.event.TableModelEvent;
+
 import org.openstreetmap.josm.Main;
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
@@ -24 +26 @@
 import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.gui.layer.OsmDataLayer;
 
-public class MemberTableModel extends AbstractTableModel {
-
-    private ArrayList<RelationMember> members;
+public class MemberTableModel extends AbstractTableModel implements TableModelListener {
+
+    /**
+     * data of the table model: The list of members and the cached WayConnectionType of each member.
+     **/
+    private ArrayList<RelationMember> members;
+    private ArrayList<WayConnectionType> connectionType = null;
+    
     private DefaultListSelectionModel listSelectionModel;
     private CopyOnWriteArrayList<IMemberModelListener> listeners;
     private OsmDataLayer layer;
@@ -37 +44 @@
     public MemberTableModel(OsmDataLayer layer) {
         members = new ArrayList<RelationMember>();
         listeners = new CopyOnWriteArrayList<IMemberModelListener>();
-        this.layer = layer;
+        this.layer = layer;
+        addTableModelListener(this);
     }
 
     public void addMemberModelListener(IMemberModelListener listener) {
@@ -100 +108 @@
         return columnIndex == 0;
     }
 
-    @Override
-    public void setValueAt(Object value, int rowIndex, int columnIndex) {
-        RelationMember member = members.get(rowIndex);
-        RelationMember newMember = new RelationMember(value.toString(), member.getMember());
-        members.remove(rowIndex);
-        members.add(rowIndex, newMember);
-    }
-
     public OsmPrimitive getReferredPrimitive(int idx) {
         return members.get(idx).getMember();
     }
@@ -658 +658 @@
             fireTableDataChanged();
         }
     }
+
+/**
+ * Determines the direction of way k with reference to the way ref_i.
+ * The direction of way ref_i is ref_direction.
+ *
+ * ref_i is usually the predecessor of k.
+ *
+ * direction:
+ * Let the relation be a route of oneway streets, and someone travels them in the given order.
+ * Direction is 1 for if it is legel and -1 if it is illegal to do so for the given way.
+ *
+ * If the two ways are not properly linked the return value is 0.
+ **/
+    private int determineDirection(int ref_i,int ref_direction, int k) {
+        if (ref_i < 0 || k < 0 || ref_i >= members.size() || k >= members.size()) {
+            return 0;
+        }
+        if (ref_direction == 0) {
+            return 0;
+        }
+        
+        RelationMember m_ref = members.get(ref_i);
+        RelationMember m = members.get(k);
+        Way way_ref = null;
+        Way way = null;
 
-    // simple version of code that was removed from GenericReleationEditor
-    // no recursion and no forward/backward support
-    // TODO: add back the number of linked elements
-    // Returns +1 if member i and (i+1) are ways and could be combined without changing
-    // the direction of one of them. If they are linked "head to head" or "tail to tail"
-    // -1 is returned.
-    // In all other cases the result is null.
-    private Integer linked(int i) {
-        // this method is aimed at finding out whether the
-        // relation member is "linked" with the next, i.e. whether
-        // (if both are ways) these ways are connected.
-
-        Integer link = null;
-        RelationMember m1 = members.get(i);
-        RelationMember m2 = members.get((i + 1) % members.size());
-        Way way1 = null;
-        Way way2 = null;
-
-        if (m1.isWay()) {
-            way1 = m1.getWay();
-        }
-        if (m2.isWay()) {
-            way2 = m2.getWay();
-        }
-        if ((way1 != null) && (way2 != null)) {
-            Node way1first = way1.firstNode();
-            Node way1last = way1.lastNode();
-            Node way2first = way2.firstNode();
-            Node way2last = way2.lastNode();
-            if (way1first != null && way2first != null && (way1first == way2first)) {
-                link = -1;
-            } else if (way1first != null && way2last != null && (way1first == way2last)) {
-                link = 1;
-            } else if (way1last != null && way2first != null && (way1last == way2first)) {
-                link = 1;
-            } else if (way1last != null && way2last != null && (way1last == way2last)) {
-                link = -1;
-            }
+        if (m_ref.isWay()) {
+            way_ref = m_ref.getWay();
         }
-
-        return link;
+        if (m.isWay()) {
+            way = m.getWay();
+        }
+        
+        if (way_ref == null || way == null) {
+            return 0;
+        }
+        
+        Node nRef = ref_direction > 0 ? way_ref.lastNode() : way_ref.firstNode();
+        if (nRef == null) {
+            return 0;
+        }
+        
+        if (nRef == way.firstNode()) {
+            return 1;
+        }
+        if (nRef == way.lastNode()) {
+            return -1;
+        }
+        return 0;
     }
 
     private WayConnectionType wayConnection(int i) {
-        RelationMember m = members.get(i);
-        if (! m.isWay())
-            return new WayConnectionType();
-        Way w = m.getWay();
-        if (w == null || w.incomplete)
-            return new WayConnectionType();
-
-        int ip = (i - 1 + members.size()) % members.size();
-        Integer link_p = linked(ip);
-        Integer link_n = linked(i);
-        Integer dir = 1;
-        // FIXME: It is somewhat stupid to loop here, but
-        // there shouldn't be a performance problem in practice.
-        for (int k = i - 1; k >= 0; --k) {
-            Integer link = linked(k);
-            if (link != null) {
-                dir *= link;
-            } else {
-                break;
+        if (connectionType == null) {
+            updateLinks();
+        }
+        return connectionType.get(i);
+    }
+
+    public void tableChanged(TableModelEvent e) {
+        connectionType = null;
+    }
+
+    public void updateLinks() {
+        connectionType = null;
+        ArrayList<WayConnectionType> con = new ArrayList<WayConnectionType>();
+
+        for (int i=0; i<members.size(); ++i) con.add(null);
+
+        int firstGroupIdx=0;
+        boolean resetFirstGoupIdx=false;
+
+        for (int i=0; i<members.size(); ++i) {
+            if (resetFirstGoupIdx) {
+                firstGroupIdx = i;
+                resetFirstGoupIdx = false;
+            }
+
+            RelationMember m = members.get(i);
+            if (! m.isWay()) {
+                con.set(i, new WayConnectionType());
+                resetFirstGoupIdx = true;
+                continue;
+            }
+
+            Way w = m.getWay();
+            if (w == null || w.incomplete) {
+                con.set(i, new WayConnectionType());
+                resetFirstGoupIdx = true;
+                continue;
+            }
+
+            boolean linkPrev = (i != firstGroupIdx);
+            boolean linkNext;
+            int dir;
+            if (linkPrev) {
+                dir = determineDirection(i-1, con.get(i-1).direction, i);
+                linkNext = (determineDirection(i, dir, i+1) != 0);
+            }
+            else {
+                dir = determineDirection(i, +1, i+1) != 0 ? +1 : 0;
+                if (dir == 0) {
+                    dir = determineDirection(i, -1, i+1) != 0 ? -1 : 0;
+                }
+                linkNext = (dir != 0);
+            }
+
+            con.set(i, new WayConnectionType(linkPrev, linkNext, dir));
+
+            if (! linkNext) {
+                boolean loop;
+                if (i == firstGroupIdx) {
+                    loop = determineDirection(i, 1, i) == 1;
+                } else {
+                    loop = determineDirection(i, dir, firstGroupIdx) == con.get(firstGroupIdx).direction;
+                }
+                if (loop) {
+                    for (int j=firstGroupIdx; j <= i; ++j) {
+                        con.get(j).isLoop = true;
+                    }
+                }
+                resetFirstGoupIdx = true;
             }
         }
-        return new WayConnectionType(link_p != null, link_n != null, dir);
-    }
-}
+        connectionType = con;
+    }
+}

src/org/openstreetmap/josm/gui/dialogs/relation/RelationNodeMap.java

@@ -1 +1 @@
 package org.openstreetmap.josm.gui.dialogs.relation;
 
 import java.util.ArrayList;
+import java.util.TreeSet;
 
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.RelationMember;
@@ -12 +13 @@
  * @author Christiaan Welvaart <cjw@time4t.net>
  *
  */
-public class RelationNodeMap {
-    private java.util.HashMap<Node, java.util.TreeSet<Integer>> points;
+public class RelationNodeMap {
+    /**
+     * For each way endpoint, list all ways that share this node
+     */
+    private java.util.HashMap<Node, TreeSet<Integer>> points;
+    /**
+     * Singleton nodes
+     */
     private java.util.HashMap<Node, Integer> nodes;
-    private java.util.Vector<Integer> remaining;
-    private ArrayList<RelationMember> members;
+    private java.util.Vector<Integer> remaining;
+    /**
+     * read only list
+     */
+    private final ArrayList<RelationMember> members;
 
     RelationNodeMap(ArrayList<RelationMember> members) {
         int i;
 
         this.members = members;
-        points = new java.util.HashMap<Node, java.util.TreeSet<Integer>>();
+        points = new java.util.HashMap<Node, TreeSet<Integer>>();
         nodes = new java.util.HashMap<Node, Integer>();
         remaining = new java.util.Vector<Integer>();
 
@@ -62 +72 @@
             Way w = m.getWay();
             if (w.lastNode() == w.firstNode())
             {
-                nodes.put(w.firstNode(), Integer.valueOf(n));
+                nodes.put(w.firstNode(), n);
             }
             else
             {
                 if (!points.containsKey(w.firstNode())) {
-                    points.put(w.firstNode(), new java.util.TreeSet<Integer>());
+                    points.put(w.firstNode(), new TreeSet<Integer>());
                 }
-                points.get(w.firstNode()).add(Integer.valueOf(n));
+                points.get(w.firstNode()).add(n);
 
                 if (!points.containsKey(w.lastNode())) {
-                    points.put(w.lastNode(), new java.util.TreeSet<Integer>());
+                    points.put(w.lastNode(), new TreeSet<Integer>());
                 }
-                points.get(w.lastNode()).add(Integer.valueOf(n));
+                points.get(w.lastNode()).add(n);
             }
         } else if (m.isNode()) {
             Node node = m.getNode();
-            nodes.put(node, Integer.valueOf(n));
+            nodes.put(node, n);
         } else {
-            remaining.add(Integer.valueOf(n));
+            remaining.add(n);
         }
     }
 
@@ -103 +113 @@
         return result;
     }
 
-    void move(int from, int to) {
-        if (from != to) {
-            RelationMember b = members.get(from);
-            RelationMember a = members.get(to);
-
-            remove(to, b);
-            add(to, a);
-        }
-    }
-
     // no node-mapped entries left
     boolean isEmpty() {
         return points.isEmpty() && nodes.isEmpty();
@@ -131 +131 @@
             result = nodes.get(node);
             nodes.remove(node);
         } else if (!points.isEmpty()) {
-            for (java.util.TreeSet<Integer> set : points.values()) {
+            for (TreeSet<Integer> set : points.values()) {
                 if (!set.isEmpty()) {
                     result = set.first();
                     Way w = members.get(result).getWay();
@@ -144 +144 @@
 
         return result;
     }
-}
+}

src/org/openstreetmap/josm/gui/dialogs/relation/WayConnectionType.java

@@ -5 +5 @@
 
 public class WayConnectionType {
 
-    public final boolean connectedToPrevious;
-    public final boolean connectedToNext;
-    /** Is 1 if way has the same direction as the first way in the set of connected ways. Else it is (-1) */
+    /** True, if the corresponding primitive is not a way or the way is incomplete */
+    private final boolean invalid;
+
+    /** True, if linked to the previous / next member.  */
+    public final boolean linkPrev;
+    public final boolean linkNext;
+
+    /** 
+     * direction is +1 if the first node of this way is connected to the previous way 
+     * and / or the last node of this way is connected to the next way. 
+     * direction is -1 if it is the other way around.
+     * If this way is neither connected to the previous nor to the next way, then
+     * direction has the value 0.
+     */
     public final int direction;
-    /** The WayConnectionType is invalid, if the corresponding primitive is not a way or the way is incomplete */
-    public final boolean invalid;
+
+    /** True, if the element is part of a closed loop of ways. */
+    public boolean isLoop;
 
-    public WayConnectionType(boolean connectedToPrevious, boolean connectedToNext, int direction) {
-        this.connectedToPrevious = connectedToPrevious;
-        this.connectedToNext = connectedToNext;
+    public WayConnectionType(boolean linkPrev, boolean linkNext, int direction) {
+        this.linkPrev = linkPrev;
+        this.linkNext = linkNext;
+        this.isLoop = false;
         this.direction = direction;
         invalid = false;
     }
 
+    /** construct invalid instance */
     public WayConnectionType() {
-        connectedToPrevious = false;
-        connectedToNext = false;
-        direction = 1;
+        this.linkPrev = false;
+        this.linkNext = false;
+        this.isLoop = false;
+        this.direction = 0;
         invalid = true;
+    }
+    
+    public boolean isValid() {
+        return !invalid;    
     }
 
-//    @Override
-//    public String toString() {
-//        return ...
-//    }
+    @Override
+    public String toString() {
+        return "[P "+linkPrev+" ;N "+linkNext+" ;D "+direction+" ;L "+isLoop+"]";
+    }
 
     public String getToolTip() {
-        if (invalid) {
+        if (!isValid()) {
             return "";
         }
-        else if (connectedToPrevious && connectedToNext) {
+        else if (linkPrev && linkNext) {
             return tr("way is connected");
         }
-        else if (connectedToPrevious) {
+        else if (linkPrev) {
             return tr("way is connected to previous relation member");
         }
-        else if (connectedToNext) {
+        else if (linkNext) {
             return tr("way is connected to next relation member");
         }
         else {
             return tr("way is not connected to previous or next relation member");
-        }
+        }//FIXME: isLoop & direction
     }
 }

src/org/openstreetmap/josm/gui/dialogs/relation/MemberTableLinkedCellRenderer.java

@@ -16 +16 @@
 
     final static Image arrowUp = ImageProvider.get("dialogs", "arrowup").getImage();
     final static Image arrowDown = ImageProvider.get("dialogs", "arrowdown").getImage();
+    final static Image corners = ImageProvider.get("dialogs", "roundedcorners").getImage();
     private WayConnectionType value = new WayConnectionType();
 
     @Override
@@ -33 +34 @@
     @Override
     public void paintComponent(Graphics g) {
         super.paintComponent(g);
-        if (value == null || value.invalid) {
+        if (value == null || !value.isValid()) {
             return;
         }
 
-        Image image = null;
+        Image arrow = null;
         switch (value.direction) {
             case 1:
-                image = arrowDown;
+                arrow = arrowDown;
                 break;
             case -1:
-                image = arrowUp;
+                arrow = arrowUp;
                 break;
         }
 
         int ymax=this.getSize().height - 1;
+        int xloop = 8;
         int xoff = this.getSize().width / 2;
+        if (value.isLoop) {
+            xoff -= xloop / 2 - 1;
+        }
         int w = 2;
         int p = 2 + w + 1;
         int y1 = 0;
         int y2 = 0;
 
-        if (image != null && (value.connectedToPrevious || value.connectedToNext)) {
-            g.drawImage(image, xoff-3, ymax / 2 - 2, null);
-        }
 
-        if (value.connectedToPrevious) {
+        if (value.linkPrev) {
             g.setColor(Color.black);
-            g.fillRect(xoff - 2, 0, 5, 2);
+            g.fillRect(xoff - 1, 0, 3, 1);
             y1 = 0;
         } else {
-            g.setColor(Color.red);
-            g.drawRect(xoff-1, p - 1 - w, w, w);
-            y1 = p;
+            if (value.isLoop) {
+                g.setColor(Color.black);
+                y1 = 5;
+                g.drawImage(corners,xoff,y1-3,xoff+3,y1, 0,0,3,3, new Color(0,0,0,0), null);
+                g.drawImage(corners,xoff+xloop-2,y1-3,xoff+xloop+1,y1, 2,0,5,3, new Color(0,0,0,0), null);
+                g.drawLine(xoff+3,y1-3,xoff+xloop-3,y1-3);
+            } 
+            else {
+                g.setColor(Color.red);
+                g.drawRect(xoff-1, p - 1 - w, w, w);
+                y1 = p;
+            }
         }
 
-        if (value.connectedToNext) {
+        if (value.linkNext) {
             g.setColor(Color.black);
-            g.fillRect(xoff - 2, ymax - 1, 5, 2);
+            g.fillRect(xoff - 1, ymax, 3, 1);
             y2 = ymax;
         } else {
-            g.setColor(Color.red);
-            g.drawRect(xoff-1, ymax - p + 1, w, w);
-            y2 = ymax - p;
+            if (value.isLoop) {
+                g.setColor(Color.black);
+                y2 = ymax - 5;
+                g.fillRect(xoff-1, y2+2, 3, 3);
+                g.drawLine(xoff, y2, xoff, y2+2);
+                g.drawImage(corners,xoff+xloop-2,y2+1,xoff+xloop+1,y2+4, 2,2,5,5, new Color(0,0,0,0), null);
+                g.drawLine(xoff+3-1,y2+3,xoff+xloop-3,y2+3);
+            } 
+            else {
+                g.setColor(Color.red);
+                g.drawRect(xoff-1, ymax - p + 1, w, w);
+                y2 = ymax - p;
+            }
+        }
+
+        if ((arrow != null) && (value.linkPrev || value.linkNext)) {
+            g.drawImage(arrow, xoff-3, (y1 + y2) / 2 - 2, null);
         }
+
         g.setColor(Color.black);
         g.drawLine(xoff, y1, xoff, y2);
+        if (value.isLoop) {
+            g.drawLine(xoff+xloop, y1, xoff+xloop, y2);
+        }
+        
     }
 }