Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Routing result simplification #391

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

farfromrefug
Copy link
Contributor

This PR changes the points for the RoutingResult.
Before we were creating the points based on the manoeuvres. However this was adding unwanted duplicate points(multiple manoeuvres at the same point?).
On an example route (computed with android dev here):

  • points returned from valhalla : 246
  • points in returned RoutingResult: 350

Now with that PR it returns the shape returned from valhalla. And the instruction indexes are still correct.

@mtehver
Copy link
Contributor

mtehver commented Nov 6, 2020

I could be wrong but I think the 'pointIndex' calculation is correct only if there is a single leg in the result. A simple check like below should remove redundant points and work in case of multiple legs also:

                    for (std::size_t j = static_cast<std::size_t>(maneuver.get("begin_shape_index").get<std::int64_t>()); j <= static_cast<std::size_t>(maneuver.get("end_shape_index").get<std::int64_t>()); j++) {
                        const valhalla::midgard::PointLL& point = shape.at(j);
                        MapPos epsg3857Point = epsg3857.fromLatLong(point.second, point.first);
                        if (epsg3857Points.empty() || epsg3857Points.back() != epsg3857Point) {
                            epsg3857Points.push_back(epsg3857Point);
                            points.push_back(proj->fromLatLong(point.second, point.first));
                        }
                    }

@farfromrefug
Copy link
Contributor Author

@mtehver yes you are right i suppoed there aws only one leg! Do you know in which case we have multiple leg so i could try?

@farfromrefug
Copy link
Contributor Author

@mtehver i finally took a look at that and i tired what you suggest and it did not work. Not sure exactly why but i still had all the points.
Now i took a second though about it and the fix was pretty easy. To ensure the point index was good with multiple shapes i simply had to do pointIndex=points.size()+maneuver.get("begin_shape_index").get<std::int64_t>()

…sult_simplification

# Conflicts:
#	all/native/routing/utils/ValhallaRoutingProxy.cpp
…le-sdk

# Conflicts:
#	all/native/routing/utils/ValhallaRoutingProxy.cpp

# Conflicts:
#	all/native/routing/utils/ValhallaRoutingProxy.cpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants