From 28d243e8064b0516b859a6182e8b63ce0b22ee7b Mon Sep 17 00:00:00 2001 From: "r.kuznetsov" Date: Thu, 13 Apr 2017 19:03:53 +0300 Subject: [PATCH] Optimized updating viewport for HLA manager --- map/framework.cpp | 10 +++--- map/framework.hpp | 2 +- map/local_ads_manager.cpp | 17 +++++---- map/local_ads_manager.hpp | 5 +-- storage/country_info_getter.cpp | 10 ++++-- storage/country_info_getter.hpp | 2 +- .../country_info_getter_test.cpp | 36 +++++++++++++++---- 7 files changed, 56 insertions(+), 26 deletions(-) diff --git a/map/framework.cpp b/map/framework.cpp index 23a86b672f..c6d309581f 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -427,10 +427,12 @@ Framework::Framework(FrameworkParams const & params) , m_isRenderingEnabled(true) , m_trackingReporter(platform::CreateSocket(), TRACKING_REALTIME_HOST, TRACKING_REALTIME_PORT, tracking::Reporter::kPushDelayMs) - , m_trafficManager(bind(&Framework::GetMwmsByRect, this, _1), kMaxTrafficCacheSizeBytes, + , m_trafficManager(bind(&Framework::GetMwmsByRect, this, _1, false /* rough */), + kMaxTrafficCacheSizeBytes, // Note. |m_routingSession| should be declared before |m_trafficManager|. m_routingSession) - , m_localAdsManager(bind(&Framework::GetMwmsByRect, this, _1), bind(&Framework::GetMwmIdByName, this, _1)) + , m_localAdsManager(bind(&Framework::GetMwmsByRect, this, _1, true /* rough */), + bind(&Framework::GetMwmIdByName, this, _1)) , m_displacementModeManager([this](bool show) { int const mode = show ? dp::displacement::kHotelMode : dp::displacement::kDefaultMode; CallDrapeFunction(bind(&df::DrapeEngine::SetDisplacementMode, _1, mode)); @@ -3563,13 +3565,13 @@ void Framework::VisualizeRoadsInRect(m2::RectD const & rect) }, kScale); } -vector Framework::GetMwmsByRect(m2::RectD const & rect) const +vector Framework::GetMwmsByRect(m2::RectD const & rect, bool rough) const { vector result; if (!m_infoGetter) return result; - auto countryIds = m_infoGetter->GetRegionsCountryIdByRect(rect); + auto countryIds = m_infoGetter->GetRegionsCountryIdByRect(rect, rough); for (auto const & countryId : countryIds) result.push_back(GetMwmIdByName(countryId)); diff --git a/map/framework.hpp b/map/framework.hpp index 92c5118bbc..50551e1451 100644 --- a/map/framework.hpp +++ b/map/framework.hpp @@ -394,7 +394,7 @@ public: /// Guarantees that listener is called in the main thread context. void SetCurrentCountryChangedListener(TCurrentCountryChanged const & listener); - vector GetMwmsByRect(m2::RectD const & rect) const; + vector GetMwmsByRect(m2::RectD const & rect, bool rough) const; MwmSet::MwmId GetMwmIdByName(std::string const & name) const; private: diff --git a/map/local_ads_manager.cpp b/map/local_ads_manager.cpp index 8f37fa9910..e1f8df9ef6 100644 --- a/map/local_ads_manager.cpp +++ b/map/local_ads_manager.cpp @@ -117,6 +117,8 @@ void LocalAdsManager::UpdateViewport(ScreenBase const & screen) // Request local ads campaigns. { std::lock_guard lock(m_mutex); + if (!m_isRunning) + return; for (auto const & mwm : mwms) { @@ -146,9 +148,8 @@ void LocalAdsManager::UpdateViewport(ScreenBase const & screen) if (!requestedCampaigns.empty()) { - m_requestedCampaigns.reserve(m_requestedCampaigns.size() + requestedCampaigns.size()); for (auto const & campaign : requestedCampaigns) - m_requestedCampaigns.push_back(std::make_pair(m_getMwmIdByNameFn(campaign), RequestType::Download)); + m_requestedCampaigns.insert(std::make_pair(m_getMwmIdByNameFn(campaign), RequestType::Download)); m_condition.notify_one(); } } @@ -165,7 +166,7 @@ void LocalAdsManager::ThreadRoutine() ReadCampaignFile(campaignFile); Invalidate(); - std::vector campaignMwms; + std::set campaignMwms; while (WaitForRequest(campaignMwms)) { for (auto const & mwm : campaignMwms) @@ -219,18 +220,16 @@ void LocalAdsManager::ThreadRoutine() } } -bool LocalAdsManager::WaitForRequest(std::vector & campaignMwms) +bool LocalAdsManager::WaitForRequest(std::set & campaignMwms) { std::unique_lock lock(m_mutex); m_condition.wait(lock, [this] {return !m_isRunning || !m_requestedCampaigns.empty();}); - if (!m_isRunning) return false; - if (!m_requestedCampaigns.empty()) - campaignMwms.swap(m_requestedCampaigns); - + campaignMwms = std::move(m_requestedCampaigns); + m_requestedCampaigns.clear(); return true; } @@ -244,7 +243,7 @@ void LocalAdsManager::OnDownloadCountry(std::string const & countryName) void LocalAdsManager::OnDeleteCountry(std::string const & countryName) { std::lock_guard lock(m_mutex); - m_requestedCampaigns.push_back(std::make_pair(m_getMwmIdByNameFn(countryName), RequestType::Delete)); + m_requestedCampaigns.insert(std::make_pair(m_getMwmIdByNameFn(countryName), RequestType::Delete)); m_condition.notify_one(); } diff --git a/map/local_ads_manager.hpp b/map/local_ads_manager.hpp index 75fbfbbfda..a3c4ea0606 100644 --- a/map/local_ads_manager.hpp +++ b/map/local_ads_manager.hpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -57,7 +58,7 @@ private: using Request = std::pair; void ThreadRoutine(); - bool WaitForRequest(std::vector & campaignMwms); + bool WaitForRequest(std::set & campaignMwms); std::string MakeRemoteURL(MwmSet::MwmId const & mwmId) const; std::vector DownloadCampaign(MwmSet::MwmId const & mwmId) const; @@ -87,7 +88,7 @@ private: bool m_isRunning = false; std::condition_variable m_condition; - std::vector m_requestedCampaigns; + std::set m_requestedCampaigns; std::mutex m_mutex; threads::SimpleThread m_thread; diff --git a/storage/country_info_getter.cpp b/storage/country_info_getter.cpp index 7d07f860cf..314430e683 100644 --- a/storage/country_info_getter.cpp +++ b/storage/country_info_getter.cpp @@ -59,7 +59,7 @@ TCountryId CountryInfoGetter::GetRegionCountryId(m2::PointD const & pt) const return id != kInvalidId ? m_countries[id].m_countryId : kInvalidCountryId; } -vector CountryInfoGetter::GetRegionsCountryIdByRect(m2::RectD const & rect) const +vector CountryInfoGetter::GetRegionsCountryIdByRect(m2::RectD const & rect, bool rough) const { size_t constexpr kAverageSize = 10; @@ -67,11 +67,15 @@ vector CountryInfoGetter::GetRegionsCountryIdByRect(m2::RectD const result.reserve(kAverageSize); for (size_t id = 0; id < m_countries.size(); ++id) { - if (rect.IsRectInside(m_countries[id].m_rect) || - (rect.IsIntersect(m_countries[id].m_rect) && IsIntersectedByRegionImpl(id, rect))) + if (rect.IsRectInside(m_countries[id].m_rect)) { result.push_back(m_countries[id].m_countryId); } + else if (rect.IsIntersect(m_countries[id].m_rect)) + { + if (rough || IsIntersectedByRegionImpl(id, rect)) + result.push_back(m_countries[id].m_countryId); + } } return result; } diff --git a/storage/country_info_getter.hpp b/storage/country_info_getter.hpp index a4a42818c2..1b398911f2 100644 --- a/storage/country_info_getter.hpp +++ b/storage/country_info_getter.hpp @@ -38,7 +38,7 @@ public: // Returns vector of countries file names without an extension for // countries belong to |rect|. - vector GetRegionsCountryIdByRect(m2::RectD const & rect) const; + vector GetRegionsCountryIdByRect(m2::RectD const & rect, bool rough) const; // Returns a list of country ids by a |pt| in mercator. // |closestCoutryIds| is filled with country ids of mwm which covers |pt| or close to it. diff --git a/storage/storage_tests/country_info_getter_test.cpp b/storage/storage_tests/country_info_getter_test.cpp index 46398cc2d6..10bd33af6e 100644 --- a/storage/storage_tests/country_info_getter_test.cpp +++ b/storage/storage_tests/country_info_getter_test.cpp @@ -56,19 +56,43 @@ UNIT_TEST(CountryInfoGetter_GetRegionsCountryIdByRect_Smoke) // Inside mwm. m2::PointD const halfSize = m2::PointD(0.1, 0.1); - auto countries = getter->GetRegionsCountryIdByRect(m2::RectD(p - halfSize, p + halfSize)); - TEST_EQUAL(countries, vector { "Belarus" }, ()); + auto const countries = + getter->GetRegionsCountryIdByRect(m2::RectD(p - halfSize, p + halfSize), false /* rough */); + TEST_EQUAL(countries, vector{"Belarus"}, ()); // Several countries. m2::PointD const halfSize2 = m2::PointD(5.0, 5.0); - auto countries2 = getter->GetRegionsCountryIdByRect(m2::RectD(p - halfSize2, p + halfSize2)); - auto expected = vector { "Belarus", "Latvia", "Lithuania", "Poland", - "Russia_Central", "Russia_Northwestern", "Ukraine" }; + auto const countries2 = getter->GetRegionsCountryIdByRect(m2::RectD(p - halfSize2, p + halfSize2), + false /* rough */); + auto const expected = vector{ + "Belarus", "Latvia", "Lithuania", "Poland", "Russia_Central", "Russia_Northwestern", + "Ukraine"}; TEST_EQUAL(countries2, expected, ()); // No one found. - auto countries3 = getter->GetRegionsCountryIdByRect(m2::RectD(-halfSize, halfSize)); + auto const countries3 = + getter->GetRegionsCountryIdByRect(m2::RectD(-halfSize, halfSize), false /* rough */); TEST_EQUAL(countries3, vector{}, ()); + + // Inside mwm (rough). + auto const countries4 = + getter->GetRegionsCountryIdByRect(m2::RectD(p - halfSize, p + halfSize), true /* rough */); + TEST_EQUAL(countries, vector{"Belarus"}, ()); + + // Several countries (rough). + auto const countries5 = + getter->GetRegionsCountryIdByRect(m2::RectD(p - halfSize2, p + halfSize2), true /* rough */); + auto const expected2 = vector{"Belarus", + "Latvia", + "Lithuania", + "Poland", + "Russia_Central", + "Russia_Far Eastern", + "Russia_Northwestern", + "Sweden", + "Ukraine", + "USA_Alaska"}; + TEST_EQUAL(countries5, expected2, ()); } UNIT_TEST(CountryInfoGetter_ValidName_Smoke)