From 3710ef8810bdab2d2e7f261c3b5c5e4053a279c9 Mon Sep 17 00:00:00 2001 From: Julian Rex Date: Mon, 9 Jul 2018 10:01:11 -0400 Subject: [PATCH] [iOS] Update annotation view touch handling (with offsets) (#12234) --- platform/ios/CHANGELOG.md | 1 + .../MGLAnnotationViewIntegrationTests.m | 108 ++++++++++++++++++ .../MGLMapViewIntegrationTest.h | 1 + .../MGLMapViewIntegrationTest.m | 8 ++ platform/ios/ios.xcodeproj/project.pbxproj | 12 ++ platform/ios/src/MGLMapView.mm | 33 +++--- 6 files changed, 143 insertions(+), 20 deletions(-) create mode 100644 platform/ios/Integration Tests/Annotation Tests/MGLAnnotationViewIntegrationTests.m diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md index 83a2d0ef950..0f6aba3cfd3 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 00000000000..ebd587ebe69 --- /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 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 1b680f2b712..4884f30b774 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 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 8fb41d0d114..f67150ba9bc 100644 --- a/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m +++ b/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m @@ -37,6 +37,14 @@ - (void)tearDown { #pragma mark - MGLMapViewDelegate +- (MGLAnnotationView*)mapView:(MGLMapView *)mapView viewForAnnotation:(id)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 bc7aa64293d..e53e649c02c 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 = ""; }; CA55CD3E202C16AA00CE7095 /* MGLCameraChangeReason.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MGLCameraChangeReason.h; sourceTree = ""; }; CA5E5042209BDC5F001A8A81 /* MGLTestUtility.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = MGLTestUtility.h; path = ../../darwin/test/MGLTestUtility.h; sourceTree = ""; }; + CA6914B420E67F50002DB0EE /* MGLAnnotationViewIntegrationTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; name = MGLAnnotationViewIntegrationTests.m; path = "Annotation Tests/MGLAnnotationViewIntegrationTests.m"; sourceTree = ""; }; DA00FC8C1D5EEB0D009AABC8 /* MGLAttributionInfo.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MGLAttributionInfo.h; sourceTree = ""; }; DA00FC8D1D5EEB0D009AABC8 /* MGLAttributionInfo.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MGLAttributionInfo.mm; sourceTree = ""; }; DA0CD58F1CF56F6A00A5F5A5 /* MGLFeatureTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = MGLFeatureTests.mm; path = ../../darwin/test/MGLFeatureTests.mm; sourceTree = ""; }; @@ -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 = ""; }; + CA6914B320E67F07002DB0EE /* Annotations */ = { + isa = PBXGroup; + children = ( + CA6914B420E67F50002DB0EE /* MGLAnnotationViewIntegrationTests.m */, + ); + name = Annotations; + sourceTree = ""; + }; 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 1bbda94387e..14f6960f264 100644 --- a/platform/ios/src/MGLMapView.mm +++ b/platform/ios/src/MGLMapView.mm @@ -1658,23 +1658,6 @@ - (void)handleSingleTapGesture:(UITapGestureRecognizer *)singleTap } } - // 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 @@ - (MGLAnnotationView *)annotationViewForAnnotation:(id)annotation 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 @@ - (MGLAnnotationTag)annotationTagAtPoint:(CGPoint)point persistingResults:(BOOL) { 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