Use geo ref to identify copied edges (#2985)

core/src/main/java/com/graphhopper/routing/util/parsers/RestrictionSetter.java

@@ -72,12 +72,7 @@ private void addArtificialEdges(List<Pair<GraphRestriction, RestrictionType>> re
                 int viaEdge = p.first.getViaEdges().get(0);
                 int artificialEdge = artificialEdgesByEdges.getOrDefault(viaEdge, -1);
                 if (artificialEdge < 0) {
-                    EdgeIteratorState viaEdgeState = baseGraph.getEdgeIteratorState(p.first.getViaEdges().get(0), Integer.MIN_VALUE);
-                    EdgeIteratorState artificialEdgeState = baseGraph.edge(viaEdgeState.getBaseNode(), viaEdgeState.getAdjNode())
-                            .setFlags(viaEdgeState.getFlags())
-                            .setWayGeometry(viaEdgeState.fetchWayGeometry(FetchMode.PILLAR_ONLY))
-                            .setDistance(viaEdgeState.getDistance())
-                            .setKeyValues(viaEdgeState.getKeyValues());
+                    EdgeIteratorState artificialEdgeState = baseGraph.copyEdge(viaEdge, true);
                     artificialEdge = artificialEdgeState.getEdge();
                     artificialEdgesByEdges.put(viaEdge, artificialEdge);
                 }

core/src/main/java/com/graphhopper/storage/BaseGraph.java

@@ -28,6 +28,7 @@
 
 import java.io.Closeable;
 import java.util.Map;
+import java.util.function.Consumer;
 
 import static com.graphhopper.util.Helper.nf;
 import static com.graphhopper.util.Parameters.Details.STREET_NAME;
@@ -56,6 +57,7 @@ public class BaseGraph implements Graph, Closeable {
     private final Directory dir;
     private final int segmentSize;
     private boolean initialized = false;
+    private long minGeoRef;
     private long maxGeoRef;
 
     public BaseGraph(Directory dir, boolean withElevation, boolean withTurnCosts, int segmentSize, int bytesForFlags) {
@@ -110,16 +112,22 @@ void checkNotInitialized() {
     private void loadWayGeometryHeader() {
         int geometryVersion = wayGeometry.getHeader(0);
         GHUtility.checkDAVersion(wayGeometry.getName(), Constants.VERSION_GEOMETRY, geometryVersion);
-        maxGeoRef = bitUtil.toLong(
+        minGeoRef = bitUtil.toLong(
                 wayGeometry.getHeader(4),
                 wayGeometry.getHeader(8)
         );
+        maxGeoRef = bitUtil.toLong(
+                wayGeometry.getHeader(12),
+                wayGeometry.getHeader(16)
+        );
     }
 
     private void setWayGeometryHeader() {
         wayGeometry.setHeader(0, Constants.VERSION_GEOMETRY);
-        wayGeometry.setHeader(4, bitUtil.getIntLow(maxGeoRef));
-        wayGeometry.setHeader(8, bitUtil.getIntHigh(maxGeoRef));
+        wayGeometry.setHeader(4, bitUtil.getIntLow(minGeoRef));
+        wayGeometry.setHeader(8, bitUtil.getIntHigh(minGeoRef));
+        wayGeometry.setHeader(12, bitUtil.getIntLow(maxGeoRef));
+        wayGeometry.setHeader(16, bitUtil.getIntHigh(maxGeoRef));
     }
 
     private void setInitialized() {
@@ -172,15 +180,16 @@ public BaseGraph create(long initSize) {
             turnCostStorage.create(initSize);
         }
         setInitialized();
-        // 0 stands for no separate geoRef
+        // 0 stands for no separate geoRef, <0 stands for no separate geoRef but existing edge copies
+        minGeoRef = -1;
         maxGeoRef = 1;
         return this;
     }
 
     public String toDetailsString() {
         return store.toDetailsString() + ", "
                 + "name:(" + edgeKVStorage.getCapacity() / Helper.MB + "MB), "
-                + "geo:" + nf(maxGeoRef) + "(" + wayGeometry.getCapacity() / Helper.MB + "MB)";
+                + "geo:" + nf(maxGeoRef) + "/" + nf(minGeoRef) + "(" + wayGeometry.getCapacity() / Helper.MB + "MB)";
     }
 
     /**
@@ -293,6 +302,58 @@ public EdgeIteratorState edge(int nodeA, int nodeB) {
         return edge;
     }
 
+    /**
+     * Creates a copy of a given edge with the same properties.
+     *
+     * @param reuseGeometry If true the copy uses the same pointer to the geometry,
+     *                      so changing the geometry would alter the geometry for both edges!
+     */
+    public EdgeIteratorState copyEdge(int edge, boolean reuseGeometry) {
+        EdgeIteratorStateImpl edgeState = (EdgeIteratorStateImpl) getEdgeIteratorState(edge, Integer.MIN_VALUE);
+        EdgeIteratorStateImpl newEdge = (EdgeIteratorStateImpl) edge(edgeState.getBaseNode(), edgeState.getAdjNode())
+                .setFlags(edgeState.getFlags())
+                .setDistance(edgeState.getDistance())
+                .setKeyValues(edgeState.getKeyValues());
+        if (reuseGeometry) {
+            // We use the same geo ref for the copied edge. This saves memory because we are not duplicating
+            // the geometry, and it allows to identify the copies of a given edge.
+            long edgePointer = edgeState.edgePointer;
+            long geoRef = store.getGeoRef(edgePointer);
+            if (geoRef == 0) {
+                // No geometry for this edge, but we need to be able to identify the copied edges later, so
+                // we use a dedicated negative value for the geo ref.
+                geoRef = minGeoRef;
+                store.setGeoRef(edgePointer, geoRef);
+                minGeoRef--;
+            }
+            store.setGeoRef(newEdge.edgePointer, geoRef);
+        } else {
+            newEdge.setWayGeometry(edgeState.fetchWayGeometry(FetchMode.PILLAR_ONLY));
+        }
+        return newEdge;
+    }
+
+    /**
+     * Runs the given action on the given edge and all its copies that were created with 'reuseGeometry=true'.
+     */
+    public void forEdgeAndCopiesOfEdge(EdgeExplorer explorer, EdgeIteratorState edge, Consumer<EdgeIteratorState> consumer) {
+        final long geoRef = store.getGeoRef(((EdgeIteratorStateImpl) edge).edgePointer);
+        if (geoRef == 0) {
+            // 0 means there is no geometry (and no copy of this edge), but of course not all edges
+            // without geometry are copies of each other, so we need to return early
+            consumer.accept(edge);
+            return;
+        }
+        EdgeIterator iter = explorer.setBaseNode(edge.getBaseNode());
+        while (iter.next()) {
+            long geoRefBefore = store.getGeoRef(((EdgeIteratorStateImpl) iter).edgePointer);
+            if (geoRefBefore == geoRef)
+                consumer.accept(iter);
+            if (store.getGeoRef(((EdgeIteratorStateImpl) iter).edgePointer) != geoRefBefore)
+                throw new IllegalStateException("The consumer must not change the geo ref");
+        }
+    }
+
     @Override
     public EdgeIteratorState getEdgeIteratorState(int edgeId, int adjNode) {
         EdgeIteratorStateImpl edge = new EdgeIteratorStateImpl(this);
@@ -353,6 +414,10 @@ private void setWayGeometry_(PointList pillarNodes, long edgePointer, boolean re
                         + "D for graph which is " + nodeAccess.getDimension() + "D");
 
             long existingGeoRef = store.getGeoRef(edgePointer);
+            if (existingGeoRef < 0)
+                // users of this method might not be aware that after changing the geo ref it is no
+                // longer possible to find the copies corresponding to an edge, so we deny this
+                throw new IllegalStateException("This edge has already been copied so we can no longer change the geometry, pointer=" + edgePointer);
 
             int len = pillarNodes.size();
             int dim = nodeAccess.getDimension();
@@ -361,9 +426,10 @@ private void setWayGeometry_(PointList pillarNodes, long edgePointer, boolean re
                 if (len <= count) {
                     setWayGeometryAtGeoRef(pillarNodes, edgePointer, reverse, existingGeoRef);
                     return;
+                } else {
+                    throw new IllegalStateException("This edge already has a way geometry so it cannot be changed to a bigger geometry, pointer=" + edgePointer);
                 }
             }
-
             long nextGeoRef = nextGeoRef(4 + len * dim * 4);
             setWayGeometryAtGeoRef(pillarNodes, edgePointer, reverse, nextGeoRef);
         } else {

core/src/main/java/com/graphhopper/util/Constants.java

@@ -64,7 +64,7 @@ public class Constants {
     public static final int VERSION_EM = 4;
     public static final int VERSION_SHORTCUT = 9;
     public static final int VERSION_NODE_CH = 0;
-    public static final int VERSION_GEOMETRY = 6;
+    public static final int VERSION_GEOMETRY = 7;
     public static final int VERSION_TURN_COSTS = 0;
     public static final int VERSION_LOCATION_IDX = 5;
     public static final int VERSION_KV_STORAGE = 2;

core/src/main/java/com/graphhopper/util/EdgeIteratorState.java

@@ -146,8 +146,9 @@ public boolean isStoreTwoDirections() {
 
     /**
      * @param list is a sorted collection of coordinates between the base node and the current adjacent node. Specify
-     *             the list without the adjacent and base node. This method can be called multiple times, but if the
-     *             distance changes, the setDistance method is not called automatically.
+     *             the list without the adjacent and base node. This method can be called multiple times, unless the
+     *             given point list is longer than the first time the method was called. Also keep in
+     *             mind that if the distance changes the setDistance method is not called automatically.
      */
     EdgeIteratorState setWayGeometry(PointList list);
 

core/src/test/java/com/graphhopper/storage/AbstractGraphStorageTester.java

@@ -725,7 +725,7 @@ public void testDontGrowOnUpdate() {
         na.setNode(2, 12, 12, 0.4);
 
         EdgeIteratorState iter2 = graph.edge(0, 1).setDistance(100).set(carAccessEnc, true, true);
-        final BaseGraph baseGraph = (BaseGraph) graph.getBaseGraph();
+        final BaseGraph baseGraph = graph.getBaseGraph();
         assertEquals(1, baseGraph.getMaxGeoRef());
         iter2.setWayGeometry(Helper.createPointList3D(1, 2, 3, 3, 4, 5, 5, 6, 7, 7, 8, 9));
         assertEquals(1 + 4 * (1 + 12), baseGraph.getMaxGeoRef());
@@ -735,11 +735,11 @@ public void testDontGrowOnUpdate() {
         assertEquals(1 + 4 * (1 + 12), baseGraph.getMaxGeoRef());
         iter2.setWayGeometry(Helper.createPointList3D(1, 2, 3));
         assertEquals(1 + 4 * (1 + 12), baseGraph.getMaxGeoRef());
-        iter2.setWayGeometry(Helper.createPointList3D(1.5, 1, 0, 2, 3, 0));
-        assertEquals(1 + 4 * (1 + 12) + 4 * (1 + 6), baseGraph.getMaxGeoRef());
+        assertThrows(IllegalStateException.class, () -> iter2.setWayGeometry(Helper.createPointList3D(1.5, 1, 0, 2, 3, 0)));
+        assertEquals(1 + 4 * (1 + 12), baseGraph.getMaxGeoRef());
         EdgeIteratorState iter1 = graph.edge(0, 2).setDistance(200).set(carAccessEnc, true, true);
         iter1.setWayGeometry(Helper.createPointList3D(3.5, 4.5, 0, 5, 6, 0));
-        assertEquals(1 + 4 * (1 + 12) + 4 * (1 + 6) + 4 * (1 + 6), baseGraph.getMaxGeoRef());
+        assertEquals(1 + 4 * (1 + 12) + 4 * (1 + 6), baseGraph.getMaxGeoRef());
     }
 
     @Test

core/src/test/java/com/graphhopper/storage/BaseGraphTest.java

@@ -17,12 +17,15 @@
  */
 package com.graphhopper.storage;
 
+import com.carrotsearch.hppc.IntArrayList;
 import com.graphhopper.routing.ev.EnumEncodedValue;
 import com.graphhopper.routing.ev.RoadClass;
 import com.graphhopper.search.KVStorage.KValue;
 import com.graphhopper.util.*;
 import com.graphhopper.util.shapes.BBox;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 import java.util.LinkedHashMap;
 import java.util.Map;
@@ -292,6 +295,103 @@ public void setGetFlags() {
         assertEquals(RoadClass.CORRIDOR, edge.get(rcEnc));
     }
 
+    @Test
+    public void copyEdge() {
+        BaseGraph graph = createGHStorage();
+        EnumEncodedValue<RoadClass> rcEnc = encodingManager.getEnumEncodedValue(RoadClass.KEY, RoadClass.class);
+        EdgeIteratorState edge1 = graph.edge(3, 5).set(rcEnc, RoadClass.LIVING_STREET);
+        EdgeIteratorState edge2 = graph.edge(3, 5).set(rcEnc, RoadClass.MOTORWAY);
+        EdgeIteratorState edge3 = graph.copyEdge(edge1.getEdge(), true);
+        EdgeIteratorState edge4 = graph.copyEdge(edge1.getEdge(), false);
+        assertEquals(RoadClass.LIVING_STREET, edge1.get(rcEnc));
+        assertEquals(RoadClass.MOTORWAY, edge2.get(rcEnc));
+        assertEquals(edge1.get(rcEnc), edge3.get(rcEnc));
+        assertEquals(edge1.get(rcEnc), edge4.get(rcEnc));
+        graph.forEdgeAndCopiesOfEdge(graph.createEdgeExplorer(), edge1, e -> e.set(rcEnc, RoadClass.FOOTWAY));
+        assertEquals(RoadClass.FOOTWAY, edge1.get(rcEnc));
+        assertEquals(RoadClass.FOOTWAY, edge3.get(rcEnc));
+        // edge4 was not changed because it was copied with reuseGeometry=false
+        assertEquals(RoadClass.LIVING_STREET, edge4.get(rcEnc));
+    }
+
+    @ParameterizedTest
+    @ValueSource(booleans = {true, false})
+    public void copyEdge_multiple(boolean withGeometries) {
+        BaseGraph graph = createGHStorage();
+        EnumEncodedValue<RoadClass> rcEnc = encodingManager.getEnumEncodedValue(RoadClass.KEY, RoadClass.class);
+        EdgeIteratorState edge1 = graph.edge(1, 2).set(rcEnc, RoadClass.FOOTWAY);
+        EdgeIteratorState edge2 = graph.edge(1, 3).set(rcEnc, RoadClass.MOTORWAY);
+        EdgeIteratorState edge3 = graph.edge(1, 4).set(rcEnc, RoadClass.CYCLEWAY);
+        if (withGeometries) {
+            edge1.setWayGeometry(Helper.createPointList(1.5, 1, 0, 2, 3, 0));
+            edge2.setWayGeometry(Helper.createPointList(1.5, 1, 1, 2, 3, 5));
+            edge3.setWayGeometry(Helper.createPointList(1.5, 1, 2, 2, 3, 6));
+        }
+        EdgeIteratorState edge4 = graph.copyEdge(edge1.getEdge(), true);
+        EdgeIteratorState edge5 = graph.copyEdge(edge3.getEdge(), true);
+        EdgeIteratorState edge6 = graph.copyEdge(edge3.getEdge(), true);
+        EdgeExplorer explorer = graph.createEdgeExplorer();
+        graph.forEdgeAndCopiesOfEdge(explorer, edge1, e -> e.set(rcEnc, RoadClass.PATH));
+        assertEquals(RoadClass.PATH, edge1.get(rcEnc));
+        assertEquals(RoadClass.CYCLEWAY, edge3.get(rcEnc));
+        assertEquals(RoadClass.PATH, edge4.get(rcEnc));
+        assertEquals(RoadClass.CYCLEWAY, edge5.get(rcEnc));
+        assertEquals(RoadClass.CYCLEWAY, edge6.get(rcEnc));
+        graph.forEdgeAndCopiesOfEdge(explorer, edge6, e -> e.set(rcEnc, RoadClass.OTHER));
+        assertEquals(RoadClass.PATH, edge1.get(rcEnc));
+        assertEquals(RoadClass.OTHER, edge3.get(rcEnc));
+        assertEquals(RoadClass.PATH, edge4.get(rcEnc));
+        assertEquals(RoadClass.OTHER, edge5.get(rcEnc));
+        assertEquals(RoadClass.OTHER, edge6.get(rcEnc));
+    }
+
+    @ParameterizedTest
+    @ValueSource(booleans = {true, false})
+    public void forEdgeAndCopiesOfEdge_noCopyNoGeo(boolean withGeometries) {
+        BaseGraph graph = createGHStorage();
+        EdgeIteratorState edge1 = graph.edge(0, 1);
+        EdgeIteratorState edge2 = graph.edge(1, 2);
+        if (withGeometries) {
+            edge1.setWayGeometry(Helper.createPointList(1.5, 1, 0, 2, 3, 0));
+            edge2.setWayGeometry(Helper.createPointList(3, 0, 4, 5, 4.5, 5.5));
+        }
+        IntArrayList edges = new IntArrayList();
+        graph.forEdgeAndCopiesOfEdge(graph.createEdgeExplorer(), edge2, e -> {
+            edges.add(e.getEdge());
+        });
+        assertEquals(IntArrayList.from(edge2.getEdge()), edges);
+
+        edges.clear();
+        EdgeIteratorState edge3 = graph.copyEdge(edge2.getEdge(), true);
+        graph.forEdgeAndCopiesOfEdge(graph.createEdgeExplorer(), edge2, e -> {
+            edges.add(e.getEdge());
+        });
+        assertEquals(IntArrayList.from(edge3.getEdge(), edge2.getEdge()), edges);
+    }
+
+    @Test
+    public void copyEdge_changeGeometry() {
+        BaseGraph graph = createGHStorage();
+        EnumEncodedValue<RoadClass> rcEnc = encodingManager.getEnumEncodedValue(RoadClass.KEY, RoadClass.class);
+        EdgeIteratorState edge1 = graph.edge(1, 2).set(rcEnc, RoadClass.FOOTWAY);
+        EdgeIteratorState edge2 = graph.edge(1, 3).set(rcEnc, RoadClass.FOOTWAY).setWayGeometry(Helper.createPointList(0, 1, 2, 3));
+        EdgeIteratorState edge3 = graph.edge(1, 4).set(rcEnc, RoadClass.FOOTWAY).setWayGeometry(Helper.createPointList(4, 5, 6, 7));
+        EdgeIteratorState edge4 = graph.copyEdge(edge1.getEdge(), true);
+        EdgeIteratorState edge5 = graph.copyEdge(edge3.getEdge(), true);
+
+        // after copying an edge we can no longer change the geometry
+        assertThrows(IllegalStateException.class, () -> graph.getEdgeIteratorState(edge1.getEdge(), Integer.MIN_VALUE).setWayGeometry(Helper.createPointList(1.5, 1, 5, 4)));
+        // after setting the geometry once we can change it again
+        graph.getEdgeIteratorState(edge2.getEdge(), Integer.MIN_VALUE).setWayGeometry(Helper.createPointList(2, 3, 4, 5));
+        // ... but not if it is longer than before
+        IllegalStateException e = assertThrows(IllegalStateException.class, () -> graph.getEdgeIteratorState(edge2.getEdge(), Integer.MIN_VALUE).setWayGeometry(Helper.createPointList(2, 3, 4, 5, 6, 7)));
+        assertTrue(e.getMessage().contains("This edge already has a way geometry so it cannot be changed to a bigger geometry"), e.getMessage());
+        // it's the same for edges with geometry that were copied:
+        graph.getEdgeIteratorState(edge3.getEdge(), Integer.MIN_VALUE).setWayGeometry(Helper.createPointList(6, 7, 8, 9));
+        e = assertThrows(IllegalStateException.class, () -> graph.getEdgeIteratorState(edge3.getEdge(), Integer.MIN_VALUE).setWayGeometry(Helper.createPointList(0, 1, 6, 7, 8, 9)));
+        assertTrue(e.getMessage().contains("This edge already has a way geometry so it cannot be changed to a bigger geometry"), e.getMessage());
+    }
+
     @Test
     public void testGeoRef() {
         BaseGraph graph = createGHStorage();