From afe80909822ccc00c4f1680c43d061cfe9a837de Mon Sep 17 00:00:00 2001 From: Alexander Borsuk Date: Sun, 18 Aug 2024 16:31:45 +0200 Subject: [PATCH] Refactored optional position parameter passing Signed-off-by: Alexander Borsuk --- .../src/main/cpp/app/organicmaps/Framework.cpp | 16 +++++++++------- .../src/main/cpp/app/organicmaps/Framework.hpp | 2 +- .../src/main/java/app/organicmaps/Framework.java | 2 +- drape_frontend/drape_engine.cpp | 6 +++--- drape_frontend/drape_engine.hpp | 3 +-- drape_frontend/frontend_renderer.cpp | 7 +++---- drape_frontend/message_subclasses.hpp | 16 ++++++++-------- .../MWMMapViewControlsManager+AddPlace.h | 2 +- .../Components/MWMAddPlaceNavigationBar.h | 3 +-- .../Components/MWMAddPlaceNavigationBar.mm | 11 +++++------ .../MapViewControls/MWMMapViewControlsManager.mm | 7 +++---- .../PlacePageManager/MWMPlacePageManager.mm | 5 +++-- map/framework.cpp | 5 ++--- map/framework.hpp | 3 +-- qt/draw_widget.cpp | 6 ++---- 15 files changed, 44 insertions(+), 50 deletions(-) diff --git a/android/app/src/main/cpp/app/organicmaps/Framework.cpp b/android/app/src/main/cpp/app/organicmaps/Framework.cpp index fe3b5435cf..51f3bdaa6b 100644 --- a/android/app/src/main/cpp/app/organicmaps/Framework.cpp +++ b/android/app/src/main/cpp/app/organicmaps/Framework.cpp @@ -448,12 +448,11 @@ void Framework::Get3dMode(bool & allow3d, bool & allow3dBuildings) m_work.Load3dMode(allow3d, allow3dBuildings); } -void Framework::SetChoosePositionMode(ChoosePositionMode mode, bool isBusiness, - bool hasPosition, m2::PointD const & position) +void Framework::SetChoosePositionMode(ChoosePositionMode mode, bool isBusiness, m2::PointD const * optionalPosition) { m_isChoosePositionMode = mode; m_work.BlockTapEvents(mode != ChoosePositionMode::None); - m_work.EnableChoosePositionMode(mode != ChoosePositionMode::None, isBusiness, hasPosition, position); + m_work.EnableChoosePositionMode(mode != ChoosePositionMode::None, isBusiness, optionalPosition); } ChoosePositionMode Framework::GetChoosePositionMode() @@ -1775,10 +1774,13 @@ JNIEXPORT void JNICALL Java_app_organicmaps_Framework_nativeSetChoosePositionMode(JNIEnv *, jclass, jint mode, jboolean isBusiness, jboolean applyPosition) { - auto const pos = applyPosition && frm()->HasPlacePageInfo() - ? g_framework->GetPlacePageInfo().GetMercator() - : m2::PointD(); - g_framework->SetChoosePositionMode(static_cast(mode), isBusiness, applyPosition, pos); + // TODO(AB): Move this code into the Framework to share with iOS and other platforms. + auto const f = frm(); + if (applyPosition && f->HasPlacePageInfo()) + g_framework->SetChoosePositionMode(static_cast(mode), isBusiness, + &f->GetCurrentPlacePageInfo().GetMercator()); + else + g_framework->SetChoosePositionMode(static_cast(mode), isBusiness, nullptr); } JNIEXPORT jint JNICALL diff --git a/android/app/src/main/cpp/app/organicmaps/Framework.hpp b/android/app/src/main/cpp/app/organicmaps/Framework.hpp index fd40fea44c..c8253d270f 100644 --- a/android/app/src/main/cpp/app/organicmaps/Framework.hpp +++ b/android/app/src/main/cpp/app/organicmaps/Framework.hpp @@ -188,7 +188,7 @@ namespace android void Set3dMode(bool allow3d, bool allow3dBuildings); void Get3dMode(bool & allow3d, bool & allow3dBuildings); - void SetChoosePositionMode(ChoosePositionMode mode, bool isBusiness, bool hasPosition, m2::PointD const & position); + void SetChoosePositionMode(ChoosePositionMode mode, bool isBusiness, m2::PointD const * optionalPosition); ChoosePositionMode GetChoosePositionMode(); void UpdateMyPositionRoutingOffset(int offsetY); diff --git a/android/app/src/main/java/app/organicmaps/Framework.java b/android/app/src/main/java/app/organicmaps/Framework.java index 2a076225c3..ebc4a3a80a 100644 --- a/android/app/src/main/java/app/organicmaps/Framework.java +++ b/android/app/src/main/java/app/organicmaps/Framework.java @@ -399,7 +399,7 @@ public class Framework } /** * @param mode - see ChoosePositionMode values. - * @param isBusiness selection area will be bounded by building borders, if its true(eg. true for businesses in buildings). + * @param isBusiness selection area will be bounded by building borders, if its true (eg. true for businesses in buildings). * @param applyPosition if true, map'll be animated to currently selected object. */ public static native void nativeSetChoosePositionMode(@ChoosePositionMode int mode, boolean isBusiness, diff --git a/drape_frontend/drape_engine.cpp b/drape_frontend/drape_engine.cpp index 96430728ad..b201a8730b 100644 --- a/drape_frontend/drape_engine.cpp +++ b/drape_frontend/drape_engine.cpp @@ -105,7 +105,7 @@ DrapeEngine::DrapeEngine(Params && params) RecacheMapShapes(); if (params.m_showChoosePositionMark) - EnableChoosePositionMode(true, std::move(params.m_boundAreaTriangles), false, m2::PointD()); + EnableChoosePositionMode(true, std::move(params.m_boundAreaTriangles), nullptr); ResizeImpl(m_viewport.GetWidth(), m_viewport.GetHeight()); } @@ -673,7 +673,7 @@ void DrapeEngine::ClearGpsTrackPoints() } void DrapeEngine::EnableChoosePositionMode(bool enable, std::vector && boundAreaTriangles, - bool hasPosition, m2::PointD const & position) + m2::PointD const * optionalPosition) { m_choosePositionMode = enable; bool kineticScroll = m_kineticScrollEnabled; @@ -691,7 +691,7 @@ void DrapeEngine::EnableChoosePositionMode(bool enable, std::vectorPostMessage(ThreadsCommutator::RenderThread, make_unique_dp(enable, std::move(boundAreaTriangles), - kineticScroll, hasPosition, position), + kineticScroll, optionalPosition), MessagePriority::Normal); } diff --git a/drape_frontend/drape_engine.hpp b/drape_frontend/drape_engine.hpp index 445ac66f3a..ddcd7bc0e1 100644 --- a/drape_frontend/drape_engine.hpp +++ b/drape_frontend/drape_engine.hpp @@ -191,8 +191,7 @@ public: std::vector && toRemove); void ClearGpsTrackPoints(); - void EnableChoosePositionMode(bool enable, std::vector && boundAreaTriangles, - bool hasPosition, m2::PointD const & position); + void EnableChoosePositionMode(bool enable, std::vector && boundAreaTriangles, m2::PointD const * optionalPosition); void BlockTapEvents(bool block); void SetKineticScrollEnabled(bool enabled); diff --git a/drape_frontend/frontend_renderer.cpp b/drape_frontend/frontend_renderer.cpp index 32958c5657..7be6b64384 100755 --- a/drape_frontend/frontend_renderer.cpp +++ b/drape_frontend/frontend_renderer.cpp @@ -833,14 +833,13 @@ void FrontendRenderer::AcceptMessage(ref_ptr message) else { // Exact position for POI or screen's center for Add place on map. - m2::PointD const pt = msg->HasPosition() ? msg->GetPosition() : - m_userEventStream.GetCurrentScreen().GlobalRect().Center(); - int zoom = kDoNotChangeZoom; if (GetCurrentZoom() < scales::GetAddNewPlaceScale()) zoom = scales::GetAddNewPlaceScale(); - AddUserEvent(make_unique_dp(pt, zoom, true /* isAnim */, false /* trackVisibleViewport */, + auto const pt = msg->GetOptionalPosition(); + AddUserEvent(make_unique_dp(pt ? *pt : m_userEventStream.GetCurrentScreen().GlobalRect().Center(), + zoom, true /* isAnim */, false /* trackVisibleViewport */, nullptr /* parallelAnimCreator */)); } } diff --git a/drape_frontend/message_subclasses.hpp b/drape_frontend/message_subclasses.hpp index f7180eae84..4c4563e6c3 100644 --- a/drape_frontend/message_subclasses.hpp +++ b/drape_frontend/message_subclasses.hpp @@ -39,6 +39,7 @@ #include #include #include +#include #include #include @@ -405,28 +406,27 @@ class SetAddNewPlaceModeMessage : public Message { public: SetAddNewPlaceModeMessage(bool enable, std::vector && boundArea, - bool enableKineticScroll, bool hasPosition, m2::PointD const & position) + bool enableKineticScroll, m2::PointD const * optionalPosition) : m_enable(enable) , m_boundArea(std::move(boundArea)) , m_enableKineticScroll(enableKineticScroll) - , m_hasPosition(hasPosition) - , m_position(position) - {} + { + if (optionalPosition) + m_position = *optionalPosition; + } Type GetType() const override { return Type::SetAddNewPlaceMode; } std::vector && AcceptBoundArea() { return std::move(m_boundArea); } bool IsEnabled() const { return m_enable; } bool IsKineticScrollEnabled() const { return m_enableKineticScroll; } - bool HasPosition() const { return m_hasPosition; } - m2::PointD const & GetPosition() const { return m_position; } + auto const & GetOptionalPosition() const { return m_position; } private: bool m_enable; std::vector m_boundArea; bool m_enableKineticScroll; - bool m_hasPosition; - m2::PointD m_position; + std::optional m_position; }; class BlockTapEventsMessage : public Message diff --git a/iphone/Maps/Categories/MWMMapViewControlsManager+AddPlace.h b/iphone/Maps/Categories/MWMMapViewControlsManager+AddPlace.h index 309275ced2..5061ffca1d 100644 --- a/iphone/Maps/Categories/MWMMapViewControlsManager+AddPlace.h +++ b/iphone/Maps/Categories/MWMMapViewControlsManager+AddPlace.h @@ -5,7 +5,7 @@ NS_ASSUME_NONNULL_BEGIN @interface MWMMapViewControlsManager (AddPlace) -- (void)addPlace:(BOOL)isBusiness hasPoint:(BOOL)hasPoint point:(m2::PointD const &)point; +- (void)addPlace:(BOOL)isBusiness position:(m2::PointD const *)optionalPosition; @end diff --git a/iphone/Maps/Classes/Components/MWMAddPlaceNavigationBar.h b/iphone/Maps/Classes/Components/MWMAddPlaceNavigationBar.h index 9fc53f3730..e7622ec994 100644 --- a/iphone/Maps/Classes/Components/MWMAddPlaceNavigationBar.h +++ b/iphone/Maps/Classes/Components/MWMAddPlaceNavigationBar.h @@ -4,8 +4,7 @@ + (void)showInSuperview:(UIView *)superview isBusiness:(BOOL)isBusiness - applyPosition:(BOOL)applyPosition - position:(m2::PointD const &)position + position:(m2::PointD const *)optionalPosition doneBlock:(MWMVoidBlock)done cancelBlock:(MWMVoidBlock)cancel; diff --git a/iphone/Maps/Classes/Components/MWMAddPlaceNavigationBar.mm b/iphone/Maps/Classes/Components/MWMAddPlaceNavigationBar.mm index f5c23eba82..53a91ab2a5 100644 --- a/iphone/Maps/Classes/Components/MWMAddPlaceNavigationBar.mm +++ b/iphone/Maps/Classes/Components/MWMAddPlaceNavigationBar.mm @@ -14,8 +14,7 @@ + (void)showInSuperview:(UIView *)superview isBusiness:(BOOL)isBusiness - applyPosition:(BOOL)applyPosition - position:(m2::PointD const &)position + position:(m2::PointD const *)optionalPosition doneBlock:(MWMVoidBlock)done cancelBlock:(MWMVoidBlock)cancel { @@ -32,13 +31,13 @@ navBar.topConstraint.constant = -navBar.height; [navBar.trailingAnchor constraintEqualToAnchor:superview.trailingAnchor].active = true; [navBar.leadingAnchor constraintEqualToAnchor:superview.leadingAnchor].active = true; - [navBar show:isBusiness applyPosition:applyPosition position:position]; + [navBar show:isBusiness position:optionalPosition]; } -- (void)show:(BOOL)enableBounds applyPosition:(BOOL)applyPosition position:(m2::PointD const &)position +- (void)show:(BOOL)enableBounds position:(m2::PointD const *)optionalPosition { auto & f = GetFramework(); - f.EnableChoosePositionMode(true /* enable */, enableBounds, applyPosition, position); + f.EnableChoosePositionMode(true /* enable */, enableBounds, optionalPosition); f.BlockTapEvents(true); [UIView animateWithDuration:kDefaultAnimationDuration animations:^ @@ -50,7 +49,7 @@ - (void)dismissWithBlock:(MWMVoidBlock)block { auto & f = GetFramework(); - f.EnableChoosePositionMode(false /* enable */, false /* enableBounds */, false /* applyPosition */, m2::PointD()); + f.EnableChoosePositionMode(false /* enable */, false /* enableBounds */, nullptr /* optionalPosition */); f.BlockTapEvents(false); [UIView animateWithDuration:kDefaultAnimationDuration animations:^ diff --git a/iphone/Maps/Classes/CustomViews/MapViewControls/MWMMapViewControlsManager.mm b/iphone/Maps/Classes/CustomViews/MapViewControls/MWMMapViewControlsManager.mm index 652404be7d..7f7905e6ac 100644 --- a/iphone/Maps/Classes/CustomViews/MapViewControls/MWMMapViewControlsManager.mm +++ b/iphone/Maps/Classes/CustomViews/MapViewControls/MWMMapViewControlsManager.mm @@ -130,10 +130,10 @@ NSString *const kMapToCategorySelectorSegue = @"MapToCategorySelectorSegue"; } - (void)addPlace { - [self addPlace:NO hasPoint:NO point:m2::PointD()]; + [self addPlace:NO position:nullptr]; } -- (void)addPlace:(BOOL)isBusiness hasPoint:(BOOL)hasPoint point:(m2::PointD const &)point { +- (void)addPlace:(BOOL)isBusiness position:(m2::PointD const *)optionalPosition { MapViewController *ownerController = self.ownerController; self.isAddingPlace = YES; @@ -145,8 +145,7 @@ NSString *const kMapToCategorySelectorSegue = @"MapToCategorySelectorSegue"; [MWMAddPlaceNavigationBar showInSuperview:ownerController.view isBusiness:isBusiness - applyPosition:hasPoint - position:point + position:optionalPosition doneBlock:^{ if ([MWMFrameworkHelper canEditMapAtViewportCenter]) [ownerController performSegueWithIdentifier:kMapToCategorySelectorSegue sender:nil]; diff --git a/iphone/Maps/UI/PlacePage/PlacePageManager/MWMPlacePageManager.mm b/iphone/Maps/UI/PlacePage/PlacePageManager/MWMPlacePageManager.mm index 56c2166adf..8101db24af 100644 --- a/iphone/Maps/UI/PlacePage/PlacePageManager/MWMPlacePageManager.mm +++ b/iphone/Maps/UI/PlacePage/PlacePageManager/MWMPlacePageManager.mm @@ -150,12 +150,13 @@ using namespace storage; - (void)addBusiness { - [[MWMMapViewControlsManager manager] addPlace:YES hasPoint:NO point:m2::PointD()]; + [[MWMMapViewControlsManager manager] addPlace:YES position:nullptr]; } - (void)addPlace:(CLLocationCoordinate2D)coordinate { - [[MWMMapViewControlsManager manager] addPlace:NO hasPoint:YES point:location_helpers::ToMercator(coordinate)]; + auto const position = location_helpers::ToMercator(coordinate); + [[MWMMapViewControlsManager manager] addPlace:NO position:&position]; } - (void)addBookmark:(PlacePageData *)data { diff --git a/map/framework.cpp b/map/framework.cpp index e81b282ceb..1a646092ec 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -2523,13 +2523,12 @@ void Framework::SaveOutdoorsEnabled(bool enabled) settings::Set(kOutdoorsEnabledKey, enabled); } -void Framework::EnableChoosePositionMode(bool enable, bool enableBounds, bool applyPosition, - m2::PointD const & position) +void Framework::EnableChoosePositionMode(bool enable, bool enableBounds, m2::PointD const * optionalPosition) { if (m_drapeEngine != nullptr) { m_drapeEngine->EnableChoosePositionMode(enable, - enableBounds ? GetSelectedFeatureTriangles() : vector(), applyPosition, position); + enableBounds ? GetSelectedFeatureTriangles() : vector(), optionalPosition); } } diff --git a/map/framework.hpp b/map/framework.hpp index e345b7d721..6a3bfc7896 100644 --- a/map/framework.hpp +++ b/map/framework.hpp @@ -60,7 +60,6 @@ #include #include -#include #include #include @@ -341,7 +340,7 @@ public: void InvalidateRendering(); void EnableDebugRectRendering(bool enabled); - void EnableChoosePositionMode(bool enable, bool enableBounds, bool applyPosition, m2::PointD const & position); + void EnableChoosePositionMode(bool enable, bool enableBounds, m2::PointD const * optionalPosition); void BlockTapEvents(bool block); using TCurrentCountryChanged = std::function; diff --git a/qt/draw_widget.cpp b/qt/draw_widget.cpp index a979e93d1c..6ed66b365e 100644 --- a/qt/draw_widget.cpp +++ b/qt/draw_widget.cpp @@ -167,14 +167,12 @@ void DrawWidget::ShowAll() void DrawWidget::ChoosePositionModeEnable() { m_framework.BlockTapEvents(true /* block */); - m_framework.EnableChoosePositionMode(true /* enable */, false /* enableBounds */, - false /* applyPosition */, m2::PointD() /* position */); + m_framework.EnableChoosePositionMode(true /* enable */, false /* enableBounds */, nullptr /* optionalPosition */); } void DrawWidget::ChoosePositionModeDisable() { - m_framework.EnableChoosePositionMode(false /* enable */, false /* enableBounds */, - false /* applyPosition */, m2::PointD() /* position */); + m_framework.EnableChoosePositionMode(false /* enable */, false /* enableBounds */, nullptr /* optionalPosition */); m_framework.BlockTapEvents(false /* block */); }