From f6e0a0a686f95b2a3589d5b29c8a1dc2c5509698 Mon Sep 17 00:00:00 2001 From: Viktor Govako Date: Wed, 10 May 2023 14:37:24 -0300 Subject: [PATCH 1/6] Removed dummy HighwayClass::Error. Signed-off-by: Viktor Govako --- indexer/ftypes_matcher.cpp | 12 ++++++------ indexer/ftypes_matcher.hpp | 1 - indexer/indexer_tests/checker_test.cpp | 2 +- .../openlr_assessment_tool/mainwindow.cpp | 4 +++- openlr/score_paths_connector.cpp | 2 ++ routing/directions_engine.cpp | 6 ++---- 6 files changed, 14 insertions(+), 13 deletions(-) diff --git a/indexer/ftypes_matcher.cpp b/indexer/ftypes_matcher.cpp index e357993173..e3548f2ee4 100644 --- a/indexer/ftypes_matcher.cpp +++ b/indexer/ftypes_matcher.cpp @@ -31,6 +31,8 @@ public: auto const & c = classif(); m_map[c.GetTypeByPath({"route", "ferry"})] = ftypes::HighwayClass::Transported; m_map[c.GetTypeByPath({"route", "shuttle_train"})] = ftypes::HighwayClass::Transported; + /// @todo Wow, actual highway type is railway-rail-motor_vehicle. + /// Should be carefull with GetHighwayClass function. m_map[c.GetTypeByPath({"railway", "rail"})] = ftypes::HighwayClass::Transported; m_map[c.GetTypeByPath({"highway", "motorway"})] = ftypes::HighwayClass::Trunk; @@ -70,7 +72,7 @@ public: { auto const it = m_map.find(t); if (it == m_map.cend()) - return ftypes::HighwayClass::Error; + return ftypes::HighwayClass::Undefined; return it->second; } }; @@ -80,7 +82,6 @@ char const * HighwayClassToString(ftypes::HighwayClass const cls) switch (cls) { case ftypes::HighwayClass::Undefined: return "Undefined"; - case ftypes::HighwayClass::Error: return "Error"; case ftypes::HighwayClass::Transported: return "Transported"; case ftypes::HighwayClass::Trunk: return "Trunk"; case ftypes::HighwayClass::Primary: return "Primary"; @@ -120,18 +121,17 @@ string DebugPrint(LocalityType const localityType) HighwayClass GetHighwayClass(feature::TypesHolder const & types) { - uint8_t constexpr kTruncLevel = 2; static HighwayClasses highwayClasses; for (auto t : types) { - ftype::TruncValue(t, kTruncLevel); + ftype::TruncValue(t, 2); HighwayClass const hc = highwayClasses.Get(t); - if (hc != HighwayClass::Error) + if (hc != HighwayClass::Undefined) return hc; } - return HighwayClass::Error; + return HighwayClass::Undefined; } uint32_t BaseChecker::PrepareToMatch(uint32_t type, uint8_t level) diff --git a/indexer/ftypes_matcher.hpp b/indexer/ftypes_matcher.hpp index 7f53f0f070..a8d1b7817b 100644 --- a/indexer/ftypes_matcher.hpp +++ b/indexer/ftypes_matcher.hpp @@ -622,7 +622,6 @@ bool IsTypeConformed(uint32_t type, base::StringIL const & path); enum class HighwayClass { Undefined = 0, // There has not been any attempt of calculating HighwayClass. - Error, // There was an attempt of calculating HighwayClass but it was not successful. Trunk, Primary, Secondary, diff --git a/indexer/indexer_tests/checker_test.cpp b/indexer/indexer_tests/checker_test.cpp index f720f40eed..c7a4fc54ff 100644 --- a/indexer/indexer_tests/checker_test.cpp +++ b/indexer/indexer_tests/checker_test.cpp @@ -177,7 +177,7 @@ UNIT_TEST(GetHighwayClassTest) feature::TypesHolder types4; types4.Add(c.GetTypeByPath({"highway"})); - TEST_EQUAL(ftypes::GetHighwayClass(types4), ftypes::HighwayClass::Error, ()); + TEST_EQUAL(ftypes::GetHighwayClass(types4), ftypes::HighwayClass::Undefined, ()); } UNIT_TEST(IsPoiChecker) diff --git a/openlr/openlr_match_quality/openlr_assessment_tool/mainwindow.cpp b/openlr/openlr_match_quality/openlr_assessment_tool/mainwindow.cpp index 8f4f4cc24d..c81be4ba86 100644 --- a/openlr/openlr_match_quality/openlr_assessment_tool/mainwindow.cpp +++ b/openlr/openlr_match_quality/openlr_assessment_tool/mainwindow.cpp @@ -152,8 +152,10 @@ public: { if (ft.GetGeomType() != feature::GeomType::Line) return; + + /// @todo Transported (railway=rail) are also present here :) auto const roadClass = ftypes::GetHighwayClass(feature::TypesHolder(ft)); - if (roadClass == ftypes::HighwayClass::Error || + if (roadClass == ftypes::HighwayClass::Undefined || roadClass == ftypes::HighwayClass::Pedestrian) { return; diff --git a/openlr/score_paths_connector.cpp b/openlr/score_paths_connector.cpp index 8fdd52c0af..5cd76f7e3b 100644 --- a/openlr/score_paths_connector.cpp +++ b/openlr/score_paths_connector.cpp @@ -46,6 +46,8 @@ public: } else { + CHECK(hwClass != ftypes::HighwayClass::Undefined, (edge)); + m_minHwClass = static_cast( min(base::Underlying(m_minHwClass), base::Underlying(hwClass))); m_maxHwClass = static_cast( diff --git a/routing/directions_engine.cpp b/routing/directions_engine.cpp index 3deae2df81..d94beaac15 100644 --- a/routing/directions_engine.cpp +++ b/routing/directions_engine.cpp @@ -78,8 +78,7 @@ void DirectionsEngine::LoadPathAttributes(FeatureID const & featureId, LoadLanes(pathSegment, *ft, isForward); pathSegment.m_highwayClass = GetHighwayClass(types); - ASSERT_NOT_EQUAL(pathSegment.m_highwayClass, HighwayClass::Error, ()); - ASSERT_NOT_EQUAL(pathSegment.m_highwayClass, HighwayClass::Undefined, ()); + ASSERT(pathSegment.m_highwayClass != HighwayClass::Undefined, (featureId)); pathSegment.m_isLink = m_linkChecker(types); pathSegment.m_onRoundabout = m_roundAboutChecker(types); @@ -120,8 +119,7 @@ void DirectionsEngine::GetSegmentRangeAndAdjacentEdges(IRoadGraph::EdgeListT con feature::TypesHolder types(*ft); auto const highwayClass = GetHighwayClass(types); - ASSERT_NOT_EQUAL(highwayClass, HighwayClass::Error, (edge.PrintLatLon())); - ASSERT_NOT_EQUAL(highwayClass, HighwayClass::Undefined, (edge.PrintLatLon())); + ASSERT(highwayClass != HighwayClass::Undefined, (edge.PrintLatLon())); double angle = 0; -- 2.45.3 From 5a80072dcddd94a6fdfe245bf290c7545e73ee6a Mon Sep 17 00:00:00 2001 From: Viktor Govako Date: Wed, 10 May 2023 21:21:40 -0300 Subject: [PATCH 2/6] Refactor ftypes::IsBridgeOrTunnelChecker. Signed-off-by: Viktor Govako --- drape_frontend/rule_drawer.cpp | 2 +- indexer/classificator.cpp | 18 ++---- indexer/classificator.hpp | 3 +- indexer/ftypes_matcher.cpp | 40 ++++--------- indexer/ftypes_matcher.hpp | 21 +------ indexer/indexer_tests/checker_test.cpp | 79 +++++--------------------- indexer/indexer_tests/test_type.cpp | 19 ++----- 7 files changed, 40 insertions(+), 142 deletions(-) diff --git a/drape_frontend/rule_drawer.cpp b/drape_frontend/rule_drawer.cpp index 74b907ceb3..a44366c093 100644 --- a/drape_frontend/rule_drawer.cpp +++ b/drape_frontend/rule_drawer.cpp @@ -263,7 +263,7 @@ void RuleDrawer::ProcessAreaStyle(FeatureType & f, Stylist const & s, // Looks like nonsense, but there are some osm objects with types // highway-path-bridge and building (sic!) at the same time (pedestrian crossing). isBuilding = (isPart || IsBuildingChecker::Instance()(types)) && - !IsBridgeChecker::Instance()(types) && !IsTunnelChecker::Instance()(types); + !IsBridgeOrTunnelChecker::Instance()(types); isBuildingOutline = isBuilding && hasParts && !isPart; is3dBuilding = m_context->Is3dBuildingsEnabled() && (isBuilding && !isBuildingOutline); diff --git a/indexer/classificator.cpp b/indexer/classificator.cpp index 221d39688a..8b93f8ed1d 100644 --- a/indexer/classificator.cpp +++ b/indexer/classificator.cpp @@ -179,14 +179,9 @@ namespace ftype set_value(type, cl+1, 1); } - bool GetValue(uint32_t type, uint8_t level, uint8_t & value) + uint8_t GetValue(uint32_t type, uint8_t level) { - if (level < get_control_level(type)) - { - value = get_value(type, level); - return true; - } - return false; + return get_value(type, level); } void PopValue(uint32_t & type) @@ -420,13 +415,10 @@ void Classificator::Clear() template void Classificator::ForEachPathObject(uint32_t type, ToDo && toDo) const { ClassifObject const * p = &m_root; - uint8_t i = 0; - - uint8_t v; - while (ftype::GetValue(type, i, v)) + uint8_t const level = ftype::GetLevel(type); + for (uint8_t i = 0; i < level; ++i) { - ++i; - p = p->GetObject(v); + p = p->GetObject(ftype::GetValue(type, i)); toDo(p); } } diff --git a/indexer/classificator.hpp b/indexer/classificator.hpp index c0cd39f23a..d72787451c 100644 --- a/indexer/classificator.hpp +++ b/indexer/classificator.hpp @@ -22,7 +22,8 @@ namespace ftype inline uint32_t GetEmptyValue() { return 1; } void PushValue(uint32_t & type, uint8_t value); - bool GetValue(uint32_t type, uint8_t level, uint8_t & value); + /// @pre level < GetLevel(type). + uint8_t GetValue(uint32_t type, uint8_t level); void PopValue(uint32_t & type); void TruncValue(uint32_t & type, uint8_t level); uint8_t GetLevel(uint32_t type); diff --git a/indexer/ftypes_matcher.cpp b/indexer/ftypes_matcher.cpp index e3548f2ee4..8c065b6990 100644 --- a/indexer/ftypes_matcher.cpp +++ b/indexer/ftypes_matcher.cpp @@ -564,18 +564,20 @@ IsPlaceChecker::IsPlaceChecker() : BaseChecker(1 /* level */) m_types.push_back(classif().GetTypeByPath({"place"})); } -IsBridgeChecker::IsBridgeChecker() : BaseChecker(3 /* level */) {} +IsBridgeOrTunnelChecker::IsBridgeOrTunnelChecker() : BaseChecker(3 /* level */) {} -bool IsBridgeChecker::IsMatched(uint32_t type) const +bool IsBridgeOrTunnelChecker::IsMatched(uint32_t type) const { - return IsTypeConformed(type, {"highway", "*", "bridge"}); -} + if (ftype::GetLevel(type) != 3) + return false; -IsTunnelChecker::IsTunnelChecker() : BaseChecker(3 /* level */) {} + ClassifObject const * p = classif().GetRoot()->GetObject(ftype::GetValue(type, 0)); + if (p->GetName() != "highway") + return false; -bool IsTunnelChecker::IsMatched(uint32_t type) const -{ - return IsTypeConformed(type, {"highway", "*", "tunnel"}); + p = p->GetObject(ftype::GetValue(type, 1)); + p = p->GetObject(ftype::GetValue(type, 2)); + return (p->GetName() == "bridge" || p->GetName() == "tunnel"); } IsHotelChecker::IsHotelChecker() @@ -948,26 +950,4 @@ uint64_t GetPopulationByRadius(double r) return base::SignedRound(pow(r / 550.0, 3.6)); } -bool IsTypeConformed(uint32_t type, base::StringIL const & path) -{ - ClassifObject const * p = classif().GetRoot(); - ASSERT(p, ()); - - uint8_t val = 0, i = 0; - for (char const * s : path) - { - if (!ftype::GetValue(type, i, val)) - return false; - - p = p->GetObject(val); - if (p == 0) - return false; - - if (p->GetName() != s && strcmp(s, "*") != 0) - return false; - - ++i; - } - return true; -} } // namespace ftypes diff --git a/indexer/ftypes_matcher.hpp b/indexer/ftypes_matcher.hpp index a8d1b7817b..65eb507c9f 100644 --- a/indexer/ftypes_matcher.hpp +++ b/indexer/ftypes_matcher.hpp @@ -336,22 +336,13 @@ public: DECLARE_CHECKER_INSTANCE(IsPlaceChecker); }; -class IsBridgeChecker : public BaseChecker +class IsBridgeOrTunnelChecker : public BaseChecker { virtual bool IsMatched(uint32_t type) const override; - IsBridgeChecker(); + IsBridgeOrTunnelChecker(); public: - DECLARE_CHECKER_INSTANCE(IsBridgeChecker); -}; - -class IsTunnelChecker : public BaseChecker -{ - virtual bool IsMatched(uint32_t type) const override; - - IsTunnelChecker(); -public: - DECLARE_CHECKER_INSTANCE(IsTunnelChecker); + DECLARE_CHECKER_INSTANCE(IsBridgeOrTunnelChecker); }; class IsIslandChecker : public BaseChecker @@ -611,12 +602,6 @@ double GetRadiusByPopulationForRouting(uint64_t p, LocalityType localityType); uint64_t GetPopulationByRadius(double r); //@} -/// Check if type conforms the path. Strings in the path can be -/// feature types like "highway", "living_street", "bridge" and so on -/// or *. * means any class. -/// The root name ("world") is ignored -bool IsTypeConformed(uint32_t type, base::StringIL const & path); - // Highway class. The order is important. // The enum values follow from the biggest roads (Trunk) to the smallest ones (Service). enum class HighwayClass diff --git a/indexer/indexer_tests/checker_test.cpp b/indexer/indexer_tests/checker_test.cpp index c7a4fc54ff..304b605fd9 100644 --- a/indexer/indexer_tests/checker_test.cpp +++ b/indexer/indexer_tests/checker_test.cpp @@ -2,15 +2,9 @@ #include "indexer/classificator.hpp" #include "indexer/classificator_loader.hpp" -#include "indexer/data_source.hpp" #include "indexer/ftypes_matcher.hpp" -#include "base/logging.hpp" - -#include -#include #include -#include #include namespace checker_test @@ -61,36 +55,6 @@ vector GetLinkTypes() return GetTypes(arr, ARRAY_SIZE(arr)); } -vector GetBridgeTypes() -{ - char const * arr[][roadArrColumnCount] = - { - { "highway", "trunk", "bridge" }, - { "highway", "tertiary", "bridge" } - }; - return GetTypes(arr, ARRAY_SIZE(arr)); -} - -vector GetTunnelTypes() -{ - char const * arr[][roadArrColumnCount] = - { - { "highway", "trunk", "tunnel" }, - { "highway", "motorway_link", "tunnel" } - }; - return GetTypes(arr, ARRAY_SIZE(arr)); -} - -vector GetBridgeAndTunnelTypes() -{ - char const * arr[][roadArrColumnCount] = - { - { "highway", "trunk", "bridge" }, - { "highway", "motorway_link", "tunnel" } - }; - return GetTypes(arr, ARRAY_SIZE(arr)); -} - uint32_t GetMotorwayJunctionType() { Classificator const & c = classif(); @@ -99,23 +63,26 @@ uint32_t GetMotorwayJunctionType() } // namespace -UNIT_TEST(IsTypeConformed) +UNIT_TEST(IsBridgeOrTunnelChecker) { classificator::Load(); + auto const & c = classif(); - char const * arr[][roadArrColumnCount] = + base::StringIL arrYes[] = { {"highway", "trunk", "bridge"}, - {"highway", "motorway_link", "tunnel"} + {"highway", "motorway_link", "tunnel"}, }; - vector types = GetTypes(arr, ARRAY_SIZE(arr)); - TEST(ftypes::IsTypeConformed(types[0], {"highway", "trunk", "bridge"}), ()); - TEST(ftypes::IsTypeConformed(types[0], {"highway", "trunk"}), ()); - TEST(ftypes::IsTypeConformed(types[1], {"highway", "*", "tunnel"}), ()); - TEST(!ftypes::IsTypeConformed(types[0], {"highway", "trunk", "tunnel"}), ()); - TEST(!ftypes::IsTypeConformed(types[0], {"highway", "abcd", "tunnel"}), ()); - TEST(!ftypes::IsTypeConformed(types[1], {"highway", "efgh", "*"}), ()); - TEST(!ftypes::IsTypeConformed(types[1], {"*", "building", "*"}), ()); + for (auto const & e : arrYes) + TEST(ftypes::IsBridgeOrTunnelChecker::Instance()(c.GetTypeByPath(e)), ()); + + base::StringIL arrNo[] = + { + {"highway", "motorway_junction"}, + {"highway", "service", "busway"}, + }; + for (auto const & e : arrNo) + TEST(!ftypes::IsBridgeOrTunnelChecker::Instance()(c.GetTypeByPath(e)), ()); } UNIT_TEST(IsWayChecker) @@ -139,24 +106,6 @@ UNIT_TEST(IsLinkChecker) TEST(!ftypes::IsLinkChecker::Instance()(GetStreetTypes()), ()); } -UNIT_TEST(IsBridgeChecker) -{ - classificator::Load(); - - TEST(ftypes::IsBridgeChecker::Instance()(GetBridgeTypes()), ()); - TEST(ftypes::IsBridgeChecker::Instance()(GetBridgeAndTunnelTypes()), ()); - TEST(!ftypes::IsBridgeChecker::Instance()(GetTunnelTypes()), ()); -} - -UNIT_TEST(IsTunnelChecker) -{ - classificator::Load(); - - TEST(ftypes::IsTunnelChecker::Instance()(GetTunnelTypes()), ()); - TEST(ftypes::IsTunnelChecker::Instance()(GetBridgeAndTunnelTypes()), ()); - TEST(!ftypes::IsTunnelChecker::Instance()(GetBridgeTypes()), ()); -} - UNIT_TEST(GetHighwayClassTest) { classificator::Load(); diff --git a/indexer/indexer_tests/test_type.cpp b/indexer/indexer_tests/test_type.cpp index eaa44d1639..b67cdbaf76 100644 --- a/indexer/indexer_tests/test_type.cpp +++ b/indexer/indexer_tests/test_type.cpp @@ -7,30 +7,21 @@ namespace void check_values_array(uint8_t values[], uint8_t count) { uint32_t type = ftype::GetEmptyValue(); - uint8_t value; - bool res = ftype::GetValue(type, 0, value); - TEST_EQUAL(res, false, ()); - res = ftype::GetValue(type, 4, value); - TEST_EQUAL(res, false, ()); for (uint8_t i = 0; i < count; ++i) ftype::PushValue(type, values[i]); for (uint8_t i = 0; i < count; ++i) - { - res = ftype::GetValue(type, i, value); - TEST_EQUAL(res, true, ()); - TEST_EQUAL(value, values[i], (value, values[i])); - } + TEST_EQUAL(ftype::GetValue(type, i), values[i], ()); - for (char i = count-1; i >= 0; --i) + while (count > 0) { + TEST_EQUAL(ftype::GetLevel(type), count, ()); ftype::PopValue(type); - - res = ftype::GetValue(type, i, value); - TEST_EQUAL(res, false, ()); + --count; } + TEST_EQUAL(ftype::GetLevel(type), 0, ()); TEST_EQUAL(type, ftype::GetEmptyValue(), (type)); } } -- 2.45.3 From ae69070db299744c711e5b0d1758e2e86adb0d81 Mon Sep 17 00:00:00 2001 From: Viktor Govako Date: Sun, 14 May 2023 12:27:45 -0300 Subject: [PATCH 3/6] Added base::Unique. Signed-off-by: Viktor Govako --- base/stl_helpers.hpp | 8 +++++++- drape_frontend/gui/gui_text.cpp | 4 +--- indexer/search_string_utils.cpp | 4 ++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/base/stl_helpers.hpp b/base/stl_helpers.hpp index da658080e0..173c512469 100644 --- a/base/stl_helpers.hpp +++ b/base/stl_helpers.hpp @@ -116,11 +116,17 @@ void SortUnique(Cont & c) /// @name Use std::ref to pass functors into std, since all algorithm functions here get forwarding reference. /// @{ +template +void Unique(Cont & c, Equals && equals) +{ + c.erase(std::unique(c.begin(), c.end(), std::ref(equals)), c.end()); +} + template void SortUnique(Cont & c, Less && less, Equals && equals) { std::sort(c.begin(), c.end(), std::ref(less)); - c.erase(std::unique(c.begin(), c.end(), std::ref(equals)), c.end()); + Unique(c, equals); } template diff --git a/drape_frontend/gui/gui_text.cpp b/drape_frontend/gui/gui_text.cpp index 1a929749b9..b7c18c82bd 100644 --- a/drape_frontend/gui/gui_text.cpp +++ b/drape_frontend/gui/gui_text.cpp @@ -306,9 +306,7 @@ ref_ptr MutableLabel::SetAlphabet(std::string const & alphabet, ref_ptr mng) { strings::UniString str = strings::MakeUniString(alphabet + "."); - std::sort(str.begin(), str.end()); - strings::UniString::iterator it = std::unique(str.begin(), str.end()); - str.resize(std::distance(str.begin(), it)); + base::SortUnique(str); dp::TextureManager::TGlyphsBuffer buffer; mng->GetGlyphRegions(str, dp::GlyphManager::kDynamicGlyphSize, buffer); diff --git a/indexer/search_string_utils.cpp b/indexer/search_string_utils.cpp index c55c55ba87..7ded9149e9 100644 --- a/indexer/search_string_utils.cpp +++ b/indexer/search_string_utils.cpp @@ -143,10 +143,10 @@ UniString NormalizeAndSimplifyString(std::string_view s) }); // Replace sequence of spaces with single one. - uniString.erase(std::unique(uniString.begin(), uniString.end(), [](UniChar l, UniChar r) + base::Unique(uniString, [](UniChar l, UniChar r) { return (l == r && l == ' '); - }), uniString.end()); + }); return uniString; -- 2.45.3 From 5d4b790c819d4a171821f1670e9fe28ae01722dc Mon Sep 17 00:00:00 2001 From: Viktor Govako Date: Sun, 14 May 2023 12:39:10 -0300 Subject: [PATCH 4/6] [drape] Avoid duplicating points in DrapeApi::TLines. Signed-off-by: Viktor Govako --- drape/batcher.cpp | 8 ++++---- drape/batcher.hpp | 4 ++-- drape_frontend/drape_api_builder.cpp | 18 ++++++++++++------ drape_frontend/drape_api_builder.hpp | 4 ++-- drape_frontend/gui/gui_text.cpp | 4 ++-- drape_frontend/gui/gui_text.hpp | 4 +--- drape_frontend/message_subclasses.hpp | 2 +- 7 files changed, 24 insertions(+), 20 deletions(-) diff --git a/drape/batcher.cpp b/drape/batcher.cpp index f11cf05472..c34ce65480 100644 --- a/drape/batcher.cpp +++ b/drape/batcher.cpp @@ -199,9 +199,9 @@ IndicesRange Batcher::InsertLineRaw(ref_ptr context, RenderStat 0 /* vertexStride */, indices); } -void Batcher::StartSession(TFlushFn const & flusher) +void Batcher::StartSession(TFlushFn && flusher) { - m_flushInterface = flusher; + m_flushInterface = std::move(flusher); } void Batcher::EndSession(ref_ptr context) @@ -314,11 +314,11 @@ Batcher * BatcherFactory::GetNew() const } SessionGuard::SessionGuard(ref_ptr context, Batcher & batcher, - Batcher::TFlushFn const & flusher) + Batcher::TFlushFn && flusher) : m_context(context) , m_batcher(batcher) { - m_batcher.StartSession(flusher); + m_batcher.StartSession(std::move(flusher)); } SessionGuard::~SessionGuard() diff --git a/drape/batcher.hpp b/drape/batcher.hpp index 0f7f6049f6..7a0c3cc5e2 100644 --- a/drape/batcher.hpp +++ b/drape/batcher.hpp @@ -67,7 +67,7 @@ public: drape_ptr && handle); using TFlushFn = std::function &&)>; - void StartSession(TFlushFn const & flusher); + void StartSession(TFlushFn && flusher); void EndSession(ref_ptr context); void ResetSession(); @@ -121,7 +121,7 @@ class SessionGuard { public: SessionGuard(ref_ptr context, Batcher & batcher, - Batcher::TFlushFn const & flusher); + Batcher::TFlushFn && flusher); ~SessionGuard(); private: diff --git a/drape_frontend/drape_api_builder.cpp b/drape_frontend/drape_api_builder.cpp index d9ecaf80bf..bc4ee0b46d 100644 --- a/drape_frontend/drape_api_builder.cpp +++ b/drape_frontend/drape_api_builder.cpp @@ -34,7 +34,7 @@ void BuildText(ref_ptr context, std::string const & str, namespace df { void DrapeApiBuilder::BuildLines(ref_ptr context, - DrapeApi::TLines const & lines, ref_ptr textures, + DrapeApi::TLines & lines, ref_ptr textures, std::vector> & properties) { properties.reserve(lines.size()); @@ -43,10 +43,16 @@ void DrapeApiBuilder::BuildLines(ref_ptr context, uint32_t constexpr kFontSize = 14; FeatureID fakeFeature; - for (auto const & line : lines) + for (auto & line : lines) { - std::string id = line.first; - DrapeApiLineData const & data = line.second; + std::string const & id = line.first; + + DrapeApiLineData & data = line.second; + base::Unique(data.m_points, [](m2::PointD const & p1, m2::PointD const & p2) + { + return p1.EqualDxDy(p2, kMwmPointAccuracy); + }); + m2::RectD rect; for (auto const & p : data.m_points) rect.Add(p); @@ -56,8 +62,8 @@ void DrapeApiBuilder::BuildLines(ref_ptr context, auto property = make_unique_dp(); property->m_center = rect.Center(); { - dp::SessionGuard guard(context, batcher, [&property, id](dp::RenderState const & state, - drape_ptr && b) + dp::SessionGuard guard(context, batcher, [&property, &id](dp::RenderState const & state, + drape_ptr && b) { property->m_id = id; property->m_buckets.emplace_back(state, std::move(b)); diff --git a/drape_frontend/drape_api_builder.hpp b/drape_frontend/drape_api_builder.hpp index 4de290782e..eac3d477c8 100644 --- a/drape_frontend/drape_api_builder.hpp +++ b/drape_frontend/drape_api_builder.hpp @@ -1,9 +1,9 @@ #pragma once #include "drape_frontend/drape_api.hpp" -#include "drape_frontend/render_state_extension.hpp" #include "drape/render_bucket.hpp" +#include "drape/render_state.hpp" #include "drape/texture_manager.hpp" #include @@ -22,7 +22,7 @@ struct DrapeApiRenderProperty class DrapeApiBuilder { public: - void BuildLines(ref_ptr context, DrapeApi::TLines const & lines, + void BuildLines(ref_ptr context, DrapeApi::TLines & lines, ref_ptr textures, std::vector> & properties); }; diff --git a/drape_frontend/gui/gui_text.cpp b/drape_frontend/gui/gui_text.cpp index b7c18c82bd..99c29ded9e 100644 --- a/drape_frontend/gui/gui_text.cpp +++ b/drape_frontend/gui/gui_text.cpp @@ -547,7 +547,7 @@ void MutableLabelHandle::SetContent(std::string const & content) m2::PointF MutableLabelDrawer::Draw(ref_ptr context, Params const & params, ref_ptr mng, - dp::Batcher::TFlushFn const & flushFn) + dp::Batcher::TFlushFn && flushFn) { uint32_t vertexCount = dp::Batcher::VertexPerQuad * params.m_maxLength; uint32_t indexCount = dp::Batcher::IndexPerQuad * params.m_maxLength; @@ -579,7 +579,7 @@ m2::PointF MutableLabelDrawer::Draw(ref_ptr context, Params { dp::Batcher batcher(indexCount, vertexCount); batcher.SetBatcherHash(static_cast(df::BatcherBucket::Default)); - dp::SessionGuard guard(context, batcher, flushFn); + dp::SessionGuard guard(context, batcher, std::move(flushFn)); batcher.InsertListOfStrip(context, staticData.m_state, make_ref(&provider), std::move(handle), dp::Batcher::VertexPerQuad); } diff --git a/drape_frontend/gui/gui_text.hpp b/drape_frontend/gui/gui_text.hpp index 71680e28d8..c01b34de8d 100644 --- a/drape_frontend/gui/gui_text.hpp +++ b/drape_frontend/gui/gui_text.hpp @@ -1,14 +1,12 @@ #pragma once #include "drape_frontend/gui/shape.hpp" -#include "drape_frontend/render_state_extension.hpp" #include "drape/binding_info.hpp" #include "drape/drape_global.hpp" #include "drape/glsl_types.hpp" #include "drape/texture_manager.hpp" -#include #include #include #include @@ -188,7 +186,7 @@ public: // Return maximum pixel size. static m2::PointF Draw(ref_ptr context, Params const & params, - ref_ptr mng, dp::Batcher::TFlushFn const & flushFn); + ref_ptr mng, dp::Batcher::TFlushFn && flushFn); }; class StaticLabelHandle : public Handle diff --git a/drape_frontend/message_subclasses.hpp b/drape_frontend/message_subclasses.hpp index 059ceff4f4..1620dce98c 100644 --- a/drape_frontend/message_subclasses.hpp +++ b/drape_frontend/message_subclasses.hpp @@ -1198,7 +1198,7 @@ public: Type GetType() const override { return Type::DrapeApiAddLines; } - DrapeApi::TLines const & GetLines() const { return m_lines; } + DrapeApi::TLines & GetLines() { return m_lines; } private: DrapeApi::TLines m_lines; -- 2.45.3 From 4c649aad181b8df4a9e9c9b8b343f0e6d63a0643 Mon Sep 17 00:00:00 2001 From: Viktor Govako Date: Sun, 14 May 2023 13:40:06 -0300 Subject: [PATCH 5/6] [drape] Avoid temporary vector in ClipPathByRect. Signed-off-by: Viktor Govako --- drape_frontend/apply_feature_functors.cpp | 12 ++---- geometry/clipping.cpp | 48 ++++++++++++----------- geometry/clipping.hpp | 5 +-- geometry/spline.cpp | 12 +++--- geometry/spline.hpp | 2 +- 5 files changed, 39 insertions(+), 40 deletions(-) diff --git a/drape_frontend/apply_feature_functors.cpp b/drape_frontend/apply_feature_functors.cpp index bb6c54f5b2..1db172551f 100644 --- a/drape_frontend/apply_feature_functors.cpp +++ b/drape_frontend/apply_feature_functors.cpp @@ -842,16 +842,12 @@ void ApplyLineFeatureGeometry::ProcessLineRule(Stylist::TRuleWrapper const & rul if (clippedPaths.empty()) return; - size_t const kExtraPointsPerSegment = 4; - m2::SmoothPaths(guidePointsForSmooth, kExtraPointsPerSegment, m2::kCentripetalAlpha, - clippedPaths); + m2::SmoothPaths(guidePointsForSmooth, 4 /* newPointsPerSegmentCount */, m2::kCentripetalAlpha, clippedPaths); m_clippedSplines.clear(); - for (auto const & path : clippedPaths) - { - auto splines = m2::ClipPathByRect(m_tileRect, path); - std::move(splines.begin(), splines.end(), std::back_inserter(m_clippedSplines)); - } + std::function inserter = base::MakeBackInsertFunctor(m_clippedSplines); + for (auto & path : clippedPaths) + m2::ClipPathByRect(m_tileRect, std::move(path), inserter); } if (m_clippedSplines.empty()) diff --git a/geometry/clipping.cpp b/geometry/clipping.cpp index 00a7a590dc..558e6d0b7d 100644 --- a/geometry/clipping.cpp +++ b/geometry/clipping.cpp @@ -1,6 +1,9 @@ #include "geometry/clipping.hpp" #include "geometry/rect_intersect.hpp" +#include "geometry/triangle2d.hpp" + +#include "base/stl_helpers.hpp" #include @@ -131,49 +134,46 @@ void ClipTriangleByRect(m2::RectD const & rect, m2::PointD const & p1, m2::Point resultIterator(polygon[0], polygon[i + 1], polygon[i + 2]); } -std::vector ClipPathByRectImpl(m2::RectD const & rect, - std::vector const & path) +template +void ClipPathByRectImpl(m2::RectD const & rect, std::vector const & path, FnT && fn) { - std::vector result; - - if (path.size() < 2) - return result; + size_t const sz = path.size(); + if (sz < 2) + return; // Divide spline into parts. - result.reserve(2); m2::PointD p1, p2; int code1 = 0; int code2 = 0; m2::SharedSpline s; - s.Reset(new m2::Spline(path.size())); + s.Reset(new m2::Spline(sz)); - for (size_t i = 0; i < path.size() - 1; i++) + for (size_t i = 0; i < sz - 1; i++) { p1 = path[i]; p2 = path[i + 1]; if (m2::Intersect(rect, p1, p2, code1, code2)) { if (s.IsNull()) - s.Reset(new m2::Spline(path.size() - i)); + s.Reset(new m2::Spline(sz - i)); s->AddPoint(p1); s->AddPoint(p2); - if (code2 != 0 || i + 2 == path.size()) + if (code2 != 0 || i + 2 == sz) { if (s->GetSize() > 1) - result.push_back(s); + fn(std::move(s)); s.Reset(nullptr); } } else if (!s.IsNull() && !s->IsEmpty()) { if (s->GetSize() > 1) - result.push_back(s); + fn(std::move(s)); s.Reset(nullptr); } } - return result; } enum class RectCase @@ -204,23 +204,27 @@ std::vector ClipSplineByRect(m2::RectD const & rect, m2::Share { case RectCase::Inside: return {spline}; case RectCase::Outside: return {}; - case RectCase::Intersect: return ClipPathByRectImpl(rect, spline->GetPath()); + case RectCase::Intersect: + { + std::vector res; + res.reserve(2); // keep previous behavior, but not sure that its actually needed + ClipPathByRectImpl(rect, spline->GetPath(), base::MakeBackInsertFunctor(res)); + return res; + } } CHECK(false, ("Unreachable")); return {}; } -std::vector ClipPathByRect(m2::RectD const & rect, - std::vector const & path) +void ClipPathByRect(m2::RectD const & rect, std::vector && path, + std::function const & fn) { switch (GetRectCase(rect, path)) { - case RectCase::Inside: return {m2::SharedSpline(path)}; - case RectCase::Outside: return {}; - case RectCase::Intersect: return ClipPathByRectImpl(rect, path); + case RectCase::Inside: fn(m2::SharedSpline(std::move(path))); break; + case RectCase::Outside: break; + case RectCase::Intersect: ClipPathByRectImpl(rect, path, fn); break; } - CHECK(false, ("Unreachable")); - return {}; } void ClipPathByRectBeforeSmooth(m2::RectD const & rect, std::vector const & path, diff --git a/geometry/clipping.hpp b/geometry/clipping.hpp index e85d31d28e..4ba9063514 100644 --- a/geometry/clipping.hpp +++ b/geometry/clipping.hpp @@ -3,7 +3,6 @@ #include "geometry/point2d.hpp" #include "geometry/rect2d.hpp" #include "geometry/spline.hpp" -#include "geometry/triangle2d.hpp" #include #include @@ -16,8 +15,8 @@ using ClipTriangleByRectResultIt = void ClipTriangleByRect(m2::RectD const & rect, m2::PointD const & p1, m2::PointD const & p2, m2::PointD const & p3, ClipTriangleByRectResultIt const & resultIterator); -std::vector ClipPathByRect(m2::RectD const & rect, - std::vector const & path); +void ClipPathByRect(m2::RectD const & rect, std::vector && path, + std::function const & fn); std::vector ClipSplineByRect(m2::RectD const & rect, m2::SharedSpline const & spline); diff --git a/geometry/spline.cpp b/geometry/spline.cpp index 1ecd97735d..aea5c865af 100644 --- a/geometry/spline.cpp +++ b/geometry/spline.cpp @@ -106,7 +106,7 @@ double Spline::GetLength() const template void Spline::Init(T && path) { - ASSERT(path.size() > 1, ("Wrong path size!")); + ASSERT_GREATER(path.size(), 1, ()); m_position = std::forward(path); size_t cnt = m_position.size() - 1; m_direction = std::vector(cnt); @@ -243,7 +243,7 @@ SharedSpline::SharedSpline(std::vector && path) bool SharedSpline::IsNull() const { - return m_spline == NULL; + return m_spline == nullptr; } void SharedSpline::Reset(Spline * spline) @@ -251,10 +251,10 @@ void SharedSpline::Reset(Spline * spline) m_spline.reset(spline); } -void SharedSpline::Reset(std::vector const & path) -{ - m_spline.reset(new Spline(path)); -} +//void SharedSpline::Reset(std::vector const & path) +//{ +// m_spline.reset(new Spline(path)); +//} Spline::iterator SharedSpline::CreateIterator() const { diff --git a/geometry/spline.hpp b/geometry/spline.hpp index b11abe39cf..b06eee3af5 100644 --- a/geometry/spline.hpp +++ b/geometry/spline.hpp @@ -96,7 +96,7 @@ public: bool IsNull() const; void Reset(Spline * spline); - void Reset(std::vector const & path); + //void Reset(std::vector const & path); Spline::iterator CreateIterator() const; -- 2.45.3 From 8ba757473d8210b3e6a731247db8fc16e7f76e4d Mon Sep 17 00:00:00 2001 From: Arnaud Vergnet Date: Tue, 16 May 2023 11:48:42 +0200 Subject: [PATCH 6/6] [android] fix wiki description not always shown Description was not reappearing when switching back from a map object with only a link Signed-off-by: Arnaud Vergnet --- .../widget/placepage/sections/PlacePageWikipediaFragment.java | 1 + 1 file changed, 1 insertion(+) diff --git a/android/src/app/organicmaps/widget/placepage/sections/PlacePageWikipediaFragment.java b/android/src/app/organicmaps/widget/placepage/sections/PlacePageWikipediaFragment.java index 53acc16492..51b0a5544a 100644 --- a/android/src/app/organicmaps/widget/placepage/sections/PlacePageWikipediaFragment.java +++ b/android/src/app/organicmaps/widget/placepage/sections/PlacePageWikipediaFragment.java @@ -95,6 +95,7 @@ public class PlacePageWikipediaFragment extends Fragment implements Observer { -- 2.45.3