Code-review fixes

drape_frontend/base_renderer.cpp

@@ -11,11 +11,32 @@ BaseRenderer::BaseRenderer()
 {
 }
 
-void BaseRenderer::SetRenderingEnabled(bool const isEnabled, completion_handler_t completionHandler)
+void BaseRenderer::SetRenderingEnabled(bool const isEnabled)
+{
+  // here we have to wait for completion of internal SetRenderingEnabled
+  mutex completionMutex;
+  condition_variable completionCondition;
+  bool notified = false;
+  auto completionHandler = [&]()
+  {
+    lock_guard<mutex> lock(completionMutex);
+    notified = true;
+    completionCondition.notify_one();
+  };
+
+  SetRenderingEnabled(isEnabled, completionHandler);
+
+  unique_lock<mutex> lock(completionMutex);
+  completionCondition.wait(lock, [&notified] { return notified; });
+}
+
+void BaseRenderer::SetRenderingEnabled(bool const isEnabled, TCompletionHandler completionHandler)
 {
   if (isEnabled == m_isEnabled)
   {
-    if (completionHandler != nullptr) completionHandler();
+    if (completionHandler != nullptr)
+      completionHandler();
+
     return;
   }
 
@@ -23,7 +44,7 @@ void BaseRenderer::SetRenderingEnabled(bool const isEnabled, completion_handler_
   if (isEnabled)
   {
     // wake up rendering thread
-    unique_lock<mutex> lock(m_renderingEnablingMutex);
+    lock_guard<mutex> lock(m_renderingEnablingMutex);
     m_wasNotified = true;
     m_renderingEnablingCondition.notify_one();
   }
@@ -42,24 +63,28 @@ void BaseRenderer::CheckRenderingEnabled()
   if (!m_isEnabled)
   {
     // nofity initiator-thread about rendering disabling
-    if (m_renderingEnablingCompletionHandler != nullptr)
-      m_renderingEnablingCompletionHandler();
+    Notify();
 
     // wait for signal
     unique_lock<mutex> lock(m_renderingEnablingMutex);
-    while(!m_wasNotified)
-    {
-      m_renderingEnablingCondition.wait(lock);
-    }
+    m_renderingEnablingCondition.wait(lock, [this] { return m_wasNotified; });
 
     // here rendering is enabled again
     m_wasNotified = false;
     m_isEnabled = true;
 
     // nofity initiator-thread about rendering enabling
-    if (m_renderingEnablingCompletionHandler != nullptr)
-      m_renderingEnablingCompletionHandler();
+    // m_renderingEnablingCompletionHandler will be setup before awakening of this thread
+    Notify();
   }
 }
 
+void BaseRenderer::Notify()
+{
+  if (m_renderingEnablingCompletionHandler != nullptr)
+    m_renderingEnablingCompletionHandler();
+
+  m_renderingEnablingCompletionHandler = nullptr;
+}
+
 } // namespace df

drape_frontend/base_renderer.hpp

@@ -1,10 +1,10 @@
 #pragma once
 
 #include "message_acceptor.hpp"
-#include "../std/condition_variable.hpp"
-#include "../std/mutex.hpp"
 #include "../std/atomic.hpp"
+#include "../std/condition_variable.hpp"
 #include "../std/function.hpp"
+#include "../std/mutex.hpp"
 
 namespace df
 {
@@ -12,11 +12,10 @@ namespace df
 class BaseRenderer : public MessageAcceptor
 {
 public:
-  using completion_handler_t = function<void()>;
+  using TCompletionHandler = function<void()>;
 
   BaseRenderer();
-
-  void SetRenderingEnabled(bool const isEnabled, completion_handler_t completionHandler);
+  void SetRenderingEnabled(bool const isEnabled);
 
 protected:
   void CheckRenderingEnabled();
@@ -25,8 +24,11 @@ private:
   mutex m_renderingEnablingMutex;
   condition_variable m_renderingEnablingCondition;
   atomic<bool> m_isEnabled;
-  completion_handler_t m_renderingEnablingCompletionHandler;
+  TCompletionHandler m_renderingEnablingCompletionHandler;
   bool m_wasNotified;
+
+  void SetRenderingEnabled(bool const isEnabled, TCompletionHandler completionHandler);
+  void Notify();
 };
 
 } // namespace df

drape_frontend/drape_engine.cpp

@@ -96,32 +96,10 @@ void DrapeEngine::UpdateUserMarksLayer(TileKey const & tileKey, UserMarksProvide
 
 void DrapeEngine::SetRenderingEnabled(bool const isEnabled)
 {
-  SetRenderingEnabled(m_frontend.GetRefPointer(), isEnabled);
-  SetRenderingEnabled(m_backend.GetRefPointer(), isEnabled);
+  m_frontend->SetRenderingEnabled(isEnabled);
+  m_backend->SetRenderingEnabled(isEnabled);
 
   LOG(LDEBUG, (isEnabled ? "Rendering enabled" : "Rendering disabled"));
 }
 
-void DrapeEngine::SetRenderingEnabled(dp::RefPointer<BaseRenderer> renderer, bool const isEnabled)
-{
-  // here we have to wait for finishing of message processing
-  mutex completionMutex;
-  condition_variable completionCondition;
-  bool notified = false;
-  auto completionHandler = [&]()
-  {
-    unique_lock<mutex> lock(completionMutex);
-    notified = true;
-    completionCondition.notify_one();
-  };
-
-  renderer->SetRenderingEnabled(isEnabled, completionHandler);
-
-  unique_lock<mutex> lock(completionMutex);
-  while (!notified) // loop to avoid spurious wakeups
-  {
-    completionCondition.wait(lock);
-  }
-}
-
 } // namespace df

drape_frontend/drape_engine.hpp

@@ -43,8 +43,6 @@ private:
   dp::MasterPointer<dp::TextureManager> m_textureManager;
 
   Viewport m_viewport;
-
-  void SetRenderingEnabled(dp::RefPointer<BaseRenderer> renderer, bool const isEnabled);
 };
 
 } // namespace df

drape_frontend/drape_frontend.pro

@@ -9,72 +9,72 @@ INCLUDEPATH *= $$ROOT_DIR/3party/protobuf/src
 DEFINES += DRAW_INFO
 
 SOURCES += \
-    engine_context.cpp \
-    memory_feature_index.cpp \
-    message_queue.cpp \
-    threads_commutator.cpp \
-    message_acceptor.cpp \
-    backend_renderer.cpp \
-    read_mwm_task.cpp \
-    batchers_pool.cpp \
-    frontend_renderer.cpp \
-    drape_engine.cpp \
-    area_shape.cpp \
-    read_manager.cpp \
-    tile_info.cpp \
-    stylist.cpp \
-    line_shape.cpp \
-    rule_drawer.cpp \
-    viewport.cpp \
-    tile_key.cpp \
     apply_feature_functors.cpp \
-    visual_params.cpp \
-    poi_symbol_shape.cpp \
+    area_shape.cpp \
+    backend_renderer.cpp \
+    base_renderer.cpp \
+    batchers_pool.cpp \
     circle_shape.cpp \
-    render_group.cpp \
-    text_shape.cpp \
-    path_text_shape.cpp \
-    path_symbol_shape.cpp \
-    text_layout.cpp \
+    drape_engine.cpp \
+    engine_context.cpp \
+    frontend_renderer.cpp \
+    line_shape.cpp \
     map_data_provider.cpp \
+    memory_feature_index.cpp \
+    message_acceptor.cpp \
+    message_queue.cpp \
+    path_symbol_shape.cpp \
+    path_text_shape.cpp \
+    poi_symbol_shape.cpp \
+    read_manager.cpp \
+    read_mwm_task.cpp \
+    render_group.cpp \
+    rule_drawer.cpp \
+    stylist.cpp \
+    text_layout.cpp \
+    text_shape.cpp \
+    threads_commutator.cpp \
+    tile_info.cpp \
+    tile_key.cpp \
     user_mark_shapes.cpp \
     user_marks_provider.cpp \
-    base_renderer.cpp
+    viewport.cpp \
+    visual_params.cpp \
 
 HEADERS += \
-    engine_context.hpp \
-    memory_feature_index.hpp \
-    tile_info.hpp \
-    message_queue.hpp \
-    message.hpp \
-    threads_commutator.hpp \
-    message_acceptor.hpp \
-    backend_renderer.hpp \
-    read_mwm_task.hpp \
-    message_subclasses.hpp \
-    map_shape.hpp \
-    batchers_pool.hpp \
-    frontend_renderer.hpp \
-    drape_engine.hpp \
-    area_shape.hpp \
-    read_manager.hpp \
-    stylist.hpp \
-    line_shape.hpp \
-    shape_view_params.hpp \
-    rule_drawer.hpp \
-    viewport.hpp \
-    tile_key.hpp \
     apply_feature_functors.hpp \
-    visual_params.hpp \
-    poi_symbol_shape.hpp \
+    area_shape.hpp \
+    backend_renderer.hpp \
+    base_renderer.hpp \
+    batchers_pool.hpp \
     circle_shape.hpp \
-    render_group.hpp \
-    text_shape.hpp \
-    path_text_shape.hpp \
-    path_symbol_shape.hpp \
-    text_layout.hpp \
+    drape_engine.hpp \
+    engine_context.hpp \
+    frontend_renderer.hpp \
     intrusive_vector.hpp \
+    line_shape.hpp \
     map_data_provider.hpp \
+    map_shape.hpp \
+    memory_feature_index.hpp \
+    message.hpp \
+    message_acceptor.hpp \
+    message_queue.hpp \
+    message_subclasses.hpp \
+    path_symbol_shape.hpp \
+    path_text_shape.hpp \
+    poi_symbol_shape.hpp \
+    read_manager.hpp \
+    read_mwm_task.hpp \
+    render_group.hpp \
+    rule_drawer.hpp \
+    shape_view_params.hpp \
+    stylist.hpp \
+    text_layout.hpp \
+    text_shape.hpp \
+    threads_commutator.hpp \
+    tile_info.hpp \
+    tile_key.hpp \
     user_mark_shapes.hpp \
     user_marks_provider.hpp \
-    base_renderer.hpp
+    viewport.hpp \
+    visual_params.hpp \

drape_frontend/engine_context.cpp

@@ -21,12 +21,12 @@ EngineContext::EngineContext(dp::RefPointer<ThreadsCommutator> commutator)
 
 void EngineContext::BeginReadTile(TileKey const & key)
 {
-  PostMessage(new TileReadStartMessage(key), MessagePriority::Normal);
+  PostMessage(new TileReadStartMessage(key));
 }
 
 void EngineContext::InsertShape(TileKey const & key, dp::TransferPointer<MapShape> shape)
 {
-  PostMessage(new MapShapeReadedMessage(key, shape), MessagePriority::Normal);
+  PostMessage(new MapShapeReadedMessage(key, shape));
 }
 
 void EngineContext::EndReadTile(TileKey const & key)
@@ -63,12 +63,14 @@ void EngineContext::EndReadTile(TileKey const & key)
   InsertShape(key, dp::MovePointer<df::MapShape>(new TextShape(r.Center(), tp)));
 #endif
 
-  PostMessage(new TileReadEndMessage(key), MessagePriority::Normal);
+  PostMessage(new TileReadEndMessage(key));
 }
 
-void EngineContext::PostMessage(Message * message, MessagePriority priority)
+void EngineContext::PostMessage(Message * message)
 {
-  m_commutator->PostMessage(ThreadsCommutator::ResourceUploadThread, dp::MovePointer(message), priority);
+  m_commutator->PostMessage(ThreadsCommutator::ResourceUploadThread,
+                            dp::MovePointer(message),
+                            MessagePriority::Normal);
 }
 
 } // namespace df

drape_frontend/engine_context.hpp

@@ -7,9 +7,8 @@
 namespace df
 {
 
-class Message;
-enum class MessagePriority;
 class MapShape;
+class Message;
 struct TileKey;
 
 class EngineContext
@@ -24,7 +23,7 @@ public:
   void EndReadTile(TileKey const & key);
 
 private:
-  void PostMessage(Message * message, MessagePriority priority);
+  void PostMessage(Message * message);
 
 private:
   dp::RefPointer<ThreadsCommutator> m_commutator;

drape_frontend/message.hpp

@@ -21,8 +21,7 @@ public:
     ClearUserMarkLayer,
     ChangeUserMarkLayerVisibility,
     UpdateUserMarkLayer,
-    GuiLayerRecached,
-    RenderingEnabled
+    GuiLayerRecached
   };
 
   virtual ~Message() {}

drape_frontend/threads_commutator.cpp

@@ -14,7 +14,7 @@ void ThreadsCommutator::RegisterThread(ThreadName name, MessageAcceptor * accept
 
 void ThreadsCommutator::PostMessage(ThreadName name, dp::TransferPointer<Message> message, MessagePriority priority)
 {
-  acceptors_map_t::iterator it = m_acceptors.find(name);
+  TAcceptorsMap::iterator it = m_acceptors.find(name);
   ASSERT(it != m_acceptors.end(), ());
   if (it != m_acceptors.end())
     it->second->PostMessage(message, priority);

drape_frontend/threads_commutator.hpp

@@ -23,8 +23,8 @@ public:
   void PostMessage(ThreadName name, dp::TransferPointer<Message> message, MessagePriority priority);
 
 private:
-  typedef map<ThreadName, MessageAcceptor *> acceptors_map_t;
-  acceptors_map_t m_acceptors;
+  typedef map<ThreadName, MessageAcceptor *> TAcceptorsMap;
+  TAcceptorsMap m_acceptors;
 };
 
 } // namespace df

std/utility.hpp

@@ -8,7 +8,6 @@
 using std::pair;
 using std::make_pair;
 using std::move;
-using std::forward;
 
 #ifdef DEBUG_NEW
 #define new DEBUG_NEW