Changed OpenGL retain management to use NSMutableSet.upstream/ahm-custom-layer-fix

6 files changed, 28 insertions, 21 deletions

diff --git a/include/mbgl/style/layers/custom_layer.hpp b/include/mbgl/style/layers/custom_layer.hpp
index 0eb2f27852..bf3387f95b 100644
--- a/include/mbgl/style/layers/custom_layer.hpp
+++ b/include/mbgl/style/layers/custom_layer.hpp
@@ -2,8 +2,6 @@
 
 #include <mbgl/style/layer.hpp>
 
-#include <functional>
-
 namespace mbgl {
 namespace style {
 
@@ -46,7 +44,7 @@ using CustomLayerRenderFunction = void (*)(void* context, const CustomLayerRende
  * `CustomLayerInitializeFunction` will be called instead to prepare for a new render.
  *
  */
-using CustomLayerContextLostFunction = std::function<void(void*)>;
+using CustomLayerContextLostFunction = void (*)(void* context);
 
 /**
  * Destroy any GL state needed by the custom layer, and deallocate context, if necessary. This
diff --git a/platform/darwin/src/MGLOpenGLStyleLayer.mm b/platform/darwin/src/MGLOpenGLStyleLayer.mm
index 5d7beac040..35acdd5d08 100644
--- a/platform/darwin/src/MGLOpenGLStyleLayer.mm
+++ b/platform/darwin/src/MGLOpenGLStyleLayer.mm
@@ -16,7 +16,7 @@
  */
 void MGLPrepareCustomStyleLayer(void *context) {
     MGLOpenGLStyleLayer *layer = (__bridge MGLOpenGLStyleLayer *)context;
-    (__bridge_retained void *)layer;
+    (void)(__bridge_retained void *)layer;
     [layer didMoveToMapView:layer.style.mapView];
 }
 
@@ -117,9 +117,9 @@ void MGLFinishCustomStyleLayer(void *context) {
         [NSException raise:@"MGLLayerReuseException"
                     format:@"%@ cannot be added to more than one MGLStyle at a time.", self];
     }
-    _style.openGLLayers[self.identifier] = nil;
+    [_style.openGLLayers removeObject:self];
     _style = style;
-    _style.openGLLayers[self.identifier] = self;
+    [_style.openGLLayers addObject:self];
 }
 
 - (void)addToStyle:(MGLStyle *)style belowLayer:(MGLStyleLayer *)otherLayer {
diff --git a/platform/darwin/src/MGLStyle.mm b/platform/darwin/src/MGLStyle.mm
index 0106ea7601..892a22b834 100644
--- a/platform/darwin/src/MGLStyle.mm
+++ b/platform/darwin/src/MGLStyle.mm
@@ -83,7 +83,7 @@
 @property (nonatomic, readonly, weak) MGLMapView *mapView;
 @property (nonatomic, readonly) mbgl::style::Style *rawStyle;
 @property (readonly, copy, nullable) NSURL *URL;
-@property (nonatomic, readwrite, strong) NS_MUTABLE_DICTIONARY_OF(NSString *, MGLOpenGLStyleLayer *) *openGLLayers;
+@property (nonatomic, readwrite, strong) NS_MUTABLE_SET_OF(MGLOpenGLStyleLayer *) *openGLLayers;
 @property (nonatomic) NS_MUTABLE_DICTIONARY_OF(NSString *, NS_DICTIONARY_OF(NSObject *, MGLTextLanguage *) *) *localizedLayersByIdentifier;
 
 @end
@@ -175,7 +175,7 @@ static NSURL *MGLStyleURL_trafficNight;
     if (self = [super init]) {
         _mapView = mapView;
         _rawStyle = rawStyle;
-        _openGLLayers = [NSMutableDictionary dictionary];
+        _openGLLayers = [NSMutableSet set];
         _localizedLayersByIdentifier = [NSMutableDictionary dictionary];
     }
     return self;
diff --git a/platform/darwin/src/MGLStyle_Private.h b/platform/darwin/src/MGLStyle_Private.h
index 3597a0a0a6..668f4e1710 100644
--- a/platform/darwin/src/MGLStyle_Private.h
+++ b/platform/darwin/src/MGLStyle_Private.h
@@ -25,7 +25,7 @@ namespace mbgl {
 @property (nonatomic, readonly) mbgl::style::Style *rawStyle;
 
 - (nullable NS_ARRAY_OF(MGLAttributionInfo *) *)attributionInfosWithFontSize:(CGFloat)fontSize linkColor:(nullable MGLColor *)linkColor;
-@property (nonatomic, readonly, strong) NS_MUTABLE_DICTIONARY_OF(NSString *, MGLOpenGLStyleLayer *) *openGLLayers;
+@property (nonatomic, readonly, strong) NS_MUTABLE_SET_OF(MGLOpenGLStyleLayer *) *openGLLayers;
 - (void)setStyleClasses:(NS_ARRAY_OF(NSString *) *)appliedClasses transitionDuration:(NSTimeInterval)transitionDuration;
 
 @end
diff --git a/platform/ios/Integration Tests/MBGLIntegrationTests.m b/platform/ios/Integration Tests/MBGLIntegrationTests.m
index 9b5281183a..2fb077145c 100644
--- a/platform/ios/Integration Tests/MBGLIntegrationTests.m
+++ b/platform/ios/Integration Tests/MBGLIntegrationTests.m
@@ -2,10 +2,6 @@
 #import <objc/message.h>
 #import <Mapbox/Mapbox.h>
 
-
-#define TRACE(message) NSLog(@"%@: %@", NSStringFromSelector(_cmd), message)
-
-
 @interface MBGLIntegrationTests : XCTestCase <MGLMapViewDelegate>
 
 @property (nonatomic) MGLMapView *mapView;
@@ -76,8 +72,8 @@
 #pragma mark - Tests
 
 - (void)testStoringOpenGLLayerInCollections {
-    // If we change the meaning of equality for MGLStyleLayer we want to know about it - since we
-    // store layers in both a NSMutableSet and in a NSMapTable
+    // If we change the meaning of equality for MGLOpenGLStyleLayer we want to know about it - since we
+    // store layers in a NSMutableSet
     MGLStyleLayer *layer1 = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"];
     MGLStyleLayer *layer2 = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"];
 
@@ -225,7 +221,6 @@
 
 - (void)testOpenGLLayerDoesNotLeakWhenRemovedFromStyle {
 
-    //    MGLOpenGLStyleLayer *layer;
     __weak id weakLayer;
     @autoreleasepool {
         MGLOpenGLStyleLayer *layer = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"];
@@ -235,7 +230,6 @@
 
         // Run the render loop, so the layer gets used/managed.
         [self waitForMapViewToBeRendered];
-        //        [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]];
 
         [self.style removeLayer:[self.style layerWithIdentifier:@"gl-layer"]];
     }
@@ -334,10 +328,27 @@
     [self.mapView setStyleURL:styleURL];
     [self waitForExpectations:@[_styleLoadingExpectation] timeout:10];
 
-    // At this point the C++ CustomLayer will have been destroyed, and the rawLayer pointer has been NULLed
+    // At this point the C++ CustomLayer will have been destroyed. Ideally the
+    // Obj-C rawLayer pointer will have been NULL'd at this point, but that is still
+    // TBD.
     XCTAssert(weakLayer == layer2);
     XCTAssertNotNil(weakLayer);
 
+    /*
+    // Attempting to re-insert this layer2 will crash the app, since the rawLayer
+    // pointer is now (potentially) pointing at garbage (i.e. it's a dangling
+    // pointer). When this issue has been fixed, we can uncomment the follow checks...
+    XCTAssert(weakLayer.rawLayer == NULL);
+
+    @try {
+        [self.style insertLayer:layer2 atIndex:0];
+        XCTFail();
+    }
+    @catch (NSException *exception) {
+        // Success, we're expecting an exception
+    }
+    */
+
     // Asking the style for the layer should return nil
     MGLStyleLayer *layer3 = [self.mapView.style layerWithIdentifier:@"gl-layer"];
     XCTAssertNil(layer3);
diff --git a/platform/ios/ios.xcodeproj/project.pbxproj b/platform/ios/ios.xcodeproj/project.pbxproj
index d1140f93ef..07fae5945c 100644
--- a/platform/ios/ios.xcodeproj/project.pbxproj
+++ b/platform/ios/ios.xcodeproj/project.pbxproj
@@ -1908,6 +1908,7 @@
 				DA8848231CBAFA6200AB86E3 /* MGLOfflineStorage_Private.h in Headers */,
 				404326891D5B9B27007111BD /* MGLAnnotationContainerView_Private.h in Headers */,
 				CA55CD41202C16AA00CE7095 /* MGLCameraChangeReason.h in Headers */,
+				1FB7DAAF1F2A4DBD00410606 /* MGLVectorSource+MGLAdditions.h in Headers */,
 				DA88483B1CBAFB8500AB86E3 /* MGLCalloutView.h in Headers */,
 				35E0CFE61D3E501500188327 /* MGLStyle_Private.h in Headers */,
 				3510FFF01D6D9D8C00F413B2 /* NSExpression+MGLAdditions.h in Headers */,
@@ -2340,7 +2341,6 @@
 					};
 					16376B2E1FFDB4B40000563E = {
 						CreatedOnToolsVersion = 9.2;
-						DevelopmentTeam = GJZR2MEM28;
 						ProvisioningStyle = Automatic;
 					};
 					DA1DC9491CB6C1C2006E619F = {
@@ -3072,7 +3072,6 @@
 				CLANG_WARN_UNGUARDED_AVAILABILITY = YES_AGGRESSIVE;
 				CODE_SIGN_IDENTITY = "iPhone Developer";
 				CODE_SIGN_STYLE = Automatic;
-				DEVELOPMENT_TEAM = GJZR2MEM28;
 				GCC_C_LANGUAGE_STANDARD = gnu11;
 				INFOPLIST_FILE = "Integration Test Harness/Info.plist";
 				IPHONEOS_DEPLOYMENT_TARGET = 11.2;
@@ -3098,7 +3097,6 @@
 				CLANG_WARN_UNGUARDED_AVAILABILITY = YES_AGGRESSIVE;
 				CODE_SIGN_IDENTITY = "iPhone Developer";
 				CODE_SIGN_STYLE = Automatic;
-				DEVELOPMENT_TEAM = GJZR2MEM28;
 				GCC_C_LANGUAGE_STANDARD = gnu11;
 				INFOPLIST_FILE = "Integration Test Harness/Info.plist";
 				IPHONEOS_DEPLOYMENT_TARGET = 11.2;