[generator] Fixed foot/bicycle tag from Relation. #8034
Conversation
| } | ||
|
|
||
| bool OsmElement::HasTag(std::string const & key) const | ||
| bool OsmElement::HasTag(std::string_view const & key) const |
There was a problem hiding this comment.
| bool OsmElement::HasTag(std::string_view const & key) const | |
| bool OsmElement::HasTag(std::string_view key) const |
There was a problem hiding this comment.
Like we always do m2::PointD const &, I don't see a reason to bother/change here.
Anyway, both variants are ok.
There was a problem hiding this comment.
There is a difference, of course. The reference version reads memory, the value version reads registers, and two fewer assembler lines: https://godbolt.org/z/351fvnhc4
The same for PointD, memory reads are replaced with register operations https://godbolt.org/z/KT9KPjzcq . So if we migrate to pass-by-value, 64 bit devices will definitely win. 32 bit ones at least work at the same speed:
| { | ||
| return base::AnyOf(m_tags, [&](auto const & t) { return t.m_key == key; }); | ||
| } | ||
|
|
||
| bool OsmElement::HasTag(std::string const & key, std::string const & value) const | ||
| bool OsmElement::HasTag(std::string_view const & key, std::string_view const & value) const |
There was a problem hiding this comment.
| bool OsmElement::HasTag(std::string_view const & key, std::string_view const & value) const | |
| bool OsmElement::HasTag(std::string_view key, std::string_view value) const |
There was a problem hiding this comment.
At least it is important for consistency. We don't want to have double standards, right? )
| bool HasTag(std::string_view const & key) const; | ||
| bool HasTag(std::string_view const & key, std::string_view const & value) const; |
There was a problem hiding this comment.
| bool HasTag(std::string_view const & key) const; | |
| bool HasTag(std::string_view const & key, std::string_view const & value) const; | |
| bool HasTag(std::string_view key) const; | |
| bool HasTag(std::string_view key, std::string_view value) const; |
| @@ -23,18 +23,23 @@ bool RelationTagsBase::IsSkipRelation(std::string_view type) | |||
| return type == "multipolygon" || type == "bridge"; | |||
| } | |||
|
|
|||
| bool RelationTagsBase::IsKeyTagExists(std::string const & key) const | |||
| bool RelationTagsBase::IsKeyTagExists(std::string_view const & key) const | |||
There was a problem hiding this comment.
| bool RelationTagsBase::IsKeyTagExists(std::string_view const & key) const | |
| bool RelationTagsBase::IsKeyTagExists(std::string_view key) const |
| @@ -142,9 +147,9 @@ void RelationTagsWay::Process(RelationElement const & e) | |||
| if (isHighway) | |||
| { | |||
| if (route == "bicycle") | |||
| Base::AddCustomTag({"bicycle", "yes"}); | |||
| Base::AddTagIfNotExist("bicycle", "yes"); | |||
There was a problem hiding this comment.
This code will always be called after the correct bicycle tag has been already added, right?
| @@ -39,8 +39,14 @@ class RelationTagsBase | |||
|
|
|||
| protected: | |||
| static bool IsSkipRelation(std::string_view type); | |||
| bool IsKeyTagExists(std::string const & key) const; | |||
| void AddCustomTag(std::pair<std::string, std::string> const & p); | |||
| bool IsKeyTagExists(std::string_view const & key) const; | |||
There was a problem hiding this comment.
| bool IsKeyTagExists(std::string_view const & key) const; | |
| bool IsKeyTagExists(std::string_view key) const; |
| void AddCustomTag(std::pair<std::string, std::string> const & p); | ||
| bool IsKeyTagExists(std::string_view const & key) const; | ||
| void AddCustomTag(std::string_view key, std::string_view value); | ||
| void AddCustomTag(std::pair<std::string, std::string> const & p) |
generator/osm_element.cpp
Outdated
| SKIP_KEY_BY_PREFIX("note"); | ||
| SKIP_KEY_BY_PREFIX("fixme"); | ||
| SKIP_KEY_BY_PREFIX("iemv"); | ||
| std::string_view kUselessKeys[] = { |
| } | ||
|
|
||
| bool OsmElement::HasTag(std::string const & key) const | ||
| bool OsmElement::HasTag(std::string_view const & key) const |
There was a problem hiding this comment.
There is a difference, of course. The reference version reads memory, the value version reads registers, and two fewer assembler lines: https://godbolt.org/z/351fvnhc4
The same for PointD, memory reads are replaced with register operations https://godbolt.org/z/KT9KPjzcq . So if we migrate to pass-by-value, 64 bit devices will definitely win. 32 bit ones at least work at the same speed:
| { | ||
| return base::AnyOf(m_tags, [&](auto const & t) { return t.m_key == key; }); | ||
| } | ||
|
|
||
| bool OsmElement::HasTag(std::string const & key, std::string const & value) const | ||
| bool OsmElement::HasTag(std::string_view const & key, std::string_view const & value) const |
There was a problem hiding this comment.
At least it is important for consistency. We don't want to have double standards, right? )
… yet. Signed-off-by: Viktor Govako <viktor.govako@gmail.com>
Signed-off-by: Viktor Govako <viktor.govako@gmail.com>
Signed-off-by: Viktor Govako <viktor.govako@gmail.com>
#8028