From c94745676714ee4744161b2db5e27cfed213907c Mon Sep 17 00:00:00 2001
From: Yuri Gorshenin <y@maps.me>
Date: Fri, 19 Aug 2016 16:46:58 +0300
Subject: [PATCH] Review fixes.

---
 search/nearby_points_sweeper.cpp              |  6 +--
 search/nearby_points_sweeper.hpp              | 13 +++++--
 search/pre_ranker.cpp                         |  7 +++-
 search/pre_ranker.hpp                         |  4 +-
 .../nearby_points_sweeper_test.cpp            | 37 +++++++++++++++++++
 5 files changed, 57 insertions(+), 10 deletions(-)

diff --git a/search/nearby_points_sweeper.cpp b/search/nearby_points_sweeper.cpp
index cc7b92712e..0ec0bf9bfd 100644
--- a/search/nearby_points_sweeper.cpp
+++ b/search/nearby_points_sweeper.cpp
@@ -23,11 +23,11 @@ bool NearbyPointsSweeper::Event::operator<(Event const & rhs) const
 }
 
 // NearbyPointsSweeper -----------------------------------------------------------------------------
-NearbyPointsSweeper::NearbyPointsSweeper(double eps) : m_eps(eps) {}
+NearbyPointsSweeper::NearbyPointsSweeper(double eps) : m_eps(eps), m_heps(max(eps * 0.5, 0.0)) {}
 
 void NearbyPointsSweeper::Add(double x, double y, size_t index)
 {
-  m_events.emplace_back(Event::TYPE_SEGMENT_START, y - m_eps * 0.5, x, index);
-  m_events.emplace_back(Event::TYPE_SEGMENT_END, y + m_eps * 0.5, x, index);
+  m_events.emplace_back(Event::TYPE_SEGMENT_START, y - m_heps, x, index);
+  m_events.emplace_back(Event::TYPE_SEGMENT_END, y + m_heps, x, index);
 }
 }  // namespace search
diff --git a/search/nearby_points_sweeper.hpp b/search/nearby_points_sweeper.hpp
index 9fbeef7228..3bf6db6e2b 100644
--- a/search/nearby_points_sweeper.hpp
+++ b/search/nearby_points_sweeper.hpp
@@ -11,10 +11,14 @@
 
 namespace search
 {
-// This class can be used to sweep points on a plane that are too
-// close to each other.  Two points are considered to be "too close"
-// when Manhattan distance between them is less than or equal to some
-// preselected epsilon.
+// This class can be used to greedily sweep points on a plane that are
+// too close to each other.  Two points are considered to be "too
+// close" when Manhattan distance between them is less than or equal
+// to some preselected epsilon. Note, the result is not the largest
+// subset of points that can be selected, but it can be computed quite
+// fast and gives satisfactory results.
+//
+// *NOTE* The class is NOT thread-safe.
 class NearbyPointsSweeper
 {
 public:
@@ -120,5 +124,6 @@ private:
 
   vector<Event> m_events;
   double const m_eps;
+  double const m_heps;
 };
 }  // namespace search
diff --git a/search/pre_ranker.cpp b/search/pre_ranker.cpp
index ed006f9eb3..9a6898eab6 100644
--- a/search/pre_ranker.cpp
+++ b/search/pre_ranker.cpp
@@ -228,7 +228,12 @@ void PreRanker::UpdateResults(bool lastUpdate)
     m_currEmit.swap(m_prevEmit);
 }
 
-void PreRanker::ClearCaches() { m_pivotFeatures.Clear(); }
+void PreRanker::ClearCaches()
+{
+  m_pivotFeatures.Clear();
+  m_prevEmit.clear();
+  m_currEmit.clear();
+}
 
 void PreRanker::FilterForViewportSearch()
 {
diff --git a/search/pre_ranker.hpp b/search/pre_ranker.hpp
index 1e51574cf2..7ef8b14682 100644
--- a/search/pre_ranker.hpp
+++ b/search/pre_ranker.hpp
@@ -95,10 +95,10 @@ private:
   // Cache of nested rects used to estimate distance from a feature to the pivot.
   NestedRectsCache m_pivotFeatures;
 
-  // A set of ids for features that are emit during the current search session.
+  // A set of ids for features that are emitted during the current search session.
   set<FeatureID> m_currEmit;
 
-  // A set of ids for features that were emit during the previous
+  // A set of ids for features that were emitted during the previous
   // search session.  They're used for filtering of current search in
   // viewport results, because we need to give more priority to
   // results that were on map previously.
diff --git a/search/search_tests/nearby_points_sweeper_test.cpp b/search/search_tests/nearby_points_sweeper_test.cpp
index 9cf99f8c3f..6b6e4a803c 100644
--- a/search/search_tests/nearby_points_sweeper_test.cpp
+++ b/search/search_tests/nearby_points_sweeper_test.cpp
@@ -79,5 +79,42 @@ UNIT_TEST(NearbyPointsSweeper_Smoke)
     TEST_EQUAL(expected, actual, ());
   }
 }
+
+UNIT_TEST(NearbyPointsSweeper_CornerCases)
+{
+  vector<m2::PointD> const points = {m2::PointD(0, 0),     m2::PointD(0, 0), m2::PointD(1, 0),
+                                     m2::PointD(0, 1),     m2::PointD(1, 1), m2::PointD(1, 0),
+                                     m2::PointD(0.5, 0.5), m2::PointD(0, 1)};
+
+  {
+    // In accordance with the specification, all points must be left
+    // as is when epsilon is negative.
+    NearbyPointsSweeper sweeper(-1.0);
+
+    TIndexSet expected;
+    for (size_t i = 0; i < points.size(); ++i)
+    {
+      sweeper.Add(points[i].x, points[i].y, i);
+      expected.insert(i);
+    }
+
+    TIndexSet actual;
+    sweeper.Sweep(MakeInsertFunctor(actual));
+
+    TEST_EQUAL(expected, actual, ());
+  }
+
+  {
+    NearbyPointsSweeper sweeper(10.0);
+    for (size_t i = 0; i < points.size(); ++i)
+      sweeper.Add(points[i].x, points[i].y, i);
+
+    TIndexSet expected = {0};
+    TIndexSet actual;
+    sweeper.Sweep(MakeInsertFunctor(actual));
+
+    TEST_EQUAL(expected, actual, ());
+  }
+}
 }  // namespace
 }  // namespace search