[iOS] Update annotation view touch handling (with offsets) (#12234)

6 files changed, 143 insertions, 20 deletions

diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md
index 83a2d0ef95..0f6aba3cfd 100644
--- a/platform/ios/CHANGELOG.md
+++ b/platform/ios/CHANGELOG.md
@@ -23,6 +23,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT
 * Added `-[MGLMapView camera:fittingShape:edgePadding:]` and `-[MGLMapView camera:fittingCoordinateBounds:edgePadding:]` allowing you specify the pitch and direction for the calculated camera. ([#12213](https://github.com/mapbox/mapbox-gl-native/pull/12213))
 * Added `-[MGLMapSnapshot coordinateForPoint:]` that returns a map coordinate for a specified snapshot image point. ([#12221](https://github.com/mapbox/mapbox-gl-native/pull/12221))
 * Reduced memory usage when collision debug mode is disabled. ([#12294](https://github.com/mapbox/mapbox-gl-native/issues/12294))
+* Fixed a bug with annotation view touch handling when a non-zero `centerOffset` is specified. ([#12234](https://github.com/mapbox/mapbox-gl-native/pull/12234))
 
 ## 4.0.3 - June 22, 2018
 
diff --git a/platform/ios/Integration Tests/Annotation Tests/MGLAnnotationViewIntegrationTests.m b/platform/ios/Integration Tests/Annotation Tests/MGLAnnotationViewIntegrationTests.m
new file mode 100644
index 0000000000..ebd587ebe6
--- /dev/null
+++ b/platform/ios/Integration Tests/Annotation Tests/MGLAnnotationViewIntegrationTests.m
@@ -0,0 +1,108 @@
+#import "MGLMapViewIntegrationTest.h"
+#import "MGLTestUtility.h"
+#import "MGLMapAccessibilityElement.h"
+
+@interface MGLMapView (Tests)
+- (MGLAnnotationTag)annotationTagAtPoint:(CGPoint)point persistingResults:(BOOL)persist;
+@end
+
+@interface MGLAnnotationViewIntegrationTests : MGLMapViewIntegrationTest
+@end
+
+@implementation MGLAnnotationViewIntegrationTests
+
+- (void)testSelectingAnnotationWithCenterOffset {
+
+    for (CGFloat dx = -100.0; dx <= 100.0; dx += 100.0 ) {
+        for (CGFloat dy = -100.0; dy <= 100.0; dy += 100.0 ) {
+            CGVector offset = CGVectorMake(dx, dy);
+            [self internalTestSelectingAnnotationWithCenterOffsetWithOffset:offset];
+        }
+    }
+}
+
+- (void)internalTestSelectingAnnotationWithCenterOffsetWithOffset:(CGVector)offset {
+
+    NSString * const MGLTestAnnotationReuseIdentifer = @"MGLTestAnnotationReuseIdentifer";
+
+    CGFloat epsilon = 0.0000001;
+    CGSize size = self.mapView.bounds.size;
+
+    CGSize annotationSize = CGSizeMake(40.0, 40.0);
+
+    self.viewForAnnotation = ^MGLAnnotationView*(MGLMapView *view, id<MGLAnnotation> annotation) {
+
+        if (![annotation isKindOfClass:[MGLPointAnnotation class]]) {
+            return nil;
+        }
+
+        // No dequeue
+        MGLAnnotationView *annotationView = [[MGLAnnotationView alloc] initWithAnnotation:annotation reuseIdentifier:MGLTestAnnotationReuseIdentifer];
+        annotationView.bounds             = (CGRect){ .origin = CGPointZero, .size = annotationSize };
+        annotationView.backgroundColor    = UIColor.redColor;
+        annotationView.enabled            = YES;
+        annotationView.centerOffset       = offset;
+
+        return annotationView;
+    };
+
+    MGLPointAnnotation *point = [[MGLPointAnnotation alloc] init];
+    point.title = NSStringFromSelector(_cmd);
+    point.coordinate = CLLocationCoordinate2DMake(0.0, 0.0);
+    [self.mapView addAnnotation:point];
+
+    // From https://github.com/mapbox/mapbox-gl-native/issues/12259#issuecomment-401414168
+    //
+    //      queryRenderedFeatures depends on collision detection having been run
+    //      before it shows results [...]. Collision detection runs asynchronously
+    //      (at least every 300ms, sometimes more often), and therefore the results
+    //      of queryRenderedFeatures are similarly asynchronous.
+    //
+    // So, we need to wait before `annotationTagAtPoint:persistingResults:` will
+    // return out newly added annotation
+
+    [self waitForCollisionDetectionToRun];
+
+    // Check that the annotation is in the center of the view
+    CGPoint annotationPoint = [self.mapView convertCoordinate:point.coordinate toPointToView:self.mapView];
+    XCTAssertEqualWithAccuracy(annotationPoint.x, size.width/2.0, epsilon);
+    XCTAssertEqualWithAccuracy(annotationPoint.y, size.height/2.0, epsilon);
+
+    // Now test taps around the annotation
+    CGPoint tapPoint = CGPointMake(annotationPoint.x + offset.dx, annotationPoint.y + offset.dy);
+
+    MGLAnnotationTag tagAtPoint = [self.mapView annotationTagAtPoint:tapPoint persistingResults:YES];
+    XCTAssert(tagAtPoint != UINT32_MAX, @"Should have tapped on annotation");
+
+    CGPoint testPoints[] = {
+        { tapPoint.x - annotationSize.width, tapPoint.y },
+        { tapPoint.x + annotationSize.width, tapPoint.y },
+        { tapPoint.x, tapPoint.y - annotationSize.height },
+        { tapPoint.x, tapPoint.y + annotationSize.height },
+        CGPointZero
+    };
+
+    CGPoint *testPoint = testPoints;
+
+    while (!CGPointEqualToPoint(*testPoint, CGPointZero)) {
+        tagAtPoint = [self.mapView annotationTagAtPoint:*testPoints persistingResults:YES];
+        XCTAssert(tagAtPoint == UINT32_MAX, @"Tap should to the side of the annotation");
+        testPoint++;
+    }
+}
+
+- (void)waitForCollisionDetectionToRun {
+    XCTAssertNil(self.renderFinishedExpectation, @"Incorrect test setup");
+
+    self.renderFinishedExpectation = [self expectationWithDescription:@"Map view should be rendered"];
+    XCTestExpectation *timerExpired = [self expectationWithDescription:@"Timer expires"];
+
+    // Wait 1/2 second
+    dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(NSEC_PER_SEC >> 1)), dispatch_get_main_queue(), ^{
+        [timerExpired fulfill];
+    });
+
+    [self waitForExpectations:@[timerExpired, self.renderFinishedExpectation] timeout:1.0];
+}
+
+@end
diff --git a/platform/ios/Integration Tests/MGLMapViewIntegrationTest.h b/platform/ios/Integration Tests/MGLMapViewIntegrationTest.h
index 1b680f2b71..4884f30b77 100644
--- a/platform/ios/Integration Tests/MGLMapViewIntegrationTest.h
+++ b/platform/ios/Integration Tests/MGLMapViewIntegrationTest.h
@@ -22,6 +22,7 @@
 @property (nonatomic) MGLStyle *style;
 @property (nonatomic) XCTestExpectation *styleLoadingExpectation;
 @property (nonatomic) XCTestExpectation *renderFinishedExpectation;
+@property (nonatomic) MGLAnnotationView * (^viewForAnnotation)(MGLMapView *mapView, id<MGLAnnotation> annotation);
 @property (nonatomic) void (^regionWillChange)(MGLMapView *mapView, BOOL animated);
 @property (nonatomic) void (^regionIsChanging)(MGLMapView *mapView);
 @property (nonatomic) void (^regionDidChange)(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated);
diff --git a/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m b/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m
index 8fb41d0d11..f67150ba9b 100644
--- a/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m
+++ b/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m
@@ -37,6 +37,14 @@
 
 #pragma mark - MGLMapViewDelegate
 
+- (MGLAnnotationView*)mapView:(MGLMapView *)mapView viewForAnnotation:(id<MGLAnnotation>)annotation {
+    if (self.viewForAnnotation) {
+        return self.viewForAnnotation(mapView, annotation);
+    }
+    
+    return nil;
+}
+
 - (void)mapView:(MGLMapView *)mapView didFinishLoadingStyle:(MGLStyle *)style {
     XCTAssertNotNil(mapView.style);
     XCTAssertEqual(mapView.style, style);
diff --git a/platform/ios/ios.xcodeproj/project.pbxproj b/platform/ios/ios.xcodeproj/project.pbxproj
index bc7aa64293..e53e649c02 100644
--- a/platform/ios/ios.xcodeproj/project.pbxproj
+++ b/platform/ios/ios.xcodeproj/project.pbxproj
@@ -370,6 +370,7 @@
 		CA4EB8C720863487006AB465 /* MGLStyleLayerIntegrationTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CA4EB8C620863487006AB465 /* MGLStyleLayerIntegrationTests.m */; };
 		CA55CD41202C16AA00CE7095 /* MGLCameraChangeReason.h in Headers */ = {isa = PBXBuildFile; fileRef = CA55CD3E202C16AA00CE7095 /* MGLCameraChangeReason.h */; settings = {ATTRIBUTES = (Public, ); }; };
 		CA55CD42202C16AA00CE7095 /* MGLCameraChangeReason.h in Headers */ = {isa = PBXBuildFile; fileRef = CA55CD3E202C16AA00CE7095 /* MGLCameraChangeReason.h */; settings = {ATTRIBUTES = (Public, ); }; };
+		CA6914B520E67F50002DB0EE /* MGLAnnotationViewIntegrationTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CA6914B420E67F50002DB0EE /* MGLAnnotationViewIntegrationTests.m */; };
 		CAA69DA4206DCD0E007279CD /* Mapbox.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = DA4A26961CB6E795000B7809 /* Mapbox.framework */; };
 		CAA69DA5206DCD0E007279CD /* Mapbox.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = DA4A26961CB6E795000B7809 /* Mapbox.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
 		CABE5DAD2072FAB40003AF3C /* Mapbox.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = DA8847D21CBAF91600AB86E3 /* Mapbox.framework */; };
@@ -1005,6 +1006,7 @@
 		CA4EB8C620863487006AB465 /* MGLStyleLayerIntegrationTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MGLStyleLayerIntegrationTests.m; sourceTree = "<group>"; };
 		CA55CD3E202C16AA00CE7095 /* MGLCameraChangeReason.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MGLCameraChangeReason.h; sourceTree = "<group>"; };
 		CA5E5042209BDC5F001A8A81 /* MGLTestUtility.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = MGLTestUtility.h; path = ../../darwin/test/MGLTestUtility.h; sourceTree = "<group>"; };
+		CA6914B420E67F50002DB0EE /* MGLAnnotationViewIntegrationTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; name = MGLAnnotationViewIntegrationTests.m; path = "Annotation Tests/MGLAnnotationViewIntegrationTests.m"; sourceTree = "<group>"; };
 		DA00FC8C1D5EEB0D009AABC8 /* MGLAttributionInfo.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MGLAttributionInfo.h; sourceTree = "<group>"; };
 		DA00FC8D1D5EEB0D009AABC8 /* MGLAttributionInfo.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MGLAttributionInfo.mm; sourceTree = "<group>"; };
 		DA0CD58F1CF56F6A00A5F5A5 /* MGLFeatureTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = MGLFeatureTests.mm; path = ../../darwin/test/MGLFeatureTests.mm; sourceTree = "<group>"; };
@@ -1352,6 +1354,7 @@
 		16376B081FFD9DAF0000563E /* Integration Tests */ = {
 			isa = PBXGroup;
 			children = (
+				CA6914B320E67F07002DB0EE /* Annotations */,
 				CA1B4A4F2099FA2800EDD491 /* Snapshotter Tests */,
 				16376B091FFD9DAF0000563E /* MBGLIntegrationTests.m */,
 				16376B0B1FFD9DAF0000563E /* Info.plist */,
@@ -1715,6 +1718,14 @@
 			path = "Snapshotter Tests";
 			sourceTree = "<group>";
 		};
+		CA6914B320E67F07002DB0EE /* Annotations */ = {
+			isa = PBXGroup;
+			children = (
+				CA6914B420E67F50002DB0EE /* MGLAnnotationViewIntegrationTests.m */,
+			);
+			name = Annotations;
+			sourceTree = "<group>";
+		};
 		DA1DC9411CB6C1C2006E619F = {
 			isa = PBXGroup;
 			children = (
@@ -2809,6 +2820,7 @@
 				16376B0A1FFD9DAF0000563E /* MBGLIntegrationTests.m in Sources */,
 				CA0C27942076CA19001CE5B7 /* MGLMapViewIntegrationTest.m in Sources */,
 				CA0C27922076C804001CE5B7 /* MGLShapeSourceTests.m in Sources */,
+				CA6914B520E67F50002DB0EE /* MGLAnnotationViewIntegrationTests.m in Sources */,
 				CA1B4A512099FB2200EDD491 /* MGLMapSnapshotterTest.m in Sources */,
 			);
 			runOnlyForDeploymentPostprocessing = 0;
diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm
index 1bbda94387..14f6960f26 100644
--- a/platform/ios/src/MGLMapView.mm
+++ b/platform/ios/src/MGLMapView.mm
@@ -1658,23 +1658,6 @@ public:
         }
     }
     
-    // Handle the case of an offset annotation view by converting the tap point to be the geo location
-    // of the annotation itself that the view represents
-    for (MGLAnnotationView *view in self.annotationContainerView.annotationViews)
-    {
-        if (view.centerOffset.dx != 0 || view.centerOffset.dy != 0) {
-            if (CGRectContainsPoint(view.frame, tapPoint)) {
-                if (!view.annotation) {
-                    [NSException raise:NSInvalidArgumentException
-                                format:@"Annotation view's annotation property should not be nil."];
-                }
-                
-                CGPoint annotationPoint = [self convertCoordinate:view.annotation.coordinate toPointToView:self];
-                tapPoint = annotationPoint;
-            }
-        }
-    }
-
     MGLAnnotationTag hitAnnotationTag = [self annotationTagAtPoint:tapPoint persistingResults:persist];
     if (hitAnnotationTag != MGLAnnotationTagNotFound)
     {
@@ -3819,8 +3802,12 @@ public:
         annotationView.mapView = self;
         CGRect bounds = UIEdgeInsetsInsetRect({ CGPointZero, annotationView.frame.size }, annotationView.alignmentRectInsets);
 
-        _largestAnnotationViewSize = CGSizeMake(MAX(_largestAnnotationViewSize.width, CGRectGetWidth(bounds)),
-                                                MAX(_largestAnnotationViewSize.height, CGRectGetHeight(bounds)));
+        // Take any offset into consideration
+        CGFloat adjustedAnnotationWidth = CGRectGetWidth(bounds) + fabs(annotationView.centerOffset.dx);
+        CGFloat adjustedAnnotationHeight = CGRectGetHeight(bounds) + fabs(annotationView.centerOffset.dx);
+
+        _largestAnnotationViewSize = CGSizeMake(MAX(_largestAnnotationViewSize.width, adjustedAnnotationWidth),
+                                                MAX(_largestAnnotationViewSize.height, adjustedAnnotationHeight));
 
         _unionedAnnotationRepresentationSize = CGSizeMake(MAX(_unionedAnnotationRepresentationSize.width, _largestAnnotationViewSize.width),
                                                           MAX(_unionedAnnotationRepresentationSize.height, _largestAnnotationViewSize.height));
@@ -4089,9 +4076,15 @@ public:
                 {
                     return true;
                 }
-                
+
                 CGPoint calloutAnchorPoint = MGLPointRounded([self convertCoordinate:annotation.coordinate toPointToView:self]);
                 CGRect frame = CGRectInset({ calloutAnchorPoint, CGSizeZero }, -CGRectGetWidth(annotationView.frame) / 2, -CGRectGetHeight(annotationView.frame) / 2);
+
+                // We need to take any offset into consideration. Note that a large offset will result in a
+                // large value for `_unionedAnnotationRepresentationSize` (and thus a larger feature query rect).
+                // Aim to keep the offset as small as possible.
+                frame = CGRectOffset(frame, annotationView.centerOffset.dx, annotationView.centerOffset.dy);
+
                 annotationRect = UIEdgeInsetsInsetRect(frame, annotationView.alignmentRectInsets);
             }
             else