Use geo ref to identify copied edges #2985
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Conflicts: # core/src/main/java/com/graphhopper/storage/BaseGraph.java
# Conflicts: # core/src/test/java/com/graphhopper/storage/BaseGraphTest.java
easbar
commented
May 4, 2024
| // 0 stands for no separate geoRef | ||
| maxGeoRef = 4; | ||
| // 0 stands for no separate geoRef, <0 stands for no separate geoRef but existing edge copies | ||
| maxGeoRef = 1; |
There was a problem hiding this comment.
Not really related to this PR but there did not seem to be a good reason this was set to 4.
# Conflicts: # core/src/main/java/com/graphhopper/storage/BaseGraph.java # core/src/test/java/com/graphhopper/storage/AbstractGraphStorageTester.java
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For e.g. #2914 we need to be able to tell which edges are copies of which. One possible solution is to assign a common identifier to each original edge and the corresponding copied edges, because then we can find the copies by iterating the edges at one of the nodes of an edge.
I added a new method
forEdgeAndCopiedEdges()toBaseGraphthat can be used to modify edges and their corresponding copied edges. We mainly use these copied edges for via-way turn restrictions so far. So basically whenever one needs to modify an edge after the via-way turn restriction handling went through it should be done using this method!A memory efficient way to do this is using the geometry ref since the geometry for copied edges should be identical to the geometry of the original edge anyway. But unfortunately, not every edge has a geometry (and thus no geo ref). This makes it a bit more tricky but it is still doable by using a dedicated (negative) value for the geo ref for each copied edge (that has no geometry).
The other tricky bit is that by setting the geometry of an edge (using
setWayGeometrywhich changes the inner geometry of an edge, i.e. the positions of the pillar nodes) a second time or by setting the geometry after copying an edge the possibility to identify the copied edges gets lost. Therefore, with this PR we now get an error when we try to set the geometry when it was set already or the edge was copied before. The only exception that still works is when the new geometry has a smaller or equal number of pillar nodes than before, because in this case the geo ref stays the same. Otherwise the current edge elevation interpolation wouldn't work for example.