Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Commit

Permalink
[ios] Guard against annotation reuse when there is no identifier
Browse files Browse the repository at this point in the history
This protects against a crash that would occur when view reuse was
attempted in cases where the map view delegate did not ask for it.
It also fixes a bug where annotation views were attempted when
the map view delegate did not implement the correct delegate method.

It also moves the call to `updateAnnotationViews` back to
`glkView:drawInRect` since doing it in `notifyMapChange`
introduced lag. Additional optimizations may occur in future commits
to make the work required to hide and show views and handle view
reuse less intensive.

In addition, the example MBXAnnotationView is simplified to just
a programatic view with a center panel that can change color. The
color changing center panel is used to indicate if a vie has
gone through the reuse queue.
  • Loading branch information
boundsj committed May 11, 2016
1 parent 9560691 commit 684480a
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 62 deletions.
18 changes: 7 additions & 11 deletions platform/ios/app/MBXAnnotationView.m
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,20 @@

@interface MBXAnnotationView ()

@property (weak, nonatomic) IBOutlet UIView *centerView;
@property (nonatomic) UIView *centerView;

@end

@implementation MBXAnnotationView

- (instancetype)initWithCoder:(NSCoder *)coder
{
self = [super initWithCoder:coder];
if (self) {
self.layer.cornerRadius = CGRectGetWidth(self.bounds) / 2.0;
}
return self;
}

- (void)layoutSubviews {
[super layoutSubviews];
self.centerView.layer.cornerRadius = CGRectGetWidth(self.centerView.bounds) / 2.0;
if (!self.centerView) {
self.backgroundColor = [UIColor blueColor];
self.centerView = [[UIView alloc] initWithFrame:CGRectMake(5,5, CGRectGetWidth(self.bounds) - 10, CGRectGetWidth(self.bounds) - 10)];
self.centerView.backgroundColor = self.centerColor;
[self addSubview:self.centerView];
}
}

- (void)setCenterColor:(UIColor *)centerColor {
Expand Down
28 changes: 0 additions & 28 deletions platform/ios/app/MBXAnnotationView.xib

This file was deleted.

7 changes: 5 additions & 2 deletions platform/ios/app/MBXViewController.m
Original file line number Diff line number Diff line change
Expand Up @@ -612,10 +612,13 @@ - (void)dealloc
- (MGLAnnotationView *)mapView:(MGLMapView *)mapView viewForAnnotation:(id<MGLAnnotation>)annotation
{
MBXAnnotationView *annotationView = (MBXAnnotationView *)[mapView dequeueReusableAnnotationViewWithIdentifier:@"MBXAnnotationView"];
annotationView.centerColor = [UIColor redColor];
if (!annotationView)
{
annotationView = [[[NSBundle mainBundle] loadNibNamed:@"MBXAnnotationView" owner:self options:nil] objectAtIndex:0];
annotationView = [[MBXAnnotationView alloc] initWithFrame:CGRectMake(0, 0, 40, 40)];
annotationView.centerColor = [UIColor whiteColor];
} else {
// orange indicates that the annotation view was reused
annotationView.centerColor = [UIColor orangeColor];
}
return annotationView;
}
Expand Down
3 changes: 0 additions & 3 deletions platform/ios/ios.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
objects = {

/* Begin PBXBuildFile section */
400165041CCFC618005583CC /* MBXAnnotationView.xib in Resources */ = {isa = PBXBuildFile; fileRef = 400165031CCFC618005583CC /* MBXAnnotationView.xib */; };
40FC668F1CCE8D6000AFCF7D /* MGLAnnotationView_Private.h in Headers */ = {isa = PBXBuildFile; fileRef = 40FC668B1CCE8CFC00AFCF7D /* MGLAnnotationView_Private.h */; };
40FC66901CCE8D6A00AFCF7D /* MGLAnnotationView.h in Headers */ = {isa = PBXBuildFile; fileRef = 40FC668C1CCE8D4F00AFCF7D /* MGLAnnotationView.h */; settings = {ATTRIBUTES = (Public, ); }; };
40FC66911CCE8D6A00AFCF7D /* MGLAnnotationView.m in Sources */ = {isa = PBXBuildFile; fileRef = 40FC668D1CCE8D4F00AFCF7D /* MGLAnnotationView.m */; };
Expand Down Expand Up @@ -311,7 +310,6 @@
/* End PBXCopyFilesBuildPhase section */

/* Begin PBXFileReference section */
400165031CCFC618005583CC /* MBXAnnotationView.xib */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = file.xib; path = MBXAnnotationView.xib; sourceTree = "<group>"; };
40FC668B1CCE8CFC00AFCF7D /* MGLAnnotationView_Private.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = MGLAnnotationView_Private.h; path = src/MGLAnnotationView_Private.h; sourceTree = "<group>"; };
40FC668C1CCE8D4F00AFCF7D /* MGLAnnotationView.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = MGLAnnotationView.h; path = src/MGLAnnotationView.h; sourceTree = "<group>"; };
40FC668D1CCE8D4F00AFCF7D /* MGLAnnotationView.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = MGLAnnotationView.m; path = src/MGLAnnotationView.m; sourceTree = "<group>"; };
Expand Down Expand Up @@ -558,7 +556,6 @@
DA1DC9981CB6E054006E619F /* MBXAppDelegate.m */,
40FDA7691CCAAA6800442548 /* MBXAnnotationView.h */,
40FDA76A1CCAAA6800442548 /* MBXAnnotationView.m */,
400165031CCFC618005583CC /* MBXAnnotationView.xib */,
DA1DC9661CB6C6B7006E619F /* MBXCustomCalloutView.h */,
DA1DC9671CB6C6B7006E619F /* MBXCustomCalloutView.m */,
DA1DC9681CB6C6B7006E619F /* MBXOfflinePacksTableViewController.h */,
Expand Down
50 changes: 32 additions & 18 deletions platform/ios/src/MGLMapView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -861,6 +861,7 @@ - (void)glkView:(__unused GLKView *)view drawInRect:(__unused CGRect)rect
{
_mbglMap->render();

[self updateAnnotationViews];
[self updateUserLocationAnnotationView];
}
}
Expand Down Expand Up @@ -2792,8 +2793,9 @@ - (void)addAnnotations:(NS_ARRAY_OF(id <MGLAnnotation>) *)annotations

if (delegateImplementsViewForAnnotation)
{
annotationView = [self addAnnotationViewWithAnnotation:annotation];
if (annotationView) {
annotationView = [self annotationViewForAnnotation:annotation];
if (annotationView)
{
annotationViewsForAnnotation[annotationValue] = annotationView;
annotationView.center = [self convertCoordinate:annotation.coordinate toPointToView:self];
[self.glView addSubview:annotationView];
Expand Down Expand Up @@ -2901,7 +2903,7 @@ - (MGLAnnotationImage *)defaultAnnotationImage
return annotationImage;
}

- (MGLAnnotationView *)addAnnotationViewWithAnnotation:(id<MGLAnnotation>)annotation
- (MGLAnnotationView *)annotationViewForAnnotation:(id<MGLAnnotation>)annotation
{
MGLAnnotationView *annotationView = (MGLAnnotationView *)[self.delegate mapView:self viewForAnnotation:annotation];

Expand Down Expand Up @@ -4236,9 +4238,6 @@ - (void)notifyMapChange:(mbgl::MapChange)change
{
[self.delegate mapViewRegionIsChanging:self];
}

[self updateAnnotationViews];

break;
}
case mbgl::MapChangeRegionDidChange:
Expand Down Expand Up @@ -4326,21 +4325,29 @@ - (void)updateUserLocationAnnotationView

- (void)updateAnnotationViews
{
BOOL delegateImplementsViewForAnnotation = [self.delegate respondsToSelector:@selector(mapView:viewForAnnotation:)];

if (!delegateImplementsViewForAnnotation)
{
return;
}

// Update all visible annotation views
std::set<MGLAnnotationTag> visibleTags;
std::vector<MGLAnnotationTag> annotationTags = [self annotationTagsInRect:CGRectInset(self.bounds, -50, -50)];
std::vector<MGLAnnotationTag> annotationTags = [self annotationTagsInRect:self.bounds];

for(auto const& annotationTag: annotationTags)
{
auto &annotationContext = _annotationContextsByAnnotationTag[annotationTag];
id<MGLAnnotation> annotation = annotationContext.annotation;

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

// If there is no annotation view at this point, it means the context's view was reused by some
// other context so we need to reuse or make a new view. If we do get here, it does mean
// that th delegate supports annotation views so it is safe to ask it for another one
// other context so we need to reuse or make a new view.
if (!annotationContext.annotationView)
{
MGLAnnotationView *annotationView = [self addAnnotationViewWithAnnotation:annotation];
MGLAnnotationView *annotationView = [self annotationViewForAnnotation:annotation];

if (annotationView)
{
Expand All @@ -4353,27 +4360,34 @@ - (void)updateAnnotationViews
annotationContext.annotationView = annotationView;
}
}

annotationContext.annotationView.center = annotationPoint;
annotationContext.annotationView.hidden = NO;

visibleTags.insert(annotationTag);
}

// Add offscreen annotation views to reuse queue
// Hide and add offscreen annotation views to reuse queue
for (auto &pair : _annotationContextsByAnnotationTag)
{
MGLAnnotationTag annotationTag = pair.first;
MGLAnnotationContext &annotationContext = pair.second;
MGLAnnotationView *annotationView = annotationContext.annotationView;
NSMutableArray *annotationViewReuseQueue = [self annotationViewReuseQueueForIdentifier:annotationContext.viewReuseIdentifier];

const bool tagIsNotVisible = visibleTags.find(annotationTag) == visibleTags.end();

if (tagIsNotVisible && annotationView && ![annotationViewReuseQueue containsObject:annotationView])
if (annotationView && tagIsNotVisible)
{
[annotationViewReuseQueue addObject:annotationView];
annotationView.hidden = YES;
annotationContext.annotationView = NULL;

if (annotationContext.viewReuseIdentifier)
{
NSMutableArray *annotationViewReuseQueue = [self annotationViewReuseQueueForIdentifier:annotationContext.viewReuseIdentifier];
if (![annotationViewReuseQueue containsObject:annotationView])
{
[annotationViewReuseQueue addObject:annotationView];
annotationContext.annotationView = nil;
}
}
}
}
}
Expand Down

0 comments on commit 684480a

Please sign in to comment.