Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[iOS] add-camera-padding #1348

Closed
wants to merge 5 commits into from
Closed

[iOS] add-camera-padding #1348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
069b76f
Adjust MLNMapCamera and MLNMapView to respect new padding value
JulianBissekkou Jul 12, 2023
90891ee
update .gitignore to exclude all .gradle directories and generate-sty…
JulianBissekkou Jul 12, 2023
a97dcc6
update CHANGELOG.md
JulianBissekkou Jul 12, 2023
dc42c1e
update iOS CHANGELOG.md
JulianBissekkou Jul 13, 2023
e656886
Merge branch 'main' into add-camera-padding
JulianBissekkou Jul 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
/platform/android/MapboxGLAndroidSDK/build
/platform/android/MapboxGLAndroidSDKTestApp/.cxx
/platform/android/MapboxGLAndroidSDKTestApp/build
/platform/android/.gradle
/platform/android/x86
/platform/android/x86_64
/cmake-build-debug
Expand All @@ -19,6 +18,10 @@
/platform/windows/vendor/mesa3d/
*.code-workspace

/scripts/generate-style-code.list

**/.gradle

.DS_Store
.cache
/build
Expand Down
43 changes: 43 additions & 0 deletions platform/ios/platform/darwin/src/MLNMapCamera.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,22 @@

#import "MLNFoundation.h"

#if TARGET_OS_IPHONE
#import <UIKit/UIKit.h>
#endif

#if TARGET_OS_IPHONE
#define MGLEdgeInsets UIEdgeInsets
#define MGLEdgeInsetsMake UIEdgeInsetsMake
#define MGLEdgeInsetsEqual UIEdgeInsetsEqualToEdgeInsets
#define MGLEdgeInsetsZero UIEdgeInsetsZero
#else
#define MGLEdgeInsets NSEdgeInsets
#define MGLEdgeInsetsMake NSEdgeInsetsMake
#define MGLEdgeInsetsEqual NSEdgeInsetsEqual
#define MGLEdgeInsetsZero NSEdgeInsetsZero
#endif

NS_ASSUME_NONNULL_BEGIN

/**
Expand Down Expand Up @@ -54,6 +70,11 @@ MLN_EXPORT
*/
@property (nonatomic) CLLocationDistance viewingDistance;

/**
Padding around the interior of the view that affects the frame of reference for `center`.
*/
@property (nonatomic) MGLEdgeInsets padding;

/** Returns a new camera with all properties set to 0. */
+ (instancetype)camera;

Expand Down Expand Up @@ -133,6 +154,28 @@ MLN_EXPORT
__attribute__((deprecated("Use -cameraLookingAtCenterCoordinate:acrossDistance:pitch:heading: "
"or -cameraLookingAtCenterCoordinate:altitude:pitch:heading:.")));

/**
Returns a new camera with the given altitude, pitch, and heading.
@param centerCoordinate The geographic coordinate on which the map should be
centered.
@param altitude The altitude (measured in meters) above the map at which the
camera should be situated. The altitude may be less than the distance from
the camera’s viewpoint to the camera’s focus point.
@param pitch The viewing angle of the camera, measured in degrees. A value of
`0` results in a camera pointed straight down at the map. Angles greater
than `0` result in a camera angled toward the horizon.
@param heading The camera’s heading, measured in degrees clockwise from true
north. A value of `0` means that the top edge of the map view corresponds to
true north. The value `90` means the top of the map is pointing due east.
The value `180` means the top of the map points due south, and so on.
@param padding Padding around the interior of the view that affects the frame of reference for `center`.
*/
+ (instancetype)cameraLookingAtCenterCoordinate:(CLLocationCoordinate2D)centerCoordinate
altitude:(CLLocationDistance)altitude
pitch:(CGFloat)pitch
heading:(CLLocationDirection)heading
padding:(MGLEdgeInsets)padding;

/**
Returns a Boolean value indicating whether the given camera is functionally
equivalent to the receiver.
Expand Down
50 changes: 41 additions & 9 deletions platform/ios/platform/darwin/src/MLNMapCamera.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ + (instancetype)cameraLookingAtCenterCoordinate:(CLLocationCoordinate2D)centerCo
return [[self alloc] initWithCenterCoordinate:centerCoordinate
altitude:eyeAltitude
pitch:pitch
heading:heading];
heading:heading
padding:MGLEdgeInsetsZero];
}

+ (instancetype)cameraLookingAtCenterCoordinate:(CLLocationCoordinate2D)centerCoordinate
Expand All @@ -60,7 +61,8 @@ + (instancetype)cameraLookingAtCenterCoordinate:(CLLocationCoordinate2D)centerCo
return [[self alloc] initWithCenterCoordinate:centerCoordinate
altitude:altitude
pitch:pitch
heading:heading];
heading:heading
padding:MGLEdgeInsetsZero];
}

+ (instancetype)cameraLookingAtCenterCoordinate:(CLLocationCoordinate2D)centerCoordinate
Expand All @@ -71,7 +73,8 @@ + (instancetype)cameraLookingAtCenterCoordinate:(CLLocationCoordinate2D)centerCo
return [[self alloc] initWithCenterCoordinate:centerCoordinate
altitude:altitude
pitch:pitch
heading:heading];
heading:heading
padding:MGLEdgeInsetsZero];
}

+ (instancetype)cameraLookingAtCenterCoordinate:(CLLocationCoordinate2D)centerCoordinate
Expand All @@ -87,18 +90,33 @@ + (instancetype)cameraLookingAtCenterCoordinate:(CLLocationCoordinate2D)centerCo
heading:heading];
}

+ (instancetype)cameraLookingAtCenterCoordinate:(CLLocationCoordinate2D)centerCoordinate
altitude:(CLLocationDistance)altitude
pitch:(CGFloat)pitch
heading:(CLLocationDirection)heading
padding:(MGLEdgeInsets)padding
{
return [[self alloc] initWithCenterCoordinate:centerCoordinate
altitude:altitude
pitch:pitch
heading:heading
padding:padding];
}

- (instancetype)initWithCenterCoordinate:(CLLocationCoordinate2D)centerCoordinate
altitude:(CLLocationDistance)altitude
pitch:(CGFloat)pitch
heading:(CLLocationDirection)heading
padding:(MGLEdgeInsets)padding
{
MLNLogDebug(@"Initializing withCenterCoordinate: %@ altitude: %.0fm pitch: %f° heading: %f°", MLNStringFromCLLocationCoordinate2D(centerCoordinate), altitude, pitch, heading);
MLNLogDebug(@"Initializing withCenterCoordinate: %@ altitude: %.0fm pitch: %f° heading: %f° padding:%@ padding; %f, %f, %f, %f", MLNStringFromCLLocationCoordinate2D(centerCoordinate), altitude, pitch, heading, padding.top, padding.left, padding.bottom, padding.right);
if (self = [super init])
{
_centerCoordinate = centerCoordinate;
_altitude = altitude;
_pitch = pitch;
_heading = heading;
_padding = padding;
}
return self;
}
Expand All @@ -113,6 +131,10 @@ - (nullable instancetype)initWithCoder:(NSCoder *)coder
_altitude = [coder decodeDoubleForKey:@"altitude"];
_pitch = [coder decodeDoubleForKey:@"pitch"];
_heading = [coder decodeDoubleForKey:@"heading"];
_padding.left = [coder decodeDoubleForKey:@"paddingLeft"];
_padding.right = [coder decodeDoubleForKey:@"paddingRight"];
_padding.top = [coder decodeDoubleForKey:@"paddingTop"];
_padding.bottom = [coder decodeDoubleForKey:@"paddingBottom"];
}
return self;
}
Expand All @@ -124,14 +146,19 @@ - (void)encodeWithCoder:(NSCoder *)coder
[coder encodeDouble:_altitude forKey:@"altitude"];
[coder encodeDouble:_pitch forKey:@"pitch"];
[coder encodeDouble:_heading forKey:@"heading"];
[coder encodeDouble:_padding.left forKey:@"paddingLeft"];
[coder encodeDouble:_padding.right forKey:@"paddingRight"];
[coder encodeDouble:_padding.top forKey:@"paddingTop"];
[coder encodeDouble:_padding.bottom forKey:@"paddingBottom"];
}

- (id)copyWithZone:(nullable NSZone *)zone
{
return [[[self class] allocWithZone:zone] initWithCenterCoordinate:_centerCoordinate
altitude:_altitude
pitch:_pitch
heading:_heading];
heading:_heading
padding:_padding];
}

+ (NSSet<NSString *> *)keyPathsForValuesAffectingViewingDistance {
Expand All @@ -151,8 +178,8 @@ - (void)setViewingDistance:(CLLocationDistance)distance {

- (NSString *)description
{
return [NSString stringWithFormat:@"<%@: %p; centerCoordinate = %f, %f; altitude = %.0fm; heading = %.0f°; pitch = %.0f°>",
NSStringFromClass([self class]), (void *)self, _centerCoordinate.latitude, _centerCoordinate.longitude, _altitude, _heading, _pitch];
return [NSString stringWithFormat:@"<%@: %p; centerCoordinate = %f, %f; altitude = %.0fm; heading = %.0f°; pitch = %.0f° ; padding = %f, %f, %f, %f>",
NSStringFromClass([self class]), (void *)self, _centerCoordinate.latitude, _centerCoordinate.longitude, _altitude, _heading, _pitch, _padding.top, _padding.left, _padding.bottom, _padding.right];
}

- (BOOL)isEqual:(id)other
Expand All @@ -170,7 +197,8 @@ - (BOOL)isEqual:(id)other
return (_centerCoordinate.latitude == otherCamera.centerCoordinate.latitude
&& _centerCoordinate.longitude == otherCamera.centerCoordinate.longitude
&& _altitude == otherCamera.altitude
&& _pitch == otherCamera.pitch && _heading == otherCamera.heading);
&& _pitch == otherCamera.pitch && _heading == otherCamera.heading
&& MGLEdgeInsetsEqual(_padding, otherCamera.padding));
}

- (BOOL)isEqualToMapCamera:(MLNMapCamera *)otherCamera
Expand All @@ -184,7 +212,11 @@ - (BOOL)isEqualToMapCamera:(MLNMapCamera *)otherCamera
&& MLNEqualFloatWithAccuracy(_centerCoordinate.longitude, otherCamera.centerCoordinate.longitude, 1e-6)
&& MLNEqualFloatWithAccuracy(_altitude, otherCamera.altitude, 1e-6)
&& MLNEqualFloatWithAccuracy(_pitch, otherCamera.pitch, 1)
&& MLNEqualFloatWithAccuracy(_heading, otherCamera.heading, 1));
&& MLNEqualFloatWithAccuracy(_heading, otherCamera.heading, 1))
&& MLNEqualFloatWithAccuracy(_padding.left, otherCamera.padding.left, 1e-6)
&& MLNEqualFloatWithAccuracy(_padding.right, otherCamera.padding.right, 1e-6)
&& MLNEqualFloatWithAccuracy(_padding.top, otherCamera.padding.top, 1e-6)
&& MLNEqualFloatWithAccuracy(_padding.bottom, otherCamera.padding.bottom, 1e-6);
}

- (NSUInteger)hash
Expand Down
5 changes: 3 additions & 2 deletions platform/ios/platform/ios/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ MapLibre welcomes participation and contributions from everyone. Please read [`C

## main

* 💥 Breaking: Changed the prefix of files, classes, methods, variables and everything from `MGL` to `MLN`. ([#919](https://github.com/maplibre/maplibre-native/pull/919)).
* Add padding to `MLNMapCamera` [#1361](https://github.com/maplibre/maplibre-native/issues/1361)
* 💥 Breaking: Changed the prefix of files, classes, methods, variables and everything from `MGL` to `MLN`. [#919](https://github.com/maplibre/maplibre-native/pull/919).

> To migrate:
> Change all your `MGL` prefixes to `MLN`. If you are using `NSKeyedArchiver` or similar mechanishm to save the state, the app may crash after this change when trying to unarchive the state using old names of the classes. You need to clean the saved state of the app and save it using new classes.
> Change all your `MGL` prefixes to `MLN`. If you are using `NSKeyedArchiver` or similar mechanism to save the state, the app may crash after this change when trying to unarchive the state using old names of the classes. You need to clean the saved state of the app and save it using new classes.

## 5.13.0 - January 05, 2023

Expand Down
32 changes: 27 additions & 5 deletions platform/ios/platform/ios/app/MBXViewController.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ typedef NS_ENUM(NSInteger, MBXSettingsAnnotationsRows) {
MBXSettingsAnnotationsCustomUserDot,
MBXSettingsAnnotationsRemoveAnnotations,
MBXSettingsAnnotationSelectRandomOffscreenPointAnnotation,
MBXSettingsAnnotationCenterSelectedAnnotation,
MBXSettingsAnnotationCenterSelectedAnnotationWithoutPadding,
MBXSettingsAnnotationCenterSelectedAnnotationWithPadding,
MBXSettingsAnnotationAddVisibleAreaPolyline
};

Expand Down Expand Up @@ -391,7 +392,8 @@ - (void)dismissSettings:(__unused id)sender
[NSString stringWithFormat:@"%@ Custom User Dot", (_customUserLocationAnnnotationEnabled ? @"Disable" : @"Enable")],
@"Remove Annotations",
@"Select an offscreen point annotation",
@"Center selected annotation",
@"Center selected annotation without padding",
@"Center selected annotation with padding",
@"Add visible area polyline"
]];
break;
Expand Down Expand Up @@ -548,8 +550,11 @@ - (void)performActionForSettingAtIndexPath:(NSIndexPath *)indexPath
case MBXSettingsAnnotationSelectRandomOffscreenPointAnnotation:
[self selectAnOffscreenPointAnnotation];
break;
case MBXSettingsAnnotationCenterSelectedAnnotation:
[self centerSelectedAnnotation];
case MBXSettingsAnnotationCenterSelectedAnnotationWithPadding:
[self centerSelectedAnnotationWithPadding];
break;
case MBXSettingsAnnotationCenterSelectedAnnotationWithoutPadding:
[self centerSelectedAnnotationWithoutPadding];
break;
case MBXSettingsAnnotationAddVisibleAreaPolyline:
[self addVisibleAreaPolyline];
Expand Down Expand Up @@ -1672,7 +1677,24 @@ - (void)selectAnOffscreenPointAnnotation {
}
}

- (void)centerSelectedAnnotation {
- (void)centerSelectedAnnotationWithPadding {
id<MLNAnnotation> annotation = self.mapView.selectedAnnotations.firstObject;

if (!annotation)
return;

CGPoint point = [self.mapView convertCoordinate:annotation.coordinate toPointToView:self.mapView];

// Animate, so that point becomes the the center
CLLocationCoordinate2D center = [self.mapView convertPoint:point toCoordinateFromView:self.mapView];
MLNMapCamera *camera = self.mapView.camera.copy;
camera.centerCoordinate = center;
camera.padding = UIEdgeInsetsMake(200, 50, 10, 4);

[self.mapView setCamera:camera animated:YES];
}

- (void)centerSelectedAnnotationWithoutPadding {
Comment on lines +1680 to +1697
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that this adjustment on the example app is not perfect.
It's been a while since I wrote some objective-c so that's why I quickly adjusted an existing entry in the feature list instead of creating a new one.

If someone has a better idea and can maybe help me out then let me know 👍🏽

id<MLNAnnotation> annotation = self.mapView.selectedAnnotations.firstObject;

if (!annotation)
Expand Down
16 changes: 15 additions & 1 deletion platform/ios/platform/ios/src/MLNMapView.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3069,6 +3069,7 @@ - (void)resetPosition
double zoom = camera.zoom ? *camera.zoom : 0.0;
mbgl::LatLng center = camera.center ? *camera.center : mbgl::LatLng();


CLLocationDirection heading = mbgl::util::wrap(bearing, 0., 360.);
CLLocationDistance altitude = MLNAltitudeForZoomLevel(zoom, pitch, 0, self.frame.size);
self.camera = [MLNMapCamera cameraLookingAtCenterCoordinate:MLNLocationCoordinate2DFromLatLng(center)
Expand Down Expand Up @@ -4352,7 +4353,17 @@ - (MLNMapCamera *)cameraForCameraOptions:(const mbgl::CameraOptions &)cameraOpti
CLLocationDirection direction = cameraOptions.bearing ? mbgl::util::wrap(*cameraOptions.bearing, 0., 360.) : self.direction;
CGFloat pitch = cameraOptions.pitch ? *cameraOptions.pitch : *mapCamera.pitch;
CLLocationDistance altitude = MLNAltitudeForZoomLevel(zoomLevel, pitch, centerCoordinate.latitude, self.frame.size);
return [MLNMapCamera cameraLookingAtCenterCoordinate:centerCoordinate altitude:altitude pitch:pitch heading:direction];

MGLEdgeInsets padding = MGLEdgeInsetsZero;
if (cameraOptions.padding) {
padding = MGLEdgeInsetsMake(
cameraOptions.padding->top(),
cameraOptions.padding->left(),
cameraOptions.padding->bottom(),
cameraOptions.padding->right()
);
}
return [MLNMapCamera cameraLookingAtCenterCoordinate:centerCoordinate altitude:altitude pitch:pitch heading:direction padding:padding];
}

/// Returns a CameraOptions object that specifies parameters for animating to
Expand All @@ -4376,6 +4387,9 @@ - (MLNMapCamera *)cameraForCameraOptions:(const mbgl::CameraOptions &)cameraOpti
{
options.pitch = camera.pitch;
}
if (!MGLEdgeInsetsEqual(camera.padding, MGLEdgeInsetsZero)) {
options.padding = MLNEdgeInsetsFromNSEdgeInsets(camera.padding);
}
Comment on lines +4390 to +4392
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving the discussion of the old PR over to this PR:

Not sure if this is correct, as there is edgePadding as argument as well as in padding in MGLMapCamera. The padding in MGLMapCamera overwrites the param here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me do some more testing before we merge this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did my testing. However, since this is my first PR in maplibre native I would like to have feedback from someone that knows this part of the code well enough :)

Copy link
Contributor

@1ec5 1ec5 Jul 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is correct, as there is edgePadding as argument as well as in padding in MGLMapCamera. The padding in MGLMapCamera overwrites the param here.

The library currently conflates two different concepts as “padding”:

  • MLNMapView.contentInset is supposed to be persistent, something the developer sets according to UI that takes up space within the map view, such as a translucent sidebar or a camera “notch” on iOS. The developer should only have to think about this inset when sizing the map view or embedding a subview, not when manipulating or animating the camera. The resulting virtual rectangle is often called a “safe area” on Apple platforms.
  • Various camera-manipulating methods like -[MLNMapView setCamera:withDuration:animationTimingFunction:edgePadding:completionHandler:] take an edgePadding parameter for more precisely targeting the camera transition. In particular, the navigation SDK (and navigation applications in general) need to anchor the user location indicator somewhere other than the center of the safe area, typically in the lower third of the map view.

In the original design in mapbox/mapbox-gl-native#3583, the mbgl::CameraOptions::padding was the sum of the (persistent) content insets and the (temporary) edge padding. mbgl::CameraOptions::padding affected the frame of reference for mbgl::CameraOptions::center, but mbgl::Map::transform never persisted the padding because it had no context about how much of that padding was persistent and how much was temporary.

Unfortunately, a change was made to mbgl that conflated these two concepts, creating quite a headache for iOS/macOS developers: mapbox/mapbox-gl-native#15232 mapbox/mapbox-gl-native#15233. I have recommended that mbgl::Transform decouple the concepts of “padding” and “insets” and even replace “padding” with a more explicit way to anchor the center coordinate within screen space: mapbox/mapbox-gl-native#15233 (comment).

None of this was taken into account on Android; this mbgl functionality was exposed more literally without much regard for the practical use cases. Unfortunately, this means cross-platform libraries like the Flutter library will need to reconcile the two approaches.

return options;
}

Expand Down