Review fixes.

base/clustering_map.hpp

@@ -67,13 +67,13 @@ public:
   template <typename Fn>
   void ForEachCluster(Fn && fn)
   {
-    struct EntryWithKey : public std::less<Entry const *>
+    struct EntryWithKey
     {
       EntryWithKey(Entry const * entry, Key const & key) : m_entry(entry), m_key(key) {}
 
       bool operator<(EntryWithKey const & rhs) const
       {
-        return std::less<Entry const *>::operator()(m_entry, rhs.m_entry);
+        return m_entry->m_root < rhs.m_entry->m_root;
       }
 
       Entry const * m_entry;
@@ -89,7 +89,6 @@ public:
     }
     std::sort(eks.begin(), eks.end());
 
-    std::equal_to<Entry const *> equal;
     size_t i = 0;
     while (i < eks.size())
     {
@@ -97,7 +96,7 @@ public:
       keys.push_back(eks[i].m_key);
 
       size_t j = i + 1;
-      while (j < eks.size() && equal(eks[i].m_entry, eks[j].m_entry))
+      while (j < eks.size() && eks[i].m_entry->m_root == eks[j].m_entry->m_root)
       {
         keys.push_back(eks[j].m_key);
         ++j;

generator/generator_tool/generator_tool.cpp

@@ -77,7 +77,7 @@ DEFINE_bool(generate_search_index, false, "5th pass - generate search index.");
 
 DEFINE_bool(dump_cities_boundaries, false, "Dump cities boundaries to a file");
 DEFINE_bool(generate_cities_boundaries, false, "Generate cities boundaries section");
-DEFINE_string(cities_boundaries_path, "", "Path to collect cities boundaries");
+DEFINE_string(cities_boundaries_data, "", "File with cities boundaries");
 
 DEFINE_bool(generate_world, false, "Generate separate world file.");
 DEFINE_bool(split_by_polygons, false,
@@ -229,12 +229,12 @@ int main(int argc, char ** argv)
 
     if (FLAGS_dump_cities_boundaries)
     {
-      CHECK(!FLAGS_cities_boundaries_path.empty(), ());
-      LOG(LINFO, ("Dumping cities boundaries to", FLAGS_cities_boundaries_path));
-      if (!generator::SerializeBoundariesTable(FLAGS_cities_boundaries_path,
+      CHECK(!FLAGS_cities_boundaries_data.empty(), ());
+      LOG(LINFO, ("Dumping cities boundaries to", FLAGS_cities_boundaries_data));
+      if (!generator::SerializeBoundariesTable(FLAGS_cities_boundaries_data,
                                                *genInfo.m_boundariesTable))
       {
-        LOG(LCRITICAL, ("Error serializing boundaries table to", FLAGS_cities_boundaries_path));
+        LOG(LCRITICAL, ("Error serializing boundaries table to", FLAGS_cities_boundaries_data));
       }
     }
   }
@@ -308,10 +308,10 @@ int main(int argc, char ** argv)
 
     if (FLAGS_generate_cities_boundaries)
     {
-      CHECK(!FLAGS_cities_boundaries_path.empty(), ());
+      CHECK(!FLAGS_cities_boundaries_data.empty(), ());
       LOG(LINFO, ("Generating cities boundaries for", datFile));
       generator::OsmIdToBoundariesTable table;
-      if (!generator::DeserializeBoundariesTable(FLAGS_cities_boundaries_path, table))
+      if (!generator::DeserializeBoundariesTable(FLAGS_cities_boundaries_data, table))
         LOG(LCRITICAL, ("Error deserializing boundaries table"));
       if (!generator::BuildCitiesBoundaries(datFile, osmToFeatureFilename, table))
         LOG(LCRITICAL, ("Error generating cities boundaries."));

indexer/cities_boundaries_serdes.hpp

@@ -86,30 +86,11 @@ public:
 
       ASSERT_EQUAL(ps.size(), 4, ());
 
-      size_t bestCurr = ps.size();
-      double bestLength = -1;
-      for (size_t curr = 0; curr < ps.size(); ++curr)
-      {
-        size_t const next = (curr + 1) % ps.size();
-
-        auto const length = ps[curr].Length(ps[next]);
-        if (length > bestLength)
-        {
-          bestCurr = curr;
-          bestLength = length;
-        }
-      }
-
-      CHECK(bestCurr != ps.size(), ());
-      std::rotate(ps.begin(), ps.begin() + bestCurr, ps.end());
-
       auto const us = ToU(ps);
 
       (*this)(us[0]);
       EncodeDelta(us[0], us[1]);
-
-      uint64_t const width = us[3].Length(us[0]);
-      WriteVarUint(m_sink, width);
+      EncodeDelta(us[0], us[3]);
     }
 
     void operator()(m2::DiamondBox const & dbox)
@@ -246,24 +227,16 @@ public:
 
     void operator()(m2::CalipersBox & cbox)
     {
-      m2::PointU pivot;
-      (*this)(pivot);
+      std::vector<m2::PointU> us(4);
+      (*this)(us[0]);
+      us[1] = DecodeDelta(us[0]);
+      us[3] = DecodeDelta(us[0]);
 
-      std::vector<m2::PointU> points(4);
-      points[0] = pivot;
-      points[1] = DecodeDelta(pivot);
+      auto ps = FromU(us);
+      auto const dp = ps[3] - ps[0];
+      ps[2] = ps[1] + dp;
 
-      auto const width = ReadVarUint<uint64_t>(m_source);
-
-      auto r01 = m2::PointD(points[1] - points[0]);
-      if (!r01.IsAlmostZero())
-        r01 = r01.Normalize();
-      auto const r21 = r01.Ort() * width;
-
-      points[2] = points[1] + r21;
-      points[3] = points[0] + r21;
-
-      cbox = m2::CalipersBox(FromU(points));
+      cbox = m2::CalipersBox(ps);
     }
 
     void operator()(m2::DiamondBox & dbox)

indexer/indexer_tests/cities_boundaries_serdes_tests.cpp

@@ -1,7 +1,7 @@
 #include "testing/testing.hpp"
 
-#include "indexer/city_boundary.hpp"
 #include "indexer/cities_boundaries_serdes.hpp"
+#include "indexer/city_boundary.hpp"
 #include "indexer/coding_params.hpp"
 
 #include "coding/reader.hpp"
@@ -127,4 +127,36 @@ UNIT_TEST(CitiesBoundariesSerDes_Smoke)
     TestEncodeDecode(expected);
   }
 }
+
+UNIT_TEST(CitiesBoundaries_Moscow)
+{
+  vector<m2::PointD> const points = {{37.04001, 67.55964},
+                                     {37.55650, 66.96428},
+                                     {38.02513, 67.37082},
+                                     {37.50865, 67.96618}};
+
+  m2::PointD const target(37.44765, 67.65243);
+
+  vector<uint8_t> buffer;
+  {
+    CityBoundary boundary(points);
+    TEST(boundary.HasPoint(target), ());
+
+    MemWriter<decltype(buffer)> sink(buffer);
+    CitiesBoundariesSerDes::Serialize(sink, {{boundary}});
+  }
+
+  {
+    Boundaries boundaries;
+    double precision;
+
+    MemReader reader(buffer.data(), buffer.size());
+    NonOwningReaderSource source(reader);
+    CitiesBoundariesSerDes::Deserialize(source, boundaries, precision);
+
+    TEST_EQUAL(boundaries.size(), 1, ());
+    TEST_EQUAL(boundaries[0].size(), 1, ());
+    TEST(boundaries[0][0].HasPoint(target, precision), ());
+  }
+}
 }  // namespace

search/cities_boundaries_table.hpp

@@ -6,6 +6,8 @@
 #include "geometry/point2d.hpp"
 
 #include <cstdint>
+#include <sstream>
+#include <string>
 #include <unordered_map>
 #include <utility>
 #include <vector>
@@ -41,6 +43,16 @@ public:
     // |*this|.
     bool HasPoint(m2::PointD const & p) const;
 
+    friend std::string DebugPrint(Boundaries const & boundaries)
+    {
+      std::ostringstream os;
+      os << "Boundaries [";
+      os << ::DebugPrint(boundaries.m_boundaries) << ", ";
+      os << "eps: " << boundaries.m_eps;
+      os << "]";
+      return os.str();
+    }
+
   private:
     std::vector<indexer::CityBoundary> m_boundaries;
     double m_eps = 0.0;

search/locality_finder.cpp

@@ -130,12 +130,14 @@ string DebugPrint(LocalityItem const & item)
   stringstream os;
   os << "Names = " << DebugPrint(item.m_names) << ", ";
   os << "Center = " << DebugPrint(item.m_center) << ", ";
-  os << "Population = " << item.m_population;
+  os << "Population = " << item.m_population << ", ";
+  os << "Boundaries = " << DebugPrint(item.m_boundaries);
   return os.str();
 }
 
 // LocalitySelector --------------------------------------------------------------------------------
 LocalitySelector::LocalitySelector(m2::PointD const & p) : m_p(p) {}
+
 void LocalitySelector::operator()(LocalityItem const & item)
 {
   auto const inside = item.m_boundaries.HasPoint(m_p);

tools/unix/generate_planet.sh

@@ -183,7 +183,7 @@ OPENTABLE_SCRIPT="$PYTHON_SCRIPTS_PATH/opentable_restaurants.py"
 OPENTABLE_FILE="${OPENTABLE_FILE:-$INTDIR/restaurants.csv}"
 VIATOR_SCRIPT="$PYTHON_SCRIPTS_PATH/viator_cities.py"
 VIATOR_FILE="${VIATOR_FILE:-$INTDIR/viator.csv}"
-CITIES_BOUNDARIES_FILE="${CITIES_BOUNDARIES_FILE:-$INTDIR/cities_boundaries}"
+CITIES_BOUNDARIES_DATA="${CITIES_BOUNDARIES_DATA:-$INTDIR/cities_boundaries.bin}"
 TESTING_SCRIPT="$SCRIPTS_PATH/test_planet.sh"
 PYTHON="$(which python2.7)"
 MWM_VERSION_FORMAT="%s"
@@ -468,7 +468,7 @@ if [ "$MODE" == "features" ]; then
                     --data_path="$TARGET" \
                     --user_resource_path="$DATA_PATH/" \
                     --dump_cities_boundaries \
-                    --cities_boundaries_path="$CITIES_BOUNDARIES_FILE" \
+                    --cities_boundaries_data="$CITIES_BOUNDARIES_DATA" \
                     $PARAMS_SPLIT 2>> "$PLANET_LOG"
   MODE=mwm
 fi
@@ -495,10 +495,11 @@ if [ "$MODE" == "mwm" ]; then
   if [ -n "$OPT_WORLD" ]; then
     (
       "$GENERATOR_TOOL" $PARAMS --output=World 2>> "$LOG_PATH/World.log"
-      "$GENERATOR_TOOL" --data_path="$TARGET" --user_resource_path="$DATA_PATH/" \
+      "$GENERATOR_TOOL" --data_path="$TARGET" \
+                        --user_resource_path="$DATA_PATH/" \
                         --generate_search_index \
                         --generate_cities_boundaries \
-                        --cities_boundaries_path="$CITIES_BOUNDARIES_FILE" \
+                        --cities_boundaries_data="$CITIES_BOUNDARIES_DATA" \
                         --output=World 2>> "$LOG_PATH/World.log"
     ) &
     "$GENERATOR_TOOL" $PARAMS --output=WorldCoasts 2>> "$LOG_PATH/WorldCoasts.log" &