Review fixes.

This commit is contained in:
Yuri Gorshenin 2015-10-09 17:35:42 +03:00 committed by Sergey Yershov
parent 3bcfa25915
commit 2bbd263956
11 changed files with 139 additions and 77 deletions

View file

@ -14,6 +14,8 @@ namespace scales
inline int GetUpperStyleScale() { return UPPER_STYLE_SCALE; }
/// Upper scales for World visible styles and indexer buckets.
inline int GetUpperWorldScale() { return 9; }
/// Upper scale level for countries.
inline int GetUpperCountryScale() { return GetUpperWorldScale() + 1; }
/// Upper scale for user comfort view (e.g. location zoom).
inline int GetUpperComfortScale() { return UPPER_STYLE_SCALE - 2; }
/// Default navigation mode scale

View file

@ -1020,7 +1020,7 @@ void Framework::InitCountryInfoGetter()
Platform const & platform = GetPlatform();
try
{
m_infoGetter.reset(new storage::CountryInfoGetter(platform.GetReader(PACKED_POLYGONS_FILE),
m_infoGetter.reset(new storage::CountryInfoReader(platform.GetReader(PACKED_POLYGONS_FILE),
platform.GetReader(COUNTRIES_FILE)));
}
catch (RootException const & e)

View file

@ -66,7 +66,7 @@ namespace integration
unique_ptr<storage::CountryInfoGetter> CreateCountryInfoGetter()
{
Platform const & platform = GetPlatform();
return make_unique<storage::CountryInfoGetter>(platform.GetReader(PACKED_POLYGONS_FILE),
return make_unique<storage::CountryInfoReader>(platform.GetReader(PACKED_POLYGONS_FILE),
platform.GetReader(COUNTRIES_FILE));
}

View file

@ -27,7 +27,7 @@ double const kInfinity = numeric_limits<double>::infinity();
// Maximum viewport scale level when we stop to expand viewport and
// simply return all rest features from mwms.
double const kMaxViewportScaleLevel = 10.0;
double const kMaxViewportScaleLevel = scales::GetUpperCountryScale();
// Upper bound on a number of features when fast path is used.
// Otherwise, slow path is used.

View file

@ -14,6 +14,9 @@
#include "search/search_tests_support/test_search_engine.hpp"
#include "search/search_tests_support/test_search_request.hpp"
#include "storage/country_decl.hpp"
#include "storage/country_info_getter.hpp"
#include "platform/local_country_file.hpp"
#include "platform/local_country_file_utils.hpp"
#include "platform/platform.hpp"
@ -163,6 +166,7 @@ bool MatchResults(Index const & index, vector<shared_ptr<MatchingRule>> rules,
for (auto const & u : unexpected)
os << " " << u << endl;
LOG(LWARNING, (os.str()));
return false;
}
@ -465,7 +469,16 @@ UNIT_TEST(Retrieval_CafeMTV)
builder.Add(*mtvCity);
}
TestSearchEngine engine("en");
m2::RectD const mskViewport(m2::PointD(0.99, -0.1), m2::PointD(1.01, 0.1));
m2::RectD const mtvViewport(m2::PointD(-1.1, -0.1), m2::PointD(-0.99, 0.1));
// There are test requests involving locality search, thus it's
// better to mock information about countries.
vector<storage::CountryDef> countries;
countries.emplace_back(msk.GetCountryName(), mskViewport);
countries.emplace_back(mtv.GetCountryName(), mtvViewport);
TestSearchEngine engine("en", make_unique<storage::CountryInfoGetterForTesting>(countries));
TEST_EQUAL(MwmSet::RegResult::Success, engine.RegisterMap(msk).second, ());
TEST_EQUAL(MwmSet::RegResult::Success, engine.RegisterMap(mtv).second, ());
TEST_EQUAL(MwmSet::RegResult::Success, engine.RegisterMap(testWorld).second, ());
@ -474,11 +487,8 @@ UNIT_TEST(Retrieval_CafeMTV)
auto const mtvId = engine.GetMwmIdByCountryFile(mtv.GetCountryFile());
auto const testWorldId = engine.GetMwmIdByCountryFile(testWorld.GetCountryFile());
m2::RectD const moscowViewport(m2::PointD(0.99, -0.1), m2::PointD(1.01, 0.1));
m2::RectD const mtvViewport(m2::PointD(-1.1, -0.1), m2::PointD(-0.99, 0.1));
{
TestSearchRequest request(engine, "Moscow ", "en", search::SearchParams::ALL, moscowViewport);
TestSearchRequest request(engine, "Moscow ", "en", search::SearchParams::ALL, mskViewport);
request.Wait();
initializer_list<shared_ptr<MatchingRule>> mskCityAlts = {

View file

@ -1301,37 +1301,15 @@ void Query::SearchAddress(Results & res)
// Candidates for search around locality. Initially filled
// with mwms containing city center.
TMWMVector candidateMwms;
auto localityMismatch = [&cityCenter, &rect, this](shared_ptr<MwmInfo> const & info)
TMWMVector localityMwms;
string const localityFile = m_infoGetter.GetRegionFile(cityCenter);
auto localityMismatch = [&localityFile](shared_ptr<MwmInfo> const & info)
{
return !info->m_limitRect.IsIntersect(rect);
return info->GetCountryName() != localityFile;
};
remove_copy_if(mwmsInfo.begin(), mwmsInfo.end(), back_inserter(candidateMwms),
remove_copy_if(mwmsInfo.begin(), mwmsInfo.end(), back_inserter(localityMwms),
localityMismatch);
auto boundingBoxCmp = [](shared_ptr<MwmInfo> const & lhs, shared_ptr<MwmInfo> const & rhs)
{
auto const & lhsBox = lhs->m_limitRect;
double const lhsSize = max(lhsBox.SizeX(), lhsBox.SizeY());
auto & rhsBox = rhs->m_limitRect;
double const rhsSize = max(rhsBox.SizeX(), rhsBox.SizeY());
return lhsSize < rhsSize;
};
// Candidate mwm for search around locality. Among all
// candidates the one with smallest dimensions is
// selected. This hack is used here to prevent
// interferention from mwms whose bounding boxes are too
// pessimisic, say, from -180 to +180 in Mercator
// coordiantes. This is the case for Russia_Far_Eastern.
auto const candidateMwmIt = min_element(candidateMwms.begin(), candidateMwms.end(), boundingBoxCmp);
if (candidateMwmIt != candidateMwms.end())
{
TMWMVector localityMwm = {*candidateMwmIt};
SearchFeaturesInViewport(params, localityMwm, LOCALITY_V);
}
SearchFeaturesInViewport(params, localityMwms, LOCALITY_V);
}
else
{

View file

@ -7,6 +7,8 @@
#include "search/search_query_factory.hpp"
#include "search/suggest.hpp"
#include "storage/country_info_getter.hpp"
#include "platform/platform.hpp"
#include "defines.hpp"
@ -49,12 +51,24 @@ namespace tests_support
{
TestSearchEngine::TestSearchEngine(string const & locale)
: m_platform(GetPlatform())
, m_infoGetter(m_platform.GetReader(PACKED_POLYGONS_FILE), m_platform.GetReader(COUNTRIES_FILE))
, m_engine(*this, m_platform.GetReader(SEARCH_CATEGORIES_FILE_NAME), m_infoGetter, locale,
, m_infoGetter(new storage::CountryInfoReader(m_platform.GetReader(PACKED_POLYGONS_FILE),
m_platform.GetReader(COUNTRIES_FILE)))
, m_engine(*this, m_platform.GetReader(SEARCH_CATEGORIES_FILE_NAME), *m_infoGetter, locale,
make_unique<TestSearchQueryFactory>())
{
}
TestSearchEngine::TestSearchEngine(string const & locale,
unique_ptr<storage::CountryInfoGetter> && infoGetter)
: m_platform(GetPlatform())
, m_infoGetter(move(infoGetter))
, m_engine(*this, m_platform.GetReader(SEARCH_CATEGORIES_FILE_NAME), *m_infoGetter, locale,
make_unique<TestSearchQueryFactory>())
{
}
TestSearchEngine::~TestSearchEngine() {}
weak_ptr<search::QueryHandle> TestSearchEngine::Search(search::SearchParams const & params,
m2::RectD const & viewport)
{

View file

@ -6,13 +6,16 @@
#include "search/search_engine.hpp"
#include "storage/country_info_getter.hpp"
#include "std/string.hpp"
#include "std/weak_ptr.hpp"
class Platform;
namespace storage
{
class CountryInfoGetter;
}
namespace search
{
class SearchParams;
@ -23,13 +26,15 @@ class TestSearchEngine : public Index
{
public:
TestSearchEngine(string const & locale);
TestSearchEngine(string const & locale, unique_ptr<storage::CountryInfoGetter> && infoGetter);
~TestSearchEngine();
weak_ptr<search::QueryHandle> Search(search::SearchParams const & params,
m2::RectD const & viewport);
private:
Platform & m_platform;
storage::CountryInfoGetter m_infoGetter;
unique_ptr<storage::CountryInfoGetter> m_infoGetter;
search::Engine m_engine;
};
} // namespace tests_support

View file

@ -45,17 +45,7 @@ private:
};
} // namespace
CountryInfoGetter::CountryInfoGetter(ModelReaderPtr polyR, ModelReaderPtr countryR)
: m_reader(polyR), m_cache(3)
{
ReaderSource<ModelReaderPtr> src(m_reader.GetReader(PACKED_POLYGONS_INFO_TAG));
rw::Read(src, m_countries);
string buffer;
countryR.ReadAsString(buffer);
LoadCountryFile2CountryInfo(buffer, m_id2info);
}
// CountryInfoGetter -------------------------------------------------------------------------------
string CountryInfoGetter::GetRegionFile(m2::PointD const & pt) const
{
IdType const id = FindFirstCountry(pt);
@ -114,7 +104,7 @@ bool CountryInfoGetter::IsBelongToRegions(m2::PointD const & pt, IdSet const & r
{
for (auto const & id : regions)
{
if (m_countries[id].m_rect.IsPointInside(pt) && IsBelongToRegion(id, pt))
if (m_countries[id].m_rect.IsPointInside(pt) && IsBelongToRegionImpl(id, pt))
return true;
}
return false;
@ -130,7 +120,39 @@ bool CountryInfoGetter::IsBelongToRegions(string const & fileName, IdSet const &
return false;
}
void CountryInfoGetter::ClearCaches() const
CountryInfoGetter::IdType CountryInfoGetter::FindFirstCountry(m2::PointD const & pt) const
{
for (size_t id = 0; id < m_countries.size(); ++id)
{
if (m_countries[id].m_rect.IsPointInside(pt) && IsBelongToRegionImpl(id, pt))
return id;
}
return kInvalidId;
}
template <typename ToDo>
void CountryInfoGetter::ForEachCountry(string const & prefix, ToDo && toDo) const
{
for (auto const & country : m_countries)
{
if (strings::StartsWith(country.m_name, prefix.c_str()))
toDo(country);
}
}
// CountryInfoReader -------------------------------------------------------------------------------
CountryInfoReader::CountryInfoReader(ModelReaderPtr polyR, ModelReaderPtr countryR)
: m_reader(polyR), m_cache(3)
{
ReaderSource<ModelReaderPtr> src(m_reader.GetReader(PACKED_POLYGONS_INFO_TAG));
rw::Read(src, m_countries);
string buffer;
countryR.ReadAsString(buffer);
LoadCountryFile2CountryInfo(buffer, m_id2info);
}
void CountryInfoReader::ClearCachesImpl() const
{
lock_guard<mutex> lock(m_cacheMutex);
@ -138,7 +160,7 @@ void CountryInfoGetter::ClearCaches() const
m_cache.Reset();
}
bool CountryInfoGetter::IsBelongToRegion(size_t id, m2::PointD const & pt) const
bool CountryInfoReader::IsBelongToRegionImpl(size_t id, m2::PointD const & pt) const
{
lock_guard<mutex> lock(m_cacheMutex);
@ -168,23 +190,23 @@ bool CountryInfoGetter::IsBelongToRegion(size_t id, m2::PointD const & pt) const
return false;
}
CountryInfoGetter::IdType CountryInfoGetter::FindFirstCountry(m2::PointD const & pt) const
{
for (size_t id = 0; id < m_countries.size(); ++id)
{
if (m_countries[id].m_rect.IsPointInside(pt) && IsBelongToRegion(id, pt))
return id;
}
return kInvalidId;
}
template <typename ToDo>
void CountryInfoGetter::ForEachCountry(string const & prefix, ToDo && toDo) const
// CountryInfoGetterForTesting ---------------------------------------------------------------------
CountryInfoGetterForTesting::CountryInfoGetterForTesting(vector<CountryDef> const & countries)
{
m_countries.assign(countries.begin(), countries.end());
for (auto const & country : m_countries)
{
if (strings::StartsWith(country.m_name, prefix.c_str()))
toDo(country);
string const & name = country.m_name;
m_id2info[name].m_name = name;
}
}
void CountryInfoGetterForTesting::ClearCachesImpl() const {}
bool CountryInfoGetterForTesting::IsBelongToRegionImpl(size_t id,
m2::PointD const & pt) const
{
CHECK_LESS(id, m_countries.size(), ());
return m_countries[id].m_rect.IsPointInside(pt);
}
} // namespace storage

View file

@ -23,7 +23,7 @@ public:
using IdType = size_t;
using IdSet = vector<IdType>;
CountryInfoGetter(ModelReaderPtr polyR, ModelReaderPtr countryR);
virtual ~CountryInfoGetter() = default;
// Returns country file name without an extension for a country |pt|
// belongs to. If there are no such country, returns an empty
@ -58,11 +58,10 @@ public:
bool IsBelongToRegions(string const & fileName, IdSet const & regions) const;
// Clears regions cache.
void ClearCaches() const;
inline void ClearCaches() const { ClearCachesImpl(); }
private:
// Returns true when |pt| belongs to a country identified by |id|.
bool IsBelongToRegion(size_t id, m2::PointD const & pt) const;
protected:
CountryInfoGetter() = default;
// Returns identifier of a first country containing |pt|.
IdType FindFirstCountry(m2::PointD const & pt) const;
@ -71,15 +70,47 @@ private:
template <typename ToDo>
void ForEachCountry(string const & prefix, ToDo && toDo) const;
// Clears regions cache.
virtual void ClearCachesImpl() const = 0;
// Returns true when |pt| belongs to a country identified by |id|.
virtual bool IsBelongToRegionImpl(size_t id, m2::PointD const & pt) const = 0;
// List of all known countries.
vector<CountryDef> m_countries;
// Maps country file name without an extension to a country info.
map<string, CountryInfo> m_id2info;
};
// This class reads info about countries from polygons file and
// countries file and caches it.
class CountryInfoReader : public CountryInfoGetter
{
public:
CountryInfoReader(ModelReaderPtr polyR, ModelReaderPtr countryR);
protected:
// CountryInfoGetter overrides:
void ClearCachesImpl() const override;
bool IsBelongToRegionImpl(size_t id, m2::PointD const & pt) const override;
// Only cache and reader can be modified from different threads, so
// they're guarded by m_cacheMutex.
FilesContainerR m_reader;
mutable my::Cache<uint32_t, vector<m2::RegionD>> m_cache;
mutable mutex m_cacheMutex;
};
// This class allows users to get info about very simply rectangular
// countries, whose info can be described by CountryDef instances.
// It's needed for testing purposes only.
class CountryInfoGetterForTesting : public CountryInfoGetter
{
public:
CountryInfoGetterForTesting(vector<CountryDef> const & countries);
protected:
// CountryInfoGetter overrides:
void ClearCachesImpl() const override;
bool IsBelongToRegionImpl(size_t id, m2::PointD const & pt) const override;
};
} // namespace storage

View file

@ -19,7 +19,7 @@ namespace
unique_ptr<CountryInfoGetter> CreateCountryInfoGetter()
{
Platform & platform = GetPlatform();
return make_unique<CountryInfoGetter>(platform.GetReader(PACKED_POLYGONS_FILE),
return make_unique<CountryInfoReader>(platform.GetReader(PACKED_POLYGONS_FILE),
platform.GetReader(COUNTRIES_FILE));
}