From 832bc08ab4d2b1c8422a9f00566469c971c26709 Mon Sep 17 00:00:00 2001 From: Alex Zolotarev Date: Mon, 21 Dec 2015 00:45:06 +0200 Subject: [PATCH] [editor] Changed interface and internal storage for faster access and easier serialization. Introduced additional information about edits. --- indexer/index.cpp | 9 +++-- indexer/index.hpp | 26 +++++++----- indexer/osm_editor.cpp | 91 ++++++++++++++++++++++++++++-------------- indexer/osm_editor.hpp | 35 +++++++++++----- 4 files changed, 108 insertions(+), 53 deletions(-) diff --git a/indexer/index.cpp b/indexer/index.cpp index 17a0c426e2..33d55bb8c5 100644 --- a/indexer/index.cpp +++ b/indexer/index.cpp @@ -119,11 +119,12 @@ bool Index::FeaturesLoaderGuard::IsWorld() const void Index::FeaturesLoaderGuard::GetFeatureByIndex(uint32_t index, FeatureType & ft) const { - FeatureID const fid(m_handle.GetId(), index); - ASSERT(!m_editor.IsFeatureDeleted(fid), ("Deleted feature was cached. Please review your code.")); - if (!m_editor.Instance().GetEditedFeature(fid, ft)) + MwmId const & id = m_handle.GetId(); + ASSERT_NOT_EQUAL(osm::Editor::EDeleted, m_editor.GetFeatureStatus(id, index), + ("Deleted feature was cached. Please review your code.")); + if (!m_editor.Instance().GetEditedFeature(id, index, ft)) { m_vector.GetByIndex(index, ft); - ft.SetID(fid); + ft.SetID(FeatureID(id, index)); } } diff --git a/indexer/index.hpp b/indexer/index.hpp index ce87856c0f..5bbb35766d 100644 --- a/indexer/index.hpp +++ b/indexer/index.hpp @@ -134,19 +134,21 @@ private: index.ForEachInIntervalAndScale( [&](uint32_t index) { - FeatureID const fid(mwmID, index); - if (m_editor.IsFeatureDeleted(fid)) - return; FeatureType feature; - if (m_editor.GetEditedFeature(fid, feature)) + switch (m_editor.GetFeatureStatus(mwmID, index)) { + case osm::Editor::EDeleted: return; + case osm::Editor::EModified: + VERIFY(m_editor.GetEditedFeature(mwmID, index, feature), ()); m_f(feature); return; + case osm::Editor::ECreated: CHECK(false, ("Created features index should be generated.")); + case osm::Editor::EUntouched: break; } if (checkUnique(index)) { fv.GetByIndex(index, feature); - feature.SetID(fid); + feature.SetID(FeatureID(mwmID, index)); m_f(feature); } }, @@ -195,9 +197,8 @@ private: { index.ForEachInIntervalAndScale([&] (uint32_t index) { - FeatureID const fid(mwmID, index); - if (!m_editor.IsFeatureDeleted(fid) && checkUnique(index)) - m_f(fid); + if (osm::Editor::EDeleted != m_editor.GetFeatureStatus(mwmID, index) && checkUnique(index)) + m_f(FeatureID(mwmID, index)); }, i.first, i.second, scale); } } @@ -244,9 +245,14 @@ public: FeaturesVector const featureReader(pValue->m_cont, pValue->GetHeader(), pValue->m_table); do { - ASSERT(!editor.IsFeatureDeleted(*fidIter), ("Deleted feature was cached. Please review your code.")); + osm::Editor::FeatureStatus const fts = editor.GetFeatureStatus(id, fidIter->m_index); + ASSERT_NOT_EQUAL(osm::Editor::EDeleted, fts, ("Deleted feature was cached. Please review your code.")); FeatureType featureType; - if (!editor.GetEditedFeature(*fidIter, featureType)) + if (fts == osm::Editor::EModified) + { + VERIFY(editor.GetEditedFeature(id, fidIter->m_index, featureType), ()); + } + else { featureReader.GetByIndex(fidIter->m_index, featureType); featureType.SetID(*fidIter); diff --git a/indexer/osm_editor.cpp b/indexer/osm_editor.cpp index 843476ab77..293775c485 100644 --- a/indexer/osm_editor.cpp +++ b/indexer/osm_editor.cpp @@ -50,29 +50,40 @@ void Editor::Load(string const & fullFilePath) { LOG(LERROR, ("Can't load XML Edits from disk:", fullFilePath)); } - // TODO(mgsergio): Implement XML deserialization into m_[deleted|edited|created]Features. + // TODO(mgsergio): Implement XML deserialization into m_features. } void Editor::Save(string const & /*fullFilePath*/) const { - // Do not save empty xml file if no any edits were made. - if (m_deletedFeatures.empty() && m_editedFeatures.empty() && m_createdFeatures.empty()) - return; - // TODO(mgsergio): Implement XML serialization from m_[deleted|edited|created]Features. + // TODO(mgsergio): Implement XML serialization from m_features. } -bool Editor::IsFeatureDeleted(FeatureID const & fid) const +Editor::FeatureStatus Editor::GetFeatureStatus(MwmSet::MwmId const & mwmId, uint32_t offset) const { // Most popular case optimization. - if (m_deletedFeatures.empty()) - return false; + if (m_features.empty()) + return EUntouched; - return m_deletedFeatures.find(fid) != m_deletedFeatures.end(); + auto const mwmMatched = m_features.find(mwmId); + if (mwmMatched == m_features.end()) + return EUntouched; + + auto const offsetMatched = mwmMatched->second.find(offset); + if (offsetMatched == mwmMatched->second.end()) + return EUntouched; + + return offsetMatched->second.m_status; } void Editor::DeleteFeature(FeatureType const & feature) { - m_deletedFeatures.insert(feature.GetID()); + FeatureID const fid = feature.GetID(); + FeatureTypeInfo & ftInfo = m_features[fid.m_mwmId][fid.m_index]; + ftInfo.m_status = EDeleted; + ftInfo.m_feature = feature; + // TODO: What if local client time is absolutely wrong? + ftInfo.m_modificationTimestamp = time(nullptr); + // TODO(AlexZ): Synchronize Save call/make it on a separate thread. Save(GetEditorFilePath()); @@ -92,7 +103,14 @@ void Editor::DeleteFeature(FeatureType const & feature) void Editor::EditFeature(FeatureType & editedFeature) { - m_editedFeatures[editedFeature.GetID()] = editedFeature; + // TODO(AlexZ): Check if feature has not changed and reset status. + FeatureID const fid = editedFeature.GetID(); + FeatureTypeInfo & ftInfo = m_features[fid.m_mwmId][fid.m_index]; + ftInfo.m_status = EModified; + ftInfo.m_feature = editedFeature; + // TODO: What if local client time is absolutely wrong? + ftInfo.m_modificationTimestamp = time(nullptr); + // TODO(AlexZ): Synchronize Save call/make it on a separate thread. Save(GetEditorFilePath()); @@ -100,43 +118,56 @@ void Editor::EditFeature(FeatureType & editedFeature) m_invalidateFn(); } -bool Editor::IsFeatureEdited(FeatureID const & fid) const -{ - return m_editedFeatures.find(fid) != m_editedFeatures.end(); -} - void Editor::ForEachFeatureInMwmRectAndScale(MwmSet::MwmId const & id, function const & f, - m2::RectD const & /*rect*/, + m2::RectD const & rect, uint32_t /*scale*/) { - // TODO(AlexZ): Check that features are in the rect and are visible at this scale. - for (auto & feature : m_createdFeatures) + auto const mwmFound = m_features.find(id); + if (mwmFound == m_features.end()) + return; + + // TODO(AlexZ): Check that features are visible at this scale. + // Process only new (created) features. + for (auto const & offset : mwmFound->second) { - if (feature.first.m_mwmId == id) - f(feature.first); + FeatureTypeInfo const & ftInfo = offset.second; + if (ftInfo.m_status == ECreated && rect.IsPointInside(ftInfo.m_feature.GetCenter())) + f(FeatureID(id, offset.first)); } } void Editor::ForEachFeatureInMwmRectAndScale(MwmSet::MwmId const & id, function const & f, - m2::RectD const & /*rect*/, + m2::RectD const & rect, uint32_t /*scale*/) { - // TODO(AlexZ): Check that features are in the rect and are visible at this scale. - for (auto & feature : m_createdFeatures) + auto mwmFound = m_features.find(id); + if (mwmFound == m_features.end()) + return; + + // TODO(AlexZ): Check that features are visible at this scale. + // Process only new (created) features. + for (auto & offset : mwmFound->second) { - if (feature.first.m_mwmId == id) - f(feature.second); + FeatureTypeInfo & ftInfo = offset.second; + if (ftInfo.m_status == ECreated && rect.IsPointInside(ftInfo.m_feature.GetCenter())) + f(ftInfo.m_feature); } } -bool Editor::GetEditedFeature(FeatureID const & fid, FeatureType & outFeature) const +bool Editor::GetEditedFeature(MwmSet::MwmId const & mwmId, uint32_t offset, FeatureType & outFeature) const { - auto found = m_editedFeatures.find(fid); - if (found == m_editedFeatures.end()) + auto const mwmMatched = m_features.find(mwmId); + if (mwmMatched == m_features.end()) return false; - outFeature = found->second; + + auto const offsetMatched = mwmMatched->second.find(offset); + if (offsetMatched == mwmMatched->second.end()) + return false; + + // TODO(AlexZ): Should we process deleted/created features as well? + outFeature = offsetMatched->second.m_feature; return true; } diff --git a/indexer/osm_editor.hpp b/indexer/osm_editor.hpp index 98cf4c5559..21b55ea8a0 100644 --- a/indexer/osm_editor.hpp +++ b/indexer/osm_editor.hpp @@ -5,6 +5,7 @@ #include "indexer/feature_meta.hpp" #include "indexer/mwm_set.hpp" +#include "std/ctime.hpp" #include "std/function.hpp" #include "std/map.hpp" #include "std/set.hpp" @@ -24,6 +25,14 @@ class Editor final public: using TInvalidateFn = function; + enum FeatureStatus + { + EUntouched, + EDeleted, + EModified, + ECreated + }; + static Editor & Instance(); void SetInvalidateFn(TInvalidateFn && fn) { m_invalidateFn = move(fn); } @@ -41,17 +50,15 @@ public: m2::RectD const & rect, uint32_t scale); - /// True if feature was deleted by user. - bool IsFeatureDeleted(FeatureID const & fid) const; + /// Easy way to check if feature was deleted, modified, created or not changed at all. + FeatureStatus GetFeatureStatus(MwmSet::MwmId const & mwmId, uint32_t offset) const; /// Marks feature as "deleted" from MwM file. void DeleteFeature(FeatureType const & feature); - /// True if feature was edited by user. - bool IsFeatureEdited(FeatureID const & fid) const; /// @returns false if feature wasn't edited. /// @param outFeature is valid only if true was returned. - bool GetEditedFeature(FeatureID const & fid, FeatureType & outFeature) const; + bool GetEditedFeature(MwmSet::MwmId const & mwmId, uint32_t offset, FeatureType & outFeature) const; /// Original feature with same FeatureID as newFeature is replaced by newFeature. void EditFeature(FeatureType & editedFeature); @@ -59,10 +66,20 @@ public: vector EditableMetadataForType(uint32_t type) const; private: - // TODO(AlexZ): Synchronize multithread access to these structures. - set m_deletedFeatures; - map m_editedFeatures; - map m_createdFeatures; + struct FeatureTypeInfo + { + FeatureStatus m_status; + FeatureType m_feature; + time_t m_modificationTimestamp = 0; + time_t m_uploadAttemptTimestamp = 0; + /// "" | "ok" | "repeat" | "failed" + string m_uploadStatus; + string m_uploadError; + }; + + // TODO(AlexZ): Synchronize multithread access. + /// Deleted, edited and created features. + map> m_features; /// Invalidate map viewport after edits. TInvalidateFn m_invalidateFn;