Merge pull request #3727 from ygorshenin/fix-features-loading-guard

[indexer] Fixed FeaturesLoaderGuard.
This commit is contained in:
mpimenov 2016-07-07 15:35:40 +03:00 committed by GitHub
commit bcb269c84c
22 changed files with 309 additions and 153 deletions

View file

@ -176,8 +176,11 @@ void FindCrossNodes(osrm::NodeDataVectorT const & nodeData, gen::OsmID2FeatureID
{
FeatureType ft;
Index::FeaturesLoaderGuard loader(index, mwmId);
loader.GetFeatureByIndex(osm2ft.GetFeatureID(startSeg.wayId), ft);
LOG(LINFO, ("Double border intersection", wgsIntersection, "rank:", GetWarningRank(ft)));
if (loader.GetFeatureByIndex(osm2ft.GetFeatureID(startSeg.wayId), ft))
{
LOG(LINFO,
("Double border intersection", wgsIntersection, "rank:", GetWarningRank(ft)));
}
}
}
}
@ -318,7 +321,11 @@ void BuildRoutingIndex(string const & baseDir, string const & countryName, strin
FeatureType ft;
Index::FeaturesLoaderGuard loader(index, p.first);
loader.GetFeatureByIndex(fID, ft);
if (!loader.GetFeatureByIndex(fID, ft))
{
LOG(LWARNING, ("Can't read feature with id:", fID, "for way:", seg.wayId));
continue;
}
ft.ParseGeometry(FeatureType::BEST_GEOMETRY);

View file

@ -84,7 +84,8 @@ int main(int argc, char * argv[])
FeatureType ft;
Index::FeaturesLoaderGuard loader(
fetcher->GetIndex(), fetcher->GetIndex().GetMwmIdByCountryFile(file.GetCountryFile()));
loader.GetFeatureByIndex(segment.m_fid, ft);
if (!loader.GetFeatureByIndex(segment.m_fid, ft))
continue;
ft.ParseGeometry(FeatureType::BEST_GEOMETRY);
// Get points in proper direction.

View file

@ -83,36 +83,52 @@ bool Index::DeregisterMap(CountryFile const & countryFile) { return Deregister(c
// Index::FeaturesLoaderGuard implementation
//////////////////////////////////////////////////////////////////////////////////
Index::FeaturesLoaderGuard::FeaturesLoaderGuard(Index const & parent, MwmId const & id)
: m_handle(parent.GetMwmHandleById(id))
, m_vector(m_handle.GetValue<MwmValue>()->m_cont, m_handle.GetValue<MwmValue>()->GetHeader(),
m_handle.GetValue<MwmValue>()->m_table.get())
Index::FeaturesLoaderGuard::FeaturesLoaderGuard(Index const & index, MwmId const & id)
: m_handle(index.GetMwmHandleById(id))
{
if (!m_handle.IsAlive())
return;
auto const & value = *m_handle.GetValue<MwmValue>();
m_vector = make_unique<FeaturesVector>(value.m_cont, value.GetHeader(), value.m_table.get());
}
string Index::FeaturesLoaderGuard::GetCountryFileName() const
{
if (!m_handle.IsAlive())
return string();
return m_handle.GetValue<MwmValue>()->GetCountryFileName();
}
bool Index::FeaturesLoaderGuard::IsWorld() const
{
if (!m_handle.IsAlive())
return false;
return m_handle.GetValue<MwmValue>()->GetHeader().GetType() == feature::DataHeader::world;
}
void Index::FeaturesLoaderGuard::GetFeatureByIndex(uint32_t index, FeatureType & ft) const
bool Index::FeaturesLoaderGuard::GetFeatureByIndex(uint32_t index, FeatureType & ft) const
{
if (!m_handle.IsAlive())
return false;
MwmId const & id = m_handle.GetId();
ASSERT_NOT_EQUAL(osm::Editor::FeatureStatus::Deleted, m_editor.GetFeatureStatus(id, index),
("Deleted feature was cached. It should not be here. Please review your code."));
if (!m_editor.Instance().GetEditedFeature(id, index, ft))
GetOriginalFeatureByIndex(index, ft);
if (m_editor.Instance().GetEditedFeature(id, index, ft))
return true;
return GetOriginalFeatureByIndex(index, ft);
}
void Index::FeaturesLoaderGuard::GetOriginalFeatureByIndex(uint32_t index, FeatureType & ft) const
bool Index::FeaturesLoaderGuard::GetOriginalFeatureByIndex(uint32_t index, FeatureType & ft) const
{
m_vector.GetByIndex(index, ft);
if (!m_handle.IsAlive())
return false;
ASSERT(m_vector != nullptr, ());
m_vector->GetByIndex(index, ft);
ft.SetID(FeatureID(m_handle.GetId(), index));
return true;
}

View file

@ -272,20 +272,21 @@ public:
class FeaturesLoaderGuard
{
public:
FeaturesLoaderGuard(Index const & parent, MwmId const & id);
FeaturesLoaderGuard(Index const & index, MwmId const & id);
inline MwmSet::MwmId const & GetId() const { return m_handle.GetId(); }
string GetCountryFileName() const;
bool IsWorld() const;
/// Everyone, except Editor core, should use this method.
void GetFeatureByIndex(uint32_t index, FeatureType & ft) const;
WARN_UNUSED_RESULT bool GetFeatureByIndex(uint32_t index, FeatureType & ft) const;
/// Editor core only method, to get 'untouched', original version of feature.
void GetOriginalFeatureByIndex(uint32_t index, FeatureType & ft) const;
inline FeaturesVector const & GetFeaturesVector() const { return m_vector; }
WARN_UNUSED_RESULT bool GetOriginalFeatureByIndex(uint32_t index, FeatureType & ft) const;
private:
MwmHandle m_handle;
FeaturesVector m_vector;
unique_ptr<FeaturesVector> m_vector;
osm::Editor & m_editor = osm::Editor::Instance();
};

View file

@ -97,18 +97,17 @@ bool AreFeaturesEqualButStreet(FeatureType const & a, FeatureType const & b)
return true;
}
XMLFeature GetMatchingFeatureFromOSM(osm::ChangesetWrapper & cw,
unique_ptr<FeatureType const> featurePtr)
XMLFeature GetMatchingFeatureFromOSM(osm::ChangesetWrapper & cw, FeatureType const & ft)
{
ASSERT_NOT_EQUAL(featurePtr->GetFeatureType(), feature::GEOM_LINE,
ASSERT_NOT_EQUAL(ft.GetFeatureType(), feature::GEOM_LINE,
("Line features are not supported yet."));
if (featurePtr->GetFeatureType() == feature::GEOM_POINT)
return cw.GetMatchingNodeFeatureFromOSM(featurePtr->GetCenter());
if (ft.GetFeatureType() == feature::GEOM_POINT)
return cw.GetMatchingNodeFeatureFromOSM(ft.GetCenter());
// Warning: geometry is cached in FeatureType. So if it wasn't BEST_GEOMETRY,
// we can never have it. Features here came from editors loader and should
// have BEST_GEOMETRY geometry.
auto geometry = featurePtr->GetTriangesAsPoints(FeatureType::BEST_GEOMETRY);
auto geometry = ft.GetTriangesAsPoints(FeatureType::BEST_GEOMETRY);
// Filters out duplicate points for closed ways or triangles' vertices.
my::SortUnique(geometry);
@ -222,7 +221,20 @@ void Editor::LoadMapEdits()
}
else
{
fti.m_feature = *m_getOriginalFeatureFn(fid);
auto const originalFeaturePtr = m_getOriginalFeatureFn(fid);
if (!originalFeaturePtr)
{
LOG(LERROR, ("A feature with id", fid, "cannot be loaded."));
// TODO: alohalytics::LogEvent in this function leads to a linker error
// with complains on alohalytics::Stats::Instance() reference is missing.
// The problem remains even when the whole code but alohalytics::LogEvent
// is removed in this function. There are no problems with this call in
// other functions.
// alohalytics::LogEvent("Editor_MissingFeature_Error");
goto SECTION_END;
}
fti.m_feature = *originalFeaturePtr;
fti.m_feature.ApplyPatch(xml);
}
@ -450,12 +462,20 @@ Editor::SaveResult Editor::SaveEditedFeature(EditableMapObject const & emo)
{
ASSERT_NOT_EQUAL(featureStatus, FeatureStatus::Deleted, ("Unexpected feature status."));
auto const originalFeaturePtr = m_getOriginalFeatureFn(fid);
if (!originalFeaturePtr)
{
LOG(LERROR, ("A feature with id", fid, "cannot be loaded."));
alohalytics::LogEvent("Editor_MissingFeature_Error");
return SaveResult::SavingError;
}
fti.m_feature = featureStatus == FeatureStatus::Untouched
? *m_getOriginalFeatureFn(fid)
? *originalFeaturePtr
: m_features[fid.m_mwmId][fid.m_index].m_feature;
fti.m_feature.ReplaceBy(emo);
bool const sameAsInMWM = featureStatus != FeatureStatus::Created &&
AreFeaturesEqualButStreet(fti.m_feature, *m_getOriginalFeatureFn(fid)) &&
AreFeaturesEqualButStreet(fti.m_feature, *originalFeaturePtr) &&
emo.GetStreet().m_defaultName == m_getOriginalFeatureStreetFn(fti.m_feature);
if (featureStatus != FeatureStatus::Untouched)
@ -615,8 +635,15 @@ 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 & originalFeature = m_getOriginalFeatureFn(feature.GetID());
auto const & metadata = originalFeature->GetMetadata();
auto const originalFeaturePtr = m_getOriginalFeatureFn(feature.GetID());
if (originalFeaturePtr)
{
LOG(LERROR, ("A feature with id", feature.GetID(), "cannot be loaded."));
alohalytics::LogEvent("Editor_MissingFeature_Error");
return {};
}
auto const & metadata = originalFeaturePtr->GetMetadata();
auto const & featureOpeningHours = metadata.Get(feature::Metadata::FMD_OPEN_HOURS);
// Note: empty string is parsed as a valid opening hours rule.
if (!osmoh::OpeningHours(featureOpeningHours).IsValid())
@ -770,8 +797,17 @@ void Editor::UploadChanges(string const & key, string const & secret, TChangeset
feature.SetTagValue(kAddrStreetTag, fti.m_street);
ourDebugFeatureString = DebugPrint(feature);
XMLFeature osmFeature =
GetMatchingFeatureFromOSM(changeset, m_getOriginalFeatureFn(fti.m_feature.GetID()));
auto const originalFeaturePtr = m_getOriginalFeatureFn(fti.m_feature.GetID());
if (!originalFeaturePtr)
{
LOG(LERROR, ("A feature with id", fti.m_feature.GetID(), "cannot be loaded."));
alohalytics::LogEvent("Editor_MissingFeature_Error");
RemoveFeatureFromStorageIfExists(fti.m_feature.GetID());
continue;
}
XMLFeature osmFeature = GetMatchingFeatureFromOSM(
changeset, *originalFeaturePtr);
XMLFeature const osmFeatureCopy = osmFeature;
osmFeature.ApplyPatch(feature);
// Check to avoid uploading duplicates into OSM.
@ -790,8 +826,16 @@ void Editor::UploadChanges(string const & key, string const & secret, TChangeset
break;
case FeatureStatus::Deleted:
auto const originalFeaturePtr = m_getOriginalFeatureFn(fti.m_feature.GetID());
if (!originalFeaturePtr)
{
LOG(LERROR, ("A feature with id", fti.m_feature.GetID(), "cannot be loaded."));
alohalytics::LogEvent("Editor_MissingFeature_Error");
RemoveFeatureFromStorageIfExists(fti.m_feature.GetID());
continue;
}
changeset.Delete(GetMatchingFeatureFromOSM(
changeset, m_getOriginalFeatureFn(fti.m_feature.GetID())));
changeset, *originalFeaturePtr));
break;
}
fti.m_uploadStatus = kUploaded;
@ -925,6 +969,11 @@ void Editor::RemoveFeatureFromStorageIfExists(MwmSet::MwmId const & mwmId, uint3
m_features.erase(matchedMwm);
}
void Editor::RemoveFeatureFromStorageIfExists(FeatureID const & fid)
{
return RemoveFeatureFromStorageIfExists(fid.m_mwmId, fid.m_index);
}
void Editor::Invalidate()
{
if (m_invalidateFn)
@ -942,7 +991,16 @@ void Editor::MarkFeatureAsObsolete(FeatureID const & fid)
auto & fti = m_features[fid.m_mwmId][fid.m_index];
// If a feature was modified we can drop all changes since it's now obsolete.
fti.m_feature = *m_getOriginalFeatureFn(fid);
auto const originalFeaturePtr = m_getOriginalFeatureFn(fid);
if (originalFeaturePtr)
{
LOG(LERROR, ("A feature with id", fid, "cannot be loaded."));
alohalytics::LogEvent("Editor_MissingFeature_Error");
return;
}
fti.m_feature = *originalFeaturePtr;
fti.m_status = FeatureStatus::Obsolete;
fti.m_modificationTimestamp = time(nullptr);

View file

@ -113,7 +113,9 @@ public:
{
NothingWasChanged,
SavedSuccessfully,
NoFreeSpaceError
NoFreeSpaceError,
NoUnderlyingMapError,
SavingError
};
/// Editor checks internally if any feature params were actually edited.
SaveResult SaveEditedFeature(EditableMapObject const & emo);
@ -171,6 +173,7 @@ private:
/// @returns false if fails.
bool Save(string const & fullFilePath) const;
void RemoveFeatureFromStorageIfExists(MwmSet::MwmId const & mwmId, uint32_t index);
void RemoveFeatureFromStorageIfExists(FeatureID const & fid);
/// Notify framework that something has changed and should be redisplayed.
void Invalidate();

View file

@ -478,7 +478,10 @@ search::AddressInfo Framework::GetAddressInfoAtPoint(m2::PointD const & pt) cons
search::AddressInfo Framework::GetFeatureAddressInfo(FeatureID const & fid) const
{
return GetFeatureAddressInfo(*GetFeatureByID(fid));
FeatureType ft;
if (!GetFeatureByID(fid, ft))
return {};
return GetFeatureAddressInfo(ft);
}
search::AddressInfo Framework::GetFeatureAddressInfo(FeatureType & ft) const

View file

@ -394,7 +394,8 @@ Framework::Framework()
{
unique_ptr<FeatureType> feature(new FeatureType());
Index::FeaturesLoaderGuard const guard(m_model.GetIndex(), fid.m_mwmId);
guard.GetOriginalFeatureByIndex(fid.m_index, *feature);
if (!guard.GetOriginalFeatureByIndex(fid.m_index, *feature))
return nullptr;
feature->ParseEverything();
return feature;
});
@ -705,7 +706,12 @@ void Framework::FillFeatureInfo(FeatureID const & fid, place_page::Info & info)
Index::FeaturesLoaderGuard const guard(m_model.GetIndex(), fid.m_mwmId);
FeatureType ft;
guard.GetFeatureByIndex(fid.m_index, ft);
if (!guard.GetFeatureByIndex(fid.m_index, ft))
{
LOG(LERROR, ("Feature can't be loaded:", fid));
return;
}
FillInfoFromFeatureType(ft, info);
// Fill countryId for place page info
@ -1191,7 +1197,12 @@ bool Framework::SearchInDownloader(DownloaderSearchParams const & params)
FeatureID const & fid = it->GetFeatureID();
Index::FeaturesLoaderGuard loader(m_model.GetIndex(), fid.m_mwmId);
FeatureType ft;
loader.GetFeatureByIndex(fid.m_index, ft);
if (!loader.GetFeatureByIndex(fid.m_index, ft))
{
LOG(LERROR, ("Feature can't be loaded:", fid));
continue;
}
ftypes::Type const type = ftypes::IsLocalityChecker::Instance().GetType(ft);
if (type == ftypes::COUNTRY || type == ftypes::STATE)
@ -1337,7 +1348,12 @@ void Framework::LoadSearchResultMetadata(search::Result & res) const
FeatureID const & id = res.GetFeatureID();
ASSERT(id.IsValid(), ("Search result doesn't contain valid FeatureID."));
// TODO @yunikkk refactor to format search result metadata accordingly with place_page::Info
search::ProcessMetadata(*GetFeatureByID(id), res.m_metadata);
FeatureType ft;
if (!GetFeatureByID(id, ft))
return;
search::ProcessMetadata(ft, res.m_metadata);
// res.m_metadata.m_isInitialized is set to true in ProcessMetadata.
}
@ -1884,17 +1900,16 @@ unique_ptr<FeatureType> Framework::GetFeatureAtPoint(m2::PointD const & mercator
return poi ? move(poi) : (area ? move(area) : move(line));
}
unique_ptr<FeatureType> Framework::GetFeatureByID(FeatureID const & fid, bool parse) const
bool Framework::GetFeatureByID(FeatureID const & fid, FeatureType & ft) const
{
ASSERT(fid.IsValid(), ());
unique_ptr<FeatureType> feature(new FeatureType);
// Note: all parse methods should be called with guard alive.
Index::FeaturesLoaderGuard guard(m_model.GetIndex(), fid.m_mwmId);
guard.GetFeatureByIndex(fid.m_index, *feature);
if (parse)
feature->ParseEverything();
return feature;
if (!guard.GetFeatureByIndex(fid.m_index, ft))
return false;
ft.ParseEverything();
return true;
}
BookmarkAndCategory Framework::FindBookmark(UserMark const * mark) const
@ -2550,21 +2565,24 @@ void Framework::EnableChoosePositionMode(bool enable, bool enableBounds, bool ap
vector<m2::TriangleD> Framework::GetSelectedFeatureTriangles() const
{
vector<m2::TriangleD> triangles;
if (m_selectedFeature.IsValid())
if (!m_selectedFeature.IsValid())
return triangles;
Index::FeaturesLoaderGuard const guard(m_model.GetIndex(), m_selectedFeature.m_mwmId);
FeatureType ft;
if (!guard.GetFeatureByIndex(m_selectedFeature.m_index, ft))
return triangles;
if (ftypes::IsBuildingChecker::Instance()(feature::TypesHolder(ft)))
{
Index::FeaturesLoaderGuard const guard(m_model.GetIndex(), m_selectedFeature.m_mwmId);
FeatureType ft;
guard.GetFeatureByIndex(m_selectedFeature.m_index, ft);
if (ftypes::IsBuildingChecker::Instance()(feature::TypesHolder(ft)))
{
triangles.reserve(10);
ft.ForEachTriangle([&](m2::PointD const & p1, m2::PointD const & p2, m2::PointD const & p3)
{
triangles.push_back(m2::TriangleD(p1, p2, p3));
}, scales::GetUpperScale());
}
m_selectedFeature = FeatureID();
triangles.reserve(10);
ft.ForEachTriangle([&](m2::PointD const & p1, m2::PointD const & p2, m2::PointD const & p3)
{
triangles.push_back(m2::TriangleD(p1, p2, p3));
}, scales::GetUpperScale());
}
m_selectedFeature = FeatureID();
return triangles;
}
@ -2595,12 +2613,19 @@ bool Framework::ParseEditorDebugCommand(search::SearchParams const & params)
for (auto const & edit : stats.m_edits)
{
FeatureID const & fid = edit.first;
auto const feature = GetFeatureByID(fid);
FeatureType ft;
if (!GetFeatureByID(fid, ft))
{
LOG(LERROR, ("Feature can't be loaded:", fid));
return true;
}
string name;
feature->GetReadableName(name);
feature::TypesHolder const types(*feature);
ft.GetReadableName(name);
feature::TypesHolder const types(ft);
search::Result::Metadata smd;
results.AddResultNoChecks(search::Result(fid, feature::GetCenter(*feature), name, edit.second,
results.AddResultNoChecks(search::Result(fid, feature::GetCenter(ft), name, edit.second,
DebugPrint(types), types.GetBestType(), smd));
}
params.m_onResults(results);
@ -2617,14 +2642,16 @@ bool Framework::ParseEditorDebugCommand(search::SearchParams const & params)
namespace
{
osm::LocalizedStreet LocalizeStreet(Index const & index, FeatureID const & fid)
WARN_UNUSED_RESULT bool LocalizeStreet(Index const & index, FeatureID const & fid,
osm::LocalizedStreet & result)
{
osm::LocalizedStreet result;
Index::FeaturesLoaderGuard g(index, fid.m_mwmId);
FeatureType ft;
g.GetFeatureByIndex(fid.m_index, ft);
if (!g.GetFeatureByIndex(fid.m_index, ft))
return false;
ft.GetPreferredNames(result.m_defaultName, result.m_localizedName);
return result;
return true;
}
vector<osm::LocalizedStreet> TakeSomeStreetsAndLocalize(
@ -2646,7 +2673,11 @@ vector<osm::LocalizedStreet> TakeSomeStreetsAndLocalize(
if (isDuplicate)
continue;
results.push_back(LocalizeStreet(index, street.m_id));
osm::LocalizedStreet ls;
if (!LocalizeStreet(index, street.m_id, ls))
continue;
results.emplace_back(move(ls));
if (results.size() >= kMaxNumberOfNearbyStreetsToDisplay)
break;
}
@ -2690,15 +2721,22 @@ void SetStreet(search::ReverseGeocoder const & coder, Index const & index,
if (it != end(streetsPool))
{
auto const localizedStreet = LocalizeStreet(index, it->m_id);
emo.SetStreet(localizedStreet);
osm::LocalizedStreet ls;
if (!LocalizeStreet(index, it->m_id, ls))
ls.m_defaultName = street;
emo.SetStreet(ls);
// A street that a feature belongs to should alwas be in the first place in the list.
auto localizedIt = find(begin(localizedStreets), end(localizedStreets), localizedStreet);
if (localizedIt != end(localizedStreets))
iter_swap(localizedIt, begin(localizedStreets));
auto it =
find_if(begin(localizedStreets), end(localizedStreets), [&ls](osm::LocalizedStreet const & rs)
{
return ls.m_defaultName == rs.m_defaultName;
});
if (it != end(localizedStreets))
iter_swap(it, begin(localizedStreets));
else
localizedStreets.insert(begin(localizedStreets), localizedStreet);
localizedStreets.insert(begin(localizedStreets), ls);
}
else
{
@ -2722,7 +2760,8 @@ void SetHostingBuildingAddress(FeatureID const & hostingBuildingFid, Index const
FeatureType hostingBuildingFeature;
Index::FeaturesLoaderGuard g(index, hostingBuildingFid.m_mwmId);
g.GetFeatureByIndex(hostingBuildingFid.m_index, hostingBuildingFeature);
if (!g.GetFeatureByIndex(hostingBuildingFid.m_index, hostingBuildingFeature))
return;
search::ReverseGeocoder::Address address;
if (coder.GetExactAddress(hostingBuildingFeature, address))
@ -2765,8 +2804,10 @@ bool Framework::GetEditableMapObject(FeatureID const & fid, osm::EditableMapObje
if (!fid.IsValid())
return false;
auto feature = GetFeatureByID(fid);
FeatureType & ft = *feature;
FeatureType ft;
if (!GetFeatureByID(fid, ft))
return false;
emo.SetFromFeatureType(ft);
emo.SetHouseNumber(ft.GetHouseNumber());
auto const & editor = osm::Editor::Instance();
@ -2804,14 +2845,17 @@ osm::Editor::SaveResult Framework::SaveEditedMapObject(osm::EditableMapObject em
{
auto const isCreatedFeature = editor.IsCreatedFeature(emo.GetID());
search::ReverseGeocoder::Address hostingBuildingAddress;
Index::FeaturesLoaderGuard g(m_model.GetIndex(), emo.GetID().m_mwmId);
FeatureType originalFeature;
if (!isCreatedFeature)
g.GetOriginalFeatureByIndex(emo.GetID().m_index, originalFeature);
{
if (!g.GetOriginalFeatureByIndex(emo.GetID().m_index, originalFeature))
return osm::Editor::NoUnderlyingMapError;
}
else
{
originalFeature.ReplaceBy(emo);
}
// Handle only pois.
if (ftypes::IsBuildingChecker::Instance()(originalFeature))
@ -2823,9 +2867,12 @@ osm::Editor::SaveResult Framework::SaveEditedMapObject(osm::EditableMapObject em
break;
FeatureType hostingBuildingFeature;
g.GetFeatureByIndex(hostingBuildingFid.m_index, hostingBuildingFeature);
if (!g.GetFeatureByIndex(hostingBuildingFid.m_index, hostingBuildingFeature))
break;
issueLatLon = MercatorBounds::ToLatLon(feature::GetCenter(hostingBuildingFeature));
search::ReverseGeocoder::Address hostingBuildingAddress;
search::ReverseGeocoder const coder(m_model.GetIndex());
// The is no address to take from a hosting building. Fallback to simple saving.
if (!coder.GetExactAddress(hostingBuildingFeature, hostingBuildingAddress))
@ -2917,7 +2964,12 @@ osm::Editor::SaveResult Framework::SaveEditedMapObject(osm::EditableMapObject em
void Framework::DeleteFeature(FeatureID const & fid) const
{
// TODO(AlexZ): Use FeatureID in the editor interface.
osm::Editor::Instance().DeleteFeature(*GetFeatureByID(fid));
FeatureType ft;
if (!GetFeatureByID(fid, ft))
return;
osm::Editor::Instance().DeleteFeature(ft);
if (m_selectedFeature == fid)
m_selectedFeature = FeatureID();
}

View file

@ -541,7 +541,7 @@ public:
/// Set parse to false if you don't need all feature fields ready.
/// TODO(AlexZ): Refactor code which uses this method to get rid of it.
/// FeatureType instances shoud not be used outside ForEach* core methods.
unique_ptr<FeatureType> GetFeatureByID(FeatureID const & fid, bool parse = true) const;
WARN_UNUSED_RESULT bool GetFeatureByID(FeatureID const & fid, FeatureType & ft) const;
void MemoryWarning();
void EnterBackground();

View file

@ -145,8 +145,16 @@ void BicycleDirectionsEngine::Generate(IRoadGraph const & graph, vector<Junction
// Checking for if |edge| is a fake edge.
if (!outFeatureId.IsValid())
continue;
FeatureType ft;
if (!GetLoader(outFeatureId.m_mwmId).GetFeatureByIndex(outFeatureId.m_index, ft))
continue;
auto const highwayClass = ftypes::GetHighwayClass(ft);
ASSERT_NOT_EQUAL(highwayClass, ftypes::HighwayClass::Error, ());
ASSERT_NOT_EQUAL(highwayClass, ftypes::HighwayClass::Undefined, ());
adjacentEdges.m_outgoingTurns.candidates.emplace_back(0.0 /* angle */, outFeatureId.m_index,
GetHighwayClass(outFeatureId));
highwayClass);
}
LoadedPathSegment pathSegment;
@ -174,16 +182,6 @@ Index::FeaturesLoaderGuard & BicycleDirectionsEngine::GetLoader(MwmSet::MwmId co
return *m_loader;
}
ftypes::HighwayClass BicycleDirectionsEngine::GetHighwayClass(FeatureID const & featureId)
{
FeatureType ft;
GetLoader(featureId.m_mwmId).GetFeatureByIndex(featureId.m_index, ft);
auto const highwayClass = ftypes::GetHighwayClass(ft);
ASSERT_NOT_EQUAL(highwayClass, ftypes::HighwayClass::Error, ());
ASSERT_NOT_EQUAL(highwayClass, ftypes::HighwayClass::Undefined, ());
return highwayClass;
}
void BicycleDirectionsEngine::LoadPathGeometry(FeatureID const & featureId,
vector<m2::PointD> const & path,
LoadedPathSegment & pathSegment)
@ -197,7 +195,13 @@ void BicycleDirectionsEngine::LoadPathGeometry(FeatureID const & featureId,
}
FeatureType ft;
GetLoader(featureId.m_mwmId).GetFeatureByIndex(featureId.m_index, ft);
if (!GetLoader(featureId.m_mwmId).GetFeatureByIndex(featureId.m_index, ft))
{
// The feature can't be read, therefore path geometry can't be
// loaded.
return;
}
auto const highwayClass = ftypes::GetHighwayClass(ft);
ASSERT_NOT_EQUAL(highwayClass, ftypes::HighwayClass::Error, ());
ASSERT_NOT_EQUAL(highwayClass, ftypes::HighwayClass::Undefined, ());

View file

@ -33,7 +33,6 @@ public:
private:
Index::FeaturesLoaderGuard & GetLoader(MwmSet::MwmId const & id);
ftypes::HighwayClass GetHighwayClass(FeatureID const & featureId);
void LoadPathGeometry(FeatureID const & featureId, vector<m2::PointD> const & path,
LoadedPathSegment & pathSegment);

View file

@ -197,9 +197,10 @@ void FeaturesRoadGraph::GetFeatureTypes(FeatureID const & featureId, feature::Ty
{
FeatureType ft;
Index::FeaturesLoaderGuard loader(m_index, featureId.m_mwmId);
loader.GetFeatureByIndex(featureId.m_index, ft);
ASSERT_EQUAL(ft.GetFeatureType(), feature::GEOM_LINE, ());
if (!loader.GetFeatureByIndex(featureId.m_index, ft))
return;
ASSERT_EQUAL(ft.GetFeatureType(), feature::GEOM_LINE, ());
types = feature::TypesHolder(ft);
}
@ -262,8 +263,12 @@ IRoadGraph::RoadInfo const & FeaturesRoadGraph::GetCachedRoadInfo(FeatureID cons
return ri;
FeatureType ft;
Index::FeaturesLoaderGuard loader(m_index, featureId.m_mwmId);
loader.GetFeatureByIndex(featureId.m_index, ft);
if (!loader.GetFeatureByIndex(featureId.m_index, ft))
return ri;
ASSERT_EQUAL(ft.GetFeatureType(), feature::GEOM_LINE, ());
ft.ParseGeometry(FeatureType::BEST_GEOMETRY);

View file

@ -101,7 +101,8 @@ void Point2PhantomNode::CalculateWeight(OsrmMappingTypes::FtSeg const & seg,
continue;
FeatureType ft;
loader.GetFeatureByIndex(segment.m_fid, ft);
if (!loader.GetFeatureByIndex(segment.m_fid, ft))
continue;
ft.ParseGeometry(FeatureType::BEST_GEOMETRY);
// Find whole edge weight by node outgoing point.
@ -230,7 +231,9 @@ void Point2PhantomNode::MakeResult(vector<FeatureGraphNode> & res, size_t maxCou
OsrmMappingTypes::FtSeg const & node_seg = segments[j];
FeatureType feature;
Index::FeaturesLoaderGuard loader(m_index, m_routingMapping.GetMwmId());
loader.GetFeatureByIndex(node_seg.m_fid, feature);
if (!loader.GetFeatureByIndex(node_seg.m_fid, feature))
continue;
feature.ParseGeometry(FeatureType::BEST_GEOMETRY);
m2::PointD const featureDirection = feature.GetPoint(node_seg.m_pointEnd) - feature.GetPoint(node_seg.m_pointStart);
bool const sameDirection = (m2::DotProduct(featureDirection, m_direction) > 0);

View file

@ -32,10 +32,16 @@ void LoadPathGeometry(buffer_vector<TSeg, 8> const & buffer, size_t startIndex,
loadPathGeometry.m_path.clear();
return;
}
// Load data from drive.
FeatureType ft;
Index::FeaturesLoaderGuard loader(index, mapping.GetMwmId());
loader.GetFeatureByIndex(segment.m_fid, ft);
if (!loader.GetFeatureByIndex(segment.m_fid, ft))
{
loadPathGeometry.m_path.clear();
return;
}
ft.ParseGeometry(FeatureType::BEST_GEOMETRY);
// Get points in proper direction.

View file

@ -128,7 +128,9 @@ public:
FeatureType ft;
Index::FeaturesLoaderGuard loader(m_index, m_routingMapping.GetMwmId());
loader.GetFeatureByIndex(seg.m_fid, ft);
if (!loader.GetFeatureByIndex(seg.m_fid, ft))
continue;
ft.ParseGeometry(FeatureType::BEST_GEOMETRY);
m2::PointD const outgoingPoint = ft.GetPoint(
@ -244,7 +246,8 @@ void FindGraphNodeOffsets(uint32_t const nodeId, m2::PointD const & point,
FeatureType ft;
Index::FeaturesLoaderGuard loader(*pIndex, mapping->GetMwmId());
loader.GetFeatureByIndex(s.m_fid, ft);
if (!loader.GetFeatureByIndex(s.m_fid, ft))
continue;
helpers::Point2PhantomNode::Candidate mappedSeg;
size_t start_idx = min(s.m_pointStart, s.m_pointEnd);

View file

@ -59,7 +59,6 @@ UNIT_TEST(RussiaMoscowSevTushinoParkBicycleOnePointTurnTest)
TRouteResult const routeResult =
integration::CalculateRoute(integration::GetBicycleComponents(), point, {0.0, 0.0}, point);
Route const & route = *routeResult.first;
IRouter::ResultCode const result = routeResult.second;
TEST_EQUAL(result, IRouter::IRouter::RouteNotFound, ());
}

View file

@ -222,41 +222,24 @@ bool House::GetNearbyMatch(ParsedNumber const & number) const
return m_number.IsIntersect(number, HN_NEARBY_DISTANCE);
}
FeatureLoader::FeatureLoader(Index const * pIndex)
: m_pIndex(pIndex), m_pGuard(0)
{
}
FeatureLoader::~FeatureLoader()
{
Free();
}
FeatureLoader::FeatureLoader(Index const & index) : m_index(index) {}
void FeatureLoader::CreateLoader(MwmSet::MwmId const & mwmId)
{
if (m_pGuard == nullptr || mwmId != m_pGuard->GetId())
{
delete m_pGuard;
m_pGuard = new Index::FeaturesLoaderGuard(*m_pIndex, mwmId);
}
if (!m_guard || mwmId != m_guard->GetId())
m_guard = make_unique<Index::FeaturesLoaderGuard>(m_index, mwmId);
}
void FeatureLoader::Load(FeatureID const & id, FeatureType & f)
bool FeatureLoader::Load(FeatureID const & id, FeatureType & f)
{
CreateLoader(id.m_mwmId);
m_pGuard->GetFeatureByIndex(id.m_index, f);
}
void FeatureLoader::Free()
{
delete m_pGuard;
m_pGuard = 0;
return m_guard->GetFeatureByIndex(id.m_index, f);
}
template <class ToDo>
void FeatureLoader::ForEachInRect(m2::RectD const & rect, ToDo toDo)
{
m_pIndex->ForEachInRect(toDo, rect, scales::GetUpperScale());
m_index.ForEachInRect(toDo, rect, scales::GetUpperScale());
}
m2::RectD Street::GetLimitRect(double offsetMeters) const
@ -329,8 +312,8 @@ void Street::SortHousesProjection()
sort(m_houses.begin(), m_houses.end(), &LessStreetDistance);
}
HouseDetector::HouseDetector(Index const * pIndex)
: m_loader(pIndex), m_streetNum(0)
HouseDetector::HouseDetector(Index const & index)
: m_loader(index), m_streetNum(0)
{
// default value for conversions
SetMetres2Mercator(360.0 / 40.0E06);
@ -463,7 +446,12 @@ int HouseDetector::LoadStreets(vector<FeatureID> const & ids)
continue;
FeatureType f;
m_loader.Load(ids[i], f);
if (!m_loader.Load(ids[i], f))
{
LOG(LWARNING, ("Can't read feature from:", ids[i].m_mwmId));
continue;
}
if (f.GetFeatureType() == feature::GEOM_LINE)
{
// Use default name as a primary compare key for merging.

View file

@ -7,6 +7,8 @@
#include "geometry/point2d.hpp"
#include "base/macros.hpp"
#include "std/string.hpp"
#include "std/queue.hpp"
@ -16,17 +18,16 @@ namespace search
class FeatureLoader
{
Index const * m_pIndex;
Index::FeaturesLoaderGuard * m_pGuard;
Index const & m_index;
unique_ptr<Index::FeaturesLoaderGuard> m_guard;
void CreateLoader(MwmSet::MwmId const & mwmId);
public:
FeatureLoader(Index const * pIndex);
~FeatureLoader();
FeatureLoader(Index const & index);
void Load(FeatureID const & id, FeatureType & f);
void Free();
WARN_UNUSED_RESULT bool Load(FeatureID const & id, FeatureType & f);
inline void Free() { m_guard.reset(); }
template <class ToDo> void ForEachInRect(m2::RectD const & rect, ToDo toDo);
};
@ -251,7 +252,7 @@ class HouseDetector
double GetApprLengthMeters(int index) const;
public:
HouseDetector(Index const * pIndex);
HouseDetector(Index const & index);
~HouseDetector();
int LoadStreets(vector<FeatureID> const & ids);

View file

@ -140,13 +140,15 @@ class PreResult2Maker
unique_ptr<Index::FeaturesLoaderGuard> m_pFV;
// For the best performance, incoming id's should be sorted by id.first (mwm file id).
void LoadFeature(FeatureID const & id, FeatureType & f, m2::PointD & center, string & name,
bool LoadFeature(FeatureID const & id, FeatureType & f, m2::PointD & center, string & name,
string & country)
{
if (m_pFV.get() == 0 || m_pFV->GetId() != id.m_mwmId)
m_pFV.reset(new Index::FeaturesLoaderGuard(m_index, id.m_mwmId));
m_pFV->GetFeatureByIndex(id.m_index, f);
if (!m_pFV->GetFeatureByIndex(id.m_index, f))
return false;
f.SetID(id);
center = feature::GetCenter(f);
@ -158,6 +160,8 @@ class PreResult2Maker
country.clear();
else
country = m_pFV->GetCountryFileName();
return true;
}
void InitRankingInfo(FeatureType const & ft, m2::PointD const & center, PreResult1 const & res,
@ -252,7 +256,8 @@ public:
string name;
string country;
LoadFeature(res1.GetId(), ft, center, name, country);
if (!LoadFeature(res1.GetId(), ft, center, name, country))
return unique_ptr<PreResult2>();
auto res2 = make_unique<PreResult2>(ft, &res1, center, m_ranker.m_params.m_position /* pivot */,
name, country);

View file

@ -509,7 +509,7 @@ UNIT_CLASS_TEST(ProcessorTest, TestPostcodes)
Index::FeaturesLoaderGuard loader(m_engine, countryId);
FeatureType ft;
loader.GetFeatureByIndex(index, ft);
TEST(loader.GetFeatureByIndex(index, ft), ());
auto rule = ExactMatch(countryId, building31);
TEST(rule->Matches(ft), ());
@ -606,7 +606,7 @@ UNIT_CLASS_TEST(ProcessorTest, TestCategories)
{
Index::FeaturesLoaderGuard loader(m_engine, wonderlandId);
FeatureType ft;
loader.GetFeatureByIndex(result.GetFeatureID().m_index, ft);
TEST(loader.GetFeatureByIndex(result.GetFeatureID().m_index, ft), ());
auto const & info = result.GetRankingInfo();

View file

@ -20,6 +20,7 @@
#include "geometry/mercator.hpp"
#include "base/macros.hpp"
#include "base/string_utils.hpp"
#include "std/fstream.hpp"
@ -54,12 +55,12 @@ struct Context
{
Context(Index & index) : m_index(index) {}
void GetFeature(FeatureID const & id, FeatureType & ft)
WARN_UNUSED_RESULT bool GetFeature(FeatureID const & id, FeatureType & ft)
{
auto const & mwmId = id.m_mwmId;
if (!m_guard || m_guard->GetId() != mwmId)
m_guard = make_unique<Index::FeaturesLoaderGuard>(m_index, mwmId);
m_guard->GetFeatureByIndex(id.m_index, ft);
return m_guard->GetFeatureByIndex(id.m_index, ft);
}
Index & m_index;
@ -83,7 +84,8 @@ bool Matches(Context & context, Sample::Result const & golden, search::Result co
return false;
FeatureType ft;
context.GetFeature(actual.GetFeatureID(), ft);
if (!context.GetFeature(actual.GetFeatureID(), ft))
return false;
string name;
if (!ft.GetName(FeatureType::DEFAULT_LANG, name))

View file

@ -197,7 +197,7 @@ UNIT_TEST(HS_StreetsMerge)
TEST_EQUAL(MwmSet::RegResult::Success, p.second, ());
{
search::HouseDetector houser(&index);
search::HouseDetector houser(index);
StreetIDsByName toDo;
toDo.streetNames.push_back("улица Володарского");
index.ForEachInScale(toDo, scales::GetUpperScale());
@ -206,7 +206,7 @@ UNIT_TEST(HS_StreetsMerge)
}
{
search::HouseDetector houser(&index);
search::HouseDetector houser(index);
StreetIDsByName toDo;
toDo.streetNames.push_back("Московская улица");
index.ForEachInScale(toDo, scales::GetUpperScale());
@ -215,7 +215,7 @@ UNIT_TEST(HS_StreetsMerge)
}
{
search::HouseDetector houser(&index);
search::HouseDetector houser(index);
StreetIDsByName toDo;
toDo.streetNames.push_back("проспект Независимости");
toDo.streetNames.push_back("Московская улица");
@ -225,7 +225,7 @@ UNIT_TEST(HS_StreetsMerge)
}
{
search::HouseDetector houser(&index);
search::HouseDetector houser(index);
StreetIDsByName toDo;
toDo.streetNames.push_back("проспект Независимости");
toDo.streetNames.push_back("Московская улица");
@ -238,7 +238,7 @@ UNIT_TEST(HS_StreetsMerge)
}
{
search::HouseDetector houser(&index);
search::HouseDetector houser(index);
StreetIDsByName toDo;
toDo.streetNames.push_back("проспект Независимости");
toDo.streetNames.push_back("Московская улица");
@ -256,7 +256,7 @@ namespace
m2::PointD FindHouse(Index & index, vector<string> const & streets,
string const & houseName, double offset)
{
search::HouseDetector houser(&index);
search::HouseDetector houser(index);
StreetIDsByName toDo;
toDo.streetNames = streets;
@ -426,7 +426,7 @@ UNIT_TEST(HS_MWMSearch)
sort(addresses.begin(), addresses.end());
search::HouseDetector detector(&index);
search::HouseDetector detector(index);
size_t all = 0, matched = 0, notMatched = 0;
size_t const percent = max(size_t(1), addresses.size() / 100);