diff --git a/map/traffic_manager.cpp b/map/traffic_manager.cpp index 016cd9140..9a48c806d 100644 --- a/map/traffic_manager.cpp +++ b/map/traffic_manager.cpp @@ -75,12 +75,12 @@ TrafficManager::CacheEntry::CacheEntry(time_point const & requestT TrafficManager::TrafficManager(DataSource & dataSource, CountryInfoGetterFn countryInfoGetter, const CountryParentNameGetterFn &countryParentNameGetter, GetMwmsByRectFn const & getMwmsByRectFn, size_t maxCacheSizeBytes, - traffic::TrafficObserver & observer) + routing::RoutingSession & routingSession) : m_dataSource(dataSource) , m_countryInfoGetterFn(countryInfoGetter) , m_countryParentNameGetterFn(countryParentNameGetter) , m_getMwmsByRectFn(getMwmsByRectFn) - , m_observer(observer) + , m_routingSession(routingSession) , m_currentDataVersion(0) , m_state(TrafficState::Disabled) // TODO no longer needed @@ -92,6 +92,11 @@ TrafficManager::TrafficManager(DataSource & dataSource, CountryInfoGetterFn coun , m_thread(&TrafficManager::ThreadRoutine, this) { CHECK(m_getMwmsByRectFn != nullptr, ()); + GetPlatform().RunTask(Platform::Thread::Gui, [this]() { + m_routingSession.SetChangeSessionStateCallback([this](routing::SessionState previous, routing::SessionState current) { + OnChangeRoutingSessionState(previous, current); + }); + }); } TrafficManager::~TrafficManager() @@ -170,7 +175,7 @@ void TrafficManager::SetEnabled(bool enabled) m_canSetMode = false; } else - m_observer.OnTrafficInfoClear(); + m_routingSession.OnTrafficInfoClear(); } void TrafficManager::Clear() @@ -194,7 +199,7 @@ void TrafficManager::Clear() // TODO figure out which of the ones below we still need m_lastDrapeMwmsByRect.clear(); - m_lastRoutingMwmsByRect.clear(); + m_lastPositionMwmsByRect.clear(); #ifdef traffic_dead_code m_requestedMwms.clear(); m_trafficETags.clear(); @@ -253,6 +258,65 @@ void TrafficManager::OnRecoverSurface() Resume(); } +void TrafficManager::OnChangeRoutingSessionState(routing::SessionState previous, routing::SessionState current) +{ + // TODO assert we’re running on the UI thread + LOG(LINFO, ("Routing session state changed from", previous, "to", current)); + LOG(LINFO, ("Running on thread", std::this_thread::get_id())); + /* + * Filter based on session state (see routing_callbacks.hpp for states and transitions). + * + * GetAllRegions() seems to get fresh data during build (presumably also rebuild), which will be + * available on the next state transition (to RouteNotStarted or OnRoute) and remain unchanged + * until the next route (re)build. Therefore, calling GetAllRegions() when new state is one of + * RouteNotStarted, OnRoute or RouteNoFollowing, and clearing MWMs when the new state is + * NoValidRoute, and ignoring all other transitions, should work for our purposes. + * There may be cases where we first calculate the route, then subscribe to the regions and only + * then get notified about a traffic problem on the route, requiring us to recalculate. + */ + std::set mwmNames; + if (current == routing::SessionState::RouteNotStarted + || current == routing::SessionState::OnRoute + || current == routing::SessionState::RouteNoFollowing) + /* + * GetAllRegions() may block when run for the first time. This should happen during routing, so + * the call here would always return cached results without blocking, causing no problems on the + * UI thread. + * + * Note that this method is called before the state is updated internally, thus `GetAllRegions()` + * and any other functions would still have `previous` as their state. + */ + m_routingSession.GetAllRegions(mwmNames); + else if (current == routing::SessionState::NoValidRoute) + mwmNames.clear(); + else + // for all other states, keep current set + return; + + LOG(LINFO, ("Router MWMs:", mwmNames)); + + std::set mwms; + for (auto const & mwmName : mwmNames) + { + MwmSet::MwmId mwmId = m_dataSource.GetMwmIdByCountryFile(platform::CountryFile(mwmName)); + if (mwmId.IsAlive()) + mwms.insert(mwmId); + } + + LOG(LINFO, ("MWM set:", mwms)); + + { + std::lock_guard lock(m_mutex); + + if (mwms != m_activeRoutingMwms) + { + m_activeMwmsChanged = true; + std::swap(mwms, m_activeRoutingMwms); + RequestTrafficData(); + } + } +} + void TrafficManager::RecalculateSubscription() { if (!IsEnabled()) @@ -262,6 +326,7 @@ void TrafficManager::RecalculateSubscription() UpdateViewport(m_currentModelView.first); if (m_currentPosition.second) UpdateMyPosition(m_currentPosition.first); + // TODO update routing – if needed } void TrafficManager::Invalidate(MwmSet::MwmId const & mwmId) @@ -351,9 +416,7 @@ void TrafficManager::UpdateMyPosition(MyPosition const & myPosition) m2::RectD const rect = mercator::RectByCenterXYAndSizeInMeters(myPosition.m_position, kSquareSideM / 2.0); // Request traffic. - UpdateActiveMwms(rect, m_lastRoutingMwmsByRect, m_activeRoutingMwms); - - // @TODO Do all routing stuff. + UpdateActiveMwms(rect, m_lastPositionMwmsByRect, m_activePositionMwms); } void TrafficManager::UpdateViewport(ScreenBase const & screen) @@ -849,8 +912,8 @@ void TrafficManager::RequestTrafficData(MwmSet::MwmId const & mwmId, bool force) void TrafficManager::RequestTrafficData() { - if ((m_activeDrapeMwms.empty() && m_activeRoutingMwms.empty()) || !IsEnabled() || - IsInvalidState() || m_isPaused) + if ((m_activeDrapeMwms.empty() && m_activePositionMwms.empty() && m_activeRoutingMwms.empty()) + || !IsEnabled() || IsInvalidState() || m_isPaused) { return; } @@ -1019,7 +1082,7 @@ void TrafficManager::OnTrafficDataUpdate() if (notifyObserver) { // Update traffic colors for routing. - m_observer.OnTrafficInfoAdded(std::move(info)); + m_routingSession.OnTrafficInfoAdded(std::move(info)); m_lastObserverUpdate = steady_clock::now(); } } @@ -1036,7 +1099,7 @@ void TrafficManager::OnTrafficDataUpdate() if (notifyObserver) { // Update traffic colors for routing. - m_observer.OnTrafficInfoRemoved(mwmId); + m_routingSession.OnTrafficInfoRemoved(mwmId); m_lastObserverUpdate = steady_clock::now(); } } @@ -1086,6 +1149,7 @@ void TrafficManager::OnTrafficDataResponse(traffic::TrafficInfo && info) void TrafficManager::UniteActiveMwms(std::set & activeMwms) const { activeMwms.insert(m_activeDrapeMwms.cbegin(), m_activeDrapeMwms.cend()); + activeMwms.insert(m_activePositionMwms.cbegin(), m_activePositionMwms.cend()); activeMwms.insert(m_activeRoutingMwms.cbegin(), m_activeRoutingMwms.cend()); } diff --git a/map/traffic_manager.hpp b/map/traffic_manager.hpp index ff615b70f..cc74b56bd 100644 --- a/map/traffic_manager.hpp +++ b/map/traffic_manager.hpp @@ -9,6 +9,8 @@ #include "indexer/mwm_set.hpp" +#include "routing/routing_session.hpp" + #include "storage/country_info_getter.hpp" #include "traffxml/traff_decoder.hpp" @@ -107,7 +109,7 @@ public: CountryInfoGetterFn countryInfoGetter, CountryParentNameGetterFn const & countryParentNameGetter, GetMwmsByRectFn const & getMwmsByRectFn, size_t maxCacheSizeBytes, - traffic::TrafficObserver & observer); + routing::RoutingSession & routingSession); ~TrafficManager(); void Teardown(); @@ -477,8 +479,8 @@ private: * @brief Updates `activeMwms` and requests traffic data. * * The old and new list of active MWMs may refer either to those used by the rendering engine - * (`m_lastDrapeMwmsByRect`/`m_activeDrapeMwms`) or to those used by the routing engine - * (`m_lastRoutingMwmsByRect`/`m_activeRoutingMwms`). + * (`m_lastDrapeMwmsByRect`/`m_activeDrapeMwms`) or to those around the current position. + * (`m_lastPositionMwmsByRect`/`m_activePositionMwms`). * * The method first determines the list of MWMs overlapping with `rect`. If it is identical to * `lastMwmsByRect`, the method returns immediately. Otherwise, it stores the new set in @@ -561,6 +563,8 @@ private: bool IsInvalidState() const; + void OnChangeRoutingSessionState(routing::SessionState previous, routing::SessionState current); + void UniteActiveMwms(std::set & activeMwms) const; void Pause(); @@ -586,7 +590,14 @@ private: CountryInfoGetterFn m_countryInfoGetterFn; CountryParentNameGetterFn m_countryParentNameGetterFn; GetMwmsByRectFn m_getMwmsByRectFn; - traffic::TrafficObserver & m_observer; + + /* + * Originally this was m_observer, of type traffic::TrafficObserver. Since routing::RoutingSession + * inherits from that class, and an interface to the routing session is needed in order to + * determine the MWMs for which we need traffic information, the type was changed and the member + * renamed to reflect that. + */ + routing::RoutingSession & m_routingSession; df::DrapeEngineSafePtr m_drapeEngine; std::atomic m_currentDataVersion; @@ -624,14 +635,16 @@ private: std::condition_variable m_condition; /* - * To determine for which MWMs we need traffic data, we need to keep track of two groups of MWMs: - * those used by the renderer (i.e. in or just around the viewport) and those used by the routing - * engine (i.e. those within a certain area around the route endpoints). + * To determine for which MWMs we need traffic data, we need to keep track of 3 groups of MWMs: + * those used by the renderer (i.e. in or just around the viewport), those within a certain area + * around the current position, and those used by the routing engine (only if currently routing). * - * Each group is stored twice: as a set and as a vector. The set always holds the MWMs which were - * last seen in use. Both get updated together when active MWMs are added or removed. However, - * the vector is used as a reference to detect changes. It may get cleared when the set is not, - * which is used to invalidate the set without destroying its contents. + * Routing MWMs are stored as a set. + * + * The other groups arestored twice: as a set and as a vector. The set always holds the MWMs which + * were last seen in use. Both get updated together when active MWMs are added or removed. + * However, the vector is used as a reference to detect changes. It may get cleared when the set + * is not, which is used to invalidate the set without destroying its contents. * * Methods which use only the set: * @@ -642,14 +655,15 @@ private: * * Methods which use both, but in a different way: * - * * ClearCache(), removes the requested MWM from the set but clears the vector completely. - * * UpdateActiveMwms(), uses the vector to detect changes. If so, it updates both vector and set. + * * (dead code) ClearCache(), removes the requested MWM from the set but clears the vector completely. + * * UpdateActiveMwms(), uses the vector to detect changes (not for routing MWMs). If so, it updates both vector and set. * * Clear() clears both the set and the vector. (Clearing the set is currently disabled as it breaks ForEachActiveMwm.) */ std::vector m_lastDrapeMwmsByRect; std::set m_activeDrapeMwms; - std::vector m_lastRoutingMwmsByRect; + std::vector m_lastPositionMwmsByRect; + std::set m_activePositionMwms; std::set m_activeRoutingMwms; // TODO no longer needed diff --git a/routing/absent_regions_finder.cpp b/routing/absent_regions_finder.cpp index 24bdc1d50..5039baa9d 100644 --- a/routing/absent_regions_finder.cpp +++ b/routing/absent_regions_finder.cpp @@ -19,6 +19,8 @@ AbsentRegionsFinder::AbsentRegionsFinder(CountryFileGetterFn const & countryFile void AbsentRegionsFinder::GenerateAbsentRegions(Checkpoints const & checkpoints, RouterDelegate const & delegate) { + m_regions.clear(); + if (m_routerThread) { m_routerThread->Cancel(); @@ -52,20 +54,21 @@ void AbsentRegionsFinder::GetAbsentRegions(std::set & regions) void AbsentRegionsFinder::GetAllRegions(std::set & countries) { - countries.clear(); - - if (!m_routerThread) - return; - - m_routerThread->Join(); - - for (auto const & mwmName : m_routerThread->GetRoutineAs()->GetMwmNames()) + // Note: if called from `RoutingSession` callback, m_state will still have its pre-update value. + if (m_routerThread) { - if (!mwmName.empty()) - countries.emplace(mwmName); + m_routerThread->Join(); + + for (auto const & mwmName : m_routerThread->GetRoutineAs()->GetMwmNames()) + { + if (!mwmName.empty()) + m_regions.emplace(mwmName); + } + + m_routerThread.reset(); } - m_routerThread.reset(); + countries = m_regions; } bool AbsentRegionsFinder::AreCheckpointsInSameMwm(Checkpoints const & checkpoints) const diff --git a/routing/absent_regions_finder.hpp b/routing/absent_regions_finder.hpp index 8028b6009..0f1541020 100644 --- a/routing/absent_regions_finder.hpp +++ b/routing/absent_regions_finder.hpp @@ -25,9 +25,27 @@ public: // Creates new thread |m_routerThread| and starts routing in it. void GenerateAbsentRegions(Checkpoints const & checkpoints, RouterDelegate const & delegate); - // Waits for the routing thread |m_routerThread| to finish and returns results from it. + + /** + * @brief Retrieves the MWMs needed to build the route. + * + * When called for the first time after `GenerateAbsentRegions()`, this method waits for the + * routing thread `m_routerThread` to finish and returns results from it. Results are cached and + * subsequent calls are served from the cache. + * + * @param countries Receives the list of MWM names. + */ void GetAllRegions(std::set & countries); - // Waits for the results from GetAllRegions() and returns only regions absent on the device. + + /** + * @brief Retrieves the missing MWMs needed to build the route. + * + * This calls `GetAllRegions()` and strips from the result all regions already present on the + * device, leaving only the missing ones. If the call to `GetAllRegions()` is the first one after + * calling `GenerateAbsentRegions()`, this involves waiting for the router thread to finish. + * + * @param absentCountries Receives the list of missing MWM names. + */ void GetAbsentRegions(std::set & absentCountries); private: @@ -40,5 +58,19 @@ private: DataSource & m_dataSource; std::unique_ptr m_routerThread; + + /** + * @brief Mutex for access to `m_regions`. + * + * Threads which access `m_regions` must lock this mutex while doing so. + */ + std::mutex m_mutex; + + /** + * @brief Regions required for building the last route. + * + * This member is cleared by `GenerateAbsentRegions()` and populated by `GetAllRegions()`. + */ + std::set m_regions; }; } // namespace routing diff --git a/routing/async_router.cpp b/routing/async_router.cpp index 161fc5102..3e96815b9 100644 --- a/routing/async_router.cpp +++ b/routing/async_router.cpp @@ -83,6 +83,13 @@ bool AsyncRouter::FindClosestProjectionToRoad(m2::PointD const & point, return m_router->FindClosestProjectionToRoad(point, direction, radius, proj); } +void AsyncRouter::GetAllRegions(std::set & countries) +{ + if (!m_absentRegionsFinder) + return; + m_absentRegionsFinder->GetAllRegions(countries); +} + void AsyncRouter::RouterDelegateProxy::OnProgress(float progress) { ProgressCallback onProgress = nullptr; diff --git a/routing/async_router.hpp b/routing/async_router.hpp index d03e13987..d9c70f09f 100644 --- a/routing/async_router.hpp +++ b/routing/async_router.hpp @@ -62,6 +62,15 @@ public: bool FindClosestProjectionToRoad(m2::PointD const & point, m2::PointD const & direction, double radius, EdgeProj & proj); + /** + * @brief Retrieves the MWMs needed to build the route. + * + * Waits for the routing thread to finish and returns the list of MWM names from it. + * + * @param countries Receives the list of MWM names. + */ + void GetAllRegions(std::set & countries); + private: /// Worker thread function void ThreadFunc(); diff --git a/routing/routing_session.cpp b/routing/routing_session.cpp index 2068825f8..fb21ab773 100644 --- a/routing/routing_session.cpp +++ b/routing/routing_session.cpp @@ -470,6 +470,13 @@ double RoutingSession::GetCompletionPercent() const return percent; } +void RoutingSession::GetAllRegions(std::set & countries) +{ + if (!m_router) + return; + m_router->GetAllRegions(countries); +} + void RoutingSession::PassCheckpoints() { CHECK_THREAD_CHECKER(m_threadChecker, ()); diff --git a/routing/routing_session.hpp b/routing/routing_session.hpp index d528430f6..95f643963 100644 --- a/routing/routing_session.hpp +++ b/routing/routing_session.hpp @@ -258,6 +258,15 @@ public: double GetCompletionPercent() const; + /** + * @brief Retrieves the MWMs needed to build the route. + * + * Waits for the `RegionsRouter` thread to finish and returns the list of MWM names from it. + * + * @param countries Receives the list of MWM names. + */ + void GetAllRegions(std::set & countries); + private: struct DoReadyCallback {