Review fixes.

This commit is contained in:
Vladimir Byko-Ianko 2016-11-07 18:27:26 +03:00
parent 3fc9a85c31
commit 4b920ba9cf
15 changed files with 62 additions and 60 deletions

View file

@ -107,9 +107,9 @@ uint32_t FeaturesCollector::operator()(FeatureBuilder1 const & fb)
{
FeatureBuilder1::TBuffer bytes;
fb.Serialize(bytes);
(void)WriteFeatureBase(bytes, fb);
uint32_t const featureId = WriteFeatureBase(bytes, fb);
CHECK_LESS(0, m_featureID, ());
return m_featureID - 1;
return featureId;
}
FeaturesAndRawGeometryCollector::FeaturesAndRawGeometryCollector(string const & featuresFileName,

View file

@ -4,6 +4,7 @@
#include "coding/file_writer.hpp"
#include "std/numeric.hpp"
#include "std/string.hpp"
#include "std/vector.hpp"
@ -19,6 +20,7 @@ class FeaturesCollector
uint32_t m_featureID = 0;
protected:
static uint32_t constexpr kInvalidFeatureId = numeric_limits<uint32_t>::max();
FileWriter m_datFile;
m2::RectD m_bounds;

View file

@ -27,8 +27,6 @@
#include "base/scope_guard.hpp"
#include "base/string_utils.hpp"
#include "std/limits.hpp"
namespace
{
typedef pair<uint64_t, uint64_t> CellAndOffsetT;
@ -522,7 +520,7 @@ namespace feature
}
}
uint32_t featureId = numeric_limits<uint32_t>::max();
uint32_t featureId = kInvalidFeatureId;
if (fb.PreSerialize(holder.m_buffer))
{
fb.Serialize(holder.m_buffer, m_header.GetDefCodingParams());

View file

@ -36,10 +36,11 @@ struct GenerateInfo
// Current generated file name if --output option is defined.
string m_fileName;
// File name with restriction in osm id terms.
// File name for a file with restrictions in osm id terms.
string m_restrictions;
// File name with mapping from feature id to osm ids.
string m_featureId2OsmIds;
// File name for a file with mapping from features id to osm ids.
// Note. One feature id maps to one or several osm ids.
string m_featureIdToOsmIds;
NodeStorageType m_nodeStorageType;
OsmSourceType m_osmFileType;

View file

@ -92,4 +92,3 @@ HEADERS += \
unpack_mwm.hpp \
ways_merger.hpp \
world_map_generator.hpp \

View file

@ -38,4 +38,3 @@ SOURCES += \
tag_admixer_test.cpp \
tesselator_test.cpp \
triangles_tree_coding_test.cpp \

View file

@ -5,13 +5,13 @@
#include "indexer/routing.hpp"
#include "coding/file_name_utils.hpp"
#include "platform/platform_tests_support/scoped_dir.hpp"
#include "platform/platform_tests_support/scoped_file.hpp"
#include "platform/platform.hpp"
#include "coding/file_name_utils.hpp"
#include "base/stl_helpers.hpp"
#include "std/string.hpp"
@ -27,7 +27,7 @@ string const kRestrictionTestDir = "test-restrictions";
UNIT_TEST(RestrictionTest_ValidCase)
{
RestrictionCollector restrictionCollector("", "");
RestrictionCollector restrictionCollector("" /* restrictionPath */, "" /* featureId2OsmIdsPath */);
// Adding restrictions and feature ids to restrictionCollector in mixed order.
restrictionCollector.AddRestriction(Restriction::Type::No, {1, 2} /* osmIds */);
restrictionCollector.AddFeatureId(30 /* featureId */, {3} /* osmIds */);
@ -53,7 +53,7 @@ UNIT_TEST(RestrictionTest_ValidCase)
UNIT_TEST(RestrictionTest_InvalidCase)
{
RestrictionCollector restrictionCollector("", "");
RestrictionCollector restrictionCollector("" /* restrictionPath */, "" /* featureId2OsmIdsPath */);
restrictionCollector.AddFeatureId(0 /* featureId */, {0} /* osmIds */);
restrictionCollector.AddRestriction(Restriction::Type::No, {0, 1} /* osmIds */);
restrictionCollector.AddFeatureId(20 /* featureId */, {2} /* osmIds */);
@ -84,21 +84,21 @@ UNIT_TEST(RestrictionTest_ParseRestrictions)
ScopedDir const scopedDir(kRestrictionTestDir);
ScopedFile const scopedFile(kRestrictionPath, kRestrictionContent);
RestrictionCollector restrictionCollector("", "");
RestrictionCollector restrictionCollector("" /* restrictionPath */, "" /* featureId2OsmIdsPath */);
Platform const & platform = Platform();
TEST(restrictionCollector.ParseRestrictions(
my::JoinFoldersToPath(platform.WritableDir(), kRestrictionPath)),
());
RestrictionVec expectedRestrictions = {{Restriction::Type::No, 2},
{Restriction::Type::Only, 2},
{Restriction::Type::Only, 2},
{Restriction::Type::No, 2},
{Restriction::Type::No, 2}};
RestrictionVec expectedRestrictions = {{Restriction::Type::No, 2 /* linkNumber */},
{Restriction::Type::Only, 2 /* linkNumber */},
{Restriction::Type::Only, 2 /* linkNumber */},
{Restriction::Type::No, 2 /* linkNumber */},
{Restriction::Type::No, 2 /* linkNumber */}};
TEST_EQUAL(restrictionCollector.m_restrictions, expectedRestrictions, ());
vector<pair<uint64_t, RestrictionCollector::Index>> const expectedRestrictionIndex = {
vector<pair<uint64_t, RestrictionCollector::LinkIndex>> const expectedRestrictionIndex = {
{1, {0, 0}}, {1, {0, 1}}, {0, {1, 0}}, {2, {1, 1}}, {2, {2, 0}},
{3, {2, 1}}, {38028428, {3, 0}}, {38028428, {3, 1}}, {4, {4, 0}}, {5, {4, 1}}};
TEST_EQUAL(restrictionCollector.m_restrictionIndex, expectedRestrictionIndex, ());
@ -117,7 +117,7 @@ UNIT_TEST(RestrictionTest_ParseFeatureId2OsmIdsMapping)
ScopedDir const scopedDir(kRestrictionTestDir);
ScopedFile const scopedFile(kFeatureIdToOsmIdsPath, kFeatureIdToOsmIdsContent);
RestrictionCollector restrictionCollector("", "");
RestrictionCollector restrictionCollector("" /* restrictionPath */, "" /* featureId2OsmIdsPath */);
Platform const & platform = Platform();
restrictionCollector.ParseFeatureId2OsmIdsMapping(

View file

@ -25,9 +25,9 @@
using namespace feature;
using namespace generator;
using namespace platform::tests_support;
using namespace platform;
using namespace routing;
using namespace platform::tests_support;
namespace
{
@ -50,10 +50,10 @@ void BuildEmptyMwm(LocalCountryFile & country)
void TestRestrictionBuilding(string const & restrictionContent, string const & mappingContent)
{
Platform & platform = GetPlatform();
string const writeableDir = platform.WritableDir();
string const writableDir = platform.WritableDir();
// Building empty mwm.
LocalCountryFile country(my::JoinFoldersToPath(writeableDir, kTestDir), CountryFile(kTestMwm), 1);
LocalCountryFile country(my::JoinFoldersToPath(writableDir, kTestDir), CountryFile(kTestMwm), 1);
ScopedDir const scopedDir(kTestDir);
string const mwmRelativePath = my::JoinFoldersToPath(kTestDir, kTestMwm + DATA_FILE_EXTENSION);
ScopedFile const scopedMwm(mwmRelativePath);
@ -67,9 +67,9 @@ void TestRestrictionBuilding(string const & restrictionContent, string const & m
ScopedFile const mappingScopedFile(mappingRelativePath, mappingContent);
// Adding restriction section to mwm.
string const restrictionFullPath = my::JoinFoldersToPath(writeableDir, restrictionRelativePath);
string const mappingFullPath = my::JoinFoldersToPath(writeableDir, mappingRelativePath);
string const mwmFullPath = my::JoinFoldersToPath(writeableDir, mwmRelativePath);
string const restrictionFullPath = my::JoinFoldersToPath(writableDir, restrictionRelativePath);
string const mappingFullPath = my::JoinFoldersToPath(writableDir, mappingRelativePath);
string const mwmFullPath = my::JoinFoldersToPath(writableDir, mwmRelativePath);
BuildRoadRestrictions(mwmFullPath, restrictionFullPath, mappingFullPath);
// Reading from mwm section and testing restrictions.

View file

@ -118,7 +118,7 @@ int main(int argc, char ** argv)
genInfo.m_osmFileName = FLAGS_osm_file_name;
genInfo.m_restrictions = FLAGS_restriction_name;
genInfo.m_featureId2OsmIds = FLAGS_feature_id_to_osm_ids_name;
genInfo.m_featureIdToOsmIds = FLAGS_feature_id_to_osm_ids_name;
genInfo.m_failOnCoasts = FLAGS_fail_on_coasts;
genInfo.m_preloadCache = FLAGS_preload_cache;
genInfo.m_bookingDatafileName = FLAGS_booking_data;
@ -163,7 +163,7 @@ int main(int argc, char ** argv)
LOG(LINFO, ("Generating final data ..."));
genInfo.m_restrictions = FLAGS_restriction_name;
genInfo.m_featureId2OsmIds = FLAGS_feature_id_to_osm_ids_name;
genInfo.m_featureIdToOsmIds = FLAGS_feature_id_to_osm_ids_name;
genInfo.m_splitByPolygons = FLAGS_split_by_polygons;
genInfo.m_createWorld = FLAGS_generate_world;
genInfo.m_makeCoasts = FLAGS_make_coasts;
@ -239,11 +239,12 @@ int main(int argc, char ** argv)
if (!FLAGS_srtm_path.empty())
routing::BuildRoadAltitudes(datFile, FLAGS_srtm_path);
// @TODO(bykoianko) generate_routing flag should be used for all routing information.
if (FLAGS_generate_routing)
{
routing::BuildRoadRestrictions(
datFile, genInfo.GetIntermediateFileName(genInfo.m_restrictions, ""),
genInfo.GetIntermediateFileName(genInfo.m_featureId2OsmIds, ""));
datFile, genInfo.GetIntermediateFileName(genInfo.m_restrictions, "" /* extention */),
genInfo.GetIntermediateFileName(genInfo.m_featureIdToOsmIds, "" /* extention */));
}
}

View file

@ -341,9 +341,8 @@ public:
if (info.m_createWorld)
m_world.reset(new TWorldGenerator(info));
// Feature id osm id to map.
string const featureId2OsmIdsFile = info.GetIntermediateFileName(info.m_featureId2OsmIds, "");
LOG(LINFO, ("Saving osm ids to feature ids map to", featureId2OsmIdsFile));
string const featureId2OsmIdsFile = info.GetIntermediateFileName(info.m_featureIdToOsmIds, "");
LOG(LINFO, ("Saving mapping from feature id osm ids to file", featureId2OsmIdsFile));
m_featureId2osmIds.Open(featureId2OsmIdsFile);
if (!m_featureId2osmIds.IsOpened())
{
@ -552,9 +551,9 @@ void SyncOfstream::Write(uint32_t featureId, vector<osm::Id> const & osmIds)
return;
lock_guard<mutex> guard(m_mutex);
m_stream << featureId << ",";
m_stream << featureId;
for (osm::Id const & osmId : osmIds)
m_stream << osmId.OsmId() << ",";
m_stream << "," << osmId.OsmId();
m_stream << endl;
}

View file

@ -117,11 +117,11 @@ bool RestrictionCollector::ParseRestrictions(string const & restrictionPath)
void RestrictionCollector::ComposeRestrictions()
{
// Going through all osm id saved in |m_restrictionIndex| (mentioned in restrictions).
size_t const restrictionSz = m_restrictions.size();
for (pair<uint64_t, Index> const & osmIdAndIndex : m_restrictionIndex)
size_t const numRestrictions = m_restrictions.size();
for (auto const & osmIdAndIndex : m_restrictionIndex)
{
Index const & index = osmIdAndIndex.second;
CHECK_LESS(index.m_restrictionNumber, restrictionSz, ());
LinkIndex const & index = osmIdAndIndex.second;
CHECK_LESS(index.m_restrictionNumber, numRestrictions, ());
Restriction & restriction = m_restrictions[index.m_restrictionNumber];
CHECK_LESS(index.m_linkNumber, restriction.m_links.size(), ());
@ -157,10 +157,10 @@ void RestrictionCollector::RemoveInvalidRestrictions()
void RestrictionCollector::AddRestriction(Restriction::Type type, vector<uint64_t> const & osmIds)
{
size_t const restrictionCount = m_restrictions.size();
m_restrictions.emplace_back(type, osmIds.size());
size_t const restrictionCount = m_restrictions.size() - 1;
for (size_t i = 0; i < osmIds.size(); ++i)
m_restrictionIndex.emplace_back(osmIds[i], Index({restrictionCount, i}));
m_restrictionIndex.emplace_back(osmIds[i], LinkIndex({restrictionCount, i}));
}
void RestrictionCollector::AddFeatureId(Restriction::FeatureId featureId,
@ -200,7 +200,8 @@ bool FromString(string str, Restriction::Type & type)
}
string DebugPrint(Restriction::Type const & type) { return ToString(type); }
string DebugPrint(RestrictionCollector::Index const & index)
string DebugPrint(RestrictionCollector::LinkIndex const & index)
{
ostringstream out;
out << "m_restrictionNumber:" << index.m_restrictionNumber

View file

@ -22,17 +22,19 @@ class RestrictionCollector
public:
/// \brief Addresses a link in vector<Restriction>.
struct Index
struct LinkIndex
{
Index(size_t restrictionNumber, size_t linkNumber)
LinkIndex(size_t restrictionNumber, size_t linkNumber)
: m_restrictionNumber(restrictionNumber), m_linkNumber(linkNumber)
{
}
size_t m_restrictionNumber = 0; // Restriction number in restriction vector.
size_t m_linkNumber =
0; // Link number for a restriction. It's equal to zero or one for most cases.
bool operator==(Index const & index) const
// Restriction number in restriction vector.
size_t m_restrictionNumber = 0;
// Link number for a restriction. It's equal to zero or one for most cases.
size_t m_linkNumber = 0;
bool operator==(LinkIndex const & index) const
{
return m_restrictionNumber == index.m_restrictionNumber && m_linkNumber == index.m_linkNumber;
}
@ -56,11 +58,11 @@ private:
/// on
/// For example:
/// 137999, 5170186,
/// 138000, 5170209,
/// 138000, 5170209, 5143342,
/// 138001, 5170228,
/// \param featureId2OsmIdsPath path to the text file.
/// \note Most restrictions consist of type and two linear(road) features.
/// \note For the time being only line-point-line restritions are supported.
/// \note For the time being only line-point-line restrictions are supported.
bool ParseFeatureId2OsmIdsMapping(string const & featureId2OsmIdsPath);
/// \brief Parses comma separated text file with line in following format:
@ -91,7 +93,7 @@ private:
void AddRestriction(Restriction::Type type, vector<uint64_t> const & osmIds);
RestrictionVec m_restrictions;
vector<pair<uint64_t, Index>> m_restrictionIndex;
vector<pair<uint64_t, LinkIndex>> m_restrictionIndex;
unordered_multimap<uint64_t, Restriction::FeatureId> m_osmIds2FeatureId;
};
@ -99,6 +101,6 @@ private:
string ToString(Restriction::Type const & type);
bool FromString(string str, Restriction::Type & type);
string DebugPrint(Restriction::Type const & type);
string DebugPrint(RestrictionCollector::Index const & index);
string DebugPrint(RestrictionCollector::LinkIndex const & index);
string DebugPrint(Restriction const & restriction);
} // namespace routing

View file

@ -78,14 +78,14 @@ void RestrictionDumper::Write(RelationElement const & relationElement)
auto const fromIt = findTag(relationElement.ways, "from");
if (fromIt == relationElement.ways.cend())
return; // No tag |from| in |relationElement.ways|.
return;
auto const toIt = findTag(relationElement.ways, "to");
if (toIt == relationElement.ways.cend())
return; // No tag |to| in |relationElement.ways|.
return;
if (findTag(relationElement.nodes, "via") == relationElement.nodes.cend())
return; // No tag |via| in |relationElement.nodes|.
return;
// Extracting type of restriction.
auto const tagIt = relationElement.tags.find("restriction");

View file

@ -16,7 +16,7 @@ public:
bool IsOpened();
/// \brief Writes |relationElement| to |m_stream| if |relationElement| is a supported restriction.
/// \note For the time being only line-point-line restritions are processed. The other
/// \note For the time being only line-point-line restrictions are processed. The other
/// restrictions
/// are ignored.
// @TODO(bykoianko) It's necessary to process all kind of restrictions.

View file

@ -35,8 +35,8 @@ bool Restriction::operator<(Restriction const & restriction) const
namespace feature
{
// For the time being only one kind of restrictions is support. It's line-point-line
// restrictions in osm ids term. Such restrictions reflects to two feature ids
// For the time being only one kind of restrictions is supported. It's line-point-line
// restrictions in osm ids term. Such restrictions correspond to two feature ids
// restrictions in feature id terms. Because of it supported number of links is two.
size_t const RestrictionSerializer::kSupportedLinkNumber = 2;
}