From 0f6a349b21dd5e05e38fae682d096b6c4f22f848 Mon Sep 17 00:00:00 2001 From: Maksim Andrianov Date: Mon, 3 Jun 2019 16:15:44 +0300 Subject: [PATCH] Review fixes. --- generator/collector_interface.hpp | 2 + generator/feature_builder.cpp | 16 +-- generator/feature_builder.hpp | 39 ++++--- generator/feature_generator.cpp | 2 +- generator/feature_maker.cpp | 4 +- generator/feature_processing_layers.cpp | 4 +- .../generator_tests/feature_builder_test.cpp | 38 +++--- generator/generator_tests/regions_tests.cpp | 110 +++++++++--------- .../test_mwm_builder.cpp | 2 +- generator/locality_sorter.cpp | 2 +- 10 files changed, 112 insertions(+), 107 deletions(-) diff --git a/generator/collector_interface.hpp b/generator/collector_interface.hpp index 840fd1a618..bf22ba6c82 100644 --- a/generator/collector_interface.hpp +++ b/generator/collector_interface.hpp @@ -4,10 +4,12 @@ struct OsmElement; class RelationElement; + namespace feature { class FeatureBuilder; } // namespace feature + namespace base { class GeoObjectId; diff --git a/generator/feature_builder.cpp b/generator/feature_builder.cpp index 53fbf34206..daea2cc762 100644 --- a/generator/feature_builder.cpp +++ b/generator/feature_builder.cpp @@ -138,7 +138,7 @@ void FeatureBuilder::SetLinear(bool reverseGeometry) } } -void FeatureBuilder::AddHoles(FeatureBuilder::Geometry const & holes) +void FeatureBuilder::SetHoles(FeatureBuilder::Geometry const & holes) { m_polygons.resize(1); @@ -256,7 +256,7 @@ bool FeatureBuilder::PreSerialize() return true; } -bool FeatureBuilder::PreSerializeAndRemoveUselessNamesForTmpMwm() +bool FeatureBuilder::PreSerializeAndRemoveUselessNamesForIntermediate() { if (!PreSerialize()) return false; @@ -348,7 +348,7 @@ void FeatureBuilder::SerializeBase(Buffer & data, serial::GeometryCodingParams c serial::SavePoint(sink, m_center, params); } -void FeatureBuilder::SerializeForTmpMwm(Buffer & data) const +void FeatureBuilder::SerializeForIntermediate(Buffer & data) const { Check(*this); @@ -377,12 +377,12 @@ void FeatureBuilder::SerializeForTmpMwm(Buffer & data) const #ifdef DEBUG Buffer tmp(data); FeatureBuilder fb; - fb.Deserialize(tmp); + fb.DeserializeFromIntermediate(tmp); ASSERT ( fb == *this, ("Source feature: ", *this, "Deserialized feature: ", fb) ); #endif } -void FeatureBuilder::SerializeBorderForTmpMwm(serial::GeometryCodingParams const & params, +void FeatureBuilder::SerializeBorderForIntermediate(serial::GeometryCodingParams const & params, Buffer & data) const { data.clear(); @@ -408,7 +408,7 @@ void FeatureBuilder::SerializeBorderForTmpMwm(serial::GeometryCodingParams const } } -void FeatureBuilder::DeserializeForTmpMwm(Buffer & data) +void FeatureBuilder::DeserializeFromIntermediate(Buffer & data) { serial::GeometryCodingParams cp; @@ -550,7 +550,7 @@ bool FeatureBuilder::PreSerializeAndRemoveUselessNamesForMwm(SupportingData cons } // we don't need empty features without geometry - return PreSerializeAndRemoveUselessNamesForTmpMwm(); + return PreSerializeAndRemoveUselessNamesForIntermediate(); } void FeatureBuilder::SerializeLocalityObject(serial::GeometryCodingParams const & params, @@ -668,8 +668,10 @@ void Check(FeatureBuilder const fb) CHECK(fb.GetOuterGeometry().size() >= 2, (fb)); if (fb.IsArea()) + { for (auto const & points : fb.GetGeometry()) CHECK(points.size() >= 3, (fb)); + } } string DebugPrint(FeatureBuilder const & fb) diff --git a/generator/feature_builder.hpp b/generator/feature_builder.hpp index 72486a235d..6c3c46b4ab 100644 --- a/generator/feature_builder.hpp +++ b/generator/feature_builder.hpp @@ -45,7 +45,7 @@ public: // To work with geometry. void AddPoint(m2::PointD const & p); - void AddHoles(Geometry const & holes); + void SetHoles(Geometry const & holes); void AddPolygon(std::vector & poly); void ResetGeometry(); m2::RectD const & GetLimitRect() const { return m_limitRect; } @@ -124,12 +124,14 @@ public: void SetType(uint32_t type) { m_params.SetType(type); } void AddType(uint32_t type) { m_params.AddType(type); } bool PopExactType(uint32_t type) { return m_params.PopExactType(type); } - template - bool RemoveTypesIf(FnT fn) + + template + bool RemoveTypesIf(Fn && fn) { - base::EraseIf(m_params.m_types, fn); + base::EraseIf(m_params.m_types, std::forward(fn)); return m_params.m_types.empty(); } + bool HasType(uint32_t t) const { return m_params.IsTypeExist(t); } uint32_t FindType(uint32_t comp, uint8_t level) const { return m_params.FindType(comp, level); } FeatureParams::Types const & GetTypes() const { return m_params.m_types; } @@ -168,10 +170,10 @@ public: void SerializeBase(Buffer & data, serial::GeometryCodingParams const & params, bool saveAddInfo) const; - bool PreSerializeAndRemoveUselessNamesForTmpMwm(); - void SerializeForTmpMwm(Buffer & data) const; - void SerializeBorderForTmpMwm(serial::GeometryCodingParams const & params, Buffer & data) const; - void DeserializeForTmpMwm(Buffer & data); + bool PreSerializeAndRemoveUselessNamesForIntermediate(); + void SerializeForIntermediate(Buffer & data) const; + void SerializeBorderForIntermediate(serial::GeometryCodingParams const & params, Buffer & data) const; + void DeserializeFromIntermediate(Buffer & data); bool PreSerializeAndRemoveUselessNamesForMwm(SupportingData const & data); void SerializeLocalityObject(serial::GeometryCodingParams const & params, @@ -181,7 +183,7 @@ public: // Get common parameters of feature. TypesHolder GetTypesHolder() const; - // TO work with osm ids. + // To work with osm ids. void AddOsmId(base::GeoObjectId id); void SetOsmId(base::GeoObjectId id); base::GeoObjectId GetFirstOsmId() const; @@ -196,8 +198,9 @@ public: void SetCoastCell(int64_t iCell) { m_coastCell = iCell; } bool IsCoastCell() const { return (m_coastCell != -1); } -protected: - template class ToDoWrapper + protected: + template + class ToDoWrapper { public: ToDoWrapper(ToDo && toDo) : m_toDo(std::forward(toDo)) {} @@ -226,27 +229,27 @@ void Check(FeatureBuilder const fb); std::string DebugPrint(FeatureBuilder const & fb); // Read feature from feature source. -template -void ReadFromSourceRawFormat(TSource & src, FeatureBuilder & fb) +template +void ReadFromSourceRawFormat(Source & src, FeatureBuilder & fb) { uint32_t const sz = ReadVarUint(src); typename FeatureBuilder::Buffer buffer(sz); src.Read(&buffer[0], sz); - fb.DeserializeForTmpMwm(buffer); + fb.DeserializeFromIntermediate(buffer); } // Process features in .dat file. template -void ForEachFromDatRawFormat(std::string const & fName, ToDo && toDo) +void ForEachFromDatRawFormat(std::string const & filename, ToDo && toDo) { - FileReader reader(fName); + FileReader reader(filename); ReaderSource src(reader); uint64_t currPos = 0; - uint64_t const fSize = reader.Size(); + uint64_t const fileSize = reader.Size(); // read features one by one - while (currPos < fSize) + while (currPos < fileSize) { FeatureBuilder fb; ReadFromSourceRawFormat(src, fb); diff --git a/generator/feature_generator.cpp b/generator/feature_generator.cpp index be8e1954b5..ea8881e396 100644 --- a/generator/feature_generator.cpp +++ b/generator/feature_generator.cpp @@ -95,7 +95,7 @@ uint32_t FeaturesCollector::WriteFeatureBase(std::vector const & bytes, Fe uint32_t FeaturesCollector::Collect(FeatureBuilder const & fb) { FeatureBuilder::Buffer bytes; - fb.SerializeForTmpMwm(bytes); + fb.SerializeForIntermediate(bytes); uint32_t const featureId = WriteFeatureBase(bytes, fb); CHECK_LESS(0, m_featureID, ()); return featureId; diff --git a/generator/feature_maker.cpp b/generator/feature_maker.cpp index fc0972c9dd..076f9230c2 100644 --- a/generator/feature_maker.cpp +++ b/generator/feature_maker.cpp @@ -54,7 +54,7 @@ bool FeatureMakerSimple::BuildFromWay(OsmElement & p, FeatureParams const & para { HolesProcessor processor(p.m_id, m_cache); m_cache.ForEachRelationByWay(p.m_id, processor); - fb.AddHoles(processor.GetHoles()); + fb.SetHoles(processor.GetHoles()); fb.SetArea(); } else @@ -86,7 +86,7 @@ bool FeatureMakerSimple::BuildFromRelation(OsmElement & p, FeatureParams const & if (!fb.IsGeometryClosed()) return; - fb.AddHoles(holesGeometry); + fb.SetHoles(holesGeometry); fb.SetParams(params); fb.SetArea(); m_queue.push(std::move(fb)); diff --git a/generator/feature_processing_layers.cpp b/generator/feature_processing_layers.cpp index 74079588a2..0c8aaac5e2 100644 --- a/generator/feature_processing_layers.cpp +++ b/generator/feature_processing_layers.cpp @@ -183,7 +183,7 @@ void PrepareFeatureLayer::Handle(FeatureBuilder & feature) auto & params = feature.GetParams(); feature::RemoveUselessTypes(params.m_types, type); - feature.PreSerializeAndRemoveUselessNamesForTmpMwm(); + feature.PreSerializeAndRemoveUselessNamesForIntermediate(); FixLandType(feature); LayerBase::Handle(feature); } @@ -292,7 +292,7 @@ void PrepareCoastlineFeatureLayer::Handle(FeatureBuilder & feature) feature::RemoveUselessTypes(params.m_types, feature.GetGeomType()); } - feature.PreSerializeAndRemoveUselessNamesForTmpMwm(); + feature.PreSerializeAndRemoveUselessNamesForIntermediate(); LayerBase::Handle(feature); } diff --git a/generator/generator_tests/feature_builder_test.cpp b/generator/generator_tests/feature_builder_test.cpp index 6b29ec8885..f661fbc5d2 100644 --- a/generator/generator_tests/feature_builder_test.cpp +++ b/generator/generator_tests/feature_builder_test.cpp @@ -54,11 +54,11 @@ UNIT_CLASS_TEST(TestWithClassificator, FBuilder_ManyTypes) Check(fb1); FeatureBuilder::Buffer buffer; - TEST(fb1.PreSerializeAndRemoveUselessNamesForTmpMwm(), ()); - fb1.SerializeForTmpMwm(buffer); + TEST(fb1.PreSerializeAndRemoveUselessNamesForIntermediate(), ()); + fb1.SerializeForIntermediate(buffer); FeatureBuilder fb2; - fb2.DeserializeForTmpMwm(buffer); + fb2.DeserializeFromIntermediate(buffer); Check(fb2); TEST_EQUAL(fb1, fb2, ()); @@ -90,11 +90,11 @@ UNIT_CLASS_TEST(TestWithClassificator, FBuilder_LineTypes) Check(fb1); FeatureBuilder::Buffer buffer; - TEST(fb1.PreSerializeAndRemoveUselessNamesForTmpMwm(), ()); - fb1.SerializeForTmpMwm(buffer); + TEST(fb1.PreSerializeAndRemoveUselessNamesForIntermediate(), ()); + fb1.SerializeForIntermediate(buffer); FeatureBuilder fb2; - fb2.DeserializeForTmpMwm(buffer); + fb2.DeserializeFromIntermediate(buffer); Check(fb2); TEST_EQUAL(fb1, fb2, ()); @@ -117,11 +117,11 @@ UNIT_CLASS_TEST(TestWithClassificator, FBuilder_Waterfall) Check(fb1); FeatureBuilder::Buffer buffer; - TEST(fb1.PreSerializeAndRemoveUselessNamesForTmpMwm(), ()); - fb1.SerializeForTmpMwm(buffer); + TEST(fb1.PreSerializeAndRemoveUselessNamesForIntermediate(), ()); + fb1.SerializeForIntermediate(buffer); FeatureBuilder fb2; - fb2.DeserializeForTmpMwm(buffer); + fb2.DeserializeFromIntermediate(buffer); Check(fb2); TEST_EQUAL(fb1, fb2, ()); @@ -233,7 +233,7 @@ UNIT_CLASS_TEST(TestWithClassificator, FeatureParams_Parsing) UNIT_CLASS_TEST(TestWithClassificator, FeatureBuilder12_SerializeLocalityObjectForBuildingPoint) { - FeatureBuilder1 fb1; + FeatureBuilder fb; FeatureParams params; char const * arr1[][1] = { @@ -246,23 +246,21 @@ UNIT_CLASS_TEST(TestWithClassificator, FeatureBuilder12_SerializeLocalityObjectF params.AddHouseName("Best House"); params.AddName("default", "Name"); - fb1.AddOsmId(base::MakeOsmNode(1)); - fb1.SetParams(params); - fb1.SetCenter(m2::PointD(10.1, 15.8)); + fb.AddOsmId(base::MakeOsmNode(1)); + fb.SetParams(params); + fb.SetCenter(m2::PointD(10.1, 15.8)); - TEST(fb1.RemoveInvalidTypes(), ()); - TEST(fb1.CheckValid(), ()); - - auto & fb2 = static_cast(fb1); + TEST(fb.RemoveInvalidTypes(), ()); + Check(fb); feature::DataHeader header; header.SetGeometryCodingParams(serial::GeometryCodingParams()); header.SetScales({scales::GetUpperScale()}); - feature::GeometryHolder holder(fb2, header, std::numeric_limits::max() /* maxTrianglesNumber */); + feature::GeometryHolder holder(fb, header, std::numeric_limits::max() /* maxTrianglesNumber */); auto & buffer = holder.GetBuffer(); - TEST(fb2.PreSerializeAndRemoveUselessNames(buffer), ()); - fb2.SerializeLocalityObject(serial::GeometryCodingParams(), buffer); + TEST(fb.PreSerializeAndRemoveUselessNamesForMwm(buffer), ()); + fb.SerializeLocalityObject(serial::GeometryCodingParams(), buffer); using indexer::LocalityObject; LocalityObject object; diff --git a/generator/generator_tests/regions_tests.cpp b/generator/generator_tests/regions_tests.cpp index 776e2c10ee..49d559040c 100644 --- a/generator/generator_tests/regions_tests.cpp +++ b/generator/generator_tests/regions_tests.cpp @@ -64,96 +64,96 @@ RegionsBuilder::Regions MakeTestDataSet1(RegionInfo & collector) { RegionsBuilder::Regions regions; { - FeatureBuilder fb1; - fb1.AddName("default", "Country_1"); - fb1.SetOsmId(MakeOsmRelation(1 /* id */)); + FeatureBuilder fb; + fb.AddName("default", "Country_1"); + fb.SetOsmId(MakeOsmRelation(1 /* id */)); vector poly = {{2, 8}, {3, 12}, {8, 15}, {13, 12}, {15, 7}, {11, 2}, {4, 4}, {2, 8}}; - fb1.AddPolygon(poly); - fb1.AddHoles({{{5, 8}, {7, 10}, {10, 10}, {11, 7}, {10, 4}, {7, 5}, {5, 8}}}); - fb1.SetArea(); - regions.emplace_back(Region(fb1, collector.Get(MakeOsmRelation(1 /* id */)))); + fb.AddPolygon(poly); + fb.SetHoles({{{5, 8}, {7, 10}, {10, 10}, {11, 7}, {10, 4}, {7, 5}, {5, 8}}}); + fb.SetArea(); + regions.emplace_back(Region(fb, collector.Get(MakeOsmRelation(1 /* id */)))); } { - FeatureBuilder fb1; - fb1.AddName("default", "Country_2"); - fb1.SetOsmId(MakeOsmRelation(2 /* id */)); + FeatureBuilder fb; + fb.AddName("default", "Country_2"); + fb.SetOsmId(MakeOsmRelation(2 /* id */)); vector poly = {{5, 8}, {7, 10}, {10, 10}, {11, 7}, {10, 4}, {7, 5}, {5, 8}}; - fb1.AddPolygon(poly); - fb1.SetArea(); - regions.emplace_back(Region(fb1, collector.Get(MakeOsmRelation(2 /* id */)))); + fb.AddPolygon(poly); + fb.SetArea(); + regions.emplace_back(Region(fb, collector.Get(MakeOsmRelation(2 /* id */)))); } { - FeatureBuilder fb1; - fb1.AddName("default", "Country_2"); - fb1.SetOsmId(MakeOsmRelation(2 /* id */)); + FeatureBuilder fb; + fb.AddName("default", "Country_2"); + fb.SetOsmId(MakeOsmRelation(2 /* id */)); vector poly = {{0, 0}, {0, 2}, {2, 2}, {2, 0}, {0, 0}}; - fb1.AddPolygon(poly); - fb1.SetArea(); - regions.emplace_back(Region(fb1, collector.Get(MakeOsmRelation(2 /* id */)))); + fb.AddPolygon(poly); + fb.SetArea(); + regions.emplace_back(Region(fb, collector.Get(MakeOsmRelation(2 /* id */)))); } { - FeatureBuilder fb1; - fb1.AddName("default", "Country_1_Region_3"); - fb1.SetOsmId(MakeOsmRelation(3 /* id */)); + FeatureBuilder fb; + fb.AddName("default", "Country_1_Region_3"); + fb.SetOsmId(MakeOsmRelation(3 /* id */)); vector poly = {{4, 4}, {7, 5}, {10, 4}, {12, 9}, {15, 7}, {11, 2}, {4, 4}}; - fb1.AddPolygon(poly); - fb1.SetArea(); - regions.emplace_back(Region(fb1, collector.Get(MakeOsmRelation(3 /* id */)))); + fb.AddPolygon(poly); + fb.SetArea(); + regions.emplace_back(Region(fb, collector.Get(MakeOsmRelation(3 /* id */)))); } { - FeatureBuilder fb1; - fb1.AddName("default", "Country_1_Region_4"); - fb1.SetOsmId(MakeOsmRelation(4 /* id */)); + FeatureBuilder fb; + fb.AddName("default", "Country_1_Region_4"); + fb.SetOsmId(MakeOsmRelation(4 /* id */)); vector poly = {{7, 10}, {9, 12}, {8, 15}, {13, 12}, {15, 7}, {12, 9}, {11, 7}, {10, 10}, {7, 10}}; - fb1.AddPolygon(poly); - fb1.SetArea(); - regions.emplace_back(Region(fb1, collector.Get(MakeOsmRelation(4 /* id */)))); + fb.AddPolygon(poly); + fb.SetArea(); + regions.emplace_back(Region(fb, collector.Get(MakeOsmRelation(4 /* id */)))); } { - FeatureBuilder fb1; - fb1.AddName("default", "Country_1_Region_5"); - fb1.SetOsmId(MakeOsmRelation(5 /* id */)); + FeatureBuilder fb; + fb.AddName("default", "Country_1_Region_5"); + fb.SetOsmId(MakeOsmRelation(5 /* id */)); vector poly = {{4, 4}, {2, 8}, {3, 12}, {8, 15}, {9, 12}, {7, 10}, {5, 8}, {7, 5}, {4, 4}}; - fb1.AddPolygon(poly); - fb1.SetArea(); - regions.emplace_back(Region(fb1, collector.Get(MakeOsmRelation(5 /* id */)))); + fb.AddPolygon(poly); + fb.SetArea(); + regions.emplace_back(Region(fb, collector.Get(MakeOsmRelation(5 /* id */)))); } { - FeatureBuilder fb1; - fb1.AddName("default", "Country_1_Region_5_Subregion_6"); - fb1.SetOsmId(MakeOsmRelation(6 /* id */)); + FeatureBuilder fb; + fb.AddName("default", "Country_1_Region_5_Subregion_6"); + fb.SetOsmId(MakeOsmRelation(6 /* id */)); vector poly = {{4, 4}, {2, 8}, {3, 12}, {4, 10}, {5, 10}, {5, 8}, {7, 5}, {4, 4}}; - fb1.AddPolygon(poly); - fb1.SetArea(); - regions.emplace_back(Region(fb1, collector.Get(MakeOsmRelation(6 /* id */)))); + fb.AddPolygon(poly); + fb.SetArea(); + regions.emplace_back(Region(fb, collector.Get(MakeOsmRelation(6 /* id */)))); } { - FeatureBuilder fb1; - fb1.AddName("default", "Country_1_Region_5_Subregion_7"); - fb1.SetOsmId(MakeOsmRelation(7 /* id */)); + FeatureBuilder fb; + fb.AddName("default", "Country_1_Region_5_Subregion_7"); + fb.SetOsmId(MakeOsmRelation(7 /* id */)); vector poly = {{3, 12}, {8, 15}, {9, 12}, {7, 10}, {5, 8}, {5, 10}, {4, 10}, {3, 12}}; - fb1.AddPolygon(poly); - fb1.SetArea(); - regions.emplace_back(Region(fb1, collector.Get(MakeOsmRelation(7 /* id */)))); + fb.AddPolygon(poly); + fb.SetArea(); + regions.emplace_back(Region(fb, collector.Get(MakeOsmRelation(7 /* id */)))); } { - FeatureBuilder fb1; - fb1.AddName("default", "Country_2_Region_8"); - fb1.SetOsmId(MakeOsmRelation(8 /* id */)); + FeatureBuilder fb; + fb.AddName("default", "Country_2_Region_8"); + fb.SetOsmId(MakeOsmRelation(8 /* id */)); vector poly = {{0, 0}, {0, 1}, {1, 1}, {1, 0}, {0, 0}}; - fb1.AddPolygon(poly); - fb1.SetArea(); - regions.emplace_back(Region(fb1, collector.Get(MakeOsmRelation(8 /* id */)))); + fb.AddPolygon(poly); + fb.SetArea(); + regions.emplace_back(Region(fb, collector.Get(MakeOsmRelation(8 /* id */)))); } return regions; diff --git a/generator/generator_tests_support/test_mwm_builder.cpp b/generator/generator_tests_support/test_mwm_builder.cpp index b014279e02..5268acdb69 100644 --- a/generator/generator_tests_support/test_mwm_builder.cpp +++ b/generator/generator_tests_support/test_mwm_builder.cpp @@ -93,7 +93,7 @@ bool TestMwmBuilder::Add(FeatureBuilder & fb) fb.SetCenter(center); } - if (!fb.PreSerializeAndRemoveUselessNamesForTmpMwm()) + if (!fb.PreSerializeAndRemoveUselessNamesForIntermediate()) { LOG(LWARNING, ("Can't pre-serialize feature.")); return false; diff --git a/generator/locality_sorter.cpp b/generator/locality_sorter.cpp index 995a239c9a..553bd64209 100644 --- a/generator/locality_sorter.cpp +++ b/generator/locality_sorter.cpp @@ -49,7 +49,7 @@ public: if (fb.IsArea()) { FeatureBuilder::Buffer buffer; - fb.SerializeBorderForTmpMwm(serial::GeometryCodingParams(), buffer); + fb.SerializeBorderForIntermediate(serial::GeometryCodingParams(), buffer); WriteFeatureBase(buffer, fb); } return 0;