From 023873e64e65730b05e8f9d9c0a6f3250c92281e Mon Sep 17 00:00:00 2001 From: Viktor Govako Date: Wed, 9 Feb 2022 20:03:05 +0300 Subject: [PATCH] [drape] Fixed OverlayTree::InsertHandle hangs up. Signed-off-by: Viktor Govako --- drape/binding_info.hpp | 1 - drape/overlay_handle.cpp | 46 +++----------- drape/overlay_handle.hpp | 42 +++++++++---- drape/overlay_tree.cpp | 130 +++++++++++++++++++++++++-------------- drape/overlay_tree.hpp | 14 ++++- 5 files changed, 136 insertions(+), 97 deletions(-) diff --git a/drape/binding_info.hpp b/drape/binding_info.hpp index 62603faed3..231b9e873f 100644 --- a/drape/binding_info.hpp +++ b/drape/binding_info.hpp @@ -31,7 +31,6 @@ class BindingInfo public: BindingInfo(); explicit BindingInfo(uint8_t count, uint8_t id = 0); - ~BindingInfo() = default; uint8_t GetCount() const; uint8_t GetID() const; diff --git a/drape/overlay_handle.cpp b/drape/overlay_handle.cpp index 999c0e42f5..c021484d47 100644 --- a/drape/overlay_handle.cpp +++ b/drape/overlay_handle.cpp @@ -38,38 +38,18 @@ OverlayHandle::OverlayHandle(OverlayID const & id, dp::Anchor anchor, , m_minVisibleScale(minVisibleScale) , m_isBillboard(isBillboard) , m_isVisible(false) - , m_enableCaching(false) + , m_caching(false) , m_extendedShapeDirty(true) , m_extendedRectDirty(true) {} -void OverlayHandle::SetCachingEnable(bool enable) +void OverlayHandle::EnableCaching(bool enable) { - m_enableCaching = enable; + m_caching = enable; m_extendedShapeDirty = true; m_extendedRectDirty = true; } -bool OverlayHandle::IsVisible() const -{ - return m_isVisible; -} - -void OverlayHandle::SetIsVisible(bool isVisible) -{ - m_isVisible = isVisible; -} - -int OverlayHandle::GetMinVisibleScale() const -{ - return m_minVisibleScale; -} - -bool OverlayHandle::IsBillboard() const -{ - return m_isBillboard; -} - m2::PointD OverlayHandle::GetPivot(ScreenBase const & screen, bool perspective) const { m2::RectD r = GetPixelRect(screen, false); @@ -133,31 +113,19 @@ bool OverlayHandle::HasDynamicAttributes() const void OverlayHandle::AddDynamicAttribute(BindingInfo const & binding, uint32_t offset, uint32_t count) { ASSERT(binding.IsDynamic(), ()); - ASSERT(std::find_if(m_offsets.begin(), m_offsets.end(), - OffsetNodeFinder(binding.GetID())) == m_offsets.end(), ()); - m_offsets.insert(std::make_pair(binding, MutateRegion(offset, count))); -} - -OverlayID const & OverlayHandle::GetOverlayID() const -{ - return m_id; -} - -uint64_t const & OverlayHandle::GetPriority() const -{ - return m_priority; + VERIFY(m_offsets.emplace(binding, MutateRegion(offset, count)).second, ()); } OverlayHandle::TOffsetNode const & OverlayHandle::GetOffsetNode(uint8_t bufferID) const { - auto const it = std::find_if(m_offsets.begin(), m_offsets.end(), OffsetNodeFinder(bufferID)); + auto const it = m_offsets.find(bufferID); ASSERT(it != m_offsets.end(), ()); return *it; } m2::RectD OverlayHandle::GetExtendedPixelRect(ScreenBase const & screen) const { - if (m_enableCaching && !m_extendedRectDirty) + if (m_caching && !m_extendedRectDirty) return m_extendedRectCache; m_extendedRectCache = GetPixelRect(screen, screen.isPerspective()); @@ -168,7 +136,7 @@ m2::RectD OverlayHandle::GetExtendedPixelRect(ScreenBase const & screen) const OverlayHandle::Rects const & OverlayHandle::GetExtendedPixelShape(ScreenBase const & screen) const { - if (m_enableCaching && !m_extendedShapeDirty) + if (m_caching && !m_extendedShapeDirty) return m_extendedShapeCache; m_extendedShapeCache.clear(); diff --git a/drape/overlay_handle.hpp b/drape/overlay_handle.hpp index 61782ef75d..4557798d5b 100644 --- a/drape/overlay_handle.hpp +++ b/drape/overlay_handle.hpp @@ -53,14 +53,23 @@ struct OverlayID : m_featureId(featureId) {} - OverlayID(FeatureID const & featureId, kml::MarkId markId, m2::PointI const & tileCoords, - uint32_t index) + OverlayID(FeatureID const & featureId, kml::MarkId markId, m2::PointI const & tileCoords, uint32_t index) : m_featureId(featureId) , m_markId(markId) , m_tileCoords(tileCoords) , m_index(index) {} + bool IsValid() const + { + return m_featureId.IsValid() || m_markId != kml::kInvalidMarkId; + } + + static OverlayID GetLowerKey(FeatureID const & featureID) + { + return {featureID, 0, {-1, -1}, 0}; + } + auto AsTupleOfRefs() const { return std::tie(m_featureId, m_markId, m_tileCoords, m_index); @@ -101,13 +110,13 @@ public: OverlayHandle(OverlayID const & id, dp::Anchor anchor, uint64_t priority, int minVisibleScale, bool isBillboard); - virtual ~OverlayHandle() {} + virtual ~OverlayHandle() = default; - bool IsVisible() const; - void SetIsVisible(bool isVisible); + bool IsVisible() const { return m_isVisible; } + void SetIsVisible(bool isVisible) { m_isVisible = isVisible; } - int GetMinVisibleScale() const; - bool IsBillboard() const; + int GetMinVisibleScale() const { return m_minVisibleScale; } + bool IsBillboard() const { return m_isBillboard; } virtual m2::PointD GetPivot(ScreenBase const & screen, bool perspective) const; @@ -135,8 +144,8 @@ public: bool HasDynamicAttributes() const; void AddDynamicAttribute(BindingInfo const & binding, uint32_t offset, uint32_t count); - OverlayID const & GetOverlayID() const; - uint64_t const & GetPriority() const; + OverlayID const & GetOverlayID() const { return m_id; } + uint64_t const & GetPriority() const { return m_priority; } virtual uint64_t GetPriorityMask() const { return kPriorityMaskAll; } @@ -148,7 +157,8 @@ public: int GetOverlayRank() const { return m_overlayRank; } void SetOverlayRank(int overlayRank) { m_overlayRank = overlayRank; } - void SetCachingEnable(bool enable); + void EnableCaching(bool enable); + bool IsCachingEnabled() const { return m_caching; } void SetReady(bool isReady) { m_isReady = isReady; } bool IsReady() const { return m_isReady; } @@ -186,17 +196,27 @@ private: dp::IndexStorage m_indexes; struct LessOffsetNode { + typedef bool is_transparent; + bool operator()(TOffsetNode const & node1, TOffsetNode const & node2) const { return node1.first.GetID() < node2.first.GetID(); } + bool operator()(uint8_t node1, TOffsetNode const & node2) const + { + return node1 < node2.first.GetID(); + } + bool operator()(TOffsetNode const & node1, uint8_t node2) const + { + return node1.first.GetID() < node2; + } }; struct OffsetNodeFinder; std::set m_offsets; - bool m_enableCaching; + bool m_caching; mutable Rects m_extendedShapeCache; mutable bool m_extendedShapeDirty; mutable m2::RectD m_extendedRectCache; diff --git a/drape/overlay_tree.cpp b/drape/overlay_tree.cpp index 0eda2f600f..ce2e289fcc 100644 --- a/drape/overlay_tree.cpp +++ b/drape/overlay_tree.cpp @@ -104,6 +104,7 @@ void OverlayTree::Clear() InvalidateOnNextFrame(); TBase::Clear(); m_handlesCache.clear(); + m_overlayIdCache.clear(); for (auto & handles : m_handles) handles.clear(); m_displacers.clear(); @@ -148,6 +149,7 @@ void OverlayTree::StartOverlayPlacing(ScreenBase const & screen, int zoomLevel) ASSERT(IsNeedUpdate(), ()); TBase::Clear(); m_handlesCache.clear(); + m_overlayIdCache.clear(); m_traits.SetModelView(screen); m_displacementInfo.clear(); m_zoomLevel = zoomLevel; @@ -162,7 +164,7 @@ bool OverlayTree::Remove(ref_ptr handle) return true; } - if (m_handlesCache.find(handle) != m_handlesCache.end()) + if (IsInCache(handle)) { Clear(); return true; @@ -172,6 +174,8 @@ bool OverlayTree::Remove(ref_ptr handle) void OverlayTree::Add(ref_ptr handle) { + /// @todo Fires when deleting downloaded country ?! + //ASSERT(handle->GetOverlayID().IsValid(), ()); ASSERT(IsNeedUpdate(), ()); ScreenBase const & modelView = GetModelView(); @@ -181,10 +185,10 @@ void OverlayTree::Add(ref_ptr handle) if (m_zoomLevel < handle->GetMinVisibleScale()) return; - handle->SetCachingEnable(true); + handle->EnableCaching(true); // Skip duplicates. - if (m_handlesCache.find(handle) != m_handlesCache.end()) + if (IsInCache(handle)) return; // Skip not-ready handles. @@ -219,6 +223,7 @@ void OverlayTree::Add(ref_ptr handle) void OverlayTree::InsertHandle(ref_ptr handle, int currentRank, ref_ptr const & parentOverlay) { + ASSERT(handle->GetOverlayID().IsValid(), ()); ASSERT(IsNeedUpdate(), ()); #ifdef DEBUG_OVERLAYS_OUTPUT @@ -228,10 +233,13 @@ void OverlayTree::InsertHandle(ref_ptr handle, int currentRank, #endif ScreenBase const & modelView = GetModelView(); + ASSERT(handle->IsCachingEnabled(), ()); m2::RectD const pixelRect = handle->GetExtendedPixelRect(modelView); + if (!m_isDisplacementEnabled) { m_handlesCache.insert(handle); + m_overlayIdCache[handle->GetOverlayID()].push_back(handle); TBase::Add(handle, pixelRect); return; } @@ -268,7 +276,6 @@ void OverlayTree::InsertHandle(ref_ptr handle, int currentRank, for (auto const & rivalHandle : rivals) { bool reject = m_selectedFeatureID.IsValid() && rivalHandle->GetOverlayID().m_featureId == m_selectedFeatureID; - if (!reject) { if (modelView.isPerspective()) @@ -288,7 +295,6 @@ void OverlayTree::InsertHandle(ref_ptr handle, int currentRank, } } - if (reject) { // Handle is displaced and bound to its parent, parent will be displaced too. @@ -309,18 +315,15 @@ void OverlayTree::InsertHandle(ref_ptr handle, int currentRank, if (rivalHandle->IsBound()) { // Delete rival handle and all handles bound to it. - for (auto it = m_handlesCache.begin(); it != m_handlesCache.end();) + auto it = m_overlayIdCache.find(rivalHandle->GetOverlayID()); + if (it != m_overlayIdCache.end()) { - if ((*it)->GetOverlayID() == rivalHandle->GetOverlayID()) + for (auto const & h : it->second) { - Erase(*it); - StoreDisplacementInfo(2 /* case index */, handle, *it); - it = m_handlesCache.erase(it); - } - else - { - ++it; + DeleteHandleImpl(h); + StoreDisplacementInfo(2 /* case index */, handle, h); } + m_overlayIdCache.erase(it); } } else @@ -331,6 +334,7 @@ void OverlayTree::InsertHandle(ref_ptr handle, int currentRank, } m_handlesCache.insert(handle); + m_overlayIdCache[handle->GetOverlayID()].push_back(handle); TBase::Add(handle, pixelRect); } @@ -349,16 +353,15 @@ void OverlayTree::EndOverlayPlacing() for (int rank = 0; rank < dp::OverlayRanksCount; rank++) { std::sort(m_handles[rank].begin(), m_handles[rank].end(), comparator); + for (auto const & handle : m_handles[rank]) { ref_ptr parentOverlay; - if (!CheckHandle(handle, rank, parentOverlay)) - continue; - - InsertHandle(handle, rank, parentOverlay); + if (CheckHandle(handle, rank, parentOverlay)) + InsertHandle(handle, rank, parentOverlay); } } - + for (int rank = 0; rank < dp::OverlayRanksCount; rank++) { for (auto const & handle : m_handles[rank]) @@ -370,7 +373,7 @@ void OverlayTree::EndOverlayPlacing() { handle->SetDisplayFlag(true); handle->SetIsVisible(true); - handle->SetCachingEnable(false); + handle->EnableCaching(false); } m_frameCounter = 0; @@ -394,32 +397,59 @@ ref_ptr OverlayTree::FindParent(ref_ptr handle, in { ASSERT_GREATER_OR_EQUAL(searchingRank, 0, ()); ASSERT_LESS(searchingRank, static_cast(m_handles.size()), ()); - for (auto const & h : m_handles[searchingRank]) + + auto it = m_overlayIdCache.find(handle->GetOverlayID()); + if (it != m_overlayIdCache.end()) { - if (h->GetOverlayID() == handle->GetOverlayID() && m_handlesCache.find(h) != m_handlesCache.end()) - return h; + for (auto const & h : it->second) + if (h->GetOverlayRank() == searchingRank) + return h; } return nullptr; } -void OverlayTree::DeleteHandle(ref_ptr const & handle) +bool OverlayTree::DeleteHandleImpl(ref_ptr handle) { - size_t const deletedCount = m_handlesCache.erase(handle); - if (deletedCount != 0) + if (m_handlesCache.erase(handle) > 0) + { Erase(handle); + return true; + } + return false; +} + +void OverlayTree::DeleteHandle(ref_ptr handle) +{ + if (DeleteHandleImpl(handle)) + { + auto it = m_overlayIdCache.find(handle->GetOverlayID()); + ASSERT(it != m_overlayIdCache.end(), ()); + + auto & v = it->second; + v.erase_if([&handle](ref_ptr const & h) { return handle == h; }); + if (v.empty()) + m_overlayIdCache.erase(it); + } } void OverlayTree::DeleteHandleWithParents(ref_ptr handle, int currentRank) { - currentRank--; - while (currentRank >= dp::OverlayRank0) + auto it = m_overlayIdCache.find(handle->GetOverlayID()); + ASSERT(it != m_overlayIdCache.end(), ()); + + auto & v = it->second; + v.erase_if([&](ref_ptr const & h) { - auto parent = FindParent(handle, currentRank); - if (parent != nullptr && parent->IsBound()) - DeleteHandle(parent); - currentRank--; - } - DeleteHandle(handle); + if (h == handle || (h->GetOverlayRank() < currentRank && h->IsBound())) + { + DeleteHandleImpl(h); + return true; + } + return false; + }); + + if (v.empty()) + m_overlayIdCache.erase(it); } bool OverlayTree::GetSelectedFeatureRect(ScreenBase const & screen, m2::RectD & featureRect) @@ -428,13 +458,13 @@ bool OverlayTree::GetSelectedFeatureRect(ScreenBase const & screen, m2::RectD & return false; auto resultRect = m2::RectD::GetEmptyRect(); - for (auto const & handle : m_handlesCache) + for (auto it = m_overlayIdCache.lower_bound(OverlayID::GetLowerKey(m_selectedFeatureID)); + it != m_overlayIdCache.end() && it->first.m_featureId == m_selectedFeatureID; ++it) { - CHECK(handle != nullptr, ()); - if (handle->IsVisible() && handle->GetOverlayID().m_featureId == m_selectedFeatureID) + for (auto const & handle : it->second) { - m2::RectD rect = handle->GetPixelRect(screen, screen.isPerspective()); - resultRect.Add(rect); + if (handle->IsVisible()) + resultRect.Add(handle->GetPixelRect(screen, screen.isPerspective())); } } @@ -456,6 +486,8 @@ void OverlayTree::Select(m2::PointD const & glbPoint, TOverlayContainer & result m2::RectD rect(pxPoint, pxPoint); rect.Inflate(kSearchRectHalfSize, kSearchRectHalfSize); + /// @todo Why we can't call Select(rect) here? + /// Why we don't check handle->IsVisible(), like in Select(rect)? for (auto const & handle : m_handlesCache) { if (!handle->HasLinearFeatureShape() && rect.IsPointInside(handle->GetPivot(screen, false))) @@ -469,7 +501,9 @@ void OverlayTree::Select(m2::RectD const & rect, TOverlayContainer & result) con ForEachInRect(rect, [&](ref_ptr const & h) { auto const & overlayId = h->GetOverlayID(); - if (!h->HasLinearFeatureShape() && h->IsVisible() && (overlayId.m_featureId.IsValid() || overlayId.m_markId != kml::kInvalidMarkId)) + ASSERT(overlayId.IsValid(), ()); + + if (!h->HasLinearFeatureShape() && h->IsVisible()) { OverlayHandle::Rects shape; h->GetPixelShape(screen, screen.isPerspective(), shape); @@ -523,11 +557,17 @@ void OverlayTree::StoreDisplacementInfo(int caseIndex, ref_ptr di UNUSED_VALUE(caseIndex); #endif - if (!m_debugRectRenderer || !m_debugRectRenderer->IsEnabled()) - return; - m_displacementInfo.emplace_back(m2::PointF(displacerHandle->GetExtendedPixelRect(modelView).Center()), - m2::PointF(displacedHandle->GetExtendedPixelRect(modelView).Center()), - dp::Color(0, 0, 255, 255)); + if (m_debugRectRenderer && m_debugRectRenderer->IsEnabled()) + { + m_displacementInfo.emplace_back(m2::PointF(displacerHandle->GetExtendedPixelRect(modelView).Center()), + m2::PointF(displacedHandle->GetExtendedPixelRect(modelView).Center()), + dp::Color(0, 0, 255, 255)); + } +} + +bool OverlayTree::IsInCache(ref_ptr const & handle) const +{ + return (m_handlesCache.find(handle) != m_handlesCache.end()); } void detail::OverlayTraits::SetVisualScale(double visualScale) diff --git a/drape/overlay_tree.hpp b/drape/overlay_tree.hpp index 577716f981..ef0a59ebe6 100644 --- a/drape/overlay_tree.hpp +++ b/drape/overlay_tree.hpp @@ -106,16 +106,28 @@ private: ref_ptr const & parentOverlay); bool CheckHandle(ref_ptr handle, int currentRank, ref_ptr & parentOverlay) const; - void DeleteHandle(ref_ptr const & handle); + bool DeleteHandleImpl(ref_ptr handle); + void DeleteHandle(ref_ptr handle); ref_ptr FindParent(ref_ptr handle, int searchingRank) const; void DeleteHandleWithParents(ref_ptr handle, int currentRank); void StoreDisplacementInfo(int caseIndex, ref_ptr displacerHandle, ref_ptr displacedHandle); + + bool IsInCache(ref_ptr const & handle) const; + int m_frameCounter; std::array>, dp::OverlayRanksCount> m_handles; + HandlesCache m_handlesCache; + /// @todo By VNG: Additional cache by OverlayID for fast FindParent(OverlayHandle). + /// I did profiling here in perspective-navigation mode. + /// The best solution is to store parent in OverlayHandle, but I didn't realize + /// how to implement it in a reasonable time, except "rewrite all". + /// Probably, another good solution is to combine m_handlesCache and m_overlayIdCache and make + /// one container like unordered_map<{FeatureID, kml::MarkId}, buffer_vector>. + std::map, 4>> m_overlayIdCache; bool m_isDisplacementEnabled;