diff --git a/editor/editor_config.hpp b/editor/editor_config.hpp index f4217e1371..7e55b35549 100644 --- a/editor/editor_config.hpp +++ b/editor/editor_config.hpp @@ -2,6 +2,7 @@ #include "indexer/feature_meta.hpp" +#include "std/mutex.hpp" #include "std/shared_ptr.hpp" #include "std/string.hpp" #include "std/vector.hpp" @@ -59,10 +60,24 @@ class EditorConfigWrapper public: EditorConfigWrapper() = default; - void Set(shared_ptr config) { atomic_store(&m_config, config); } - shared_ptr Get() const { return atomic_load(&m_config); } + void Set(shared_ptr config) + { + lock_guard lock(m_mu); + m_config = config; + } + + shared_ptr Get() const + { + lock_guard lock(m_mu); + return m_config; + } private: + // It's possible to use atomic_{load|store} here instead of mutex, + // but seems that libstdc++4.9 doesn't support it. Need to rewrite + // this code as soon as libstdc++5 will be ready for lastest Debian + // release, or as soon as atomic_shared_ptr will be ready. + mutable mutex m_mu; shared_ptr m_config = make_shared(); // Just in case someone tryes to pass EditorConfigWrapper by value instead of referense. diff --git a/indexer/indexer_tests/osm_editor_test.cpp b/indexer/indexer_tests/osm_editor_test.cpp index 21bc744d23..b3e3592d0f 100644 --- a/indexer/indexer_tests/osm_editor_test.cpp +++ b/indexer/indexer_tests/osm_editor_test.cpp @@ -2,7 +2,7 @@ #include "indexer/indexer_tests/osm_editor_test.hpp" -#include "search/reverse_geocoder.hpp" +#include "search/editor_delegate.hpp" #include "indexer/classificator.hpp" #include "indexer/classificator_loader.hpp" @@ -17,6 +17,8 @@ #include "coding/file_name_utils.hpp" +#include "std/unique_ptr.hpp" + using namespace generator::tests_support; using namespace indexer::tests_support; @@ -144,7 +146,7 @@ EditorTest::EditorTest() LOG(LERROR, ("Classificator read error: ", e.what())); } - indexer::tests_support::SetUpEditorForTesting(m_index); + indexer::tests_support::SetUpEditorForTesting(make_unique(m_index)); } EditorTest::~EditorTest() @@ -232,7 +234,7 @@ void EditorTest::SetIndexTest() builder.Add(TestCafe({4.0, 4.0}, "London Cafe", "en")); }); - auto const mwmId = editor.m_mwmIdByMapNameFn("GB"); + auto const mwmId = editor.GetMwmIdByMapName("GB"); TEST_EQUAL(gbMwmId, mwmId, ()); @@ -241,28 +243,28 @@ void EditorTest::SetIndexTest() ForEachCafeAtPoint(m_index, m2::PointD(1.0, 1.0), [&editor](FeatureType & ft) { - auto const firstPtr = editor.m_getOriginalFeatureFn(ft.GetID()); + auto const firstPtr = editor.GetOriginalFeature(ft.GetID()); TEST(firstPtr, ()); SetBuildingLevelsToOne(ft); - auto const secondPtr = editor.m_getOriginalFeatureFn(ft.GetID()); + auto const secondPtr = editor.GetOriginalFeature(ft.GetID()); TEST(secondPtr, ()); TEST_EQUAL(firstPtr->GetID(), secondPtr->GetID(), ()); }); ForEachCafeAtPoint(m_index, m2::PointD(1.0, 1.0), [&editor](FeatureType & ft) { - TEST_EQUAL(editor.m_getOriginalFeatureStreetFn(ft), "Test street", ()); + TEST_EQUAL(editor.GetOriginalFeatureStreet(ft), "Test street", ()); EditFeature(ft, [](osm::EditableMapObject & emo) { osm::LocalizedStreet ls{"Some street", ""}; emo.SetStreet(ls); }); - TEST_EQUAL(editor.m_getOriginalFeatureStreetFn(ft), "Test street", ()); + TEST_EQUAL(editor.GetOriginalFeatureStreet(ft), "Test street", ()); }); uint32_t counter = 0; - editor.m_forEachFeatureAtPointFn([&counter](FeatureType & ft) + editor.ForEachFeatureAtPoint([&counter](FeatureType & ft) { ++counter; }, {100.0, 100.0}); @@ -270,7 +272,7 @@ void EditorTest::SetIndexTest() TEST_EQUAL(counter, 0, ()); counter = 0; - editor.m_forEachFeatureAtPointFn([&counter](FeatureType & ft) + editor.ForEachFeatureAtPoint([&counter](FeatureType & ft) { ++counter; }, {3.0, 3.0}); @@ -278,7 +280,7 @@ void EditorTest::SetIndexTest() TEST_EQUAL(counter, 1, ()); counter = 0; - editor.m_forEachFeatureAtPointFn([&counter](FeatureType & ft) + editor.ForEachFeatureAtPoint([&counter](FeatureType & ft) { ++counter; }, {1.0, 1.0}); @@ -286,7 +288,7 @@ void EditorTest::SetIndexTest() TEST_EQUAL(counter, 2, ()); counter = 0; - editor.m_forEachFeatureAtPointFn([&counter](FeatureType & ft) + editor.ForEachFeatureAtPoint([&counter](FeatureType & ft) { ++counter; }, {4.0, 4.0}); diff --git a/indexer/indexer_tests_support/helpers.cpp b/indexer/indexer_tests_support/helpers.cpp index 05392903de..0f748989e8 100644 --- a/indexer/indexer_tests_support/helpers.cpp +++ b/indexer/indexer_tests_support/helpers.cpp @@ -2,16 +2,14 @@ #include "editor/editor_storage.hpp" -#include "std/unique_ptr.hpp" - namespace indexer { namespace tests_support { -void SetUpEditorForTesting(Index & index) +void SetUpEditorForTesting(unique_ptr delegate) { auto & editor = osm::Editor::Instance(); - editor.SetIndex(index); + editor.SetDelegate(move(delegate)); editor.SetStorageForTesting(make_unique()); editor.ClearAllLocalEdits(); } diff --git a/indexer/indexer_tests_support/helpers.hpp b/indexer/indexer_tests_support/helpers.hpp index fd3018ffcf..e0001681d2 100644 --- a/indexer/indexer_tests_support/helpers.hpp +++ b/indexer/indexer_tests_support/helpers.hpp @@ -3,6 +3,8 @@ #include "indexer/editable_map_object.hpp" #include "indexer/osm_editor.hpp" +#include "std/unique_ptr.hpp" + #include "base/assert.hpp" class Index; @@ -11,7 +13,7 @@ namespace indexer { namespace tests_support { -void SetUpEditorForTesting(Index & index); +void SetUpEditorForTesting(unique_ptr delegate); template void EditFeature(FeatureType const & ft, TFn && fn) diff --git a/indexer/osm_editor.cpp b/indexer/osm_editor.cpp index 96f230ae09..e804dee125 100644 --- a/indexer/osm_editor.cpp +++ b/indexer/osm_editor.cpp @@ -10,8 +10,6 @@ #include "indexer/osm_editor.hpp" #include "indexer/index_helpers.hpp" -#include "search/reverse_geocoder.hpp" - #include "platform/local_country_file_utils.hpp" #include "platform/platform.hpp" #include "platform/preferred_languages.hpp" @@ -140,7 +138,8 @@ Editor::Editor() : m_configLoader(m_config) , m_notes(editor::Notes::MakeNotes()) , m_storage(make_unique()) -{} +{ +} Editor & Editor::Instance() { @@ -148,47 +147,11 @@ Editor & Editor::Instance() return instance; } -void Editor::SetIndex(Index const & index) -{ - m_mwmIdByMapNameFn = [&index](string const & name) -> MwmSet::MwmId - { - return index.GetMwmIdByCountryFile(platform::CountryFile(name)); - }; - - m_getOriginalFeatureFn = [&index](FeatureID const & fid) -> unique_ptr - { - unique_ptr feature(new FeatureType()); - Index::FeaturesLoaderGuard const guard(index, fid.m_mwmId); - if (!guard.GetOriginalFeatureByIndex(fid.m_index, *feature)) - return nullptr; - feature->ParseEverything(); - return feature; - }; - - m_getOriginalFeatureStreetFn = [&index](FeatureType & ft) -> string - { - search::ReverseGeocoder const coder(index); - auto const streets = coder.GetNearbyFeatureStreets(ft); - if (streets.second < streets.first.size()) - return streets.first[streets.second].m_name; - return {}; - }; - - // Due to floating points accuracy issues (geometry is saved in editor up to 7 digits - // after decimal point) some feature vertexes are threated as external to a given feature. - auto const toleranceInMeters = 1e-2; - m_forEachFeatureAtPointFn = - [&index, toleranceInMeters](TFeatureTypeFn && fn, m2::PointD const & mercator) - { - indexer::ForEachFeatureAtPoint(index, move(fn), mercator, toleranceInMeters); - }; -} - void Editor::LoadMapEdits() { - if (!m_mwmIdByMapNameFn) + if (!m_delegate) { - LOG(LERROR, ("Can't load any map edits, MwmIdByNameAndVersionFn has not been set.")); + LOG(LERROR, ("Can't load any map edits, delegate has not been set.")); return; } @@ -213,7 +176,7 @@ void Editor::LoadMapEdits() { string const mapName = mwm.attribute("name").as_string(""); int64_t const mapVersion = mwm.attribute("version").as_llong(0); - MwmSet::MwmId const mwmId = m_mwmIdByMapNameFn(mapName); + MwmSet::MwmId const mwmId = GetMwmIdByMapName(mapName); // TODO(mgsergio, AlexZ): Is it normal to have isMwmIdAlive and mapVersion // NOT equal to mwmId.GetInfo()->GetVersion() at the same time? auto const needMigrateEdits = !mwmId.IsAlive() || mapVersion != mwmId.GetInfo()->GetVersion(); @@ -235,10 +198,15 @@ void Editor::LoadMapEdits() goto SECTION_END; } + TForEachFeaturesNearByFn forEach = [this](TFeatureTypeFn && fn, + m2::PointD const & point) { + return ForEachFeatureAtPoint(move(fn), point); + }; + // TODO(mgsergio): Deleted features are not properly handled yet. auto const fid = needMigrateEdits ? editor::MigrateFeatureIndex( - m_forEachFeatureAtPointFn, xml, section.first, + forEach, xml, section.first, [this, &mwmId] { return GenerateNewFeatureId(mwmId); }) : FeatureID(mwmId, xml.GetMWMFeatureIndex()); @@ -253,7 +221,7 @@ void Editor::LoadMapEdits() } else { - auto const originalFeaturePtr = m_getOriginalFeatureFn(fid); + auto const originalFeaturePtr = GetOriginalFeature(fid); if (!originalFeaturePtr) { LOG(LERROR, ("A feature with id", fid, "cannot be loaded.")); @@ -451,7 +419,7 @@ bool Editor::OriginalFeatureHasDefaultName(FeatureID const & fid) const if (IsCreatedFeature(fid)) return false; - auto const originalFeaturePtr = m_getOriginalFeatureFn(fid); + auto const originalFeaturePtr = GetOriginalFeature(fid); if (!originalFeaturePtr) { LOG(LERROR, ("A feature with id", fid, "cannot be loaded.")); @@ -504,7 +472,7 @@ Editor::SaveResult Editor::SaveEditedFeature(EditableMapObject const & emo) } else { - auto const originalFeaturePtr = m_getOriginalFeatureFn(fid); + auto const originalFeaturePtr = GetOriginalFeature(fid); if (!originalFeaturePtr) { LOG(LERROR, ("A feature with id", fid, "cannot be loaded.")); @@ -518,7 +486,7 @@ Editor::SaveResult Editor::SaveEditedFeature(EditableMapObject const & emo) fti.m_feature.ReplaceBy(emo); bool const sameAsInMWM = AreFeaturesEqualButStreet(fti.m_feature, *originalFeaturePtr) && - emo.GetStreet().m_defaultName == m_getOriginalFeatureStreetFn(fti.m_feature); + emo.GetStreet().m_defaultName == GetOriginalFeatureStreet(fti.m_feature); if (featureStatus != FeatureStatus::Untouched) { @@ -675,7 +643,7 @@ EditableProperties Editor::GetEditableProperties(FeatureType const & feature) co // Disable opening hours editing if opening hours cannot be parsed. if (GetFeatureStatus(feature.GetID()) != FeatureStatus::Created) { - auto const originalFeaturePtr = m_getOriginalFeatureFn(feature.GetID()); + auto const originalFeaturePtr = GetOriginalFeature(feature.GetID()); if (!originalFeaturePtr) { LOG(LERROR, ("A feature with id", feature.GetID(), "cannot be loaded.")); @@ -837,7 +805,7 @@ void Editor::UploadChanges(string const & key, string const & secret, TChangeset feature.SetTagValue(kAddrStreetTag, fti.m_street); ourDebugFeatureString = DebugPrint(feature); - auto const originalFeaturePtr = m_getOriginalFeatureFn(fti.m_feature.GetID()); + auto const originalFeaturePtr = GetOriginalFeature(fti.m_feature.GetID()); if (!originalFeaturePtr) { LOG(LERROR, ("A feature with id", fti.m_feature.GetID(), "cannot be loaded.")); @@ -866,7 +834,7 @@ void Editor::UploadChanges(string const & key, string const & secret, TChangeset break; case FeatureStatus::Deleted: - auto const originalFeaturePtr = m_getOriginalFeatureFn(fti.m_feature.GetID()); + auto const originalFeaturePtr = GetOriginalFeature(fti.m_feature.GetID()); if (!originalFeaturePtr) { LOG(LERROR, ("A feature with id", fti.m_feature.GetID(), "cannot be loaded.")); @@ -1129,7 +1097,7 @@ void Editor::MarkFeatureWithStatus(FeatureID const & fid, FeatureStatus status) { auto & fti = m_features[fid.m_mwmId][fid.m_index]; - auto const originalFeaturePtr = m_getOriginalFeatureFn(fid); + auto const originalFeaturePtr = GetOriginalFeature(fid); if (!originalFeaturePtr) { @@ -1143,6 +1111,46 @@ void Editor::MarkFeatureWithStatus(FeatureID const & fid, FeatureStatus status) fti.m_modificationTimestamp = time(nullptr); } +MwmSet::MwmId Editor::GetMwmIdByMapName(string const & name) +{ + if (!m_delegate) + { + LOG(LERROR, ("Can't get mwm id by map name:", name, ", delegate is not set.")); + return {}; + } + return m_delegate->GetMwmIdByMapName(name); +} + +unique_ptr Editor::GetOriginalFeature(FeatureID const & fid) const +{ + if (!m_delegate) + { + LOG(LERROR, ("Can't get original feature by id:", fid, ", delegate is not set.")); + return {}; + } + return m_delegate->GetOriginalFeature(fid); +} + +string Editor::GetOriginalFeatureStreet(FeatureType & ft) const +{ + if (!m_delegate) + { + LOG(LERROR, ("Can't get feature street, delegate is not set.")); + return {}; + } + return m_delegate->GetOriginalFeatureStreet(ft); +} + +void Editor::ForEachFeatureAtPoint(TFeatureTypeFn && fn, m2::PointD const & point) const +{ + if (!m_delegate) + { + LOG(LERROR, ("Can't load features in point's vicinity, delegate is not set.")); + return; + } + m_delegate->ForEachFeatureAtPoint(move(fn), point); +} + string DebugPrint(Editor::FeatureStatus fs) { switch (fs) diff --git a/indexer/osm_editor.hpp b/indexer/osm_editor.hpp index 57044db36b..cb36ec7a94 100644 --- a/indexer/osm_editor.hpp +++ b/indexer/osm_editor.hpp @@ -51,6 +51,16 @@ public: using TForEachFeaturesNearByFn = function; + struct Delegate + { + virtual ~Delegate() = default; + + virtual MwmSet::MwmId GetMwmIdByMapName(string const & name) const = 0; + virtual unique_ptr GetOriginalFeature(FeatureID const & fid) const = 0; + virtual string GetOriginalFeatureStreet(FeatureType & ft) const = 0; + virtual void ForEachFeatureAtPoint(TFeatureTypeFn && fn, m2::PointD const & point) const = 0; + }; + enum class UploadResult { Success, @@ -72,8 +82,7 @@ public: static Editor & Instance(); - // Reference to the index will be used in editor functors, it should not be temporary object. - void SetIndex(Index const & index); + inline void SetDelegate(unique_ptr delegate) { m_delegate = move(delegate); } inline void SetStorageForTesting(unique_ptr storage) { @@ -225,20 +234,20 @@ private: void MarkFeatureWithStatus(FeatureID const & fid, FeatureStatus status); + // These methods are just checked wrappers around Delegate. + MwmSet::MwmId GetMwmIdByMapName(string const & name); + unique_ptr GetOriginalFeature(FeatureID const & fid) const; + string GetOriginalFeatureStreet(FeatureType & ft) const; + void ForEachFeatureAtPoint(TFeatureTypeFn && fn, m2::PointD const & point) const; + // TODO(AlexZ): Synchronize multithread access. /// Deleted, edited and created features. map> m_features; - /// Get MwmId for each map, used in FeatureIDs and to check if edits are up-to-date. - TMwmIdByMapNameFn m_mwmIdByMapNameFn; + unique_ptr m_delegate; + /// Invalidate map viewport after edits. TInvalidateFn m_invalidateFn; - /// Get FeatureType from mwm. - TFeatureLoaderFn m_getOriginalFeatureFn; - /// Get feature original street name or empty string. - TFeatureOriginalStreetFn m_getOriginalFeatureStreetFn; - /// Iterate over all features in some area that includes given point. - TForEachFeaturesNearByFn m_forEachFeatureAtPointFn; /// Contains information about what and how can be edited. editor::EditorConfigWrapper m_config; diff --git a/map/framework.cpp b/map/framework.cpp index c8ac3f2907..4356bb8bf7 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -13,6 +13,7 @@ #include "routing/routing_algorithm.hpp" #include "search/downloader_search_callback.hpp" +#include "search/editor_delegate.hpp" #include "search/engine.hpp" #include "search/everywhere_search_params.hpp" #include "search/geometry_utils.hpp" @@ -397,7 +398,7 @@ Framework::Framework() osm::Editor & editor = osm::Editor::Instance(); - editor.SetIndex(m_model.GetIndex()); + editor.SetDelegate(make_unique(m_model.GetIndex())); editor.SetInvalidateFn([this](){ InvalidateRect(GetCurrentViewport()); }); editor.LoadMapEdits(); diff --git a/search/editor_delegate.cpp b/search/editor_delegate.cpp new file mode 100644 index 0000000000..aa982055df --- /dev/null +++ b/search/editor_delegate.cpp @@ -0,0 +1,44 @@ +#include "search/editor_delegate.hpp" + +#include "search/reverse_geocoder.hpp" + +#include "indexer/feature_decl.hpp" +#include "indexer/index.hpp" +#include "indexer/index.hpp" +#include "indexer/index_helpers.hpp" + +namespace search +{ +EditorDelegate::EditorDelegate(Index const & index) : m_index(index) {} + +MwmSet::MwmId EditorDelegate::GetMwmIdByMapName(string const & name) const +{ + return m_index.GetMwmIdByCountryFile(platform::CountryFile(name)); +} + +unique_ptr EditorDelegate::GetOriginalFeature(FeatureID const & fid) const +{ + auto feature = make_unique(); + Index::FeaturesLoaderGuard const guard(m_index, fid.m_mwmId); + if (!guard.GetOriginalFeatureByIndex(fid.m_index, *feature)) + return unique_ptr(); + feature->ParseEverything(); + return feature; +} + +string EditorDelegate::GetOriginalFeatureStreet(FeatureType & ft) const +{ + search::ReverseGeocoder const coder(m_index); + auto const streets = coder.GetNearbyFeatureStreets(ft); + if (streets.second < streets.first.size()) + return streets.first[streets.second].m_name; + return {}; +} + +void EditorDelegate::ForEachFeatureAtPoint(osm::Editor::TFeatureTypeFn && fn, + m2::PointD const & point) const +{ + auto const kToleranceMeters = 1e-2; + indexer::ForEachFeatureAtPoint(m_index, move(fn), point, kToleranceMeters); +} +} // namespace search diff --git a/search/editor_delegate.hpp b/search/editor_delegate.hpp new file mode 100644 index 0000000000..c523cf2372 --- /dev/null +++ b/search/editor_delegate.hpp @@ -0,0 +1,24 @@ +#pragma once + +#include "indexer/osm_editor.hpp" + +class Index; + +namespace search +{ +class EditorDelegate : public osm::Editor::Delegate +{ +public: + EditorDelegate(Index const & index); + + // osm::Editor::Delegate overrides: + MwmSet::MwmId GetMwmIdByMapName(string const & name) const override; + unique_ptr GetOriginalFeature(FeatureID const & fid) const override; + string GetOriginalFeatureStreet(FeatureType & ft) const override; + void ForEachFeatureAtPoint(osm::Editor::TFeatureTypeFn && fn, + m2::PointD const & point) const override; + +private: + Index const & m_index; +}; +} // namespace search diff --git a/search/search.pro b/search/search.pro index da1ee701ea..dd1732c817 100644 --- a/search/search.pro +++ b/search/search.pro @@ -18,6 +18,7 @@ HEADERS += \ displayed_categories.hpp \ downloader_search_callback.hpp \ dummy_rank_table.hpp \ + editor_delegate.hpp \ engine.hpp \ everywhere_search_params.hpp \ feature_offset_match.hpp \ @@ -83,6 +84,7 @@ SOURCES += \ displayed_categories.cpp \ downloader_search_callback.cpp \ dummy_rank_table.cpp \ + editor_delegate.cpp \ engine.cpp \ features_filter.cpp \ features_layer.cpp \ diff --git a/search/search_integration_tests/helpers.cpp b/search/search_integration_tests/helpers.cpp index 79768d0f2d..111d08718d 100644 --- a/search/search_integration_tests/helpers.cpp +++ b/search/search_integration_tests/helpers.cpp @@ -1,5 +1,6 @@ #include "search/search_integration_tests/helpers.hpp" +#include "search/editor_delegate.hpp" #include "search/processor_factory.hpp" #include "search/search_tests_support/test_search_request.hpp" @@ -27,7 +28,7 @@ SearchTest::SearchTest() , m_engine(make_unique(), make_unique(), Engine::Params()) { - indexer::tests_support::SetUpEditorForTesting(m_engine); + indexer::tests_support::SetUpEditorForTesting(make_unique(m_engine)); } SearchTest::~SearchTest()