gpesquero/organicmaps
1
Fork 0
forked from organicmaps/organicmaps
Code Pull requests Activity

[routing] Correct processing that a osm id of a road may correspond to several road feature ids for speed cameras case.

Browse source
This commit is contained in:
Vladimir Byko-Ianko 2020-04-16 17:11:58 +03:00 • committed by Tatiana Yan
parent c97310b4ae
commit 8a86ca95d6
8 changed files with 77 additions and 54 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

36
generator/camera_info_collector.cpp
View file

@ -24,14 +24,14 @@ CamerasInfoCollector::CamerasInfoCollector(std::string const & dataFilePath,
std::string const & camerasInfoPath,
std::string const & osmIdsToFeatureIdsPath)
{
std::map<base::GeoObjectId, uint32_t> osmIdToFeatureId;
if (!routing::ParseRoadsOsmIdToFeatureIdMapping(osmIdsToFeatureIdsPath, osmIdToFeatureId))
std::map<base::GeoObjectId, std::vector<uint32_t>> osmIdToFeatureIds;
if (!routing::ParseRoadsOsmIdToFeatureIdMapping(osmIdsToFeatureIdsPath, osmIdToFeatureIds))
{
LOG(LCRITICAL, ("An error happened while parsing feature id to osm ids mapping from file:",
osmIdsToFeatureIdsPath));
}
if (!ParseIntermediateInfo(camerasInfoPath, osmIdToFeatureId))
if (!ParseIntermediateInfo(camerasInfoPath, osmIdToFeatureIds))
LOG(LCRITICAL, ("Unable to parse intermediate file(", camerasInfoPath, ") about cameras info"));
platform::LocalCountryFile file = platform::LocalCountryFile::MakeTemporary(dataFilePath);
@ -96,8 +96,9 @@ void CamerasInfoCollector::Serialize(FileWriter & writer) const
camera.Serialize(writer, prevFeatureId);
}
bool CamerasInfoCollector::ParseIntermediateInfo(std::string const & camerasInfoPath,
std::map<base::GeoObjectId, uint32_t> const & osmIdToFeatureId)
bool CamerasInfoCollector::ParseIntermediateInfo(
std::string const & camerasInfoPath,
std::map<base::GeoObjectId, std::vector<uint32_t>> const & osmIdToFeatureIds)
{
FileReader reader(camerasInfoPath);
ReaderSource<FileReader> src(reader);
@ -144,9 +145,14 @@ bool CamerasInfoCollector::ParseIntermediateInfo(std::string const & camerasInfo
{
ReadPrimitiveFromSource(src, wayOsmId);
auto const it = osmIdToFeatureId.find(base::MakeOsmWay(wayOsmId));
if (it != osmIdToFeatureId.cend())
ways.emplace_back(it->second /* featureId */, 0 /* segmentId */, 0 /* coef */);
auto const it = osmIdToFeatureIds.find(base::MakeOsmWay(wayOsmId));
if (it != osmIdToFeatureIds.cend())
{
auto const & featureIdsVec = it->second;
// Note. One |wayOsmId| may correspond several feature ids.
for (auto const featureId : featureIdsVec)
ways.emplace_back(featureId, 0 /* segmentId */, 0 /* coef */);
}
}
auto const speed = base::asserted_cast<uint8_t>(maxSpeedKmPH);
@ -282,10 +288,6 @@ std::optional<std::pair<double, uint32_t>> CamerasInfoCollector::Camera::FindMys
ft.ForEachPoint(findPoint, scales::GetUpperScale());
// CHECK(found, ("Cannot find camera point at the feature:", wayFeatureId,
// "; camera center:", mercator::ToLatLon(m_data.m_center)));
// @TODO(bykoianko) The invariant above is valid. But as a fast fix, let's remove
// all the cameras in case of no feature ends matches.
if (!found)
{
cannotFindMyself = true;
@ -308,8 +310,14 @@ std::optional<std::pair<double, uint32_t>> CamerasInfoCollector::Camera::FindMys
if (cannotFindMyself)
{
LOG(LWARNING, ("Cannot find camera point at the feature:", wayFeatureId,
"; camera center:", mercator::ToLatLon(m_data.m_center)));
// Note. Speed camera with coords |m_data.m_center| is not located at any point of |wayFeatureId|.
// It may happens if osm feature corresponds to |wayFeatureId| is split by a mini_roundabout
// or turning_loop. There are two cases:
// * |m_data.m_center| is located at a point of another part of the whole osm feature. In
// that case it will be found on another call of FindMyself() method.
// * |m_data.m_center| coincides with point of mini_roundabout or turning_loop. It means
// there on feature point which coincides with speed camera (|m_data.m_center|).
// These camera (notification) will be lost.
return {};
}

5
generator/camera_info_collector.hpp
View file

@ -80,8 +80,9 @@ private:
inline static double constexpr kMaxDistFromCameraToClosestSegmentMeters = 20.0;
inline static double constexpr kSearchCameraRadiusMeters = 10.0;
bool ParseIntermediateInfo(std::string const & camerasInfoPath,
std::map<base::GeoObjectId, uint32_t> const & osmIdToFeatureId);
bool ParseIntermediateInfo(
std::string const & camerasInfoPath,
std::map<base::GeoObjectId, std::vector<uint32_t>> const & osmIdToFeatureIds);
std::vector<Camera> m_cameras;
};

12
generator/metalines_builder.cpp
View file

@ -225,8 +225,8 @@ void MetalinesBuilder::MergeInto(MetalinesBuilder & collector) const
bool WriteMetalinesSection(std::string const & mwmPath, std::string const & metalinesPath,
std::string const & osmIdsToFeatureIdsPath)
{
std::map<base::GeoObjectId, uint32_t> osmIdToFeatureId;
if (!routing::ParseRoadsOsmIdToFeatureIdMapping(osmIdsToFeatureIdsPath, osmIdToFeatureId))
std::map<base::GeoObjectId, std::vector<uint32_t>> osmIdToFeatureIds;
if (!routing::ParseRoadsOsmIdToFeatureIdMapping(osmIdsToFeatureIdsPath, osmIdToFeatureIds))
return false;
FileReader reader(metalinesPath);
@ -243,12 +243,14 @@ bool WriteMetalinesSection(std::string const & mwmPath, std::string const & meta
for (auto const wayId : ways)
{
// We get a negative wayId when a feature direction should be reversed.
auto fid = osmIdToFeatureId.find(base::MakeOsmWay(std::abs(wayId)));
if (fid == osmIdToFeatureId.cend())
auto fids = osmIdToFeatureIds.find(base::MakeOsmWay(std::abs(wayId)));
if (fids == osmIdToFeatureIds.cend())
break;
// Keeping the sign for the feature direction.
int32_t const featureId = static_cast<int32_t>(fid->second);
// @TODO(bykoianko) All the feature ids should be used instead of |fids->second[0]|.
CHECK(!fids->second.empty(), ());
int32_t const featureId = static_cast<int32_t>(fids->second.front());
featureIds.push_back(wayId > 0 ? featureId : -featureId);
}

16
generator/restriction_collector.cpp
View file

@ -52,7 +52,7 @@ RestrictionCollector::RestrictionCollector(std::string const & osmIdsToFeatureId
std::unique_ptr<IndexGraph> && graph)
: m_indexGraph(std::move(graph))
{
CHECK(ParseRoadsOsmIdToFeatureIdMapping(osmIdsToFeatureIdPath, m_osmIdToFeatureId),
CHECK(ParseRoadsOsmIdToFeatureIdMapping(osmIdsToFeatureIdPath, m_osmIdToFeatureIds),
("An error happened while parsing feature id to "
"osm ids mapping from file:", osmIdsToFeatureIdPath));
}
@ -62,7 +62,7 @@ bool RestrictionCollector::Process(std::string const & restrictionPath)
CHECK(m_indexGraph, ());
SCOPE_GUARD(clean, [this]() {
m_osmIdToFeatureId.clear();
m_osmIdToFeatureIds.clear();
m_restrictions.clear();
});
@ -311,16 +311,18 @@ bool RestrictionCollector::AddRestriction(m2::PointD const & coords,
std::vector<uint32_t> featureIds(osmIds.size());
for (size_t i = 0; i < osmIds.size(); ++i)
{
auto const result = m_osmIdToFeatureId.find(osmIds[i]);
if (result == m_osmIdToFeatureId.cend())
auto const result = m_osmIdToFeatureIds.find(osmIds[i]);
if (result == m_osmIdToFeatureIds.cend())
{
// It could happend near mwm border when one of a restriction lines is not included in mwm
// It could happens near mwm border when one of a restriction lines is not included in mwm
// but the restriction is included.
return false;
}
// Only one feature id is found for |osmIds[i]|.
featureIds[i] = result->second;
// @TODO(bykoianko) All the feature ids should be used instead of |result->second.front()|.
CHECK(!result->second.empty(), ());
featureIds[i] = result->second.front();
}
if (!IsRestrictionValid(restrictionType, coords, featureIds))
@ -332,7 +334,7 @@ bool RestrictionCollector::AddRestriction(m2::PointD const & coords,
void RestrictionCollector::AddFeatureId(uint32_t featureId, base::GeoObjectId osmId)
{
::routing::AddFeatureId(osmId, featureId, m_osmIdToFeatureId);
::routing::AddFeatureId(osmId, featureId, m_osmIdToFeatureIds);
}
void FromString(std::string const & str, Restriction::Type & type)

2
generator/restriction_collector.hpp
View file

@ -91,7 +91,7 @@ private:
std::vector<base::GeoObjectId> const & osmIds);
std::vector<Restriction> m_restrictions;
std::map<base::GeoObjectId, uint32_t> m_osmIdToFeatureId;
std::map<base::GeoObjectId, std::vector<uint32_t>> m_osmIdToFeatureIds;
std::unique_ptr<IndexGraph> m_indexGraph;

29
generator/road_access_generator.cpp
View file

@ -163,7 +163,7 @@ set<OsmElement::Tag> const kHighwaysWhereIgnoreBarriersWithoutAccess = {
};
bool ParseRoadAccess(string const & roadAccessPath,
map<base::GeoObjectId, uint32_t> const & osmIdToFeatureId,
map<base::GeoObjectId, vector<uint32_t>> const & osmIdToFeatureIds,
RoadAccessCollector::RoadAccessByVehicleType & roadAccessByVehicleType)
{
ifstream stream(roadAccessPath);
@ -240,14 +240,16 @@ bool ParseRoadAccess(string const & roadAccessPath,
}
++iter;
auto const it = osmIdToFeatureId.find(base::MakeOsmWay(osmId));
auto const it = osmIdToFeatureIds.find(base::MakeOsmWay(osmId));
// Even though this osm element has a tag that is interesting for us,
// we have not created a feature from it. Possible reasons:
// no primary tag, unsupported type, etc.
if (it == osmIdToFeatureId.cend())
if (it == osmIdToFeatureIds.cend())
continue;
uint32_t const featureId = it->second;
// @TODO(bykoianko) All the feature ids should be used instead of |it->second.front()|.
CHECK(!it->second.empty(), ());
uint32_t const featureId = it->second.front();
if (pointIdx == 0)
addFeature(featureId, vehicleType, roadAccessType, osmId);
@ -262,7 +264,8 @@ bool ParseRoadAccess(string const & roadAccessPath,
}
void ParseRoadAccessConditional(
string const & roadAccessPath, map<base::GeoObjectId, uint32_t> const & osmIdToFeatureId,
string const & roadAccessPath,
map<base::GeoObjectId, vector<uint32_t>> const & osmIdToFeatureIds,
RoadAccessCollector::RoadAccessByVehicleType & roadAccessByVehicleType)
{
ifstream stream(roadAccessPath);
@ -312,10 +315,12 @@ void ParseRoadAccessConditional(
if (conditional.IsEmpty())
continue;
auto const it = osmIdToFeatureId.find(base::MakeOsmWay(osmId));
if (it == osmIdToFeatureId.end())
auto const it = osmIdToFeatureIds.find(base::MakeOsmWay(osmId));
if (it == osmIdToFeatureIds.end())
continue;
uint32_t const featureId = it->second;
// @TODO(bykoianko) All the feature ids should be used instead of |it->second.front()|.
CHECK(!osmIdToFeatureIds.empty(), ());
uint32_t const featureId = it->second.front();
wayToAccessConditional[static_cast<size_t>(vehicleType)].emplace(featureId, move(conditional));
}
@ -643,8 +648,8 @@ void RoadAccessWriter::MergeInto(RoadAccessWriter & collector) const
RoadAccessCollector::RoadAccessCollector(string const & dataFilePath, string const & roadAccessPath,
string const & osmIdsToFeatureIdsPath)
{
map<base::GeoObjectId, uint32_t> osmIdToFeatureId;
if (!ParseRoadsOsmIdToFeatureIdMapping(osmIdsToFeatureIdsPath, osmIdToFeatureId))
map<base::GeoObjectId, vector<uint32_t>> osmIdToFeatureIds;
if (!ParseRoadsOsmIdToFeatureIdMapping(osmIdsToFeatureIdsPath, osmIdToFeatureIds))
{
LOG(LWARNING, ("An error happened while parsing feature id to osm ids mapping from file:",
osmIdsToFeatureIdsPath));
@ -653,7 +658,7 @@ RoadAccessCollector::RoadAccessCollector(string const & dataFilePath, string con
}
RoadAccessCollector::RoadAccessByVehicleType roadAccessByVehicleType;
if (!ParseRoadAccess(roadAccessPath, osmIdToFeatureId, roadAccessByVehicleType))
if (!ParseRoadAccess(roadAccessPath, osmIdToFeatureIds, roadAccessByVehicleType))
{
LOG(LWARNING, ("An error happened while parsing road access from file:", roadAccessPath));
m_valid = false;
@ -661,7 +666,7 @@ RoadAccessCollector::RoadAccessCollector(string const & dataFilePath, string con
}
auto const roadAccessConditional = roadAccessPath + ROAD_ACCESS_CONDITIONAL_EXT;
ParseRoadAccessConditional(roadAccessConditional, osmIdToFeatureId, roadAccessByVehicleType);
ParseRoadAccessConditional(roadAccessConditional, osmIdToFeatureIds, roadAccessByVehicleType);
m_valid = true;
m_roadAccessByVehicleType.swap(roadAccessByVehicleType);

13
generator/routing_helpers.cpp
View file

@ -27,19 +27,18 @@ bool ForEachRoadFromFile(string const & filename, ToDo && toDo)
namespace routing
{
void AddFeatureId(base::GeoObjectId osmId, uint32_t featureId,
map<base::GeoObjectId, uint32_t> & osmIdToFeatureId)
map<base::GeoObjectId, std::vector<uint32_t>> & osmIdToFeatureIds)
{
// Failing to insert here usually means that two features were created
// from one osm id, for example an area and its boundary.
osmIdToFeatureId.emplace(osmId, featureId);
osmIdToFeatureIds[osmId].push_back(featureId);
}
bool ParseRoadsOsmIdToFeatureIdMapping(string const & osmIdsToFeatureIdPath,
map<base::GeoObjectId, uint32_t> & osmIdToFeatureId)
bool ParseRoadsOsmIdToFeatureIdMapping(
string const & osmIdsToFeatureIdPath,
map<base::GeoObjectId, std::vector<uint32_t>> & osmIdToFeatureIds)
{
return ForEachRoadFromFile(osmIdsToFeatureIdPath,
[&](uint32_t featureId, base::GeoObjectId osmId) {
AddFeatureId(osmId, featureId, osmIdToFeatureId);
AddFeatureId(osmId, featureId, osmIdToFeatureIds);
});
}

18
generator/routing_helpers.hpp
View file

@ -5,24 +5,30 @@
#include <cstdint>
#include <map>
#include <string>
#include <vector>
namespace routing
{
// Adds feature id and corresponding |osmId| to |osmIdToFeatureId|.
// Adds |featureId| and corresponding |osmId| to |osmIdToFeatureId|.
// Note. In general, one |featureId| may correspond to several osm ids.
// But for a road feature |featureId| corresponds to exactly one osm id.
// Or on the contrary one osm id may correspond to several feature ids. It may happens for example
// when an area and its boundary may correspond to the same osm id.
// As for road features a road |osmId| may correspond to several feature ids if
// the |osmId| is split by a mini_roundabout or a turning_loop.
void AddFeatureId(base::GeoObjectId osmId, uint32_t featureId,
std::map<base::GeoObjectId, uint32_t> & osmIdToFeatureId);
std::map<base::GeoObjectId, std::vector<uint32_t>> & osmIdToFeatureIds);
// Parses comma separated text file with line in following format:
// <feature id>, <osm id 1 corresponding feature id>, <osm id 2 corresponding feature id>, and so
// on
// on. It may contain several line with the same feature ids.
// For example:
// 137999, 5170186,
// 138000, 5170209, 5143342,
// 138001, 5170228,
bool ParseRoadsOsmIdToFeatureIdMapping(std::string const & osmIdsToFeatureIdPath,
std::map<base::GeoObjectId, uint32_t> & osmIdToFeatureId);
// 137999, 5170197,
bool ParseRoadsOsmIdToFeatureIdMapping(
std::string const & osmIdsToFeatureIdPath,
std::map<base::GeoObjectId, std::vector<uint32_t>> & osmIdToFeatureIds);
bool ParseRoadsFeatureIdToOsmIdMapping(std::string const & osmIdsToFeatureIdPath,
std::map<uint32_t, base::GeoObjectId> & featureIdToOsmId);
} // namespace routing