Framework::GetAllFeaturesAtMercatorPoint().

map/framework.cpp

@@ -1687,53 +1687,74 @@ bool Framework::ShowMapForURL(string const & url)
   return false;
 }
 
-unique_ptr<FeatureType> Framework::GetFeatureAtMercatorPoint(m2::PointD const & mercator) const
+size_t Framework::ForEachFeatureAtMercatorPoint(TFeatureTypeFn && fn, m2::PointD const & mercator) const
 {
-  constexpr double const kRectWidthInMeters = 1.1;
-  int const kScale = scales::GetUpperScale();
-  m2::RectD const rect = MercatorBounds::RectByCenterXYAndSizeInMeters(mercator, kRectWidthInMeters);
-  unique_ptr<FeatureType> pointFt, areaFt;
-  double minPointDistance = kRectWidthInMeters;
-  auto const & isBuilding = ftypes::IsBuildingChecker::Instance();
-  m_model.ForEachFeature(rect, [&](FeatureType const & ft)
+  constexpr double kSelectRectWidthInMeters = 1.1;
+  constexpr double kMetersToLinearFeature = 3;
+  constexpr int kScale = scales::GetUpperScale();
+  m2::RectD const rect = MercatorBounds::RectByCenterXYAndSizeInMeters(mercator, kSelectRectWidthInMeters);
+  size_t processedFeaturesCount = 0;
+  m_model.ForEachFeature(rect, [&](FeatureType & ft)
   {
     switch (ft.GetFeatureType())
     {
-      case feature::GEOM_POINT:
+    case feature::GEOM_POINT:
+      if (rect.IsPointInside(ft.GetCenter()))
       {
-        double const distance = MercatorBounds::DistanceOnEarth(ft.GetCenter(), mercator);
-        if (distance < minPointDistance)
-        {
-          pointFt.reset(new FeatureType(ft));
-          pointFt->ParseMetadata();
-          minPointDistance = distance;
-        }
-        break;
+        fn(ft);
+        ++processedFeaturesCount;
       }
-      case feature::GEOM_AREA:
+      break;
+    case feature::GEOM_LINE:
+      if (feature::GetMinDistanceMeters(ft, mercator) < kMetersToLinearFeature)
       {
-        // Quick rough check.
-        if (!ft.GetLimitRect(kScale).IsPointInside(mercator))
-          return;
-        // Distance is 0.0 if point is inside area feature.
-        if (0.0 != feature::GetMinDistanceMeters(ft, mercator))
-          return;
-        // Buildings have higher priority over other types.
-        if (areaFt && isBuilding(*areaFt))
-          return;
-        areaFt.reset(new FeatureType(ft));
-        areaFt->ParseMetadata();
-        break;
+        fn(ft);
+        ++processedFeaturesCount;
       }
-      // TODO(AlexZ): At the moment, ignore linear features.
-      default: return;
+      break;
+    case feature::GEOM_AREA:
+      if (ft.GetLimitRect(kScale).IsPointInside(mercator) && feature::GetMinDistanceMeters(ft, mercator) == 0.0)
+      {
+        fn(ft);
+        ++processedFeaturesCount;
+      }
+      break;
+    case feature::GEOM_UNDEFINED:
+      ASSERT(false, ("case feature::GEOM_UNDEFINED"));
+      break;
     }
   }, kScale);
-
-  return pointFt ? move(pointFt) : move(areaFt);
+  return processedFeaturesCount;
 }
 
-unique_ptr<FeatureType> Framework::GetFeatureByID(FeatureID const & fid) const
+unique_ptr<FeatureType> Framework::GetFeatureAtMercatorPoint(m2::PointD const & mercator) const
+{
+  unique_ptr<FeatureType> poi, line, area;
+  ForEachFeatureAtMercatorPoint([&](FeatureType & ft)
+  {
+    switch (ft.GetFeatureType())
+    {
+    case feature::GEOM_POINT:
+      poi.reset(new FeatureType(ft));
+      break;
+    case feature::GEOM_LINE:
+      line.reset(new FeatureType(ft));
+      break;
+    case feature::GEOM_AREA:
+      // Buildings have higher priority over other types.
+      if (area && ftypes::IsBuildingChecker::Instance()(*area))
+        return;
+      area.reset(new FeatureType(ft));
+      break;
+    case feature::GEOM_UNDEFINED:
+      ASSERT(false, ("case feature::GEOM_UNDEFINED"));
+      break;
+    }
+  }, mercator);
+  return poi ? move(poi) : (area ? move(area) : move(line));
+}
+
+unique_ptr<FeatureType> Framework::GetFeatureByID(FeatureID const & fid, bool parse) const
 {
   ASSERT(fid.IsValid(), ());
 
@@ -1741,7 +1762,8 @@ unique_ptr<FeatureType> Framework::GetFeatureByID(FeatureID const & fid) const
   // Note: all parse methods should be called with guard alive.
   Index::FeaturesLoaderGuard guard(m_model.GetIndex(), fid.m_mwmId);
   guard.GetFeatureByIndex(fid.m_index, *feature);
-  feature->ParseEverything();
+  if (parse)
+    feature->ParseEverything();
   return feature;
 }
 

map/framework.hpp

@@ -490,12 +490,13 @@ public:
   /// If it does have explicit street name in OSM, it goes first in the returned vector.
   /// @returns empty vector if no named streets were found around feature.
   vector<string> GetNearbyFeatureStreets(FeatureType const & ft) const;
-  /// Get feature at given point even if it's invisible on the screen.
-  /// TODO(AlexZ): Refactor out other similar methods.
+  /// Get "best for the user" feature at given point even if it's invisible on the screen.
   /// @returns nullptr if no feature was found at the given mercator point.
   unique_ptr<FeatureType> GetFeatureAtMercatorPoint(m2::PointD const & mercator) const;
-  // TODO(AlexZ): Do we really need to avoid linear features?
-  unique_ptr<FeatureType> GetFeatureByID(FeatureID const & fid) const;
+  using TFeatureTypeFn = function<void(FeatureType &)>;
+  size_t ForEachFeatureAtMercatorPoint(TFeatureTypeFn && fn, m2::PointD const & mercator) const;
+  /// Set parse to false if you don't need all feature fields ready.
+  unique_ptr<FeatureType> GetFeatureByID(FeatureID const & fid, bool parse = true) const;
 
   void MemoryWarning();
   void EnterBackground();

map/map_tests/feature_getters_tests.cpp

@@ -0,0 +1,56 @@
+#include "testing/testing.hpp"
+
+#include "map/framework.hpp"
+
+#include "indexer/classificator.hpp"
+
+#include "platform/local_country_file.hpp"
+
+#include "geometry/mercator.hpp"
+
+#include "std/algorithm.hpp"
+#include "std/vector.hpp"
+
+UNIT_TEST(Framework_ForEachFeatureAtMercatorPoint_And_Others)
+{
+  Framework frm;
+  frm.DeregisterAllMaps();
+  frm.RegisterMap(platform::LocalCountryFile::MakeForTesting("minsk-pass"));
+
+  vector<char const *> types =
+  {
+    "highway|footway|",
+    "hwtag|yesfoot|",
+    "highway|service|parking_aisle|",
+    "amenity|parking|",
+    "barrier|lift_gate|"
+  };
+  TEST_EQUAL(4, frm.ForEachFeatureAtMercatorPoint([&](FeatureType & ft)
+  {
+    ft.ForEachType([&types](uint32_t type)
+    {
+      string const strType = classif().GetFullObjectName(type);
+      auto found = find(types.begin(), types.end(), strType);
+      TEST(found != types.end(), ());
+      types.erase(found);
+    });
+  }, MercatorBounds::FromLatLon(53.882663, 27.537788)), ());
+  TEST_EQUAL(0, types.size(), ());
+
+  ftypes::IsBuildingChecker const & isBuilding = ftypes::IsBuildingChecker::Instance();
+  {
+    // Restaurant in the building.
+    auto const feature = frm.GetFeatureAtMercatorPoint(MercatorBounds::FromLatLon(53.89395, 27.567365));
+    string name;
+    TEST(feature->GetName(FeatureType::DEFAULT_LANG, name), ());
+    TEST_EQUAL("Родны Кут", name, ());
+    TEST(!isBuilding(*feature), ());
+  }
+
+  {
+    // Same building as above, very close to the restaurant.
+    auto const feature = frm.GetFeatureAtMercatorPoint(MercatorBounds::FromLatLon(53.893603, 27.567032));
+    TEST(feature, ());
+    TEST(isBuilding(*feature), ());
+  }
+}

map/map_tests/map_tests.pro

@@ -30,6 +30,7 @@ SOURCES += \
   ../../testing/testingmain.cpp \
   address_tests.cpp \
   bookmarks_test.cpp \
+  feature_getters_tests.cpp \
   ge0_parser_tests.cpp  \
   geourl_test.cpp \
   gps_track_collection_test.cpp \

qt/draw_widget.cpp

@@ -494,19 +494,24 @@ void DrawWidget::ShowInfoPopup(QMouseEvent * e, m2::PointD const & pt)
     menu.addAction(QString::fromUtf8(s.c_str()));
   };
 
-  search::AddressInfo const info = m_framework->GetMercatorAddressInfo(m_framework->PtoG(pt));
-  for (auto const & type : info.m_types)
-    addStringFn(type);
-
-  menu.addSeparator();
-
-  if (!info.m_name.empty())
+  m_framework->ForEachFeatureAtMercatorPoint([&](FeatureType const & ft)
   {
-    addStringFn(info.m_name);
-    menu.addSeparator();
-  }
+    search::AddressInfo const info = m_framework->GetFeatureAddressInfo(ft);
 
-  addStringFn(info.FormatAddress());
+    string concat;
+    for (auto const & type : info.m_types)
+      concat += type + " ";
+    addStringFn(concat);
+
+    if (!info.m_name.empty())
+      addStringFn(info.m_name);
+
+    string const addr = info.FormatAddress();
+    if (!addr.empty())
+      addStringFn(addr);
+
+    menu.addSeparator();
+  }, m_framework->PtoG(pt));
 
   menu.exec(e->pos());
 }