Speed up path simplification with path details/instructions - fixed #1802
Conversation
…ere are also path details)
# Conflicts: # reader-osm/src/test/java/com/graphhopper/GraphHopperIT.java
# Conflicts: # api/src/main/java/com/graphhopper/util/ShallowImmutablePointList.java # reader-osm/src/test/java/com/graphhopper/GraphHopperIT.java
| @Override | ||
| public void setInterval(int index, int start, int end) { | ||
| Instruction instruction = instructions.get(index); | ||
| if (instruction instanceof ViaInstruction || instruction instanceof FinishInstruction) { | ||
| if (start != end) { | ||
| throw new IllegalStateException("via- and finish-instructions are expected to have zero length"); | ||
| } | ||
| // have to make sure that via instructions and finish instructions contain a single point | ||
| // even though their 'instruction length' is zero. | ||
| end++; | ||
| } | ||
| instruction.setPoints(pointList.shallowCopy(start, end, false)); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
This is the fix compared to #1782. For via and finish instructions I increase the interval length, such that they contain a single point. Pretty ugly...
There was a problem hiding this comment.
But isn't this similar to how point count and edge count behave? E.g. a path with 1 edge contains 2 points and so 0 edge has 1 point? Hmmh, but yes. A JSON line of 1 point is not allowed.
There was a problem hiding this comment.
But isn't this similar to how point count and edge count behave? E.g. a path with 1 edge contains 2 points and so 0 edge has 1 point? Hmmh, but yes. A JSON line of 1 point is not allowed.
Ah you mean instruction length = number of edges of the interval ? Yes this works, but like you say for a single point its a bit unclear.
There was a problem hiding this comment.
Yes. It is similar, so if you have a Via- or FinishInstruction it should still have a location (one point) but no extent or distance.
| assertEquals(expectedLength, instruction.getLength()); | ||
| assertEquals(expectedPoints, instruction.getPoints().size()); |
There was a problem hiding this comment.
Compared to #1782 here I am now testing the instruction length and instructions' point list size separately, because they are different for via and finish instructions.
|
I think this can be merged, but maybe its better to wait for graphhopper/map-matching#160 and release before merging this. |
|
I think I'll just release 1.0-pre7.1 for the map matching bug fix. IMO no need to delay this here. Or do we need something from core? |
|
No we can wait a bit until tagging 1.0-pre8 |
This is a revised version of the previously reverted #1782 and should fix #1764.
When trying to update the navigation sdk it turned out the point lists for via- and finish instructions were empty after #1782, which makes problems for both navigation and map matching. For via- and finish-instructions the pointlists must contain a single point even though their
instruction.getLength()is zero. I have now fixed this and navigation and map-matching tests are green with this change.