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

If there are too many annotation images, some annotation images are invisible #3185

Closed
skajake opened this issue Dec 4, 2015 · 23 comments · Fixed by #9213
Closed

If there are too many annotation images, some annotation images are invisible #3185

skajake opened this issue Dec 4, 2015 · 23 comments · Fixed by #9213
Labels
bug iOS Mapbox Maps SDK for iOS performance Speed, stability, CPU usage, memory usage, or power usage

Comments

@skajake
Copy link

skajake commented Dec 4, 2015

My use case:
I have 19,000 annotations spread across the US.
Each annotation has a unique image.
I only show 20 annotations at a time as the user pans.

Problem:
After panning for awhile and loading a few hundred annotations (not all at once), it fails to show any more.

Here is how I remove and add annotations as the user pans:

-(void)mapView:(MGLMapView *)mapView regionDidChangeAnimated:(BOOL)animated {
    //This method only returns 20 objects
    NSArray *myObjects = [dummyService getObjectsInVisibleRect:[mapView visibleCoordinateBounds].sw ne:[mapView visibleCoordinateBounds].ne];
    NSMutableArray *newAnnotations = [[NSMutableArray alloc] init];
    for(MyObject *myObject in myObjects) {
        MGLPointAnnotation *point = [[MGLPointAnnotation alloc] init];
        point.coordinate = [[myObject location] coordinate];
        point.title = [myObject myIdentifier];
        point.subtitle = [myObject myName];
        [newAnnotations addObject:point];
    }
    NSArray *existingAnnotations = [_mapView annotations];
    NSMutableArray *annotationsToRemove = [[NSMutableArray alloc] init];
    for(MGLPointAnnotation *existingAnnotation in existingAnnotations) {
        //I actually subclass MGLPointAnnotation, such that isEqual and hash are implemented.  In any case, the containsObject logic here works as intended
        if([newAnnotations containsObject:existingAnnotation]) {
            [newAnnotations removeObject:existingAnnotation];
        } else {
            [annotationsToRemove addObject:existingAnnotation];
        }
    }
    if([annotationsToRemove count] > 0) {
        [_mapView removeAnnotations:annotationsToRemove];
    }
    if([newAnnotations count] > 0) {
        [_mapView addAnnotations:newAnnotations];
    }
}

To be clear, the imageForAnnotation method fires, and a valid MGLAnnotationImage (inited with UIImage and a unique reuseIdentifier) is returned. However, the annotations stop showing up on the map.

@1ec5 1ec5 added iOS Mapbox Maps SDK for iOS performance Speed, stability, CPU usage, memory usage, or power usage labels Dec 4, 2015
@1ec5
Copy link
Contributor

1ec5 commented Dec 4, 2015

I actually subclass MGLPointAnnotation, such that isEqual and hash are implemented.

Oh, good catch. Please feel free to submit a pull request with that functionality.

The iosapp demo application loads up to 10,000 points more or less simultaneously, and the only hard limit I’m aware of is UINT32_MAX (due to the type used for annotation IDs internally). So it sounds like this might be related to the overall number of additions and removals over time. Another possibility is that you’re running into a different form of #2956.

@jfirebaugh
Copy link
Contributor

I think this is a duplicate of #1883.

@skajake
Copy link
Author

skajake commented Dec 5, 2015

I tried using the same reuseIdentifier (and by extension image) and it has no trouble showing as many annotations as I want. Is there a way to disable caching, or whatever the reuse identifier is used for?

@s0meone
Copy link

s0meone commented Dec 13, 2015

I'm having the same issue. Just built from source (d11d53d).

The workaround described in #2693 (comment) is not working for me. It looks like it's the same issue as #1883 or #2956.

I'm clustering around 16k annotations and only show 16 annotations at the same time, which are removed and added every time the map region changes. Everything works fine when using a single image with a single reuse id. But when I'm using unique reuse ids, annotations start disappearing.

out

@s0meone
Copy link

s0meone commented Dec 13, 2015

Tried to debug this some more. The problem only seems to exist when there are a lot of different reuse ids. It doesn't matter if the image is drawn using UIGraphicsGetImageFromCurrentImageContext or loaded from file with UIImage(named: "test").

@skajake
Copy link
Author

skajake commented Dec 21, 2015

This seems fairly trivial to reproduce in the iosapp.

Here is how the app looks with 10,000 annotations with the current logic that uses a reuse identifier of 2 characters (99 permutations)
screen shot 2015-12-20 at 9 17 03 pm

Here is how the app looks when you change the reuse identifier to be the entire string, not just the last 2 characters:
screen shot 2015-12-20 at 9 17 40 pm

This was accomplished by changing line 482 of MBXViewController.mm to:

NSString *lastTwoCharacters = title;  //[title substringFromIndex:title.length - 2];

@JohnRbk
Copy link

JohnRbk commented Dec 21, 2015

FWIW, I'm able to reproduce this as well. Some observations:

  1. For the annotations that don't render, its interesting that the callout views still do appear when the annotation is selected. (Try using @skajake's sample code to reproduce the issue; click on the areas with missing annotations to see the callouts appear)
  2. While debugging in Xcode, I see that all the images returned by dequeueReusableAnnotationImageWithIdentifier are all valid images. For whatever reason, they do not all render on the map.

Let me know if anyone has found a workaround.

@ChrisLowe-Takor
Copy link

I've got a sample project at https://github.com/ChrisLowe-Takor/MapboxAnnotationLimitBug showing this bug.

screen shot 2016-01-08 at 10 11 26 am

This should be a 20x20 grid of numbered annotations. As @JohnRbk mentioned imageForAnnotation correctly returns the desired image and the accessory callout can be clicked but the annotation isn't rendered on the map.

I've played around with it a little and found that using smaller graphics increases the number of custom annotations that can be shown (over 100 in this demo, whilst my app bombs out around 30).

@1ec5
Copy link
Contributor

1ec5 commented Jan 8, 2016

The root cause of this bug is #489. The sprite image data is still present, which is why hit testing continues to work. However, the SpriteAtlas is failing to allocate a new image based on the data, because with more annotation images or much larger annotation images, we start to run up against the 512×512 sprite sheet limit.

@jfirebaugh @ansis, can we at least increase the limit to 1024×1024 to mitigate #489?

@1ec5
Copy link
Contributor

1ec5 commented Jan 8, 2016

I’m wondering whether another workaround would be for either MGLMapView or client code to remove annotation images that are unneeded (i.e., out of view), now that we have #3146 #3320. That’d only address some of the examples posted above, unfortunately.

@1ec5 1ec5 changed the title Annotation Limit If there are too many annotation images, some annotation images are invisible Jan 8, 2016
@1ec5 1ec5 added this to the ios-v3.1.0 milestone Jan 14, 2016
@1ec5
Copy link
Contributor

1ec5 commented Jan 14, 2016

Unfortunately, #3530 exacerbates this bug. Now that sprites in the SpriteAtlas are unscaled, on a 2× display, we’re only able to squeeze in something like a fourth of the sprites. Even the “Add 100 Points” demo runs up against the limit:

100

@1ec5 1ec5 added the bug label Jan 14, 2016
@jfirebaugh
Copy link
Contributor

@1ec5 Can you elaborate? It seems like the only case in which #3530 would exacerbate this bug was if you were supplying lots of @2x icons on a device with @1x pixel ratio, which seems uncommon. The more common case of supplying @1x icons on a @2x device should actually consume less space in the atlas, since they no longer get upscaled.

@mb12
Copy link

mb12 commented Jan 14, 2016

We can set debug::spriteWarnings to true to troubleshoot this. It will spew out a warning for SpriteAtlas overflow. IMHO, we should change the default value of this to true since it can happen even without annotations (Its more serious in that case since the map will silently be missing some icons).

@1ec5
Copy link
Contributor

1ec5 commented Jan 14, 2016

@jfirebaugh, my bad, this regression wasn’t caused by #3530; I simply misremembered, expecting the 100 points to be distributed in the same way as in the 10,000-point demo. In fact, the stock demos are fine, and the steps in #3185 (comment) reproduce exactly as before:

10000

Sorry for the false alarm.

@ChrisLowe-Takor
Copy link

My project is severely hampered by this bug so I'll put my 2c.

  • Logging the failed allocation as @mb12 is preferable to silently failing.
  • Increasing the size of the sprite sheet helps but doesn't really solve it.
  • Being able to remove annotations and free up it's place in the sprite sheet is a good compromise. Ideally it would be handled automatically when an annotation is off-screen but I'd settle for leaving this up to the client. If it is left up to the client there should be a hook in the MGLMapViewDelegate so they can attempt to deal with it.
- (void) mapView:(MGLMapView *)mapView didFailToAllocationAnnotationImage: (id<MGLAnnotation) annotation
  • Ideally, the sprite sheet would resize when it runs out of space.

@1ec5
Copy link
Contributor

1ec5 commented Jan 26, 2016

#3703 increases the SpriteAtlas size to 1024×1024.

I’m still working on a way for developers to remove annotation images, building off #3146. The main holdup right now is that annotations whose images have been deleted wind up producing “sprite not found” messages in the console (and hurting performance slightly, probably related). So I’m looking at replacing the deleted image with the default image. Failing that, I’ll have MGLMapView automatically remove those annotations and readd them with the default annotation image.

@1ec5 1ec5 self-assigned this Jan 28, 2016
1ec5 added a commit that referenced this issue Feb 5, 2016
Added an API for deleting unused annotation images’ images. When you nil out the image of an MGLAnnotationImage, MGLMapView deletes the sprite from the style and recreates any annotation associated with the MGLAnnotationImage instance; the MGLAnnotationImage’s falls back to SDK’s default annotation image.

In iosapp, deselecting an annotation resets its image to the default; deselecting it again restores the image.

ref #3185
@1ec5
Copy link
Contributor

1ec5 commented Feb 5, 2016

I’m still working on a way for developers to remove annotation images, building off #3146.

Proposed in #3835. Given the high impact, however, I’m putting that PR on the 3.2.0 milestone. 3.1.0 is going to ship with a 1024×1024 SpriteAtlas limitation.

@1ec5 1ec5 modified the milestones: ios-v3.2.0, ios-v3.1.0 Feb 5, 2016
@postmechanical
Copy link

Just ran into this issue, even though we add and remove annotations as need when the map region changes.

1ec5 added a commit that referenced this issue Apr 17, 2016
Added an API for deleting unused annotation images’ images. When you nil out the image of an MGLAnnotationImage, MGLMapView deletes the sprite from the style and recreates any annotation associated with the MGLAnnotationImage instance; the MGLAnnotationImage’s falls back to SDK’s default annotation image.

In iosapp, deselecting an annotation resets its image to the default; deselecting it again restores the image.

ref #3185
1ec5 added a commit that referenced this issue Apr 19, 2016
Added an API for deleting unused annotation images’ images. When you nil out the image of an MGLAnnotationImage, MGLMapView deletes the sprite from the style and recreates any annotation associated with the MGLAnnotationImage instance; the MGLAnnotationImage’s falls back to SDK’s default annotation image.

In iosapp, deselecting an annotation resets its image to the default; deselecting it again restores the image.

ref #3185
@1ec5
Copy link
Contributor

1ec5 commented May 13, 2016

#3835 landed, by the way, but the real solution for the large-image use case will come with #4801.

@1ec5
Copy link
Contributor

1ec5 commented May 27, 2016

#4801 landed. As of the iOS SDK v3.3.0 alphas, if you run into this issue, you have several recourses:

  • If you’re still relocating a point annotation by removing the annotation and readding it to the map view, you should instead set the MGLAnnotation’s coordinate property.
  • Similarly, if you want to change the appearance of a point annotation, set its MGLAnnotationImage’s image property in a KVO-compliant way.
  • Consider using MGLAnnotationView instead of MGLAnnotationImage. Depending on your usage, MGLAnnotationView may have poorer performance than MGLAnnotationImage (which is rendered in OpenGL), but as a native UIView subclass, it behaves exactly as you’d expect with respect to images.

The bug that this ticket tracks is still present in the SDK, but with these workarounds, this is as far as we expect to take the v3.3.0 milestone.

@ppoh71
Copy link

ppoh71 commented Aug 11, 2016

have a same issue. ios/swift/mapbox v.3.3.3

i generate many different images on the fly with CGContext for different annotations.
as soon as i have more than around 500 unique reuseIdentifier for the annotations image, everything above 500 isn't displayed anymore. annotation is set, but the image not.

extension showRouteController: MGLMapViewDelegate {

func mapView(mapView: MGLMapView, imageForAnnotation annotation: MGLAnnotation) -> MGLAnnotationImage? {

        var image: UIImage
        var reuseIdentifier = ""

        //reuse identifier
        switch(self.funcType) {

        case .PrintMarker:
            reuseIdentifier =  "MarkerSpeed\(utils.getSpeed(globalSpeed.gSpeed))-1"
        case .PrintBaseHeight:
            reuseIdentifier =  "MarkerSpeedBase\(utils.getSpeed(globalSpeed.gSpeed))-2"    
        case .PrintAltitude:
            reuseIdentifier =  "MarkerAlt\(Int(round(globalAltitude.gAltitude)))-3"        
        case .Recording:
            reuseIdentifier =  "MarkerCircleSpeed\(utils.getSpeed(globalSpeed.gSpeed))-4"        
        case .PrintCircles:
            reuseIdentifier =  "MarkerCircle\(utils.getSpeed(globalSpeed.gSpeed))-5"         
        default:
            print("marker image default break")
            break   
        }

        var annotationImage = mapView.dequeueReusableAnnotationImageWithIdentifier(reuseIdentifier)

        if annotationImage == nil { 
              image = imageUtils.drawLineOnImage(self.funcType)
              annotationImage = MGLAnnotationImage(image: image, reuseIdentifier: reuseIdentifier)
        }
         return annotationImage
    }

@friedbunny
Copy link
Contributor

Since iOS SDK v3.3.x added view-based annotations, that’s our suggested workaround for use cases that require many unique or large images. You can see a basic implemention example here.

There are performance trade-offs, but views are generally more flexible and nicer to work with, anyway. 😉

@friedbunny
Copy link
Contributor

To make sure the loop here is obviously and fully closed: the fix for this issue will be available in iOS 3.7.0, pre-releases of which are available now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug iOS Mapbox Maps SDK for iOS performance Speed, stability, CPU usage, memory usage, or power usage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants