From 0ace2e69035a52ef6f0cb82c2b9e4a4e52f4e977 Mon Sep 17 00:00:00 2001 From: Sergey Magidovich Date: Thu, 28 Jan 2016 13:28:16 +0300 Subject: [PATCH] Implement simplified comparison. --- editor/changeset_wrapper.cpp | 5 +- .../editor_tests/osm_feature_matcher_test.cpp | 95 ++++++++++++---- editor/osm_feature_matcher.cpp | 107 +++--------------- editor/osm_feature_matcher.hpp | 8 -- indexer/osm_editor.cpp | 3 + 5 files changed, 98 insertions(+), 120 deletions(-) diff --git a/editor/changeset_wrapper.cpp b/editor/changeset_wrapper.cpp index 0e946028a3..35ad6a1fa9 100644 --- a/editor/changeset_wrapper.cpp +++ b/editor/changeset_wrapper.cpp @@ -58,7 +58,6 @@ XMLFeature ChangesetWrapper::GetMatchingNodeFeatureFromOSM(m2::PointD const & ce // Throws! LoadXmlFromOSM(ll, doc); - // feature must be the original one, not patched! pugi::xml_node const bestNode = GetBestOsmNode(doc, ll); if (bestNode.empty()) MYTHROW(OsmObjectWasDeletedException, @@ -77,7 +76,6 @@ XMLFeature ChangesetWrapper::GetMatchingAreaFeatureFromOSM(set const // Throws! LoadXmlFromOSM(ll, doc); - // TODO(AlexZ): Select best matching OSM way from possible many ways. pugi::xml_node const bestWay = GetBestOsmWay(doc, geometry); if (bestWay.empty()) continue; @@ -86,7 +84,8 @@ XMLFeature ChangesetWrapper::GetMatchingAreaFeatureFromOSM(set const if (!way.IsArea()) continue; - // TODO: Check that this way is really match our feature. + // 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; } diff --git a/editor/editor_tests/osm_feature_matcher_test.cpp b/editor/editor_tests/osm_feature_matcher_test.cpp index 306d0a24b7..fae7f32e6f 100644 --- a/editor/editor_tests/osm_feature_matcher_test.cpp +++ b/editor/editor_tests/osm_feature_matcher_test.cpp @@ -6,7 +6,7 @@ #include "3party/pugixml/src/pugixml.hpp" -static char const * const osmRawResponse = R"SEP( +static char const * const osmRawResponseWay = R"SEP( @@ -53,33 +53,90 @@ static char const * const osmRawResponse = R"SEP( )SEP"; +static char const * const osmRawResponseNode = R"SEP( + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +)SEP"; + UNIT_TEST(GetBestOsmWay_test) { - pugi::xml_document osmResponse; - TEST(osmResponse.load_buffer(osmRawResponse, ::strlen(osmRawResponse)), ()); - set geometry = { - {53.8977484, 27.557359}, - {53.8978710, 27.557681}, - {53.8978034, 27.557764}, - {53.8977652, 27.557803}, - {53.8977254, 27.557837}, - {53.8976570, 27.557661}, - {53.8976041, 27.557518}, - {53.8977484, 27.557359} - }; + { + pugi::xml_document osmResponse; + TEST(osmResponse.load_buffer(osmRawResponseWay, ::strlen(osmRawResponseWay)), ()); + set geometry = { + MercatorBounds::FromLatLon(53.8977484, 27.557359), + MercatorBounds::FromLatLon(53.8978710, 27.557681), + MercatorBounds::FromLatLon(53.8978034, 27.557764), + MercatorBounds::FromLatLon(53.8977652, 27.557803), + MercatorBounds::FromLatLon(53.8977254, 27.557837), + MercatorBounds::FromLatLon(53.8976570, 27.557661), + MercatorBounds::FromLatLon(53.8976041, 27.557518), + }; - auto const bestWay = osm::GetBestOsmWay(osmResponse, geometry); - TEST_EQUAL(editor::XMLFeature(bestWay).GetName(), "Беллесбумпром", ()); + auto const bestWay = osm::GetBestOsmWay(osmResponse, geometry); + TEST_EQUAL(editor::XMLFeature(bestWay).GetName(), "Беллесбумпром", ()); + } + { + pugi::xml_document osmResponse; + TEST(osmResponse.load_buffer(osmRawResponseWay, ::strlen(osmRawResponseWay)), ()); + set geometry = { + MercatorBounds::FromLatLon(53.8975484, 27.557359), // diff + MercatorBounds::FromLatLon(53.8978710, 27.557681), + MercatorBounds::FromLatLon(53.8975034, 27.557764), // diff + MercatorBounds::FromLatLon(53.8977652, 27.557803), + MercatorBounds::FromLatLon(53.8975254, 27.557837), // diff + MercatorBounds::FromLatLon(53.8976570, 27.557661), + MercatorBounds::FromLatLon(53.8976041, 27.557318), // diff + }; + + auto const bestWay = osm::GetBestOsmWay(osmResponse, geometry); + TEST(!bestWay, ()); + } } UNIT_TEST(GetBestOsmNode_test) { - pugi::xml_document osmResponse; - TEST(osmResponse.load_buffer(osmRawResponse, ::strlen(osmRawResponse)), ()); + { + pugi::xml_document osmResponse; + TEST(osmResponse.load_buffer(osmRawResponseNode, ::strlen(osmRawResponseNode)), ()); - auto const bestWay = osm::GetBestOsmWay(osmResponse, geometry); - TEST_EQUAL(editor::XMLFeature(bestWay).GetCenter(), "Беллесбумпром", ()); + auto const bestNode = osm::GetBestOsmNode(osmResponse, ms::LatLon(53.8977398, 27.5579251)); + TEST_EQUAL(editor::XMLFeature(bestNode).GetName(), "Главное управление капитального строительства", ()); + } + { + pugi::xml_document osmResponse; + TEST(osmResponse.load_buffer(osmRawResponseNode, ::strlen(osmRawResponseNode)), ()); + auto const bestNode = osm::GetBestOsmNode(osmResponse, ms::LatLon(53.8978398, 27.5579251)); + TEST(bestNode, ()); + } } diff --git a/editor/osm_feature_matcher.cpp b/editor/osm_feature_matcher.cpp index 92f835299a..989f7da3b9 100644 --- a/editor/osm_feature_matcher.cpp +++ b/editor/osm_feature_matcher.cpp @@ -4,16 +4,18 @@ namespace osm { +double constexpr kPointDiffEps = MercatorBounds::GetCellID2PointAbsEpsilon(); + bool LatLonEqual(ms::LatLon const & a, ms::LatLon const & b) { - double constexpr eps = MercatorBounds::GetCellID2PointAbsEpsilon(); - return a.EqualDxDy(b, eps); + return a.EqualDxDy(b, kPointDiffEps); } double ScoreLatLon(XMLFeature const & xmlFt, ms::LatLon const & latLon) { - // TODO: Find proper score values; - return LatLonEqual(latLon, xmlFt.GetCenter()) ? 10 : -10; + auto const a = MercatorBounds::FromLatLon(xmlFt.GetCenter()); + auto const b = MercatorBounds::FromLatLon(latLon); + return 1.0 - (a.Length(b) / kPointDiffEps); } double ScoreGeometry(pugi::xml_document const & osmResponse, pugi::xml_node const & way, @@ -31,7 +33,7 @@ double ScoreGeometry(pugi::xml_document const & osmResponse, pugi::xml_node cons { ++total; string const nodeRef = xNodeRef.attribute().value(); - auto const node = osmResponse.select_node(("node[@ref='" + nodeRef + "']").data()).node(); + auto const node = osmResponse.select_node(("osm/node[@id='" + nodeRef + "']").data()).node(); if (!node) { LOG(LDEBUG, ("OSM response have ref", nodeRef, @@ -46,93 +48,14 @@ double ScoreGeometry(pugi::xml_document const & osmResponse, pugi::xml_node cons { ++matched; geometry.erase(pointIt); + break; } } } - return static_cast(matched) / total * 100; + return static_cast(matched) / total - 0.5; } -// double ScoreNames(XMLFeature const & xmlFt, StringUtf8Multilang const & names) -// { -// double score = 0; -// names.ForEach([&score, &xmlFt](uint8_t const langCode, string const & name) -// { -// if (xmlFt.GetName(langCode) == name) -// score += 1; -// return true; -// }); - -// // TODO(mgsergio): Deafult name match should have greater wieght. Should it? - -// return score; -// } - -// vector GetOsmOriginalTagsForType(feature::Metadata::EType const et) -// { -// static multimap const kFmd2Osm = { -// {feature::Metadata::EType::FMD_CUISINE, "cuisine"}, -// {feature::Metadata::EType::FMD_OPEN_HOURS, "opening_hours"}, -// {feature::Metadata::EType::FMD_PHONE_NUMBER, "phone"}, -// {feature::Metadata::EType::FMD_PHONE_NUMBER, "contact:phone"}, -// {feature::Metadata::EType::FMD_FAX_NUMBER, "fax"}, -// {feature::Metadata::EType::FMD_FAX_NUMBER, "contact:fax"}, -// {feature::Metadata::EType::FMD_STARS, "stars"}, -// {feature::Metadata::EType::FMD_OPERATOR, "operator"}, -// {feature::Metadata::EType::FMD_URL, "url"}, -// {feature::Metadata::EType::FMD_WEBSITE, "website"}, -// {feature::Metadata::EType::FMD_WEBSITE, "contact:website"}, -// // TODO: {feature::Metadata::EType::FMD_INTERNET, ""}, -// {feature::Metadata::EType::FMD_ELE, "ele"}, -// {feature::Metadata::EType::FMD_TURN_LANES, "turn:lanes"}, -// {feature::Metadata::EType::FMD_TURN_LANES_FORWARD, "turn:lanes:forward"}, -// {feature::Metadata::EType::FMD_TURN_LANES_BACKWARD, "turn:lanes:backward"}, -// {feature::Metadata::EType::FMD_EMAIL, "email"}, -// {feature::Metadata::EType::FMD_EMAIL, "contact:email"}, -// {feature::Metadata::EType::FMD_POSTCODE, "addr:postcode"}, -// {feature::Metadata::EType::FMD_WIKIPEDIA, "wikipedia"}, -// {feature::Metadata::EType::FMD_MAXSPEED, "maxspeed"}, -// {feature::Metadata::EType::FMD_FLATS, "addr:flats"}, -// {feature::Metadata::EType::FMD_HEIGHT, "height"}, -// {feature::Metadata::EType::FMD_MIN_HEIGHT, "min_height"}, -// {feature::Metadata::EType::FMD_MIN_HEIGHT, "building:min_level"}, -// {feature::Metadata::EType::FMD_DENOMINATION, "denomination"}, -// {feature::Metadata::EType::FMD_BUILDING_LEVELS, "building:levels"}, -// }; - -// vector result; -// auto const range = kFmd2Osm.equal_range(et); -// for (auto it = range.first; it != range.second; ++it) -// result.emplace_back(it->second); - -// return result; -// } - -// double ScoreMetadata(XMLFeature const & xmlFt, feature::Metadata const & metadata) -// { -// double score = 0; - -// for (auto const type : metadata.GetPresentTypes()) -// { -// for (auto const osm_tag : GetOsmOriginalTagsForType(static_cast(type))) -// { -// if (xmlFt.GetTagValue(osm_tag) == metadata.Get(type)) -// { -// score += 1; -// break; -// } -// } -// } - -// return score; -// } - -// bool TypesEqual(XMLFeature const & xmlFt, feature::TypesHolder const & types) -// { -// // TODO: Use mapcss-mapping.csv to correctly match types. -// return true; -// } - pugi::xml_node GetBestOsmNode(pugi::xml_document const & osmResponse, ms::LatLon const latLon) { double bestScore = -1; @@ -144,6 +67,7 @@ pugi::xml_node GetBestOsmNode(pugi::xml_document const & osmResponse, ms::LatLon { XMLFeature xmlFt(xNode.node()); + // TODO: // if (!TypesEqual(xmlFt, feature::TypesHolder(ft))) // continue; @@ -151,6 +75,7 @@ pugi::xml_node GetBestOsmNode(pugi::xml_document const & osmResponse, ms::LatLon if (nodeScore < 0) continue; + // TODO: // nodeScore += ScoreNames(xmlFt, ft.GetNames()); // nodeScore += ScoreMetadata(xmlFt, ft.GetMetadata()); @@ -167,8 +92,8 @@ pugi::xml_node GetBestOsmNode(pugi::xml_document const & osmResponse, ms::LatLon } } - // TODO(mgsergio): Add a properly defined threshold. - // if (bestScore < minimumScoreThreshold) + // TODO(mgsergio): Add a properly defined threshold when more fields will be compared. + // if (bestScore < kMiniScoreThreshold) // return pugi::xml_node; return bestMatchNode; @@ -185,6 +110,7 @@ pugi::xml_node GetBestOsmWay(pugi::xml_document const & osmResponse, set geometry); -// double ScoreNames(XMLFeature const & xmlFt, StringUtf8Multilang const & names); - -// vector GetOsmOriginalTagsForType(feature::Metadata::EType const et); - -// double ScoreMetadata(XMLFeature const & xmlFt, feature::Metadata const & metadata); - -// bool TypesEqual(XMLFeature const & xmlFt, feature::TypesHolder const & types); - pugi::xml_node GetBestOsmNode(pugi::xml_document const & osmResponse, ms::LatLon const latLon); pugi::xml_node GetBestOsmWay(pugi::xml_document const & osmResponse, set const & geometry); diff --git a/indexer/osm_editor.cpp b/indexer/osm_editor.cpp index d737501e54..a517a8270b 100644 --- a/indexer/osm_editor.cpp +++ b/indexer/osm_editor.cpp @@ -262,6 +262,9 @@ XMLFeature GetMatchingFeatureFromOSM(osm::ChangesetWrapper & cw, geometry.insert(p1); geometry.insert(p2); geometry.insert(p3); + // Warning: geometry is cached in FeatureTyped. So if it wasn't BEST_GEOMETRY + // We can never have it. Features here came from editors loader and should + // have BEST_GEOMETRY geometry. }, FeatureType::BEST_GEOMETRY); ASSERT_GREATER_OR_EQUAL(geometry.size(), 3, ("Is it an area feature?"));