Review fixes

10 files changed, 64 insertions, 57 deletions

diff --git a/drape/batcher.cpp b/drape/batcher.cpp
index c8f9b5ac31..682ead9183 100644
--- a/drape/batcher.cpp
+++ b/drape/batcher.cpp
@@ -179,8 +179,8 @@ void Batcher::StartFeatureRecord(FeatureGeometryId feature, m2::RectD const & li
   if (!m_currentFeature.IsValid())
     return;
 
-  for (auto it = m_buckets.begin(); it != m_buckets.end(); ++it)
-    it->second->StartFeatureRecord(feature, limitRect);
+  for (auto const & bucket : m_buckets)
+    bucket.second->StartFeatureRecord(feature, limitRect);
 }
 
 void Batcher::EndFeatureRecord()
@@ -190,8 +190,8 @@ void Batcher::EndFeatureRecord()
 
   m_currentFeature = FeatureGeometryId();
 
-  for (auto it = m_buckets.begin(); it != m_buckets.end(); ++it)
-    it->second->EndFeatureRecord(true);
+  for (auto const & bucket : m_buckets)
+    bucket.second->EndFeatureRecord(true);
 }
 
 void Batcher::ChangeBuffer(ref_ptr<CallbacksWrapper> wrapper)
diff --git a/drape/batcher.hpp b/drape/batcher.hpp
index 3c71dc7002..01f51b8f66 100644
--- a/drape/batcher.hpp
+++ b/drape/batcher.hpp
@@ -50,6 +50,7 @@ public:
   void StartSession(TFlushFn const & flusher);
   void EndSession();
 
+  // Begin/end processing of feature with FeatureGeometryId in the batcher.
   void StartFeatureRecord(FeatureGeometryId feature, m2::RectD const & limitRect);
   void EndFeatureRecord();
 
@@ -79,13 +80,13 @@ private:
 
     bool operator < (BucketId const & other) const
     {
-      if (m_state == other.m_state)
-        return m_sharedFeatures < other.m_sharedFeatures;
-      return m_state < other.m_state;
+      if (m_state != other.m_state)
+        return m_state < other.m_state;
+      return m_sharedFeatures < other.m_sharedFeatures;
     }
 
     GLState m_state;
-    bool m_sharedFeatures;
+    bool m_sharedFeatures = false;
   };
 
   using TBuckets = map<BucketId, drape_ptr<RenderBucket>>;
diff --git a/drape/feature_geometry_decl.hpp b/drape/feature_geometry_decl.hpp
index 80745fa22e..7299ac3ec7 100644
--- a/drape/feature_geometry_decl.hpp
+++ b/drape/feature_geometry_decl.hpp
@@ -10,18 +10,18 @@ struct FeatureGeometryId
   FeatureID m_featureId;
   uint32_t m_shapeInd = 0;
 
-  FeatureGeometryId() {}
+  FeatureGeometryId() = default;
   FeatureGeometryId(FeatureID feature, uint32_t shapeInd)
     : m_featureId(feature)
     , m_shapeInd(shapeInd)
   {}
 
   bool IsValid() const { return m_featureId.IsValid(); }
-  inline bool operator<(FeatureGeometryId const & r) const
+  bool operator<(FeatureGeometryId const & r) const
   {
-    if (m_featureId == r.m_featureId)
-      return m_shapeInd < r.m_shapeInd;
-    return m_featureId < r.m_featureId;
+    if (m_featureId != r.m_featureId)
+      return m_featureId < r.m_featureId;
+    return m_shapeInd < r.m_shapeInd;
   }
 };
 
diff --git a/drape/glstate.cpp b/drape/glstate.cpp
index 83b0763050..091ba984c3 100644
--- a/drape/glstate.cpp
+++ b/drape/glstate.cpp
@@ -103,6 +103,11 @@ bool GLState::operator==(GLState const & other) const
          m_maskTexture == other.m_maskTexture;
 }
 
+bool GLState::operator!=(GLState const & other) const
+{
+  return !operator==(other);
+}
+
 namespace
 {
 
diff --git a/drape/glstate.hpp b/drape/glstate.hpp
index 3fa7788128..5946d6f493 100644
--- a/drape/glstate.hpp
+++ b/drape/glstate.hpp
@@ -69,6 +69,7 @@ public:
 
   bool operator<(GLState const & other) const;
   bool operator==(GLState const & other) const;
+  bool operator!=(GLState const & other) const;
 
 private:
   uint32_t m_gpuProgramIndex;
diff --git a/drape/overlay_handle.cpp b/drape/overlay_handle.cpp
index 761da448fd..fb9ea1d419 100644
--- a/drape/overlay_handle.cpp
+++ b/drape/overlay_handle.cpp
@@ -35,14 +35,14 @@ OverlayHandle::OverlayHandle(FeatureID const & id,
   , m_isBillboard(isBillboard)
   , m_isVisible(false)
   , m_enableCaching(false)
-  , m_extendedRectsDirty(true)
+  , m_extendedShapeDirty(true)
   , m_extendedRectDirty(true)
 {}
 
 void OverlayHandle::SetCachingEnable(bool enable)
 {
   m_enableCaching = enable;
-  m_extendedRectsDirty = true;
+  m_extendedShapeDirty = true;
   m_extendedRectDirty = true;
 }
 
@@ -155,15 +155,15 @@ m2::RectD OverlayHandle::GetExtendedPixelRect(ScreenBase const & screen) const
 
 OverlayHandle::Rects const & OverlayHandle::GetExtendedPixelShape(ScreenBase const & screen) const
 {
-  if (m_enableCaching && !m_extendedRectsDirty)
-    return m_extendedRectsCache;
+  if (m_enableCaching && !m_extendedShapeDirty)
+    return m_extendedShapeCache;
 
-  m_extendedRectsCache.clear();
-  GetPixelShape(screen, m_extendedRectsCache, screen.isPerspective());
-  for (auto & rect : m_extendedRectsCache)
+  m_extendedShapeCache.clear();
+  GetPixelShape(screen, m_extendedShapeCache, screen.isPerspective());
+  for (auto & rect : m_extendedShapeCache)
     rect.Inflate(m_extendingSize, m_extendingSize);
-  m_extendedRectsDirty = false;
-  return m_extendedRectsCache;
+  m_extendedShapeDirty = false;
+  return m_extendedShapeCache;
 }
 
 m2::RectD OverlayHandle::GetPerspectiveRect(m2::RectD const & pixelRect, ScreenBase const & screen) const
diff --git a/drape/overlay_handle.hpp b/drape/overlay_handle.hpp
index 83874ba4ba..1277ab25ae 100644
--- a/drape/overlay_handle.hpp
+++ b/drape/overlay_handle.hpp
@@ -128,8 +128,8 @@ private:
   set<TOffsetNode, LessOffsetNode> m_offsets;
 
   bool m_enableCaching;
-  mutable Rects m_extendedRectsCache;
-  mutable bool m_extendedRectsDirty;
+  mutable Rects m_extendedShapeCache;
+  mutable bool m_extendedShapeDirty;
   mutable m2::RectD m_extendedRectCache;
   mutable bool m_extendedRectDirty;
 };
diff --git a/drape/overlay_tree.cpp b/drape/overlay_tree.cpp
index cfc3fd401d..893b15c44c 100644
--- a/drape/overlay_tree.cpp
+++ b/drape/overlay_tree.cpp
@@ -6,7 +6,7 @@
 namespace dp
 {
 
-int const kFrameUpdarePeriod = 10;
+int const kFrameUpdatePeriod = 10;
 int const kAverageHandlesCount[dp::OverlayRanksCount] = { 300, 200, 50 };
 
 namespace
@@ -72,7 +72,7 @@ bool OverlayTree::Frame()
     return true;
 
   m_frameCounter++;
-  if (m_frameCounter >= kFrameUpdarePeriod)
+  if (m_frameCounter >= kFrameUpdatePeriod)
     m_frameCounter = -1;
 
   return IsNeedUpdate();
@@ -100,7 +100,7 @@ void OverlayTree::Remove(ref_ptr<OverlayHandle> handle)
   if (m_frameCounter == -1)
     return;
 
-  if (m_handlesCache.find(GetHandleKey(handle)) != m_handlesCache.end())
+  if (m_handlesCache.find(handle) != m_handlesCache.end())
     m_frameCounter = -1;
 }
 
@@ -113,7 +113,7 @@ void OverlayTree::Add(ref_ptr<OverlayHandle> handle)
   handle->SetIsVisible(false);
   handle->SetCachingEnable(true);
 
-  if (m_handlesCache.find(GetHandleKey(handle)) != m_handlesCache.end())
+  if (m_handlesCache.find(handle) != m_handlesCache.end())
     return;
 
   if (!handle->Update(modelView))
@@ -233,17 +233,17 @@ void OverlayTree::InsertHandle(ref_ptr<OverlayHandle> handle,
       // Delete rival handle and all handles bound to it.
       for (auto it = m_handlesCache.begin(); it != m_handlesCache.end();)
       {
-        if (it->second->GetFeatureID() == rivalHandle->GetFeatureID())
+        if ((*it)->GetFeatureID() == rivalHandle->GetFeatureID())
         {
-          Erase(it->second);
+          Erase(*it);
 
         #ifdef DEBUG_OVERLAYS_OUTPUT
-          LOG(LINFO, ("Displace (2):", handle->GetOverlayDebugInfo(), "->", it->second->GetOverlayDebugInfo()));
+          LOG(LINFO, ("Displace (2):", handle->GetOverlayDebugInfo(), "->", it->GetOverlayDebugInfo()));
         #endif
 
         #ifdef COLLECT_DISPLACEMENT_INFO
           m_displacementInfo.emplace_back(DisplacementData(handle->GetExtendedPixelRect(modelView).Center(),
-                                                           it->second->GetExtendedPixelRect(modelView).Center(),
+                                                           it->GetExtendedPixelRect(modelView).Center(),
                                                            dp::Color(0, 0, 255, 255)));
         #endif
 
@@ -271,7 +271,7 @@ void OverlayTree::InsertHandle(ref_ptr<OverlayHandle> handle,
     }
   }
 
-  m_handlesCache.insert(make_pair(GetHandleKey(handle), handle));
+  m_handlesCache.insert(handle);
   TBase::Add(handle, pixelRect);
 }
 
@@ -300,10 +300,10 @@ void OverlayTree::EndOverlayPlacing()
     m_handles[rank].clear();
   }
 
-  for (auto it = m_handlesCache.begin(); it != m_handlesCache.end(); ++it)
+  for (auto const & handle : m_handlesCache)
   {
-    it->second->SetIsVisible(true);
-    it->second->SetCachingEnable(false);
+    handle->SetIsVisible(true);
+    handle->SetCachingEnable(false);
   }
 
   m_frameCounter = 0;
@@ -320,12 +320,12 @@ bool OverlayTree::CheckHandle(ref_ptr<OverlayHandle> handle, int currentRank,
     return true;
 
   int const seachingRank = currentRank - 1;
-  for (auto it = m_handlesCache.begin(); it != m_handlesCache.end(); ++it)
+  for (auto const & h : m_handlesCache)
   {
-    if (it->second->GetFeatureID() == handle->GetFeatureID() &&
-        it->second->GetOverlayRank() == seachingRank)
+    if (h->GetFeatureID() == handle->GetFeatureID() &&
+        h->GetOverlayRank() == seachingRank)
     {
-      parentOverlay = it->second;
+      parentOverlay = h;
       return true;
     }
   }
@@ -335,17 +335,12 @@ bool OverlayTree::CheckHandle(ref_ptr<OverlayHandle> handle, int currentRank,
 
 void OverlayTree::DeleteHandle(ref_ptr<OverlayHandle> const & handle)
 {
-  size_t const deletedCount = m_handlesCache.erase(GetHandleKey(handle));
+  size_t const deletedCount = m_handlesCache.erase(handle);
   ASSERT_NOT_EQUAL(deletedCount, 0, ());
   if (deletedCount != 0)
     Erase(handle);
 }
 
-uint64_t OverlayTree::GetHandleKey(ref_ptr<OverlayHandle> const & handle) const
-{
-  return reinterpret_cast<uint64_t>(handle.get());
-}
-
 void OverlayTree::Select(m2::PointD const & glbPoint, TOverlayContainer & result) const
 {
   ScreenBase const & screen = m_traits.m_modelView;
@@ -355,10 +350,10 @@ void OverlayTree::Select(m2::PointD const & glbPoint, TOverlayContainer & result
   m2::RectD rect(pxPoint, pxPoint);
   rect.Inflate(kSearchRectHalfSize, kSearchRectHalfSize);
 
-  for (auto it = m_handlesCache.begin(); it != m_handlesCache.end(); ++it)
+  for (auto const & handle : m_handlesCache)
   {
-    if (rect.IsPointInside(it->second->GetPivot(screen, false)))
-      result.push_back(it->second);
+    if (rect.IsPointInside(handle->GetPivot(screen, false)))
+      result.push_back(handle);
   }
 }
 
diff --git a/drape/overlay_tree.hpp b/drape/overlay_tree.hpp
index 878bd1938b..817680ff45 100644
--- a/drape/overlay_tree.hpp
+++ b/drape/overlay_tree.hpp
@@ -9,7 +9,7 @@
 
 #include "std/array.hpp"
 #include "std/vector.hpp"
-#include "std/unordered_map.hpp"
+#include "std/unordered_set.hpp"
 
 namespace dp
 {
@@ -29,6 +29,16 @@ struct OverlayTraits
   }
 };
 
+struct OverlayHasher
+{
+  hash<OverlayHandle*> m_hasher;
+
+  size_t operator()(ref_ptr<OverlayHandle> const & handle) const
+  {
+    return m_hasher(handle.get());
+  }
+};
+
 }
 
 using TOverlayContainer = buffer_vector<ref_ptr<OverlayHandle>, 8>;
@@ -75,14 +85,9 @@ private:
                    ref_ptr<OverlayHandle> & parentOverlay) const;
   void DeleteHandle(ref_ptr<OverlayHandle> const & handle);
 
-  uint64_t GetHandleKey(ref_ptr<OverlayHandle> const & handle) const;
-
   int m_frameCounter;
   array<vector<ref_ptr<OverlayHandle>>, dp::OverlayRanksCount> m_handles;
-  vector<ref_ptr<OverlayHandle>> m_handlesToDelete;
-
-  unordered_map<uint64_t, ref_ptr<OverlayHandle>> m_handlesCache;
-
+  unordered_set<ref_ptr<OverlayHandle>, detail::OverlayHasher> m_handlesCache;
   bool m_followingMode;
 
 #ifdef COLLECT_DISPLACEMENT_INFO
diff --git a/drape/vertex_array_buffer.hpp b/drape/vertex_array_buffer.hpp
index 7083e816d8..2ac9c1fb09 100644
--- a/drape/vertex_array_buffer.hpp
+++ b/drape/vertex_array_buffer.hpp
@@ -67,7 +67,7 @@ public:
                      ref_ptr<AttributeBufferMutator> attrMutator);
 
   void ResetChangingTracking() { m_isChanged = false; }
-  bool IsChanged() { return m_isChanged; }
+  bool IsChanged() const { return m_isChanged; }
 
 private:
   ref_ptr<DataBuffer> GetOrCreateStaticBuffer(BindingInfo const & bindingInfo);