From c1c2841d40908486c32ebf9febdfbf3cea8929cf Mon Sep 17 00:00:00 2001 From: Daria Volvenkova Date: Wed, 10 Apr 2019 22:47:43 +0300 Subject: [PATCH] [screenshots] Fixed inaccurate notifications about ready graphics. --- drape_frontend/drape_engine.cpp | 15 ++++------ drape_frontend/drape_engine.hpp | 7 ++--- drape_frontend/frontend_renderer.cpp | 43 +++++++++++++++++++++++++-- drape_frontend/frontend_renderer.hpp | 26 ++++++++++++---- drape_frontend/message.hpp | 3 +- drape_frontend/message_subclasses.hpp | 17 +++++++++++ map/framework.cpp | 12 ++++---- map/framework.hpp | 9 +++--- qt/main.cpp | 10 +++---- qt/screenshoter.cpp | 40 ++++++++++--------------- qt/screenshoter.hpp | 1 + 11 files changed, 120 insertions(+), 63 deletions(-) diff --git a/drape_frontend/drape_engine.cpp b/drape_frontend/drape_engine.cpp index a13ec95ce6..23c2a573fd 100644 --- a/drape_frontend/drape_engine.cpp +++ b/drape_frontend/drape_engine.cpp @@ -78,7 +78,6 @@ DrapeEngine::DrapeEngine(Params && params) std::move(mpParams), m_viewport, std::bind(&DrapeEngine::ModelViewChanged, this, _1), - std::bind(&DrapeEngine::GraphicsReady, this), std::bind(&DrapeEngine::TapEvent, this, _1), std::bind(&DrapeEngine::UserPositionChanged, this, _1, _2), make_ref(m_requestedTiles), @@ -421,12 +420,6 @@ void DrapeEngine::ModelViewChanged(ScreenBase const & screen) m_modelViewChanged(screen); } -void DrapeEngine::GraphicsReady() -{ - if (m_graphicsReady != nullptr) - m_graphicsReady(); -} - void DrapeEngine::MyPositionModeChanged(location::EMyPositionMode mode, bool routingActive) { settings::Set(settings::kLocationStateMode, mode); @@ -505,10 +498,14 @@ void DrapeEngine::SetModelViewListener(TModelViewListenerFn && fn) m_modelViewChanged = std::move(fn); } -void DrapeEngine::SetGraphicsReadyListener(TGraphicsReadyFn && fn) +#if defined(OMIM_OS_MAC) || defined(OMIM_OS_LINUX) +void DrapeEngine::NotifyGraphicsReady(TGraphicsReadyFn const & fn) { - m_graphicsReady = std::move(fn); + m_threadCommutator->PostMessage(ThreadsCommutator::RenderThread, + make_unique_dp(fn), + MessagePriority::Normal); } +#endif void DrapeEngine::SetTapEventInfoListener(TTapEventInfoFn && fn) { diff --git a/drape_frontend/drape_engine.hpp b/drape_frontend/drape_engine.hpp index 235acc198b..547c81cb40 100644 --- a/drape_frontend/drape_engine.hpp +++ b/drape_frontend/drape_engine.hpp @@ -138,8 +138,10 @@ public: using TModelViewListenerFn = FrontendRenderer::TModelViewChanged; void SetModelViewListener(TModelViewListenerFn && fn); +#if defined(OMIM_OS_MAC) || defined(OMIM_OS_LINUX) using TGraphicsReadyFn = FrontendRenderer::TGraphicsReadyFn; - void SetGraphicsReadyListener(TGraphicsReadyFn && fn); + void NotifyGraphicsReady(TGraphicsReadyFn const & fn); +#endif void ClearUserMarksGroup(kml::MarkGroupId groupId); void ChangeVisibilityUserMarksGroup(kml::MarkGroupId groupId, bool isVisible); @@ -237,8 +239,6 @@ private: void AddUserEvent(drape_ptr && e); void PostUserEvent(drape_ptr && e); void ModelViewChanged(ScreenBase const & screen); - void GraphicsReady(); - void MyPositionModeChanged(location::EMyPositionMode mode, bool routingActive); void TapEvent(TapInfo const & tapInfo); void UserPositionChanged(m2::PointD const & position, bool hasPosition); @@ -263,7 +263,6 @@ private: dp::Viewport m_viewport; TModelViewListenerFn m_modelViewChanged; - TGraphicsReadyFn m_graphicsReady; TUserPositionChangedFn m_userPositionChanged; TTapEventInfoFn m_tapListener; diff --git a/drape_frontend/frontend_renderer.cpp b/drape_frontend/frontend_renderer.cpp index d5b78fe19f..0f6c06779b 100755 --- a/drape_frontend/frontend_renderer.cpp +++ b/drape_frontend/frontend_renderer.cpp @@ -192,7 +192,6 @@ FrontendRenderer::FrontendRenderer(Params && params) , m_screenshotMode(params.m_myPositionParams.m_hints.m_screenshotMode) , m_viewport(params.m_viewport) , m_modelViewChangedFn(params.m_modelViewChangedFn) - , m_graphicsReadyFn(params.m_graphicsReadyFn) , m_tapEventInfoFn(params.m_tapEventFn) , m_userPositionChangedFn(params.m_positionChangedFn) , m_requestedTiles(params.m_requestedTiles) @@ -351,8 +350,11 @@ void FrontendRenderer::AcceptMessage(ref_ptr message) DrapeMeasurer::Instance().EndScenePreparing(); #endif m_trafficRenderer->OnGeometryReady(m_currentZoomLevel); - if (m_graphicsReadyFn) - m_graphicsReadyFn(); + +#if defined(OMIM_OS_MAC) || defined(OMIM_OS_LINUX) + if (m_graphicsStage == GraphicsStage::WaitReady) + m_graphicsStage = GraphicsStage::WaitRendering; +#endif } break; } @@ -999,6 +1001,20 @@ void FrontendRenderer::AcceptMessage(ref_ptr message) break; } +#if defined(OMIM_OS_MAC) || defined(OMIM_OS_LINUX) + case Message::Type::NotifyGraphicsReady: + { + ref_ptr msg = message; + m_graphicsReadyFn = msg->GetCallback(); + if (m_graphicsReadyFn) + { + m_graphicsStage = GraphicsStage::WaitReady; + InvalidateRect(m_userEventStream.GetCurrentScreen().ClipRect()); + } + break; + } +#endif + default: ASSERT(false, ()); } @@ -1436,6 +1452,11 @@ void FrontendRenderer::RenderScene(ScreenBase const & modelView, bool activeFram m_guiRenderer->Render(m_context, make_ref(m_gpuProgramManager), m_myPositionController->IsInRouting(), modelView); } + +#if defined(OMIM_OS_MAC) || defined(OMIM_OS_LINUX) + if (m_graphicsStage == GraphicsStage::WaitRendering) + m_graphicsStage = GraphicsStage::Rendered; +#endif } void FrontendRenderer::Render2dLayer(ScreenBase const & modelView) @@ -1714,6 +1735,9 @@ void FrontendRenderer::RenderFrame() m_frameData.m_forceFullRedrawNextFrame = m_overlayTree->IsNeedUpdate(); if (canSuspend) { +#if defined(OMIM_OS_MAC) || defined(OMIM_OS_LINUX) + EmitGraphicsReady(); +#endif // Process a message or wait for a message. // IsRenderingEnabled() can return false in case of rendering disabling and we must prevent // possibility of infinity waiting in ProcessSingleMessage. @@ -2474,6 +2498,19 @@ void FrontendRenderer::EmitModelViewChanged(ScreenBase const & modelView) const m_modelViewChangedFn(modelView); } +#if defined(OMIM_OS_MAC) || defined(OMIM_OS_LINUX) +void FrontendRenderer::EmitGraphicsReady() +{ + if (m_graphicsStage == GraphicsStage::Rendered) + { + if (m_graphicsReadyFn) + m_graphicsReadyFn(); + m_graphicsStage = GraphicsStage::Unknown; + m_graphicsReadyFn = nullptr; + } +} +#endif + void FrontendRenderer::OnPrepareRouteArrows(dp::DrapeID subrouteIndex, std::vector && borders) { m_commutator->PostMessage(ThreadsCommutator::RenderThread, diff --git a/drape_frontend/frontend_renderer.hpp b/drape_frontend/frontend_renderer.hpp index 9bd71bdf03..ca9aa7b13c 100755 --- a/drape_frontend/frontend_renderer.hpp +++ b/drape_frontend/frontend_renderer.hpp @@ -84,16 +84,15 @@ public: Params(dp::ApiVersion apiVersion, ref_ptr commutator, ref_ptr factory, ref_ptr texMng, MyPositionController::Params && myPositionParams, dp::Viewport viewport, - TModelViewChanged const & modelViewChangedFn, TGraphicsReadyFn const & graphicsReayFn, - TTapEventInfoFn const & tapEventFn, TUserPositionChangedFn const & positionChangedFn, - ref_ptr requestedTiles, OverlaysShowStatsCallback && overlaysShowStatsCallback, + TModelViewChanged const & modelViewChangedFn, TTapEventInfoFn const & tapEventFn, + TUserPositionChangedFn const & positionChangedFn, ref_ptr requestedTiles, + OverlaysShowStatsCallback && overlaysShowStatsCallback, bool allow3dBuildings, bool trafficEnabled, bool blockTapEvents, std::vector && enabledEffects) : BaseRenderer::Params(apiVersion, commutator, factory, texMng) , m_myPositionParams(std::move(myPositionParams)) , m_viewport(viewport) , m_modelViewChangedFn(modelViewChangedFn) - , m_graphicsReadyFn(graphicsReayFn) , m_tapEventFn(tapEventFn) , m_positionChangedFn(positionChangedFn) , m_requestedTiles(requestedTiles) @@ -107,7 +106,6 @@ public: MyPositionController::Params m_myPositionParams; dp::Viewport m_viewport; TModelViewChanged m_modelViewChangedFn; - TGraphicsReadyFn m_graphicsReadyFn; TTapEventInfoFn m_tapEventFn; TUserPositionChangedFn m_positionChangedFn; ref_ptr m_requestedTiles; @@ -192,6 +190,10 @@ private: void EmitModelViewChanged(ScreenBase const & modelView) const; +#if defined(OMIM_OS_MAC) || defined(OMIM_OS_LINUX) + void EmitGraphicsReady(); +#endif + TTilesCollection ResolveTileKeys(ScreenBase const & screen); void ResolveZoomLevel(ScreenBase const & screen); void UpdateDisplacementEnabled(); @@ -315,7 +317,6 @@ private: dp::Viewport m_viewport; UserEventStream m_userEventStream; TModelViewChanged m_modelViewChangedFn; - TGraphicsReadyFn m_graphicsReadyFn; TTapEventInfoFn m_tapEventInfoFn; TUserPositionChangedFn m_userPositionChangedFn; @@ -373,6 +374,19 @@ private: bool m_firstLaunchAnimationTriggered = false; bool m_firstLaunchAnimationInterrupted = false; +#if defined(OMIM_OS_MAC) || defined(OMIM_OS_LINUX) + TGraphicsReadyFn m_graphicsReadyFn; + + enum class GraphicsStage + { + Unknown, + WaitReady, + WaitRendering, + Rendered + }; + GraphicsStage m_graphicsStage = GraphicsStage::Unknown; +#endif + bool m_finishTexturesInitialization = false; drape_ptr m_transitBackground; diff --git a/drape_frontend/message.hpp b/drape_frontend/message.hpp index 49f4e6c84a..1a374fa6c3 100644 --- a/drape_frontend/message.hpp +++ b/drape_frontend/message.hpp @@ -96,7 +96,8 @@ public: RegenerateTransitScheme, FlushTransitScheme, ShowDebugInfo, - NotifyRenderThread + NotifyRenderThread, + NotifyGraphicsReady, }; virtual ~Message() = default; diff --git a/drape_frontend/message_subclasses.hpp b/drape_frontend/message_subclasses.hpp index 874733974f..7d5368e400 100644 --- a/drape_frontend/message_subclasses.hpp +++ b/drape_frontend/message_subclasses.hpp @@ -936,6 +936,23 @@ private: RequestSymbolsSizeCallback m_callback; }; +class NotifyGraphicsReadyMessage : public Message +{ +public: + using GraphicsReadyCallback = std::function; + + NotifyGraphicsReadyMessage(GraphicsReadyCallback const & callback) + : m_callback(callback) + {} + + Type GetType() const override { return Type::NotifyGraphicsReady; } + + GraphicsReadyCallback GetCallback() { return m_callback; } + +private: + GraphicsReadyCallback m_callback; +}; + class EnableTrafficMessage : public Message { public: diff --git a/map/framework.cpp b/map/framework.cpp index 72177c99f7..802e24cb83 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -1267,15 +1267,13 @@ void Framework::SetViewportListener(TViewportChangedFn const & fn) m_viewportChangedFn = fn; } -void Framework::SetGraphicsReadyListener(TGraphicsReadyfn const & fn) +#if defined(OMIM_OS_MAC) || defined(OMIM_OS_LINUX) +void Framework::NotifyGraphicsReady(TGraphicsReadyFn const & fn) { - m_graphicsReadyFn = fn; - m_drapeEngine->SetGraphicsReadyListener( - m_graphicsReadyFn == nullptr ? static_cast(nullptr) : [this]() - { - GetPlatform().RunTask(Platform::Thread::Gui, [this](){ m_graphicsReadyFn(); }); - }); + if (m_drapeEngine != nullptr) + m_drapeEngine->NotifyGraphicsReady(fn); } +#endif void Framework::StopLocationFollow() { diff --git a/map/framework.hpp b/map/framework.hpp index a7b29bb96c..bf927405d4 100644 --- a/map/framework.hpp +++ b/map/framework.hpp @@ -207,9 +207,6 @@ protected: using TViewportChangedFn = df::DrapeEngine::TModelViewListenerFn; TViewportChangedFn m_viewportChangedFn; - using TGraphicsReadyfn = df::DrapeEngine::TGraphicsReadyFn; - TGraphicsReadyfn m_graphicsReadyFn; - drape_ptr m_drapeEngine; double m_startForegroundTime = 0.0; @@ -640,7 +637,11 @@ public: void GetTouchRect(m2::PointD const & center, uint32_t pxRadius, m2::AnyRectD & rect); void SetViewportListener(TViewportChangedFn const & fn); - void SetGraphicsReadyListener(TGraphicsReadyfn const & fn); + +#if defined(OMIM_OS_MAC) || defined(OMIM_OS_LINUX) + using TGraphicsReadyFn = df::DrapeEngine::TGraphicsReadyFn; + void NotifyGraphicsReady(TGraphicsReadyFn const & fn); +#endif void StopLocationFollow(); diff --git a/qt/main.cpp b/qt/main.cpp index 9982c949b9..177be2e674 100644 --- a/qt/main.cpp +++ b/qt/main.cpp @@ -26,16 +26,16 @@ #include #include -DEFINE_string(data_path, "", "Path to data directory"); +DEFINE_string(data_path, "", "Path to data directory."); DEFINE_string(log_abort_level, base::ToString(base::GetDefaultLogAbortLevel()), "Log messages severity that causes termination."); -DEFINE_string(resources_path, "", "Path to resources directory"); +DEFINE_string(resources_path, "", "Path to resources directory."); DEFINE_string(kml_path, "", "Path to a directory with kml files to take screenshots."); DEFINE_string(dst_path, "", "Path to a directory to save screenshots."); DEFINE_string(lang, "", "Device language."); -DEFINE_int32(width, 0, "Screenshot width"); -DEFINE_int32(height, 0, "Screenshot height"); -DEFINE_double(dpi_scale, 0.0, "Screenshot dpi scale"); +DEFINE_int32(width, 0, "Screenshot width."); +DEFINE_int32(height, 0, "Screenshot height."); +DEFINE_double(dpi_scale, 0.0, "Screenshot dpi scale."); namespace { diff --git a/qt/screenshoter.cpp b/qt/screenshoter.cpp index 6d645e817b..5bd65a1f7f 100644 --- a/qt/screenshoter.cpp +++ b/qt/screenshoter.cpp @@ -1,11 +1,13 @@ #include "qt/screenshoter.hpp" -#include "storage/storage.hpp" - #include "map/bookmark_helpers.hpp" #include "map/bookmark_manager.hpp" #include "map/framework.hpp" +#include "storage/storage.hpp" + +#include "platform/preferred_languages.hpp" + #include "coding/file_name_utils.hpp" #include "base/logging.hpp" @@ -25,7 +27,6 @@ Screenshoter::Screenshoter(ScreenshotParams const & screenshotParams, Framework { m_framework.GetStorage().Subscribe(std::bind(&Screenshoter::OnCountryChanged, this, std::placeholders::_1), [](storage::CountryId const &, storage::MapFilesDownloader::Progress const &) {}); - } void Screenshoter::Start() @@ -42,8 +43,6 @@ void Screenshoter::Start() return; } - m_framework.SetGraphicsReadyListener(std::bind(&Screenshoter::OnGraphicsReady, this)); - Platform::FilesList files; Platform::GetFilesByExt(m_screenshotParams.m_kmlPath, kKmlExtension, files); @@ -63,12 +62,13 @@ void Screenshoter::ProcessNextKml() CHECK_EQUAL(m_state, State::Ready, ()); ChangeState(State::LoadKml); + std::string const prefix = languages::GetCurrentNorm() + "_"; std::unique_ptr kmlData; while (kmlData == nullptr && !m_filesToProcess.empty()) { auto const filePath = m_filesToProcess.front(); m_filesToProcess.pop_front(); - m_nextScreenshotName = base::GetNameFromFullPathWithoutExt(filePath); + m_nextScreenshotName = prefix + base::GetNameFromFullPathWithoutExt(filePath); kmlData = LoadKmlFile(filePath, KmlFileType::Text); if (kmlData != nullptr && kmlData->m_bookmarksData.empty() && kmlData->m_tracksData.empty()) @@ -78,15 +78,11 @@ void Screenshoter::ProcessNextKml() if (kmlData == nullptr) { ChangeState(State::Done); - m_framework.SetGraphicsReadyListener(nullptr); return; } kmlData->m_categoryData.m_visible = true; - if (!kmlData->m_serverId.empty()) - m_nextScreenshotName = kmlData->m_serverId; - BookmarkManager::KMLDataCollection collection; collection.emplace_back("", std::move(kmlData)); @@ -126,10 +122,7 @@ void Screenshoter::PrepareCountries() } if (m_countriesToDownload.empty()) - { - ChangeState(State::WaitGraphics); - m_framework.InvalidateRect(m_framework.GetCurrentViewport()); - } + WaitGraphics(); } void Screenshoter::OnCountryChanged(storage::CountryId countryId) @@ -138,16 +131,11 @@ void Screenshoter::OnCountryChanged(storage::CountryId countryId) return; auto const status = m_framework.GetStorage().CountryStatusEx(countryId); - if (status == storage::Status::EOnDisk || - status == storage::Status::EDownloadFailed || - status == storage::Status::EOutOfMemFailed) + if (status == storage::Status::EOnDisk) { m_countriesToDownload.erase(countryId); if (m_countriesToDownload.empty()) - { - ChangeState(State::WaitGraphics); - m_framework.InvalidateRect(m_framework.GetCurrentViewport()); - } + WaitGraphics(); } } @@ -165,11 +153,15 @@ void Screenshoter::OnGraphicsReady() return; ChangeState(State::Ready); + SaveScreenshot(); +} - auto const kWaitGraphicsDelay = seconds(1); - GetPlatform().RunDelayedTask(Platform::Thread::File, kWaitGraphicsDelay, [this]() +void Screenshoter::WaitGraphics() +{ + ChangeState(State::WaitGraphics); + m_framework.NotifyGraphicsReady([this]() { - GetPlatform().RunTask(Platform::Thread::Gui, [this]() { SaveScreenshot(); }); + GetPlatform().RunTask(Platform::Thread::Gui, [this] { OnGraphicsReady(); }); }); } diff --git a/qt/screenshoter.hpp b/qt/screenshoter.hpp index d3c7cd180a..b48e010656 100644 --- a/qt/screenshoter.hpp +++ b/qt/screenshoter.hpp @@ -57,6 +57,7 @@ private: void ProcessNextKml(); void PrepareCountries(); void SaveScreenshot(); + void WaitGraphics(); void ChangeState(State newState); friend std::string DebugPrint(State state);