From f33641f1fd4127a45ef8a49cf609815f3a351f0d Mon Sep 17 00:00:00 2001 From: Daria Volvenkova Date: Mon, 13 Nov 2017 15:33:14 +0300 Subject: [PATCH] Review fixes. --- base/string_utils.cpp | 2 + base/string_utils.hpp | 2 + drape_frontend/color_constants.cpp | 32 ++++++++------- drape_frontend/route_builder.cpp | 37 +++--------------- drape_frontend/route_renderer.cpp | 2 + drape_frontend/route_shape.cpp | 40 +++++++++++++++++-- drape_frontend/route_shape.hpp | 5 ++- map/bookmark_manager.cpp | 1 - map/routing_manager.cpp | 63 +++++++++++++++--------------- 9 files changed, 102 insertions(+), 82 deletions(-) diff --git a/base/string_utils.cpp b/base/string_utils.cpp index c3398f4e40..42defe2b06 100644 --- a/base/string_utils.cpp +++ b/base/string_utils.cpp @@ -260,6 +260,8 @@ bool StartsWith(UniString const & s, UniString const & p) bool StartsWith(std::string const & s1, char const * s2) { return (s1.compare(0, strlen(s2), s2) == 0); } +bool StartsWith(std::string const & s1, std::string const & s2) { return (s1.compare(0, s2.length(), s2) == 0); } + bool EndsWith(std::string const & s1, char const * s2) { size_t const n = s1.size(); diff --git a/base/string_utils.hpp b/base/string_utils.hpp index 3e353fd3ac..fe9e99a6c1 100644 --- a/base/string_utils.hpp +++ b/base/string_utils.hpp @@ -457,6 +457,8 @@ bool StartsWith(UniString const & s, UniString const & p); bool StartsWith(std::string const & s1, char const * s2); +bool StartsWith(std::string const & s1, std::string const & s2); + bool EndsWith(std::string const & s1, char const * s2); bool EndsWith(std::string const & s1, std::string const & s2); diff --git a/drape_frontend/color_constants.cpp b/drape_frontend/color_constants.cpp index 346267b48b..9f6b09238a 100644 --- a/drape_frontend/color_constants.cpp +++ b/drape_frontend/color_constants.cpp @@ -16,12 +16,16 @@ #include #include +using namespace std; + namespace { +string const kTransitColorFileName = "transit_colors.txt"; + class TransitColorsHolder { public: - dp::Color GetColor(std::string const & name) const + dp::Color GetColor(string const & name) const { auto const style = GetStyleReader().GetCurrentStyle(); auto const isDarkStyle = style == MapStyle::MapStyleDark || style == MapStyle::MapStyleVehicleDark; @@ -37,10 +41,10 @@ public: void Load() { - std::string data; + string data; try { - ReaderPtr(GetPlatform().GetReader("transit_colors.txt")).ReadAsString(data); + ReaderPtr(GetPlatform().GetReader(kTransitColorFileName)).ReadAsString(data); } catch (RootException const & ex) { @@ -66,7 +70,7 @@ public: ASSERT(name != nullptr, ()); ASSERT(colorInfo != nullptr, ()); - std::string strValue; + string strValue; FromJSONObject(colorInfo, "clear", strValue); m_clearColors[df::GetTransitColorName(name)] = ParseColor(strValue); FromJSONObject(colorInfo, "night", strValue); @@ -74,7 +78,6 @@ public: FromJSONObject(colorInfo, "text", strValue); m_clearColors[df::GetTransitTextColorName(name)] = ParseColor(strValue); m_nightColors[df::GetTransitTextColorName(name)] = ParseColor(strValue); - } } catch (my::Json::Exception const & e) @@ -84,16 +87,17 @@ public: } private: - dp::Color ParseColor(std::string const & colorStr) + dp::Color ParseColor(string const & colorStr) { unsigned int color; if (strings::to_uint(colorStr, color, 16)) return df::ToDrapeColor(static_cast(color)); + LOG(LWARNING, ("Color parsing failed:", colorStr)); return dp::Color(); } - std::map m_clearColors; - std::map m_nightColors; + map m_clearColors; + map m_nightColors; }; TransitColorsHolder & TransitColors() @@ -101,13 +105,13 @@ TransitColorsHolder & TransitColors() static TransitColorsHolder h; return h; } -} // namespace +} // namespace namespace df { -std::string const kTransitColorPrefix = "transit_"; -std::string const kTransitTextPrefix = "text_"; -std::string const kTransitLinePrefix = "line_"; +string const kTransitColorPrefix = "transit_"; +string const kTransitTextPrefix = "text_"; +string const kTransitLinePrefix = "line_"; ColorConstant GetTransitColorName(ColorConstant const & localName) { @@ -121,7 +125,7 @@ ColorConstant GetTransitTextColorName(ColorConstant const & localName) bool IsTransitColor(ColorConstant const & constant) { - return strings::StartsWith(constant, kTransitColorPrefix.c_str()); + return strings::StartsWith(constant, kTransitColorPrefix); } dp::Color GetColorConstant(ColorConstant const & constant) @@ -136,4 +140,4 @@ void LoadTransitColors() { TransitColors().Load(); } -} // namespace df +} // namespace df diff --git a/drape_frontend/route_builder.cpp b/drape_frontend/route_builder.cpp index faddcc4a1e..aa8ff71e90 100644 --- a/drape_frontend/route_builder.cpp +++ b/drape_frontend/route_builder.cpp @@ -2,33 +2,6 @@ #include "drape_frontend/route_shape.hpp" -namespace -{ -std::map> SplitSubroute(df::SubrouteConstPtr subroute) -{ - ASSERT(subroute != nullptr, ()); - - std::map> result; - if (subroute->m_styleType == df::SubrouteStyleType::Single) - { - ASSERT(!subroute->m_style.empty(), ()); - result[0] = std::make_pair(0, subroute->m_polyline.GetSize() - 1); - return result; - } - - size_t startStyleIndex = 0; - for (size_t i = 1; i <= subroute->m_style.size(); ++i) - { - if (i == subroute->m_style.size() || subroute->m_style[i] != subroute->m_style[i - 1]) - { - result[i - 1] = std::make_pair(subroute->m_style[startStyleIndex].m_startIndex, subroute->m_style[i - 1].m_endIndex); - startStyleIndex = i; - } - } - return result; -} -} // namespace - namespace df { RouteBuilder::RouteBuilder(FlushFn && flushFn, FlushArrowsFn && flushArrowsFn, @@ -46,13 +19,13 @@ void RouteBuilder::Build(dp::DrapeID subrouteId, SubrouteConstPtr subroute, cacheData.m_baseDepthIndex = subroute->m_baseDepthIndex; m_routeCache[subrouteId] = std::move(cacheData); - auto const & subrouteIndices = SplitSubroute(subroute); std::vector> subrouteData; - subrouteData.reserve(subrouteIndices.size()); - for (auto const & indices : subrouteIndices) + subrouteData.reserve(subroute->m_style.size()); + + ASSERT(!subroute->m_style.empty(), ()); + for (size_t styleIndex = 0; styleIndex < subroute->m_style.size(); ++styleIndex) { - subrouteData.push_back(RouteShape::CacheRoute(subrouteId, subroute, indices.second.first, - indices.second.second, indices.first, recacheId, textures)); + subrouteData.push_back(RouteShape::CacheRoute(subrouteId, subroute, styleIndex, recacheId, textures)); } auto markersData = RouteShape::CacheMarkers(subrouteId, subroute, recacheId, textures); diff --git a/drape_frontend/route_renderer.cpp b/drape_frontend/route_renderer.cpp index 0b95b80aa0..8f1e86e219 100644 --- a/drape_frontend/route_renderer.cpp +++ b/drape_frontend/route_renderer.cpp @@ -59,6 +59,8 @@ void InterpolateByZoom(SubrouteConstPtr const & subroute, ScreenBase const & scr std::vector const * halfWidthInPixel = &kRouteHalfWidthInPixelOthers; if (subroute->m_routeType == RouteType::Car || subroute->m_routeType == RouteType::Taxi) halfWidthInPixel = &kRouteHalfWidthInPixelCar; + else if (subroute->m_routeType == RouteType::Transit) + halfWidthInPixel = &kRouteHalfWidthInPixelTransit; halfWidth = InterpolateByZoomLevels(index, lerpCoef, *halfWidthInPixel); halfWidth *= static_cast(df::VisualParams::Instance().GetVisualScale()); diff --git a/drape_frontend/route_shape.cpp b/drape_frontend/route_shape.cpp index 93d93db456..e9e6f3febc 100644 --- a/drape_frontend/route_shape.cpp +++ b/drape_frontend/route_shape.cpp @@ -23,6 +23,14 @@ std::vector const kRouteHalfWidthInPixelCar = 3.0f, 3.0f, 4.0f, 5.0f, 6.0, 8.0f, 10.0f, 10.0f, 18.0f, 27.0f }; +std::vector const kRouteHalfWidthInPixelTransit = +{ + // 1 2 3 4 5 6 7 8 9 10 + 1.0f, 1.0f, 1.0f, 1.0f, 1.0f, 1.0f, 1.0f, 1.0f, 1.2f, 1.2f, + //11 12 13 14 15 16 17 18 19 20 + 1.5f, 1.5f, 2.0f, 2.5f, 3.0, 4.0f, 5.0f, 5.0f, 9.0f, 13.0f +}; + std::vector const kRouteHalfWidthInPixelOthers = { // 1 2 3 4 5 6 7 8 9 10 @@ -154,6 +162,18 @@ glsl::vec3 MarkerNormal(float x, float y, float z, float cosAngle, float sinAngl } } // namespace +void Subroute::AddStyle(SubrouteStyle const & style) +{ + if (!m_style.empty() && m_style.back() == style) + { + m_style.back().m_endIndex = style.m_endIndex; + } + else + { + m_style.push_back(style); + } +} + void RouteShape::PrepareGeometry(std::vector const & path, m2::PointD const & pivot, std::vector const & segmentsColors, float baseDepth, TGeometryBuffer & geometry, TGeometryBuffer & joinsGeometry) @@ -489,10 +509,24 @@ void RouteShape::CacheRouteArrows(ref_ptr mng, m2::PolylineD AV::GetBindingInfo(), routeArrowsData.m_renderProperty); } -drape_ptr RouteShape::CacheRoute(dp::DrapeID subrouteId, SubrouteConstPtr subroute, - size_t startIndex, size_t endIndex, size_t styleIndex, int recacheId, - ref_ptr textures) +drape_ptr RouteShape::CacheRoute(dp::DrapeID subrouteId, SubrouteConstPtr subroute, size_t styleIndex, + int recacheId, ref_ptr textures) { + size_t startIndex; + size_t endIndex; + if (subroute->m_styleType == df::SubrouteStyleType::Single) + { + ASSERT_EQUAL(styleIndex, 0, ()); + startIndex = 0; + endIndex = subroute->m_polyline.GetSize() - 1; + } + else + { + auto const & style = subroute->m_style[styleIndex]; + startIndex = style.m_startIndex; + endIndex = style.m_endIndex; + } + ASSERT_LESS(startIndex, endIndex, ()); auto const points = subroute->m_polyline.ExtractSegment(startIndex, endIndex); diff --git a/drape_frontend/route_shape.hpp b/drape_frontend/route_shape.hpp index 1bd3e595a2..1523e95140 100644 --- a/drape_frontend/route_shape.hpp +++ b/drape_frontend/route_shape.hpp @@ -36,6 +36,7 @@ double const kArrowHeightFactor = kArrowTextureHeight / kArrowBodyHeight; double const kArrowAspect = kArrowTextureWidth / kArrowTextureHeight; extern std::vector const kRouteHalfWidthInPixelCar; +extern std::vector const kRouteHalfWidthInPixelTransit; extern std::vector const kRouteHalfWidthInPixelOthers; enum class RouteType : uint8_t @@ -136,6 +137,8 @@ struct SubrouteMarker struct Subroute { + void AddStyle(SubrouteStyle const & style); + df::RouteType m_routeType; m2::PolylineD m_polyline; std::vector m_turns; @@ -199,7 +202,7 @@ public: using TMarkersGeometryBuffer = buffer_vector; static drape_ptr CacheRoute(dp::DrapeID subrouteId, SubrouteConstPtr subroute, - size_t startIndex, size_t endIndex, size_t styleIndex, int recacheId, + size_t styleIndex, int recacheId, ref_ptr textures); static drape_ptr CacheMarkers(dp::DrapeID subrouteId, diff --git a/map/bookmark_manager.cpp b/map/bookmark_manager.cpp index 20b5dad853..53ecd5070e 100644 --- a/map/bookmark_manager.cpp +++ b/map/bookmark_manager.cpp @@ -70,7 +70,6 @@ BookmarkManager::BookmarkManager(GetStringsBundleFn && getStringsBundleFn) { ASSERT(m_getStringsBundle != nullptr, ()); - m_userMarkLayers.reserve(7); m_userMarkLayers.emplace_back(my::make_unique()); m_userMarkLayers.emplace_back(my::make_unique()); m_userMarkLayers.emplace_back(my::make_unique()); diff --git a/map/routing_manager.cpp b/map/routing_manager.cpp index 0baa01d632..8a52619c09 100644 --- a/map/routing_manager.cpp +++ b/map/routing_manager.cpp @@ -108,7 +108,7 @@ struct TransitMarkInfo vector m_titles; }; -using GetMwmIdFn = std::function; +using GetMwmIdFn = function; void CollectTransitDisplayInfo(vector const & segments, GetMwmIdFn const & getMwmIdFn, TransitDisplayInfos & transitDisplayInfos) { @@ -128,55 +128,58 @@ void CollectTransitDisplayInfo(vector const & segments, GetMwmIdFn { case TransitInfo::Type::Edge: { - auto const & edge = transitInfo.GetEdge(); + auto const &edge = transitInfo.GetEdge(); mwmTransit->m_stops[edge.m_stop1Id] = {}; - mwmTransit->m_stops[edge.m_stop2Id] ={}; + mwmTransit->m_stops[edge.m_stop2Id] = {}; mwmTransit->m_lines[edge.m_lineId] = {}; - for (auto const & shapeId : edge.m_shapeIds) + for (auto const &shapeId : edge.m_shapeIds) mwmTransit->m_shapes[shapeId] = {}; - } break; + } case TransitInfo::Type::Transfer: { - auto const & transfer = transitInfo.GetTransfer(); + auto const &transfer = transitInfo.GetTransfer(); mwmTransit->m_stops[transfer.m_stop1Id] = {}; mwmTransit->m_stops[transfer.m_stop2Id] = {}; - } break; + } case TransitInfo::Type::Gate: { - auto const & gate = transitInfo.GetGate(); + auto const &gate = transitInfo.GetGate(); auto const featureId = FeatureID(mwmId, gate.m_featureId); mwmTransit->m_features[featureId] = {}; - } break; + } } } } void AddTransitGateSegment(m2::PointD const & destPoint, df::Subroute & subroute) { + ASSERT_GREATER(subroute.m_polyline.GetSize(), 0, ()); df::SubrouteStyle style(df::kGateColor, df::RoutePattern(1.0, 1.0)); style.m_startIndex = subroute.m_polyline.GetSize() - 1; auto const vec = destPoint - subroute.m_polyline.Back(); subroute.m_polyline.Add(destPoint); style.m_endIndex = subroute.m_polyline.GetSize() - 1; - subroute.m_style.push_back(style); + subroute.AddStyle(style); } void AddTransitPedestrianSegment(m2::PointD const & destPoint, df::Subroute & subroute) { + ASSERT_GREATER(subroute.m_polyline.GetSize(), 0, ()); df::SubrouteStyle style(df::kRoutePedestrian, df::RoutePattern(4.0, 2.0)); style.m_startIndex = subroute.m_polyline.GetSize() - 1; subroute.m_polyline.Add(destPoint); style.m_endIndex = subroute.m_polyline.GetSize() - 1; - subroute.m_style.push_back(style); + subroute.AddStyle(style); } void AddTransitShapes(std::vector const & shapeIds, TransitShapesInfo const & shapes, df::ColorConstant const & color, bool isInverted, df::Subroute & subroute) { + ASSERT_GREATER(subroute.m_polyline.GetSize(), 0, ()); df::SubrouteStyle style(color); style.m_startIndex = subroute.m_polyline.GetSize() - 1; @@ -190,7 +193,7 @@ void AddTransitShapes(std::vector const & shapeIds, TransitSha } style.m_endIndex = subroute.m_polyline.GetSize() - 1; - subroute.m_style.emplace_back(move(style)); + subroute.AddStyle(style); } string ColorToHexStr(dp::Color const & color) @@ -289,6 +292,7 @@ void FillTransitStyleForRendering(vector const & segments, Transit AddTransitShapes(edge.m_shapeIds, displayInfo.m_shapes, currentColor, isInverted, subroute); + ASSERT_GREATER(subroute.m_polyline.GetSize(), 1, ()); auto const & p1 = *(subroute.m_polyline.End() - 2); auto const & p2 = *(subroute.m_polyline.End() - 1); m2::PointD currentDir = (p2 - p1).Normalize(); @@ -300,8 +304,8 @@ void FillTransitStyleForRendering(vector const & segments, Transit marker.m_colors.push_back(currentColor); auto const fid = FeatureID(mwmId, stop1.GetFeatureId()); - transitMarkInfo.m_titles.push_back(TransitTitle(displayInfo.m_features.at(fid).m_title, - df::GetTransitTextColorName(line.GetColor()))); + transitMarkInfo.m_titles.emplace_back(displayInfo.m_features.at(fid).m_title, + df::GetTransitTextColorName(line.GetColor())); } lastColor = currentColor; @@ -379,8 +383,8 @@ void FillTransitStyleForRendering(vector const & segments, Transit vector GetTransitMarkerSizes(float markerScale) { vector markerSizes; - markerSizes.reserve(df::kRouteHalfWidthInPixelOthers.size()); - for (auto const halfWidth : df::kRouteHalfWidthInPixelOthers) + markerSizes.reserve(df::kRouteHalfWidthInPixelTransit.size()); + for (auto const halfWidth : df::kRouteHalfWidthInPixelTransit) { float const d = 2 * halfWidth * markerScale; markerSizes.push_back(m2::PointF(d, d)); @@ -390,7 +394,7 @@ vector GetTransitMarkerSizes(float markerScale) void CreateTransitMarks(vector const & transitMarks, UserMarksController & marksController) { - static vector const kTrasferMarkerSizes = GetTransitMarkerSizes(kTransferMarkerScale); + static vector const kTransferMarkerSizes = GetTransitMarkerSizes(kTransferMarkerScale); static vector const kStopMarkerSizes = GetTransitMarkerSizes(kStopMarkerScale); for (size_t i = 0; i < transitMarks.size(); ++i) @@ -408,7 +412,7 @@ void CreateTransitMarks(vector const & transitMarks, UserMarksC if (mark.m_titles.size() > 1) { transitMark->SetTextPosition(dp::Bottom); - transitMark->SetSymbolSizes(kTrasferMarkerSizes); + transitMark->SetSymbolSizes(kTransferMarkerSizes); userMark = marksController.CreateUserMark(mark.m_point); ASSERT(dynamic_cast(userMark) != nullptr, ()); @@ -417,7 +421,7 @@ void CreateTransitMarks(vector const & transitMarks, UserMarksC transitMark2->SetPrimaryText(mark.m_titles.back().m_text); transitMark2->SetPrimaryTextColor(df::GetColorConstant(mark.m_titles.back().m_color)); transitMark2->SetTextPosition(dp::Top); - transitMark2->SetSymbolSizes(kTrasferMarkerSizes); + transitMark2->SetSymbolSizes(kTransferMarkerSizes); } else { @@ -566,19 +570,16 @@ char const * const kRoutingCalculatingRoute = "Routing_CalculatingRoute"; TransitStepInfo::TransitStepInfo(bool isPedestrian, double distance, double time, std::string const & type, std::string const & number, std::string const & color) - : m_isPedestrian(isPedestrian) - , m_distance(distance) - , m_time(time) - , m_type(type) - , m_number(number) - , m_color(color) + : m_isPedestrian(isPedestrian) + , m_distance(distance) + , m_time(time) + , m_type(type) + , m_number(number) + , m_color(color) {} bool TransitStepInfo::IsEqualType(TransitStepInfo const & ts) const { - if (m_isPedestrian) - return ts.m_isPedestrian; - return m_isPedestrian == ts.m_isPedestrian && (m_isPedestrian || (m_type == ts.m_type && m_number == ts.m_number && m_color == ts.m_color)); } @@ -864,7 +865,7 @@ void RoutingManager::InsertRoute(Route const & route) case RouterType::Taxi: subroute->m_routeType = m_currentRouterType == RouterType::Vehicle ? df::RouteType::Car : df::RouteType::Taxi; - subroute->m_style.emplace_back(df::kRouteColor, df::kRouteOutlineColor); + subroute->AddStyle(df::SubrouteStyle(df::kRouteColor, df::kRouteOutlineColor)); FillTrafficForRendering(segments, subroute->m_traffic); FillTurnsDistancesForRendering(segments, subroute->m_baseDistance, subroute->m_turns); @@ -891,11 +892,11 @@ void RoutingManager::InsertRoute(Route const & route) break; case RouterType::Pedestrian: subroute->m_routeType = df::RouteType::Pedestrian; - subroute->m_style.emplace_back(df::kRoutePedestrian, df::RoutePattern(4.0, 2.0)); + subroute->AddStyle(df::SubrouteStyle(df::kRoutePedestrian, df::RoutePattern(4.0, 2.0))); break; case RouterType::Bicycle: subroute->m_routeType = df::RouteType::Bicycle; - subroute->m_style.emplace_back(df::kRouteBicycle, df::RoutePattern(8.0, 2.0)); + subroute->AddStyle(df::SubrouteStyle(df::kRouteBicycle, df::RoutePattern(8.0, 2.0))); FillTurnsDistancesForRendering(segments, subroute->m_baseDistance, subroute->m_turns); break;