organicmaps/organicmaps-tmp

Archived

Fork 1

[routing] Show in navigation destination tags #2632

Merged

AntonM030481 merged 4 commits from navigation-link_destination into master

2022-05-29 10:19:25 +00:00

AntonM030481 commented

2022-05-27 09:57:48 +00:00

(Migrated from github.com)

1st iteration for #849

Previously only name of next street was shown.

Now

it will show "[ref] name" for next street.
for highway links it will show "[target:ref] target".

As before we go forward through segments to find fist named one (but this is limited to 400m).

Test output can be seen in Debug output of qt::DrawWidget::SubmitFakeLocationPoint().
E.g. at (34.68730, 32.91658) -> (34.69269, 32.92118)

Suggested next step is to replace [ref] and [target:ref] by icons generated by GetRoadShields().

1st iteration for #849 Previously only name of next street was shown. Now - it will show "[ref] name" for next street. - for highway links it will show "[target:ref] target". As before we go forward through segments to find fist named one (but this is limited to 400m). Test output can be seen in Debug output of qt::DrawWidget::SubmitFakeLocationPoint(). E.g. at (34.68730, 32.91658) -> (34.69269, 32.92118) Suggested next step is to replace [ref] and [target:ref] by icons generated by GetRoadShields().

vng (Migrated from github.com) requested changes 2022-05-27 13:12:52 +00:00

routing/directions_engine.cpp

vng (Migrated from github.com) commented

2022-05-27 13:09:37 +00:00

Use var:

if (auto const & rn = ft->GetRoadNumber(); !rn.empty())
  name += rn;

Use var: ``` if (auto const & rn = ft->GetRoadNumber(); !rn.empty()) name += rn; ```

vng (Migrated from github.com) commented

2022-05-27 13:10:38 +00:00

? Leave one statement.

vng (Migrated from github.com) commented

2022-05-27 13:11:48 +00:00

This looks like strange python code :) Use var and block.

vng (Migrated from github.com) commented

2022-05-27 13:12:02 +00:00

... = std::move(name)

vng (Migrated from github.com) reviewed 2022-05-27 13:17:10 +00:00

generator/pygen/pygen.cpp

vng (Migrated from github.com) commented

2022-05-27 13:17:10 +00:00

Hm, Did you get what for this pygen.cpp module?
And why "destination_ref", if we should get "destination:ref"

Hm, Did you get what for this pygen.cpp module? And why "destination_ref", if we should get "destination:ref"

AntonM030481 (Migrated from github.com) reviewed 2022-05-27 13:40:46 +00:00

routing/directions_engine.cpp

AntonM030481 (Migrated from github.com) commented

2022-05-27 13:40:46 +00:00

AntonM030481 (Migrated from github.com) reviewed 2022-05-27 13:41:36 +00:00

routing/directions_engine.cpp

AntonM030481 (Migrated from github.com) commented

2022-05-27 13:41:36 +00:00

OK. Thanks.

AntonM030481 (Migrated from github.com) reviewed 2022-05-27 13:43:18 +00:00

routing/directions_engine.cpp

AntonM030481 (Migrated from github.com) commented

2022-05-27 13:43:18 +00:00

Too much Python yesterday :)

AntonM030481 (Migrated from github.com) reviewed 2022-05-27 14:06:32 +00:00

generator/pygen/pygen.cpp

AntonM030481 (Migrated from github.com) commented

2022-05-27 14:06:32 +00:00

No, I didn't :)
I applied the same logic as for FMD_TURN_LANES_FORWARD, FMD_BUILDING_LEVELS, etc.

BTW, can't we use duplication of matching in indexer::ToString, indexer::Metadata::TypeFromString and in Pygen.
We need 2 direct and 1 reverse transformation.
E.g. create static array and initialize it as a[Metadata::EType] = value;
And in cycle populate map<string, Metadata::EType> and extend it manually where needed for reverse transformation?

No, I didn't :) I applied the same logic as for FMD_TURN_LANES_FORWARD, FMD_BUILDING_LEVELS, etc. BTW, can't we use duplication of matching in indexer::ToString, indexer::Metadata::TypeFromString and in Pygen. We need 2 direct and 1 reverse transformation. E.g. create static array and initialize it as `a[Metadata::EType] = value;` And in cycle populate `map<string, Metadata::EType>` and extend it manually where needed for reverse transformation?

AntonM030481 (Migrated from github.com) reviewed 2022-05-27 14:09:50 +00:00

routing/directions_engine.cpp

AntonM030481 (Migrated from github.com) commented

2022-05-27 14:09:50 +00:00

I decided to get rid of |name|. But next time for sure!

vng (Migrated from github.com) reviewed 2022-05-27 16:38:36 +00:00

generator/pygen/pygen.cpp

vng (Migrated from github.com) commented

2022-05-27 16:38:36 +00:00

Revert changes in this pygen.cpp at all. Seems like it used to create bindings from cpp to python, but we don't use it.

AntonM030481 commented

2022-05-27 17:16:32 +00:00

(Migrated from github.com)

@vng How can I add multiline additional text field to qt window to show in desktop navigation mode the result of
Debug output of qt::DrawWidget::SubmitFakeLocationPoint()?
I hope it will be useful in the future too, e.g. if we will add lane guidance.
In the perfect case it should be the copy of GUI guidance from mobile, but text version will be OK (ascii art!)

@vng How can I add multiline additional text field to qt window to show in desktop navigation mode the result of Debug output of qt::DrawWidget::SubmitFakeLocationPoint()? I hope it will be useful in the future too, e.g. if we will add lane guidance. In the perfect case it should be the copy of GUI guidance from mobile, but text version will be OK (ascii art!)

AntonM030481 (Migrated from github.com) reviewed 2022-05-28 16:45:03 +00:00

generator/pygen/pygen.cpp

AntonM030481 (Migrated from github.com) commented

2022-05-28 16:45:03 +00:00

AntonM030481 commented

2022-05-28 17:01:11 +00:00

(Migrated from github.com)

Now everything is OK.

But it will be better to make algorithm more flexible to handle incomplete data.

Currently it is expected that for link both destination and destination:ref tags will exist together.
But sometimes only destination tag exists for link. And destination:ref can be calculated by checking next segments of route until link will end and the main road will start (and sometimes it will have ref tag, missed in link).
It's better to go trough longer distance (compared to current 400m) looking for existing data,
if we go along the link. Sometimes links are really long (e.g. 1km+ in USA) and they have no tag it's better to go until link will end (no matter how long it is) until main road will start.
Sometimes exit is not tagged as link (e.g. if new road starts here). At the same time they can have all useful tags from link. Usually destination:ref=ref in such cases, or only 1st part of destination:ref can match. E.g. here.

Now everything is OK. But it will be better to make algorithm more flexible to handle incomplete data. - Currently it is expected that for link both destination and destination:ref tags will exist together. But sometimes only destination tag exists for link. And destination:ref can be calculated by checking next segments of route until link will end and the main road will start (and sometimes it will have ref tag, missed in link). - It's better to go trough longer distance (compared to current 400m) looking for existing data, if we go along the link. Sometimes links are really long (e.g. 1km+ in USA) and they have no tag it's better to go until link will end (no matter how long it is) until main road will start. - Sometimes exit is not tagged as link (e.g. if new road starts here). At the same time they can have all useful tags from link. Usually destination:ref=ref in such cases, or only 1st part of destination:ref can match. E.g. [here](https://www.openstreetmap.org/way/8924268).

👍 1

biodranik commented

2022-05-28 17:32:11 +00:00

(Migrated from github.com)

These are very good ideas. Let's implement them!

👍 1

vng (Migrated from github.com) approved these changes 2022-05-29 07:08:45 +00:00

vng commented

2022-05-29 07:11:15 +00:00

(Migrated from github.com)

Still in draft? Can merge it now to get "destinations" in next data snapshot.

AntonM030481 commented

2022-05-29 09:15:15 +00:00

(Migrated from github.com)

Let's merge now.
Additional improvements organicmaps/organicmaps#2632 (comment) are no trivial since currently we don't have info about "link" status of roads at level of Route class and straightforward adding it will be non elegant at all.

Let's merge now. Additional improvements https://git.omaps.dev/organicmaps/organicmaps/pulls/2632#issuecomment-1140297558 are no trivial since currently we don't have info about "link" status of roads at level of Route class and straightforward adding it will be non elegant at all.

This repo is archived. You cannot comment on pull requests.

No reviewers

vng