From 4f29050b58d3430e6b38d9e088a1ac42525eb94a Mon Sep 17 00:00:00 2001 From: Viktor Govako Date: Fri, 20 May 2022 10:11:06 +0300 Subject: [PATCH] [generator] Do not reduce speeds from parent links in MaxspeedsMwmCollector. Signed-off-by: Viktor Govako --- generator/maxspeeds_builder.cpp | 57 +++++++++++++++---- generator/mini_roundabout_transformer.cpp | 2 +- generator/routing_helpers.hpp | 1 + .../routing_integration_tests/route_test.cpp | 21 +++++++ 4 files changed, 70 insertions(+), 11 deletions(-) diff --git a/generator/maxspeeds_builder.cpp b/generator/maxspeeds_builder.cpp index 897aaaafaa..c2b00e9642 100644 --- a/generator/maxspeeds_builder.cpp +++ b/generator/maxspeeds_builder.cpp @@ -121,8 +121,39 @@ public: uint32_t const fid = seg.GetFeatureId(); return Segment(0, fid, seg.GetSegmentIdx() > 0 ? 0 : GetLastIndex(fid), seg.IsForward()); }; + auto const GetHighwayType = [&](uint32_t fid) + { + return GetRoad(fid).GetHighwayType(); + }; auto const & converter = GetMaxspeedConverter(); + using HwTypeT = std::optional; + auto const CalculateSpeed = [&](uint32_t parentFID, Maxspeed const & s, HwTypeT hwType) -> std::optional + { + HwTypeT const parentHwType = GetHighwayType(parentFID); + if (!parentHwType) + return {}; + + // Set speed as-is from parent link. + if (parentHwType == hwType) + return {{s.GetForward(), s.GetUnits()}}; + + using routing::HighwayType; + if ((*parentHwType == HighwayType::HighwayMotorway && hwType == HighwayType::HighwayMotorwayLink) || + (*parentHwType == HighwayType::HighwayTrunk && hwType == HighwayType::HighwayTrunkLink) || + (*parentHwType == HighwayType::HighwayPrimary && hwType == HighwayType::HighwayPrimaryLink) || + (*parentHwType == HighwayType::HighwaySecondary && hwType == HighwayType::HighwaySecondaryLink) || + (*parentHwType == HighwayType::HighwayTertiary && hwType == HighwayType::HighwayTertiaryLink)) + { + // Reduce factor from parent road. See DontUseLinksWhenRidingOnMotorway test. + // 0.85, this factor should be greater than sqrt(2) / 2 - prefer diagonal link to square path. + return converter.ClosestValidMacro( + { base::asserted_cast(std::lround(s.GetForward() * 0.85)), s.GetUnits() }); + } + + return {}; + }; + ForEachFeature(m_dataPath, [&](FeatureType & ft, uint32_t fid) { if (!routing::IsCarRoad(TypesHolder(ft))) @@ -149,6 +180,8 @@ public: // 0 - not updated, 1 - goto next iteration, 2 - updated int status; + HwTypeT const hwType = GetHighwayType(fid); + // Check ingoing first, then - outgoing. for (bool direction : { false, true }) { @@ -174,26 +207,30 @@ public: Segment const target = e.GetTarget(); uint32_t const targetFID = target.GetFeatureId(); - LOG_MAX_SPEED(("Edge target =", target, "; osmid =", GetOsmID(targetFID).GetSerialId())); + uint64_t const targetOsmID = GetOsmID(targetFID).GetSerialId(); + LOG_MAX_SPEED(("Edge target =", target, "; osmid =", targetOsmID)); Maxspeed const * s = GetSpeed(targetFID); if (s) { if (routing::IsNumeric(s->GetForward())) { - status = 2; + auto const speed = CalculateSpeed(targetFID, *s, hwType); + if (speed) + { + status = 2; - // Main thing here is to reduce the speed, relative to a parent. So use 0.7 factor. - auto const speed = converter.ClosestValidMacro( - SpeedInUnits(std::lround(s->GetForward() * 0.7), s->GetUnits())); - maxSpeed->SetUnits(s->GetUnits()); - maxSpeed->SetForward(speed.GetSpeed()); + maxSpeed->SetForward(speed->GetSpeed()); + maxSpeed->SetUnits(speed->GetUnits()); - LOG(LINFO, ("Updated link speed for way", osmID, "with", *maxSpeed)); - break; + LOG(LINFO, ("Updated link speed for way", osmID, "with", *maxSpeed, "from", targetOsmID)); + break; + } } else if (s->GetForward() == routing::kCommonMaxSpeedValue && - reviewed.find(targetFID) == reviewed.end()) + reviewed.size() < 4 && // limit with some reasonable transitions + reviewed.find(targetFID) == reviewed.end() && + hwType == GetHighwayType(targetFID)) { LOG_MAX_SPEED(("Add reviewed")); diff --git a/generator/mini_roundabout_transformer.cpp b/generator/mini_roundabout_transformer.cpp index f8ba552328..06fc451508 100644 --- a/generator/mini_roundabout_transformer.cpp +++ b/generator/mini_roundabout_transformer.cpp @@ -310,7 +310,7 @@ std::vector MiniRoundaboutTransformer::ProcessRoundabou if (!foundSurrogateRoad) { - LOG(LERROR, ("Not found road for mini_roundabout", mercator::FromLatLon(rb.m_coord))); + LOG(LERROR, ("Road not found for mini_roundabout", rb.m_coord)); continue; } } diff --git a/generator/routing_helpers.hpp b/generator/routing_helpers.hpp index 4621e08568..b115f7d5a7 100644 --- a/generator/routing_helpers.hpp +++ b/generator/routing_helpers.hpp @@ -10,6 +10,7 @@ namespace routing { using OsmIdToFeatureIds = std::map>; +/// @todo Make vector as FeatureID is continuous. using FeatureIdToOsmId = std::map; // Adds |featureId| and corresponding |osmId| to |osmIdToFeatureIds|. diff --git a/routing/routing_integration_tests/route_test.cpp b/routing/routing_integration_tests/route_test.cpp index b19324949c..a980e703f1 100644 --- a/routing/routing_integration_tests/route_test.cpp +++ b/routing/routing_integration_tests/route_test.cpp @@ -720,4 +720,25 @@ using namespace std; mercator::FromLatLon(55.7083688, 37.6213856), {0., 0.}, mercator::FromLatLon(55.724623, 37.62588), 1921.88); } + + // https://github.com/organicmaps/organicmaps/issues/1727 + // https://github.com/organicmaps/organicmaps/issues/2020 + // https://github.com/organicmaps/organicmaps/issues/2057 + UNIT_TEST(DontUseLinksWhenRidingOnMotorway) + { + integration::CalculateRouteAndTestRouteLength( + integration::GetVehicleComponents(VehicleType::Car), + mercator::FromLatLon(32.16881, 34.90656), {0., 0.}, + mercator::FromLatLon(32.1588823, 34.9330855), 2847.33); + + integration::CalculateRouteAndTestRouteLength( + integration::GetVehicleComponents(VehicleType::Car), + mercator::FromLatLon(43.587808, 1.495385), {0., 0.}, + mercator::FromLatLon(43.600145, 1.490489), 1457.16); + + integration::CalculateRouteAndTestRouteLength( + integration::GetVehicleComponents(VehicleType::Car), + mercator::FromLatLon(34.0175371, -84.3272339), {0., 0.}, + mercator::FromLatLon(34.0298011, -84.3182477), 1609.76); + } } // namespace route_test