Framework POI getters refactoring.

3 files changed, 50 insertions, 39 deletions

diff --git a/map/address_finder.cpp b/map/address_finder.cpp
index 01e844ebd9..58d2d81794 100644
--- a/map/address_finder.cpp
+++ b/map/address_finder.cpp
@@ -474,19 +474,32 @@ void Framework::GetAddressInfoForGlobalPoint(m2::PointD const & pt, search::Addr
   //GetLocality(pt, info);
 }
 
-void Framework::GetAddressInfo(FeatureType const & ft, m2::PointD const & pt, search::AddressInfo & info) const
+search::AddressInfo Framework::GetPOIAddressInfo(FeatureType const & ft) const
 {
-  info.Clear();
+  search::AddressInfo info;
+
+  ASSERT_NOT_EQUAL(ft.GetFeatureType(), feature::GEOM_LINE, ());
+  m2::PointD const center = feature::GetCenter(ft);
+
+  info.m_country = GetCountryName(center);
+  if (info.m_country.empty())
+  {
+    LOG(LINFO, ("Can't find region for point ", center));
+    return info;
+  }
 
   double const inf = numeric_limits<double>::max();
   double addressR[] = { inf, inf, inf };
 
   // FeatureType::WORST_GEOMETRY - no need to check on visibility
-  DoGetAddressInfo getAddress(pt, FeatureType::WORST_GEOMETRY, GetChecker(), addressR);
+  DoGetAddressInfo getAddress(center, FeatureType::WORST_GEOMETRY, GetChecker(), addressR);
   getAddress(ft);
   getAddress.FillAddress(m_searchEngine.get(), info);
 
-  GetAddressInfoForGlobalPoint(pt, info);
+  /// @todo Temporarily commented - it's slow and not used in UI
+  //GetLocality(pt, info);
+
+  return info;
 }
 
 void Framework::GetLocality(m2::PointD const & pt, search::AddressInfo & info) const
diff --git a/map/framework.cpp b/map/framework.cpp
index 5a574cf242..d65c19100a 100644
--- a/map/framework.cpp
+++ b/map/framework.cpp
@@ -1628,11 +1628,13 @@ bool Framework::ShowMapForURL(string const & url)
 bool Framework::GetVisiblePOI(m2::PointD const & glbPoint, search::AddressInfo & info, feature::Metadata & metadata) const
 {
   ASSERT(m_drapeEngine != nullptr, ());
-  FeatureID id = m_drapeEngine->GetVisiblePOI(glbPoint);
+  FeatureID const id = m_drapeEngine->GetVisiblePOI(glbPoint);
   if (!id.IsValid())
     return false;
 
-  GetVisiblePOI(id, info, metadata);
+  FeatureType const feature = GetPOIByID(id);
+  metadata = feature.GetMetadata();
+  info = GetPOIAddressInfo(feature);
   return true;
 }
 
@@ -1643,33 +1645,25 @@ bool Framework::GetVisiblePOI(m2::PointD const & ptMercator, FeatureType & outPO
   if (!fid.IsValid())
     return false;
 
-  // Note: all parse methods should be called with guard alive.
-  Index::FeaturesLoaderGuard guard(m_model.GetIndex(), fid.m_mwmId);
-  guard.GetFeatureByIndex(fid.m_index, outPOI);
-  outPOI.ParseHeader2();
-  outPOI.ParseGeometry(FeatureType::BEST_GEOMETRY);
-  outPOI.ParseTriangles(FeatureType::BEST_GEOMETRY);
-  outPOI.ParseMetadata();
+  outPOI = GetPOIByID(fid);
+
   return true;
 }
 
-m2::PointD Framework::GetVisiblePOI(FeatureID const & id, search::AddressInfo & info, feature::Metadata & metadata) const
+FeatureType Framework::GetPOIByID(FeatureID const & fid) const
 {
-  ASSERT(id.IsValid(), ());
-  Index::FeaturesLoaderGuard guard(m_model.GetIndex(), id.m_mwmId);
-
-  FeatureType ft;
-  guard.GetFeatureByIndex(id.m_index, ft);
+  ASSERT(fid.IsValid(), ());
 
-  ft.ParseMetadata();
-  metadata = ft.GetMetadata();
-
-  ASSERT_NOT_EQUAL(ft.GetFeatureType(), feature::GEOM_LINE, ());
-  m2::PointD const center = feature::GetCenter(ft);
-
-  GetAddressInfo(ft, center, info);
+  FeatureType feature;
+  // 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.ParseHeader2();
+  feature.ParseGeometry(FeatureType::BEST_GEOMETRY);
+  feature.ParseTriangles(FeatureType::BEST_GEOMETRY);
+  feature.ParseMetadata();
 
-  return m_currentModelView.isPerspective() ? GtoP3d(center) : GtoP(center);
+  return feature;
 }
 
 namespace
@@ -1818,7 +1812,7 @@ void Framework::InvalidateUserMarks()
   }
 }
 
-void Framework::OnTapEvent(m2::PointD pxPoint, bool isLong, bool isMyPosition, FeatureID const & feature)
+void Framework::OnTapEvent(m2::PointD pxPoint, bool isLong, bool isMyPosition, FeatureID const & fid)
 {
   // Back up last tap event to recover selection in case of Drape reinitialization.
   if (!m_lastTapEvent)
@@ -1826,9 +1820,9 @@ void Framework::OnTapEvent(m2::PointD pxPoint, bool isLong, bool isMyPosition, F
   m_lastTapEvent->m_pxPoint = pxPoint;
   m_lastTapEvent->m_isLong = isLong;
   m_lastTapEvent->m_isMyPosition = isMyPosition;
-  m_lastTapEvent->m_feature = feature;
+  m_lastTapEvent->m_feature = fid;
 
-  UserMark const * mark = OnTapEventImpl(pxPoint, isLong, isMyPosition, feature);
+  UserMark const * mark = OnTapEventImpl(pxPoint, isLong, isMyPosition, fid);
 
   {
     alohalytics::TStringMap details {{"isLongPress", isLong ? "1" : "0"}};
@@ -1851,7 +1845,7 @@ void Framework::InvalidateRendering()
     m_drapeEngine->Invalidate();
 }
 
-UserMark const * Framework::OnTapEventImpl(m2::PointD pxPoint, bool isLong, bool isMyPosition, FeatureID const & feature)
+UserMark const * Framework::OnTapEventImpl(m2::PointD pxPoint, bool isLong, bool isMyPosition, FeatureID const & fid)
 {
   m2::PointD const pxPoint2d = m_currentModelView.P3dtoP(pxPoint);
 
@@ -1887,26 +1881,29 @@ UserMark const * Framework::OnTapEventImpl(m2::PointD pxPoint, bool isLong, bool
     return mark;
 
   bool needMark = false;
-  m2::PointD pxPivot;
+  m2::PointD mercatorPivot;
   search::AddressInfo info;
   feature::Metadata metadata;
 
-  if (feature.IsValid())
+  if (fid.IsValid())
   {
-    pxPivot = GetVisiblePOI(feature, info, metadata);
+    FeatureType const feature = GetPOIByID(fid);
+    mercatorPivot = feature::GetCenter(feature);
+    info = GetPOIAddressInfo(feature);
+    metadata = feature.GetMetadata();
     needMark = true;
   }
   else if (isLong)
   {
-    GetAddressInfoForPixelPoint(pxPoint2d, info);
-    pxPivot = pxPoint;
+    GetAddressInfoForPixelPoint(pxPoint, info);
+    mercatorPivot = m_currentModelView.PtoG(pxPoint);
     needMark = true;
   }
 
   if (needMark)
   {
     PoiMarkPoint * poiMark = UserMarkContainer::UserMarkForPoi();
-    poiMark->SetPtOrg(m_currentModelView.PtoG(m_currentModelView.P3dtoP(pxPivot)));
+    poiMark->SetPtOrg(mercatorPivot);
     poiMark->SetInfo(info);
     poiMark->SetMetadata(move(metadata));
     return poiMark;
diff --git a/map/framework.hpp b/map/framework.hpp
index 43ac3e7778..2d5329d1f9 100644
--- a/map/framework.hpp
+++ b/map/framework.hpp
@@ -481,14 +481,15 @@ public:
   //@}
 
 private:
-  void GetAddressInfo(FeatureType const & ft, m2::PointD const & pt, search::AddressInfo & info) const;
+  search::AddressInfo GetPOIAddressInfo(FeatureType const & ft) const;
   void GetLocality(m2::PointD const & pt, search::AddressInfo & info) const;
 
 public:
   bool GetVisiblePOI(m2::PointD const & glbPoint, search::AddressInfo & info, feature::Metadata & metadata) const;
   // TODO(AlexZ): Refactor similar getters to share common interface.
   bool GetVisiblePOI(m2::PointD const & ptMercator, FeatureType & outPOI) const;
-  m2::PointD GetVisiblePOI(FeatureID const & id, search::AddressInfo & info, feature::Metadata & metadata) const;
+  // TODO(AlexZ): Do we really need to avoid linear features?
+  FeatureType GetPOIByID(FeatureID const & fid) const;
   void FindClosestPOIMetadata(m2::PointD const & pt, feature::Metadata & metadata) const;
 
   void MemoryWarning();