diff --git a/editor/CMakeLists.txt b/editor/CMakeLists.txt index b184f246d8..7d8b0691eb 100644 --- a/editor/CMakeLists.txt +++ b/editor/CMakeLists.txt @@ -12,12 +12,12 @@ set( editor_notes.hpp editor_storage.cpp editor_storage.hpp + feature_matcher.cpp + feature_matcher.hpp opening_hours_ui.cpp opening_hours_ui.hpp osm_auth.cpp osm_auth.hpp - osm_feature_matcher.cpp - osm_feature_matcher.hpp server_api.cpp server_api.hpp ui2oh.cpp diff --git a/editor/changeset_wrapper.cpp b/editor/changeset_wrapper.cpp index 73b6c39a03..1e5cc4cefc 100644 --- a/editor/changeset_wrapper.cpp +++ b/editor/changeset_wrapper.cpp @@ -1,5 +1,5 @@ #include "editor/changeset_wrapper.hpp" -#include "editor/osm_feature_matcher.hpp" +#include "editor/feature_matcher.hpp" #include "indexer/feature.hpp" @@ -157,7 +157,7 @@ XMLFeature ChangesetWrapper::GetMatchingNodeFeatureFromOSM(m2::PointD const & ce // Throws! LoadXmlFromOSM(ll, doc); - pugi::xml_node const bestNode = GetBestOsmNode(doc, ll); + pugi::xml_node const bestNode = matcher::GetBestOsmNode(doc, ll); if (bestNode.empty()) { MYTHROW(OsmObjectWasDeletedException, @@ -194,7 +194,7 @@ XMLFeature ChangesetWrapper::GetMatchingAreaFeatureFromOSM(vector co hasRelation = true; } - pugi::xml_node const bestWayOrRelation = GetBestOsmWayOrRelation(doc, geometry); + pugi::xml_node const bestWayOrRelation = matcher::GetBestOsmWayOrRelation(doc, geometry); if (!bestWayOrRelation) { if (hasRelation) @@ -202,30 +202,15 @@ XMLFeature ChangesetWrapper::GetMatchingAreaFeatureFromOSM(vector co continue; } - if (strcmp(bestWayOrRelation.name(), "relation") == 0) - { - stringstream sstr; - bestWayOrRelation.print(sstr); - LOG(LDEBUG, ("Relation is the best match", sstr.str())); - MYTHROW(RelationFeatureAreNotSupportedException, ("Got relation as the best matching")); - } - if (!OsmFeatureHasTags(bestWayOrRelation)) { stringstream sstr; bestWayOrRelation.print(sstr); - LOG(LDEBUG, ("Way or relation has no tags", sstr.str())); - MYTHROW(EmptyFeatureException, ("Way or relation has no tags")); + LOG(LDEBUG, ("The matched object has no tags", sstr.str())); + MYTHROW(EmptyFeatureException, ("The matched object has no tags")); } - // TODO: rename to wayOrRelation when relations are handled. - XMLFeature const way(bestWayOrRelation); - ASSERT(way.IsArea(), ("Best way must be an area.")); - - // AlexZ: TODO: Check that this way is really match our feature. - // If we had some way to check it, why not to use it in selecting our feature? - - return way; + return XMLFeature(bestWayOrRelation); } MYTHROW(OsmObjectWasDeletedException, ("OSM does not have any matching way for feature")); } diff --git a/editor/changeset_wrapper.hpp b/editor/changeset_wrapper.hpp index 526f4519a9..ba8293e4dd 100644 --- a/editor/changeset_wrapper.hpp +++ b/editor/changeset_wrapper.hpp @@ -30,8 +30,6 @@ public: DECLARE_EXCEPTION(CreateChangeSetFailedException, ChangesetWrapperException); DECLARE_EXCEPTION(ModifyNodeFailedException, ChangesetWrapperException); DECLARE_EXCEPTION(LinearFeaturesAreNotSupportedException, ChangesetWrapperException); - // TODO: Remove this when relations are handled properly. - DECLARE_EXCEPTION(RelationFeatureAreNotSupportedException, ChangesetWrapperException); DECLARE_EXCEPTION(EmptyFeatureException, ChangesetWrapperException); ChangesetWrapper(TKeySecret const & keySecret, diff --git a/editor/editor_tests/CMakeLists.txt b/editor/editor_tests/CMakeLists.txt index fe43bb756c..dab75909d2 100644 --- a/editor/editor_tests/CMakeLists.txt +++ b/editor/editor_tests/CMakeLists.txt @@ -5,9 +5,9 @@ set( config_loader_test.cpp editor_config_test.cpp editor_notes_test.cpp + feature_matcher_test.cpp match_by_geometry_test.cpp opening_hours_ui_test.cpp - osm_feature_matcher_test.cpp ui2oh_test.cpp user_stats_test.cpp xml_feature_test.cpp diff --git a/editor/editor_tests/osm_feature_matcher_test.cpp b/editor/editor_tests/feature_matcher_test.cpp similarity index 96% rename from editor/editor_tests/osm_feature_matcher_test.cpp rename to editor/editor_tests/feature_matcher_test.cpp index 2a87ae4799..15b7ca1204 100644 --- a/editor/editor_tests/osm_feature_matcher_test.cpp +++ b/editor/editor_tests/feature_matcher_test.cpp @@ -1,6 +1,6 @@ #include "testing/testing.hpp" -#include "editor/osm_feature_matcher.hpp" +#include "editor/feature_matcher.hpp" #include "editor/xml_feature.hpp" #include "3party/pugixml/src/pugixml.hpp" @@ -270,7 +270,7 @@ UNIT_TEST(GetBestOsmNode_Test) pugi::xml_document osmResponse; TEST(osmResponse.load_buffer(osmRawResponseNode, ::strlen(osmRawResponseNode)), ()); - auto const bestNode = osm::GetBestOsmNode(osmResponse, ms::LatLon(53.8977398, 27.5579251)); + auto const bestNode = matcher::GetBestOsmNode(osmResponse, ms::LatLon(53.8977398, 27.5579251)); TEST_EQUAL(editor::XMLFeature(bestNode).GetName(), "Главное управление капитального строительства", ()); } @@ -278,7 +278,7 @@ UNIT_TEST(GetBestOsmNode_Test) pugi::xml_document osmResponse; TEST(osmResponse.load_buffer(osmRawResponseNode, ::strlen(osmRawResponseNode)), ()); - auto const bestNode = osm::GetBestOsmNode(osmResponse, ms::LatLon(53.8977254, 27.5578377)); + auto const bestNode = matcher::GetBestOsmNode(osmResponse, ms::LatLon(53.8977254, 27.5578377)); TEST_EQUAL(bestNode.attribute("id").value(), string("277172019"), ()); } } @@ -300,7 +300,7 @@ UNIT_TEST(GetBestOsmWay_Test) {27.55768215326728, 64.237078459837136} }; - auto const bestWay = osm::GetBestOsmWayOrRelation(osmResponse, geometry); + auto const bestWay = matcher::GetBestOsmWayOrRelation(osmResponse, geometry); TEST(bestWay, ()); TEST_EQUAL(editor::XMLFeature(bestWay).GetName(), "Беллесбумпром", ()); } @@ -320,7 +320,7 @@ UNIT_TEST(GetBestOsmWay_Test) {27.55778215326728, 64.237178459837136} }; - auto const bestWay = osm::GetBestOsmWayOrRelation(osmResponse, geometry); + auto const bestWay = matcher::GetBestOsmWayOrRelation(osmResponse, geometry); TEST(!bestWay, ()); } } @@ -401,7 +401,7 @@ UNIT_TEST(GetBestOsmRealtion_Test) {37.640749645536772, 67.455726619480004} }; - auto const bestWay = osm::GetBestOsmWayOrRelation(osmResponse, geometry); + auto const bestWay = matcher::GetBestOsmWayOrRelation(osmResponse, geometry); TEST_EQUAL(bestWay.attribute("id").value(), string("365808"), ()); } @@ -461,7 +461,7 @@ UNIT_TEST(HouseBuildingMiss_test) {-0.20463511500236109, 60.333954694375052} }; - auto const bestWay = osm::GetBestOsmWayOrRelation(osmResponse, geometry); + auto const bestWay = matcher::GetBestOsmWayOrRelation(osmResponse, geometry); TEST_EQUAL(bestWay.attribute("id").value(), string("345630019"), ()); } @@ -521,7 +521,7 @@ UNIT_TEST(HouseWithSeveralEntrances) {37.56998492456961, 67.57561599892091} }; - auto const bestWay = osm::GetBestOsmWayOrRelation(osmResponse, geometry); + auto const bestWay = matcher::GetBestOsmWayOrRelation(osmResponse, geometry); TEST_EQUAL(bestWay.attribute("id").value(), string("30680719"), ()); } @@ -570,7 +570,31 @@ UNIT_TEST(RelationWithSingleWay) {37.54076493934366, 67.532212492318536} }; - auto const bestWay = osm::GetBestOsmWayOrRelation(osmResponse, geometry); + auto const bestWay = matcher::GetBestOsmWayOrRelation(osmResponse, geometry); TEST_EQUAL(bestWay.attribute("id").value(), string("26232961"), ()); } + +UNIT_TEST(ScoreTriangulatedGeometries) +{ + vector lhs = { + {0, 0}, + {10, 10}, + {10, 0}, + {0, 0}, + {10, 10}, + {0, 10} + }; + + vector rhs = { + {-1, -1}, + {9, 9}, + {9, -1}, + {-1, -1}, + {9, 9}, + {-1, 9} + }; + + auto const score = matcher::ScoreTriangulatedGeometries(lhs, rhs); + TEST_GREATER(score, 0.6, ()); +} } // namespace diff --git a/editor/editor_tests/match_by_geometry_test.cpp b/editor/editor_tests/match_by_geometry_test.cpp index ce936d200e..86b06b86ce 100644 --- a/editor/editor_tests/match_by_geometry_test.cpp +++ b/editor/editor_tests/match_by_geometry_test.cpp @@ -1,6 +1,6 @@ #include "testing/testing.hpp" -#include "editor/osm_feature_matcher.hpp" +#include "editor/feature_matcher.hpp" #include "editor/xml_feature.hpp" #include "3party/pugixml/src/pugixml.hpp" @@ -249,7 +249,7 @@ UNIT_TEST(MatchByGeometry) {37.622065377399821, 67.468244489045759} }; - auto const matched = osm::GetBestOsmWayOrRelation(osmResponse, geometry); + auto const matched = matcher::GetBestOsmWayOrRelation(osmResponse, geometry); TEST_EQUAL(matched.attribute("id").value(), string("85761"), ()); } } // namespace diff --git a/editor/editor_tests/xml_feature_test.cpp b/editor/editor_tests/xml_feature_test.cpp index c894cad48c..13ddab50a3 100644 --- a/editor/editor_tests/xml_feature_test.cpp +++ b/editor/editor_tests/xml_feature_test.cpp @@ -155,48 +155,6 @@ UNIT_TEST(XMLFeature_HasTags) TEST(emptyFeature.HasAttribute("timestamp"), ()); } -UNIT_TEST(XMLFeature_IsArea) -{ - constexpr char const * validAreaXml = R"( - - - - - - -)"; - TEST(XMLFeature(validAreaXml).IsArea(), ()); - - constexpr char const * notClosedWayXml = R"( - - - - - - -)"; - TEST(!XMLFeature(notClosedWayXml).IsArea(), ()); - - constexpr char const * invalidWayXml = R"( - - - - - -)"; - TEST(!XMLFeature(invalidWayXml).IsArea(), ()); - - constexpr char const * emptyWay = R"( - -)"; - TEST(!XMLFeature(emptyWay).IsArea(), ()); - - constexpr char const * node = R"( - -)"; - TEST(!XMLFeature(node).IsArea(), ()); -} - auto const kTestNode = R"( @@ -303,7 +261,7 @@ UNIT_TEST(XMLFeature_FromXmlNode) UNIT_TEST(XMLFeature_Geometry) { - XMLFeature::TMercatorGeometry const geometry = { + vector const geometry = { {28.7206411, 3.7182409}, {46.7569003, 47.0774689}, {22.5909217, 41.6994874}, diff --git a/editor/osm_feature_matcher.cpp b/editor/feature_matcher.cpp similarity index 83% rename from editor/osm_feature_matcher.cpp rename to editor/feature_matcher.cpp index 159c59ad88..1cdbab1e85 100644 --- a/editor/osm_feature_matcher.cpp +++ b/editor/feature_matcher.cpp @@ -1,4 +1,4 @@ -#include "editor/osm_feature_matcher.hpp" +#include "editor/feature_matcher.hpp" #include "base/logging.hpp" #include "base/stl_helpers.hpp" @@ -9,9 +9,9 @@ #include "std/utility.hpp" #include +#include #include #include -#include using editor::XMLFeature; @@ -33,7 +33,8 @@ using ForEachWayFn = function +AreaType IntersectionArea(LGeometry const & our, RGeometry const & their) { ASSERT(bg::is_valid(our), ()); ASSERT(bg::is_valid(their), ()); @@ -70,7 +71,7 @@ void AddInnerIfNeeded(pugi::xml_document const & osmResponse, pugi::xml_node con void MakeOuterRing(MultiLinestring & outerLines, Polygon & dest) { bool const needReverse = - outerLines.size() > 1 && bg::equals(outerLines[0].front(), outerLines[1].back()); + outerLines.size() > 1 && bg::equals(outerLines[0].front(), outerLines[1].back()); for (size_t i = 0; i < outerLines.size(); ++i) { @@ -81,15 +82,16 @@ void MakeOuterRing(MultiLinestring & outerLines, Polygon & dest) } } -double MatchByGeometry(MultiPolygon const & our, Polygon const & their) +template +double MatchByGeometry(LGeometry const & lhs, RGeometry const & rhs) { - if (!bg::is_valid(our) || !bg::is_valid(their)) + if (!bg::is_valid(lhs) || !bg::is_valid(rhs)) return kPenaltyScore; - auto const ourArea = bg::area(our); - auto const theirArea = bg::area(their); - auto const intersectionArea = IntersectionArea(our, their); - auto const unionArea = ourArea + theirArea - intersectionArea; + auto const lhsArea = bg::area(lhs); + auto const rhsArea = bg::area(rhs); + auto const intersectionArea = IntersectionArea(lhs, rhs); + auto const unionArea = lhsArea + rhsArea - intersectionArea; // Avoid infinity. if (my::AlmostEqualAbs(unionArea, 0.0, 1e-18)) @@ -104,7 +106,7 @@ double MatchByGeometry(MultiPolygon const & our, Polygon const & their) return score; } -MultiPolygon TriangelsToPolygon(vector const & points) +MultiPolygon TriangelsToPolygon(vector const & points) { size_t const kTriangleSize = 3; CHECK_EQUAL(points.size() % kTriangleSize, 0, ()); @@ -134,8 +136,7 @@ MultiPolygon TriangelsToPolygon(vector const & points) return result; } -/// @returns value form (-Inf, 1]. Negative values are used as penalty, -/// positive as score. +/// Returns value form (-Inf, 1]. Negative values are used as penalty, positive as score. double ScoreLatLon(XMLFeature const & xmlFt, ms::LatLon const & latLon) { auto const a = MercatorBounds::FromLatLon(xmlFt.GetCenter()); @@ -241,19 +242,19 @@ Polygon GetRelationsGeometry(pugi::xml_document const & osmResponse, } Polygon GetWaysOrRelationsGeometry(pugi::xml_document const & osmResponse, - pugi::xml_node const & wayOrRelation) + pugi::xml_node const & wayOrRelation) { if (strcmp(wayOrRelation.name(), "way") == 0) return GetWaysGeometry(osmResponse, wayOrRelation); return GetRelationsGeometry(osmResponse, wayOrRelation); } -/// @returns value form [-1, 1]. Negative values are used as penalty, positive as score. -/// @param osmResponse - nodes, ways and relations from osm; -/// @param wayOrRelation - either way or relation to be compared agains ourGeometry; -/// @param outGeometry - geometry of a FeatureType (ourGeometry must be sort-uniqued); -double ScoreGeometry(pugi::xml_document const & osmResponse, - pugi::xml_node const & wayOrRelation, vector const & ourGeometry) +/// Returns value form [-1, 1]. Negative values are used as penalty, positive as score. +/// |osmResponse| - nodes, ways and relations from osm; +/// |wayOrRelation| - either way or relation to be compared agains ourGeometry; +/// |ourGeometry| - geometry of a FeatureType; +double ScoreGeometry(pugi::xml_document const & osmResponse, pugi::xml_node const & wayOrRelation, + vector const & ourGeometry) { ASSERT(!ourGeometry.empty(), ("Our geometry cannot be empty")); @@ -269,9 +270,9 @@ double ScoreGeometry(pugi::xml_document const & osmResponse, return MatchByGeometry(our, their); } -} // namespace +} // namespace -namespace osm +namespace matcher { pugi::xml_node GetBestOsmNode(pugi::xml_document const & osmResponse, ms::LatLon const & latLon) { @@ -331,4 +332,17 @@ pugi::xml_node GetBestOsmWayOrRelation(pugi::xml_document const & osmResponse, return bestMatchWay; } -} // namespace osm + +double ScoreTriangulatedGeometries(vector const & lhs, vector const & rhs) +{ + auto const lhsPolygon = TriangelsToPolygon(lhs); + if (bg::is_empty(lhsPolygon)) + return kPenaltyScore; + + auto const rhsPolygon = TriangelsToPolygon(rhs); + if (bg::is_empty(rhsPolygon)) + return kPenaltyScore; + + return MatchByGeometry(lhsPolygon, rhsPolygon); +} +} // namespace matcher diff --git a/editor/osm_feature_matcher.hpp b/editor/feature_matcher.hpp similarity index 52% rename from editor/osm_feature_matcher.hpp rename to editor/feature_matcher.hpp index 2d3947317f..dd697b944d 100644 --- a/editor/osm_feature_matcher.hpp +++ b/editor/feature_matcher.hpp @@ -7,11 +7,14 @@ #include "std/vector.hpp" -namespace osm +namespace matcher { -/// @returns closest to the latLon node from osm or empty node if none is close enough. +/// Returns closest to the latLon node from osm or empty node if none is close enough. pugi::xml_node GetBestOsmNode(pugi::xml_document const & osmResponse, ms::LatLon const & latLon); -/// @returns a way from osm with similar geometry or empy node if can't find such way. +/// Returns a way from osm with similar geometry or empty node if can't find such way. pugi::xml_node GetBestOsmWayOrRelation(pugi::xml_document const & osmResponse, vector const & geometry); +/// Returns value form [-1, 1]. Negative values are used as penalty, positive as score. +/// |lhs| and |rhs| - triangulated polygons; +double ScoreTriangulatedGeometries(vector const & lhs, vector const & rhs); } // namespace osm diff --git a/editor/xml_feature.cpp b/editor/xml_feature.cpp index d9b58036e5..9c731b6915 100644 --- a/editor/xml_feature.cpp +++ b/editor/xml_feature.cpp @@ -15,8 +15,10 @@ constexpr char const * kUploadStatus = "upload_status"; constexpr char const * kUploadError = "upload_error"; constexpr char const * kHouseNumber = "addr:housenumber"; +constexpr char const * kUnknownType = "unknown"; constexpr char const * kNodeType = "node"; constexpr char const * kWayType = "way"; +constexpr char const * kRelationType = "relation"; pugi::xml_node FindTag(pugi::xml_document const & document, string const & key) { @@ -45,14 +47,18 @@ m2::PointD GetMercatorPointFromNode(pugi::xml_node const & node) void ValidateElement(pugi::xml_node const & nodeOrWay) { + using editor::XMLFeature; + if (!nodeOrWay) MYTHROW(editor::InvalidXML, ("Document has no valid root element.")); - string const type = nodeOrWay.name(); - if (type == kNodeType) + auto const type = XMLFeature::StringToType(nodeOrWay.name()); + + if (type == XMLFeature::Type::Unknown) + MYTHROW(editor::InvalidXML, ("XMLFeature does not support root tag", nodeOrWay.name())); + + if (type == XMLFeature::Type::Node) UNUSED_VALUE(GetLatLonFromNode(nodeOrWay)); - else if (type != kWayType) - MYTHROW(editor::InvalidXML, ("XMLFeature does not support root tag", type)); if (!nodeOrWay.attribute(kTimestamp)) MYTHROW(editor::NoTimestamp, ("Node has no timestamp attribute")); @@ -69,7 +75,9 @@ char const * const XMLFeature::kIntlLang = XMLFeature::XMLFeature(Type const type) { - m_document.append_child(type == Type::Node ? kNodeType : kWayType); + ASSERT_NOT_EQUAL(type, Type::Unknown, ()); + + m_document.append_child(TypeToString(type).c_str()); } XMLFeature::XMLFeature(string const & xml) @@ -111,17 +119,15 @@ vector XMLFeature::FromOSM(string const & osmXml) vector features; for (auto const n : doc.child("osm").children()) { - string const name(n.name()); - // TODO(AlexZ): Add relation support. - if (name == kNodeType || name == kWayType) - features.push_back(XMLFeature(n)); // TODO(AlexZ): Use emplace_back when pugi supports it. + if (StringToType(n.name()) != Type::Unknown) + features.emplace_back(n); } return features; } XMLFeature::Type XMLFeature::GetType() const { - return strcmp(GetRootNode().name(), "node") == 0 ? Type::Node : Type::Way; + return StringToType(GetRootNode().name()); } string XMLFeature::GetTypeString() const @@ -129,21 +135,6 @@ string XMLFeature::GetTypeString() const return GetRootNode().name(); } -bool XMLFeature::IsArea() const -{ - if (strcmp(GetRootNode().name(), kWayType) != 0) - return false; - - vector ndIds; - for (auto const & nd : GetRootNode().select_nodes("nd")) - ndIds.push_back(nd.node().attribute("ref").value()); - - if (ndIds.size() < 4) - return false; - - return ndIds.front() == ndIds.back(); -} - void XMLFeature::Save(ostream & ost) const { m_document.save(ost, " "); @@ -201,12 +192,13 @@ m2::PointD XMLFeature::GetMercatorCenter() const ms::LatLon XMLFeature::GetCenter() const { + ASSERT_EQUAL(GetType(), Type::Node, ()); return GetLatLonFromNode(GetRootNode()); } void XMLFeature::SetCenter(ms::LatLon const & ll) { - ASSERT_EQUAL(GetRootNode().name(), string(kNodeType), ()); + ASSERT_EQUAL(GetType(), Type::Node, ()); SetAttribute("lat", strings::to_string_dac(ll.lat, kLatLonTolerance)); SetAttribute("lon", strings::to_string_dac(ll.lon, kLatLonTolerance)); } @@ -216,10 +208,11 @@ void XMLFeature::SetCenter(m2::PointD const & mercatorCenter) SetCenter(MercatorBounds::ToLatLon(mercatorCenter)); } -XMLFeature::TMercatorGeometry XMLFeature::GetGeometry() const +vector XMLFeature::GetGeometry() const { - ASSERT_EQUAL(GetType(), Type::Way, ("Only ways have geometry")); - TMercatorGeometry geometry; + ASSERT_NOT_EQUAL(GetType(), Type::Unknown, ()); + ASSERT_NOT_EQUAL(GetType(), Type::Node, ()); + vector geometry; for (auto const xCenter : GetRootNode().select_nodes("nd")) { ASSERT(xCenter.node(), ("no nd attribute.")); @@ -394,6 +387,31 @@ bool XMLFeature::AttachToParentNode(pugi::xml_node parent) const return !parent.append_copy(GetRootNode()).empty(); } +// static +string XMLFeature::TypeToString(Type type) +{ + switch (type) + { + case Type::Unknown: return kUnknownType; + case Type::Node: return kNodeType; + case Type::Way: return kWayType; + case Type::Relation: return kRelationType; + } +} + +// static +XMLFeature::Type XMLFeature::StringToType(string const & type) +{ + if (type == kNodeType) + return Type::Node; + if (type == kWayType) + return Type::Way; + if (type == kRelationType) + return Type::Relation; + + return Type::Unknown; +} + string DebugPrint(XMLFeature const & feature) { ostringstream ost; @@ -403,10 +421,6 @@ string DebugPrint(XMLFeature const & feature) string DebugPrint(XMLFeature::Type const type) { - switch (type) - { - case XMLFeature::Type::Node: return "Node"; - case XMLFeature::Type::Way: return "Way"; - } + return XMLFeature::TypeToString(type); } } // namespace editor diff --git a/editor/xml_feature.hpp b/editor/xml_feature.hpp index d6bbf67985..92840c3b93 100644 --- a/editor/xml_feature.hpp +++ b/editor/xml_feature.hpp @@ -36,12 +36,12 @@ public: enum class Type { + Unknown, Node, - Way + Way, + Relation }; - using TMercatorGeometry = vector; - /// Creates empty node or way. XMLFeature(Type const type); XMLFeature(string const & xml); @@ -51,7 +51,7 @@ public: // TODO: It should make "deep" compare instead of converting to strings. // Strings comparison does not work if tags order is different but tags are equal. bool operator==(XMLFeature const & other) const; - /// @returns nodes and ways from osmXml. Vector can be empty. + /// @returns nodes, ways and relations from osmXml. Vector can be empty. static vector FromOSM(string const & osmXml); void Save(ostream & ost) const; @@ -63,15 +63,12 @@ public: Type GetType() const; string GetTypeString() const; - /// @returns true only if it is a way and it is closed (area). - bool IsArea() const; - m2::PointD GetMercatorCenter() const; ms::LatLon GetCenter() const; void SetCenter(ms::LatLon const & ll); void SetCenter(m2::PointD const & mercatorCenter); - TMercatorGeometry GetGeometry() const; + vector GetGeometry() const; /// Sets geometry in mercator to match against FeatureType's geometry in mwm /// when megrating to a new mwm build. @@ -81,7 +78,9 @@ public: template void SetGeometry(TIterator begin, TIterator end) { - ASSERT_EQUAL(GetType(), Type::Way, ("Only ways have geometry")); + ASSERT_NOT_EQUAL(GetType(), Type::Unknown, ()); + ASSERT_NOT_EQUAL(GetType(), Type::Node, ()); + for (; begin != end; ++begin) { auto nd = GetRootNode().append_child("nd"); @@ -162,6 +161,9 @@ public: bool AttachToParentNode(pugi::xml_node parent) const; + static string TypeToString(Type type); + static Type StringToType(string const & type); + private: pugi::xml_node const GetRootNode() const; pugi::xml_node GetRootNode(); diff --git a/indexer/edits_migration.cpp b/indexer/edits_migration.cpp index 298bee88f4..5562651254 100644 --- a/indexer/edits_migration.cpp +++ b/indexer/edits_migration.cpp @@ -4,6 +4,8 @@ #include "indexer/edits_migration.hpp" #include "indexer/feature.hpp" +#include "editor/feature_matcher.hpp" + #include "base/logging.hpp" #include "base/stl_iterator.hpp" @@ -43,7 +45,7 @@ FeatureID MigrateNodeFeatureIndex(osm::Editor::ForEachFeaturesNearByFn & forEach return feature->GetID(); } -FeatureID MigrateWayFeatureIndex( +FeatureID MigrateWayorRelatonFeatureIndex( osm::Editor::ForEachFeaturesNearByFn & forEach, XMLFeature const & xml, osm::Editor::FeatureStatus const /* Unused for now (we don't create/delete area features)*/, TGenerateIDFn const & /*Unused for the same reason*/) @@ -51,6 +53,7 @@ FeatureID MigrateWayFeatureIndex( unique_ptr feature; auto bestScore = 0.6; // initial score is used as a threshold. auto geometry = xml.GetGeometry(); + auto count = 0; if (geometry.empty()) MYTHROW(MigrationError, ("Feature has invalid geometry", xml)); @@ -58,9 +61,6 @@ FeatureID MigrateWayFeatureIndex( // This can be any point on a feature. auto const someFeaturePoint = geometry[0]; - sort(begin(geometry), end(geometry)); // Sort to use in set_intersection. - auto count = 0; - LOG(LDEBUG, ("SomePoint", someFeaturePoint)); forEach( [&feature, &xml, &geometry, &count, &bestScore](FeatureType const & ft) { @@ -68,29 +68,9 @@ FeatureID MigrateWayFeatureIndex( return; ++count; auto ftGeometry = ft.GetTriangesAsPoints(FeatureType::BEST_GEOMETRY); - sort(begin(ftGeometry), end(ftGeometry)); - // The default comparison operator used in sort above (cmp1) and one that is - // used in set_itersection (cmp2) are compatible in that sence that - // cmp2(a, b) :- cmp1(a, b) and - // cmp1(a, b) :- cmp2(a, b) || a almost equal b. - // You can think of cmp2 as !(a >= b). - // But cmp2 is not transitive: - // i.e. !cmp(a, b) && !cmp(b, c) does NOT implies !cmp(a, c), - // |a, b| < eps, |b, c| < eps. - // This could lead to unexpected results in set_itersection (with greedy implementation), - // but we assume such situation is very unlikely. - auto const matched = set_intersection(begin(geometry), end(geometry), - begin(ftGeometry), end(ftGeometry), - CounterIterator(), - [](m2::PointD const & p1, m2::PointD const & p2) - { - // TODO(mgsergio): Use 1e-7 everyware instead of - // MercatotBounds::GetCellID2PointAbsEpsilon - return p1 < p2 && !p1.EqualDxDy(p2, 1e-7); - }).GetCount(); + auto const score = matcher::ScoreTriangulatedGeometries(geometry, ftGeometry); - auto const score = static_cast(matched) / geometry.size(); if (score > bestScore) { bestScore = score; @@ -98,8 +78,10 @@ FeatureID MigrateWayFeatureIndex( } }, someFeaturePoint); + if (count == 0) MYTHROW(MigrationError, ("No ways returned for point", someFeaturePoint)); + if (!feature) { MYTHROW(MigrationError, @@ -115,10 +97,13 @@ FeatureID MigrateFeatureIndex(osm::Editor::ForEachFeaturesNearByFn & forEach, { switch (xml.GetType()) { + case XMLFeature::Type::Unknown: + MYTHROW(MigrationError, ("Migration for XMLFeature::Type::Unknown is not possible")); case XMLFeature::Type::Node: return MigrateNodeFeatureIndex(forEach, xml, featureStatus, generateID); case XMLFeature::Type::Way: - return MigrateWayFeatureIndex(forEach, xml, featureStatus, generateID); + case XMLFeature::Type::Relation: + return MigrateWayorRelatonFeatureIndex(forEach, xml, featureStatus, generateID); } } } // namespace editor diff --git a/indexer/feature.cpp b/indexer/feature.cpp index f52913981d..6a1d6d07ee 100644 --- a/indexer/feature.cpp +++ b/indexer/feature.cpp @@ -118,10 +118,7 @@ editor::XMLFeature FeatureType::ToXML(bool serializeType) const else { ParseTriangles(BEST_GEOMETRY); - vector geometry(begin(m_triangles), end(m_triangles)); - // Remove duplicates. - my::SortUnique(geometry); - feature.SetGeometry(geometry); + feature.SetGeometry(begin(m_triangles), end(m_triangles)); } ForEachName([&feature](uint8_t const & lang, string const & name) diff --git a/indexer/osm_editor.cpp b/indexer/osm_editor.cpp index d2c4a5439c..03cf10ffdb 100644 --- a/indexer/osm_editor.cpp +++ b/indexer/osm_editor.cpp @@ -62,9 +62,8 @@ constexpr char const * kAddrStreetTag = "addr:street"; constexpr char const * kUploaded = "Uploaded"; constexpr char const * kDeletedFromOSMServer = "Deleted from OSM by someone"; -constexpr char const * kRelationsAreNotSupported = "Relations are not supported yet"; constexpr char const * kNeedsRetry = "Needs Retry"; -constexpr char const * kWrongMatch = "Matched feature has no tags"; +constexpr char const * kMatchedFeatureIsEmpty = "Matched feature has no tags"; struct XmlSection { @@ -119,10 +118,7 @@ bool NeedsUpload(string const & uploadStatus) { return uploadStatus != kUploaded && uploadStatus != kDeletedFromOSMServer && - // TODO: Remove this line when relations are supported. - uploadStatus != kRelationsAreNotSupported && - // TODO: Remove this when we have better matching algorithm. - uploadStatus != kWrongMatch; + uploadStatus != kMatchedFeatureIsEmpty; } /// Compares editable fields connected with feature ignoring street. @@ -772,8 +768,7 @@ void Editor::UploadChanges(string const & key, string const & secret, TChangeset continue; } - XMLFeature osmFeature = GetMatchingFeatureFromOSM( - changeset, *originalFeaturePtr); + XMLFeature osmFeature = GetMatchingFeatureFromOSM(changeset, *originalFeaturePtr); XMLFeature const osmFeatureCopy = osmFeature; osmFeature.ApplyPatch(feature); // Check to avoid uploading duplicates into OSM. @@ -815,16 +810,9 @@ void Editor::UploadChanges(string const & key, string const & secret, TChangeset ++errorsCount; LOG(LWARNING, (ex.what())); } - catch (ChangesetWrapper::RelationFeatureAreNotSupportedException const & ex) - { - fti.m_uploadStatus = kRelationsAreNotSupported; - fti.m_uploadError = ex.Msg(); - ++errorsCount; - LOG(LWARNING, (ex.what())); - } catch (ChangesetWrapper::EmptyFeatureException const & ex) { - fti.m_uploadStatus = kWrongMatch; + fti.m_uploadStatus = kMatchedFeatureIsEmpty; fti.m_uploadError = ex.Msg(); ++errorsCount; LOG(LWARNING, (ex.what())); diff --git a/xcode/editor/editor.xcodeproj/project.pbxproj b/xcode/editor/editor.xcodeproj/project.pbxproj index 7f213fed9e..d071738b68 100644 --- a/xcode/editor/editor.xcodeproj/project.pbxproj +++ b/xcode/editor/editor.xcodeproj/project.pbxproj @@ -26,7 +26,6 @@ 3496ABE21DC2035800C5DDBA /* editor_config_test.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 3496ABD21DC2034900C5DDBA /* editor_config_test.cpp */; }; 3496ABE31DC2035800C5DDBA /* editor_notes_test.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 3496ABD31DC2034900C5DDBA /* editor_notes_test.cpp */; }; 3496ABE41DC2035800C5DDBA /* opening_hours_ui_test.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 3496ABD41DC2034900C5DDBA /* opening_hours_ui_test.cpp */; }; - 3496ABE51DC2035800C5DDBA /* osm_feature_matcher_test.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 3496ABD51DC2034900C5DDBA /* osm_feature_matcher_test.cpp */; }; 3496ABE61DC2035800C5DDBA /* ui2oh_test.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 3496ABD61DC2034900C5DDBA /* ui2oh_test.cpp */; }; 3496ABE71DC2035800C5DDBA /* user_stats_test.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 3496ABD71DC2034900C5DDBA /* user_stats_test.cpp */; }; 3496ABE81DC2035800C5DDBA /* xml_feature_test.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 3496ABD81DC2034900C5DDBA /* xml_feature_test.cpp */; }; @@ -44,10 +43,12 @@ 3496AC031DC2048E00C5DDBA /* testingmain.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 3496AC011DC2047D00C5DDBA /* testingmain.cpp */; }; 3496AC051DC204B700C5DDBA /* editor.config in Resources */ = {isa = PBXBuildFile; fileRef = 3496AC041DC204B700C5DDBA /* editor.config */; }; 3496AC071DC204D000C5DDBA /* libz.tbd in Frameworks */ = {isa = PBXBuildFile; fileRef = 3496AC061DC204D000C5DDBA /* libz.tbd */; }; - 34EB091F1C5F846900F47F1F /* osm_feature_matcher.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 34EB091D1C5F846900F47F1F /* osm_feature_matcher.cpp */; }; - 34EB09201C5F846900F47F1F /* osm_feature_matcher.hpp in Headers */ = {isa = PBXBuildFile; fileRef = 34EB091E1C5F846900F47F1F /* osm_feature_matcher.hpp */; }; 34FFB34C1C316A7600BFF6C3 /* server_api.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 34FFB34A1C316A7600BFF6C3 /* server_api.cpp */; }; 34FFB34D1C316A7600BFF6C3 /* server_api.hpp in Headers */ = {isa = PBXBuildFile; fileRef = 34FFB34B1C316A7600BFF6C3 /* server_api.hpp */; }; + 3D052487200F62EE00F24998 /* feature_matcher.hpp in Headers */ = {isa = PBXBuildFile; fileRef = 3D052485200F62ED00F24998 /* feature_matcher.hpp */; }; + 3D052488200F62EE00F24998 /* feature_matcher.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 3D052486200F62ED00F24998 /* feature_matcher.cpp */; }; + 3D05248B200F630000F24998 /* feature_matcher_test.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 3D052489200F630000F24998 /* feature_matcher_test.cpp */; }; + 3D05248C200F630000F24998 /* match_by_geometry_test.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 3D05248A200F630000F24998 /* match_by_geometry_test.cpp */; }; 3D3058741D707DBE004AC712 /* config_loader.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 3D3058721D707DBE004AC712 /* config_loader.cpp */; }; 3D3058751D707DBE004AC712 /* config_loader.hpp in Headers */ = {isa = PBXBuildFile; fileRef = 3D3058731D707DBE004AC712 /* config_loader.hpp */; }; 3D489BEF1D4F67E10052AA38 /* editor_storage.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 3D489BED1D4F67E10052AA38 /* editor_storage.cpp */; }; @@ -78,7 +79,6 @@ 3496ABD21DC2034900C5DDBA /* editor_config_test.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = editor_config_test.cpp; sourceTree = ""; }; 3496ABD31DC2034900C5DDBA /* editor_notes_test.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = editor_notes_test.cpp; sourceTree = ""; }; 3496ABD41DC2034900C5DDBA /* opening_hours_ui_test.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = opening_hours_ui_test.cpp; sourceTree = ""; }; - 3496ABD51DC2034900C5DDBA /* osm_feature_matcher_test.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = osm_feature_matcher_test.cpp; sourceTree = ""; }; 3496ABD61DC2034900C5DDBA /* ui2oh_test.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ui2oh_test.cpp; sourceTree = ""; }; 3496ABD71DC2034900C5DDBA /* user_stats_test.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = user_stats_test.cpp; sourceTree = ""; }; 3496ABD81DC2034900C5DDBA /* xml_feature_test.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = xml_feature_test.cpp; sourceTree = ""; }; @@ -95,12 +95,14 @@ 3496AC011DC2047D00C5DDBA /* testingmain.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = testingmain.cpp; path = ../../testing/testingmain.cpp; sourceTree = ""; }; 3496AC041DC204B700C5DDBA /* editor.config */ = {isa = PBXFileReference; lastKnownFileType = text.xml; name = editor.config; path = ../../data/editor.config; sourceTree = ""; }; 3496AC061DC204D000C5DDBA /* libz.tbd */ = {isa = PBXFileReference; lastKnownFileType = "sourcecode.text-based-dylib-definition"; name = libz.tbd; path = usr/lib/libz.tbd; sourceTree = SDKROOT; }; - 34EB091D1C5F846900F47F1F /* osm_feature_matcher.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = osm_feature_matcher.cpp; sourceTree = ""; }; - 34EB091E1C5F846900F47F1F /* osm_feature_matcher.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = osm_feature_matcher.hpp; sourceTree = ""; }; 34F5586E1DBF49B200A4FC11 /* common-debug.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; name = "common-debug.xcconfig"; path = "../common-debug.xcconfig"; sourceTree = ""; }; 34F5586F1DBF49B200A4FC11 /* common-release.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; name = "common-release.xcconfig"; path = "../common-release.xcconfig"; sourceTree = ""; }; 34FFB34A1C316A7600BFF6C3 /* server_api.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = server_api.cpp; sourceTree = ""; }; 34FFB34B1C316A7600BFF6C3 /* server_api.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = server_api.hpp; sourceTree = ""; }; + 3D052485200F62ED00F24998 /* feature_matcher.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = feature_matcher.hpp; sourceTree = ""; }; + 3D052486200F62ED00F24998 /* feature_matcher.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = feature_matcher.cpp; sourceTree = ""; }; + 3D052489200F630000F24998 /* feature_matcher_test.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = feature_matcher_test.cpp; sourceTree = ""; }; + 3D05248A200F630000F24998 /* match_by_geometry_test.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = match_by_geometry_test.cpp; sourceTree = ""; }; 3D3058721D707DBE004AC712 /* config_loader.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = config_loader.cpp; sourceTree = ""; }; 3D3058731D707DBE004AC712 /* config_loader.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = config_loader.hpp; sourceTree = ""; }; 3D489BED1D4F67E10052AA38 /* editor_storage.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = editor_storage.cpp; sourceTree = ""; }; @@ -164,6 +166,8 @@ 341138731C15AE02002E3B3E /* Editor */ = { isa = PBXGroup; children = ( + 3D052486200F62ED00F24998 /* feature_matcher.cpp */, + 3D052485200F62ED00F24998 /* feature_matcher.hpp */, 3D3058721D707DBE004AC712 /* config_loader.cpp */, 3D3058731D707DBE004AC712 /* config_loader.hpp */, 3D489BED1D4F67E10052AA38 /* editor_storage.cpp */, @@ -175,8 +179,6 @@ 34527C4F1C89B1770015050E /* editor_config.cpp */, 34527C501C89B1770015050E /* editor_config.hpp */, 34583BBE1C8854C100F94664 /* yes_no_unknown.hpp */, - 34EB091D1C5F846900F47F1F /* osm_feature_matcher.cpp */, - 34EB091E1C5F846900F47F1F /* osm_feature_matcher.hpp */, 340DC8271C4E71E500EAA2CC /* changeset_wrapper.cpp */, 340DC8281C4E71E500EAA2CC /* changeset_wrapper.hpp */, 340C20DC1C3E4DFD00111D22 /* osm_auth.cpp */, @@ -197,12 +199,13 @@ 3496ABD01DC2032800C5DDBA /* editor_tests */ = { isa = PBXGroup; children = ( + 3D052489200F630000F24998 /* feature_matcher_test.cpp */, + 3D05248A200F630000F24998 /* match_by_geometry_test.cpp */, 3496AC011DC2047D00C5DDBA /* testingmain.cpp */, 3496ABD11DC2034900C5DDBA /* config_loader_test.cpp */, 3496ABD21DC2034900C5DDBA /* editor_config_test.cpp */, 3496ABD31DC2034900C5DDBA /* editor_notes_test.cpp */, 3496ABD41DC2034900C5DDBA /* opening_hours_ui_test.cpp */, - 3496ABD51DC2034900C5DDBA /* osm_feature_matcher_test.cpp */, 3496ABD61DC2034900C5DDBA /* ui2oh_test.cpp */, 3496ABD71DC2034900C5DDBA /* user_stats_test.cpp */, 3496ABD81DC2034900C5DDBA /* xml_feature_test.cpp */, @@ -236,7 +239,6 @@ isa = PBXHeadersBuildPhase; buildActionMask = 2147483647; files = ( - 34EB09201C5F846900F47F1F /* osm_feature_matcher.hpp in Headers */, 34FFB34D1C316A7600BFF6C3 /* server_api.hpp in Headers */, F60F02EF1C92CBF1003A0AF6 /* editor_notes.hpp in Headers */, 3D489BF01D4F67E10052AA38 /* editor_storage.hpp in Headers */, @@ -246,6 +248,7 @@ 340C20DF1C3E4DFD00111D22 /* osm_auth.hpp in Headers */, 3D3058751D707DBE004AC712 /* config_loader.hpp in Headers */, 34527C521C89B1770015050E /* editor_config.hpp in Headers */, + 3D052487200F62EE00F24998 /* feature_matcher.hpp in Headers */, 3411387B1C15AE42002E3B3E /* ui2oh.hpp in Headers */, 3441CE491CFC1D3C00CF30D4 /* user_stats.hpp in Headers */, 341138791C15AE42002E3B3E /* opening_hours_ui.hpp in Headers */, @@ -342,17 +345,19 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( + 3D05248B200F630000F24998 /* feature_matcher_test.cpp in Sources */, 347C71281C295B1100BE9208 /* xml_feature.cpp in Sources */, 341138781C15AE42002E3B3E /* opening_hours_ui.cpp in Sources */, 340C20DE1C3E4DFD00111D22 /* osm_auth.cpp in Sources */, + 3D052488200F62EE00F24998 /* feature_matcher.cpp in Sources */, 3D489BEF1D4F67E10052AA38 /* editor_storage.cpp in Sources */, 3411387A1C15AE42002E3B3E /* ui2oh.cpp in Sources */, 340DC8291C4E71E500EAA2CC /* changeset_wrapper.cpp in Sources */, 3D3058741D707DBE004AC712 /* config_loader.cpp in Sources */, - 34EB091F1C5F846900F47F1F /* osm_feature_matcher.cpp in Sources */, 3441CE481CFC1D3C00CF30D4 /* user_stats.cpp in Sources */, 34527C511C89B1770015050E /* editor_config.cpp in Sources */, 34FFB34C1C316A7600BFF6C3 /* server_api.cpp in Sources */, + 3D05248C200F630000F24998 /* match_by_geometry_test.cpp in Sources */, F60F02EE1C92CBF1003A0AF6 /* editor_notes.cpp in Sources */, ); runOnlyForDeploymentPostprocessing = 0; @@ -366,7 +371,6 @@ 3496ABE31DC2035800C5DDBA /* editor_notes_test.cpp in Sources */, 3496ABE41DC2035800C5DDBA /* opening_hours_ui_test.cpp in Sources */, 3496AC031DC2048E00C5DDBA /* testingmain.cpp in Sources */, - 3496ABE51DC2035800C5DDBA /* osm_feature_matcher_test.cpp in Sources */, 3496ABE61DC2035800C5DDBA /* ui2oh_test.cpp in Sources */, 3496ABE71DC2035800C5DDBA /* user_stats_test.cpp in Sources */, 3496ABE81DC2035800C5DDBA /* xml_feature_test.cpp in Sources */,