From 4e88894b91cb9d20a275e71c2f9c86842a475b73 Mon Sep 17 00:00:00 2001 From: Julian Rex Date: Wed, 28 Mar 2018 18:59:38 -0400 Subject: Changed OpenGL retain management to use NSMutableSet. --- include/mbgl/style/layers/custom_layer.hpp | 4 +-- platform/darwin/src/MGLOpenGLStyleLayer.mm | 6 ++--- platform/darwin/src/MGLStyle.mm | 4 +-- platform/darwin/src/MGLStyle_Private.h | 2 +- .../ios/Integration Tests/MBGLIntegrationTests.m | 29 +++++++++++++++------- platform/ios/ios.xcodeproj/project.pbxproj | 4 +-- 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 -#include - 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; +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 #import - -#define TRACE(message) NSLog(@"%@: %@", NSStringFromSelector(_cmd), message) - - @interface MBGLIntegrationTests : XCTestCase @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; -- cgit v1.2.1