From 22a1da58765557fe37fb6461e64ec62443d9bda5 Mon Sep 17 00:00:00 2001 From: Asheem Mamoowala Date: Mon, 26 Feb 2018 13:42:25 -0800 Subject: [PATCH 1/5] [ios] Adding a few more test cases. testOpenGLLayerDoesNotLeakWhenStyleChanged is failing --- .../Integration Tests/MBGLIntegrationTests.m | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/platform/ios/Integration Tests/MBGLIntegrationTests.m b/platform/ios/Integration Tests/MBGLIntegrationTests.m index 1008ef6c9f1..cb3bdc3df68 100644 --- a/platform/ios/Integration Tests/MBGLIntegrationTests.m +++ b/platform/ios/Integration Tests/MBGLIntegrationTests.m @@ -88,6 +88,30 @@ - (void)testAddingRemovingOpenGLLayer { addRemoveGLLayer(); } +- (void)testAddingRemovingOpenGLLayerWithoutRendering { + XCTAssertNotNil(self.style); + + void(^addRemoveGLLayer)(void) = ^{ + MGLOpenGLStyleLayer *layer = nil; + __weak id weakLayer = nil; + + @autoreleasepool { + + layer = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; + [self.style insertLayer:layer atIndex:0]; + weakLayer = layer; + layer = nil; + [self.style removeLayer:weakLayer]; + } + + XCTAssertNil(weakLayer); + }; + + addRemoveGLLayer(); + addRemoveGLLayer(); + addRemoveGLLayer(); +} + - (void)testReusingOpenGLLayer { NSTimeInterval waitInterval = 0.02; @@ -118,4 +142,73 @@ - (void)testOpenGLLayerDoesNotLeakWhenCreatedAndDestroyedWithoutAddingToStyle { XCTAssertNil(weakLayer); } +- (void)testOpenGLLayerDoesNotLeakWhenRemovedFromStyle { + NSTimeInterval waitInterval = 0.02; + + MGLOpenGLStyleLayer *layer; + __weak id weakLayer; + @autoreleasepool { + layer = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; + weakLayer = layer; + [self.style insertLayer:layer atIndex:0]; + layer = nil; + + [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; + + [self.style removeLayer:[self.style layerWithIdentifier:@"gl-layer"]]; + [self.mapView setNeedsDisplay]; + + [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; + } + XCTAssertNil(weakLayer); +} + +- (void)testOpenGLLayerDoesNotLeakWhenStyleChanged { + NSTimeInterval waitInterval = 0.02; + __weak id weakLayer; + + @autoreleasepool { + MGLOpenGLStyleLayer *layer = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; + weakLayer = layer; + [self.style insertLayer:layer atIndex:0]; + layer = nil; + + [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; + + NSURL *styleURL = [[NSBundle bundleForClass:[self class]] URLForResource:@"one-liner" withExtension:@"json"]; + _styleLoadingExpectation = [self expectationWithDescription:@"Map view should finish loading style."]; + [self.mapView setStyleURL:styleURL]; + + [self waitForExpectationsWithTimeout:1 handler:nil]; + [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; + } + XCTAssertNil(weakLayer); +} + +- (void)testReusingOpenGLLayerIdentifier { + NSTimeInterval waitInterval = 0.02; + MGLOpenGLStyleLayer *layer1, *layer2; + @autoreleasepool { + layer1 = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; + [self.style insertLayer:layer1 atIndex:0]; + + [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; + + [self.style removeLayer:layer1]; + + layer2 = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; + [self.style insertLayer:layer2 atIndex:0]; + + [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; + + [self.style removeLayer:layer2]; + + [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; + } + XCTAssertNotNil(layer1); + XCTAssertNotNil(layer2); + XCTAssertNil([layer1 style]); + XCTAssertNil([layer2 style]); +} + @end From 0327167738336404ac02c2d0202b1de860dd060e Mon Sep 17 00:00:00 2001 From: Julian Rex Date: Wed, 28 Feb 2018 16:24:00 -0500 Subject: [PATCH 2/5] [ios] Added sets to manage retain/release of layers (esp custom gl layers) --- platform/darwin/src/MGLOpenGLStyleLayer.mm | 13 ++++--- platform/darwin/src/MGLStyle.mm | 25 +++++++++++++ platform/darwin/src/MGLStyleLayer.mm | 12 +++--- platform/darwin/src/MGLStyle_Private.h | 4 ++ .../Integration Tests/MBGLIntegrationTests.m | 37 +++++++++++++++++++ platform/ios/src/MGLMapView.mm | 10 +++++ 6 files changed, 89 insertions(+), 12 deletions(-) diff --git a/platform/darwin/src/MGLOpenGLStyleLayer.mm b/platform/darwin/src/MGLOpenGLStyleLayer.mm index 9f957b7082b..80805b001bf 100644 --- a/platform/darwin/src/MGLOpenGLStyleLayer.mm +++ b/platform/darwin/src/MGLOpenGLStyleLayer.mm @@ -15,10 +15,9 @@ when creating an OpenGL style layer. */ void MGLPrepareCustomStyleLayer(void *context) { - - // Pair retain/release during rendering (see MGLFinishCustomStyleLayer) - id retaineee = (__bridge id)context; - MGLOpenGLStyleLayer *layer = (__bridge MGLOpenGLStyleLayer*)CFBridgingRetain(retaineee); + // Note, that the layer is retained/released by MGLStyle, ensuring that the layer + // is alive during rendering + MGLOpenGLStyleLayer *layer = (__bridge MGLOpenGLStyleLayer*)context; [layer didMoveToMapView:layer.style.mapView]; } @@ -51,8 +50,10 @@ void MGLDrawCustomStyleLayer(void *context, const mbgl::style::CustomLayerRender when creating an OpenGL style layer. */ void MGLFinishCustomStyleLayer(void *context) { - // Release the layer (since we retained it in the initialization) - MGLOpenGLStyleLayer *layer = CFBridgingRelease(context); + + // Note, that the layer is retained/released by MGLStyle, ensuring that the layer + // is alive during rendering + MGLOpenGLStyleLayer *layer = (__bridge MGLOpenGLStyleLayer*)context; [layer willMoveFromMapView:layer.style.mapView]; } diff --git a/platform/darwin/src/MGLStyle.mm b/platform/darwin/src/MGLStyle.mm index f6fc5533be0..695f8b55e7c 100644 --- a/platform/darwin/src/MGLStyle.mm +++ b/platform/darwin/src/MGLStyle.mm @@ -85,6 +85,10 @@ @interface MGLStyle() @property (readonly, copy, nullable) NSURL *URL; @property (nonatomic) NS_MUTABLE_DICTIONARY_OF(NSString *, NS_DICTIONARY_OF(NSObject *, MGLTextLanguage *) *) *localizedLayersByIdentifier; +// Used for retain/release management +@property (nonatomic) NSMutableSet *layersForUpdating; +@property (nonatomic) NSSet *layersForRendering; + @end @implementation MGLStyle @@ -172,6 +176,8 @@ + (NSURL *)trafficNightStyleURLWithVersion:(NSInteger)version { - (instancetype)initWithRawStyle:(mbgl::style::Style *)rawStyle mapView:(MGLMapView *)mapView { if (self = [super init]) { + _layersForUpdating = [NSMutableSet set]; + _mapView = mapView; _rawStyle = rawStyle; _localizedLayersByIdentifier = [NSMutableDictionary dictionary]; @@ -534,6 +540,25 @@ - (void)insertLayer:(MGLStyleLayer *)layer aboveLayer:(MGLStyleLayer *)sibling { [self didChangeValueForKey:@"layers"]; } +#pragma mark - Layer retain/release management + +- (void)addToManagedLayers:(MGLStyleLayer*)layer { + [self.layersForUpdating addObject:layer]; +} + +- (void)removeFromManagedLayers:(MGLStyleLayer*)layer { + [self.layersForUpdating removeObject:layer]; +} + +- (void)retainLayersUsedDuringRendering { + self.layersForRendering = [self.layersForUpdating copy]; +} + +- (void)releaseLayersUsedDuringRendering { + self.layersForRendering = nil; +} + + #pragma mark Style classes - (NS_ARRAY_OF(NSString *) *)styleClasses diff --git a/platform/darwin/src/MGLStyleLayer.mm b/platform/darwin/src/MGLStyleLayer.mm index 45a44865254..444dbaeea30 100644 --- a/platform/darwin/src/MGLStyleLayer.mm +++ b/platform/darwin/src/MGLStyleLayer.mm @@ -38,9 +38,9 @@ - (void)addToStyle:(MGLStyle *)style belowLayer:(MGLStyleLayer *)otherLayer "to the style more than once is invalid.", self, style]; } - // Since we're adding self to a C++ collection, we need to retain ourselves, so that we don't - // end up with a dangling pointer - CFBridgingRetain(self); + // We need to ensure that this layer is retained, so that any references from layer impl's + // e.g. contexts) are still valid + [style addToManagedLayers:self]; if (otherLayer) { const mbgl::optional belowLayerId{otherLayer.identifier.UTF8String}; @@ -55,9 +55,9 @@ - (void)removeFromStyle:(MGLStyle *)style if (self.rawLayer == style.rawStyle->getLayer(self.identifier.UTF8String)) { _pendingLayer = style.rawStyle->removeLayer(self.identifier.UTF8String); - // Pair the retain above, and release self, since we're now removed from the collection - CFTypeRef toRelease = (__bridge CFTypeRef)self; - CFBridgingRelease(toRelease); + // We need to ensure that this layer is now released (however, if this layer is about to be + // used by the renderer then it will released once rendering is complete) + [style removeFromManagedLayers:self]; } } diff --git a/platform/darwin/src/MGLStyle_Private.h b/platform/darwin/src/MGLStyle_Private.h index 4cbe953a442..cfde78fd068 100644 --- a/platform/darwin/src/MGLStyle_Private.h +++ b/platform/darwin/src/MGLStyle_Private.h @@ -28,6 +28,10 @@ namespace mbgl { - (void)setStyleClasses:(NS_ARRAY_OF(NSString *) *)appliedClasses transitionDuration:(NSTimeInterval)transitionDuration; +- (void)addToManagedLayers:(MGLStyleLayer*)layer; +- (void)removeFromManagedLayers:(MGLStyleLayer*)layer; +- (void)retainLayersUsedDuringRendering; +- (void)releaseLayersUsedDuringRendering; @end @interface MGLStyle (MGLStreetsAdditions) diff --git a/platform/ios/Integration Tests/MBGLIntegrationTests.m b/platform/ios/Integration Tests/MBGLIntegrationTests.m index cb3bdc3df68..ea95d25c984 100644 --- a/platform/ios/Integration Tests/MBGLIntegrationTests.m +++ b/platform/ios/Integration Tests/MBGLIntegrationTests.m @@ -142,6 +142,43 @@ - (void)testOpenGLLayerDoesNotLeakWhenCreatedAndDestroyedWithoutAddingToStyle { XCTAssertNil(weakLayer); } +- (void)testOpenGLLayerDoesNotLeakWhenMapViewDeallocs { + NSTimeInterval waitInterval = 0.02; + __weak id weakLayer; + + @autoreleasepool { + + NSURL *styleURL = [[NSBundle bundleForClass:[self class]] URLForResource:@"one-liner" withExtension:@"json"]; + MGLMapView *mapView2 = [[MGLMapView alloc] initWithFrame:UIScreen.mainScreen.bounds styleURL:styleURL]; + mapView2.delegate = self; + + XCTAssertNil(mapView2.style); + + _styleLoadingExpectation = [self expectationWithDescription:@"Map view should finish loading style."]; + [self waitForExpectationsWithTimeout:1 handler:nil]; + +// UIView *superView = [[UIView alloc] initWithFrame:UIScreen.mainScreen.bounds]; +// [superView addSubview:self.mapView]; +// UIWindow *window = [[UIWindow alloc] initWithFrame:UIScreen.mainScreen.bounds]; +// [window addSubview:superView]; +// [window makeKeyAndVisible]; + + MGLOpenGLStyleLayer *layer = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; + weakLayer = layer; + [mapView2.style insertLayer:layer atIndex:0]; + layer = nil; + + [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; + [self.mapView setNeedsDisplay]; + [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; + } + XCTAssertNil(weakLayer); +} + + + + + - (void)testOpenGLLayerDoesNotLeakWhenRemovedFromStyle { NSTimeInterval waitInterval = 0.02; diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm index cad14343a8f..2df7de2c5c0 100644 --- a/platform/ios/src/MGLMapView.mm +++ b/platform/ios/src/MGLMapView.mm @@ -201,6 +201,7 @@ @interface MGLMapView () getStyle().loadURL([[styleURL absoluteString] UTF8String]); } @@ -5549,10 +5553,13 @@ - (void)mapViewDidFailLoadingMapWithError:(NSError *)error { } - (void)mapViewWillStartRenderingFrame { + if (!_mbglMap) { return; } + [self.style retainLayersUsedDuringRendering]; + if ([self.delegate respondsToSelector:@selector(mapViewWillStartRenderingFrame:)]) { [self.delegate mapViewWillStartRenderingFrame:self]; @@ -5575,6 +5582,9 @@ - (void)mapViewDidFinishRenderingFrameFullyRendered:(BOOL)fullyRendered { { [self.delegate mapViewDidFinishRenderingFrame:self fullyRendered:fullyRendered]; } + + [self.style releaseLayersUsedDuringRendering]; + } - (void)mapViewWillStartRenderingMap { From 6c9239c8746d9d3f0e3bdb340838978b7b998eba Mon Sep 17 00:00:00 2001 From: Julian Rex Date: Wed, 28 Feb 2018 16:41:16 -0500 Subject: [PATCH 3/5] [macos] Added retain/release calls to macos's MGLMapView cf. iOS. --- platform/ios/src/MGLMapView.mm | 1 - platform/macos/src/MGLMapView.mm | 4 ++++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm index 2df7de2c5c0..92ee50ce3de 100644 --- a/platform/ios/src/MGLMapView.mm +++ b/platform/ios/src/MGLMapView.mm @@ -361,7 +361,6 @@ - (void)setStyleURL:(nullable NSURL *)styleURL styleURL = styleURL.mgl_URLByStandardizingScheme; -// [self.layers removeAllObjects]; self.style = nil; _mbglMap->getStyle().loadURL([[styleURL absoluteString] UTF8String]); diff --git a/platform/macos/src/MGLMapView.mm b/platform/macos/src/MGLMapView.mm index 9ec90121985..e54b0f195d8 100644 --- a/platform/macos/src/MGLMapView.mm +++ b/platform/macos/src/MGLMapView.mm @@ -882,6 +882,8 @@ - (void)mapViewWillStartRenderingFrame { return; } + [self.style retainLayersUsedDuringRendering]; + if ([self.delegate respondsToSelector:@selector(mapViewWillStartRenderingFrame:)]) { [self.delegate mapViewWillStartRenderingFrame:self]; } @@ -899,6 +901,8 @@ - (void)mapViewDidFinishRenderingFrameFullyRendered:(BOOL)fullyRendered { if ([self.delegate respondsToSelector:@selector(mapViewDidFinishRenderingFrame:fullyRendered:)]) { [self.delegate mapViewDidFinishRenderingFrame:self fullyRendered:fullyRendered]; } + + [self.style releaseLayersUsedDuringRendering]; } - (void)mapViewWillStartRenderingMap { From 6fb2a94c5e1be320cc88bf9ed822fbd297223c4b Mon Sep 17 00:00:00 2001 From: Julian Rex Date: Mon, 5 Mar 2018 13:15:20 -0500 Subject: [PATCH 4/5] [ios, macos, core] Added retain/release manager that holds on to the MGLOpenGLStyleLayers for the 2 trips through the render loop that are needed before they can be deallocated. Added additional tests. --- include/mbgl/style/layers/custom_layer.hpp | 15 + platform/darwin/src/MGLOpenGLStyleLayer.mm | 44 +- platform/darwin/src/MGLStyle.mm | 21 +- platform/darwin/src/MGLStyleLayer.mm | 21 +- .../src/MGLStyleLayerRetentionManager.mm | 53 ++ .../MGLStyleLayerRetentionManager_Private.h | 16 + platform/darwin/src/MGLStyleLayer_Private.h | 8 +- platform/darwin/src/MGLStyle_Private.h | 3 +- .../Integration Tests/MBGLIntegrationTests.m | 247 ---------- .../Integration Tests/MBGLIntegrationTests.mm | 459 ++++++++++++++++++ platform/ios/ios.xcodeproj/project.pbxproj | 36 +- platform/ios/src/MGLMapView.mm | 16 +- platform/ios/src/MGLMapView_Private.h | 4 + src/mbgl/style/layers/custom_layer.cpp | 14 +- 14 files changed, 664 insertions(+), 293 deletions(-) create mode 100644 platform/darwin/src/MGLStyleLayerRetentionManager.mm create mode 100644 platform/darwin/src/MGLStyleLayerRetentionManager_Private.h delete mode 100644 platform/ios/Integration Tests/MBGLIntegrationTests.m create mode 100644 platform/ios/Integration Tests/MBGLIntegrationTests.mm diff --git a/include/mbgl/style/layers/custom_layer.hpp b/include/mbgl/style/layers/custom_layer.hpp index bf3387f95be..cd54c54ffb5 100644 --- a/include/mbgl/style/layers/custom_layer.hpp +++ b/include/mbgl/style/layers/custom_layer.hpp @@ -54,6 +54,17 @@ using CustomLayerContextLostFunction = void (*)(void* context); */ using CustomLayerDeinitializeFunction = void (*)(void* context); +/** + * Called from `CustomLayer`'s destructor. + * This provides a mechanism to handle any necessary clean-up using the provided `peer` object. + * For example, if a platform-native peer object has a raw pointer to the CustomLayer it could be + * set to NULL. + * + * This function is called from CustomLayer, unlike the above functions that are passed into the + * private implementation. + */ +using CustomLayerDeallocationFunction = void (*)(util::unique_any *peer); + class CustomLayer : public Layer { public: CustomLayer(const std::string& id, @@ -61,12 +72,14 @@ class CustomLayer : public Layer { CustomLayerRenderFunction, CustomLayerContextLostFunction, CustomLayerDeinitializeFunction, + CustomLayerDeallocationFunction, void* context); CustomLayer(const std::string& id, CustomLayerInitializeFunction, CustomLayerRenderFunction, CustomLayerDeinitializeFunction, + CustomLayerDeallocationFunction, void* context); ~CustomLayer() final; @@ -87,6 +100,8 @@ class CustomLayer : public Layer { std::unique_ptr cloneRef(const std::string& id) const final; CustomLayer(const CustomLayer&) = delete; + + CustomLayerDeallocationFunction deallocationFn = nullptr; }; template <> diff --git a/platform/darwin/src/MGLOpenGLStyleLayer.mm b/platform/darwin/src/MGLOpenGLStyleLayer.mm index 80805b001bf..a5f8b574a08 100644 --- a/platform/darwin/src/MGLOpenGLStyleLayer.mm +++ b/platform/darwin/src/MGLOpenGLStyleLayer.mm @@ -50,13 +50,43 @@ void MGLDrawCustomStyleLayer(void *context, const mbgl::style::CustomLayerRender when creating an OpenGL style layer. */ void MGLFinishCustomStyleLayer(void *context) { - // Note, that the layer is retained/released by MGLStyle, ensuring that the layer // is alive during rendering MGLOpenGLStyleLayer *layer = (__bridge MGLOpenGLStyleLayer*)context; + [layer willMoveFromMapView:layer.style.mapView]; } + +/** + Function to be called when the core `CustomLayer` (not the Impl) gets deallocated. + It's possible taht at this stage the Obj-C style layer is being deallocated (but that case is detected). + */ +void MGLDeallocateCustomStyleLayer(mbgl::util::unique_any *peer) { + + // We know that the peer object contains a LayerWrapper with a weak pointer to + // our custom layer. We can use this to safely access the layer, and clear out the + // raw pointer. + // + // If we don't do this rawLayer can become a dangling pointer (which was previously being + // accessed via the description method) + + if (!(peer && peer->has_value())) + return; + + LayerWrapper *wrapper = mbgl::util::any_cast(peer); + + if (!wrapper) + return; + + // If the MGLStyleLayer is currently being dealloc'd (and trigger the CustomLayer destructor, and + // this function) then layer here will be nil (even though wrapper->layer may appear to be non-nil) + MGLStyleLayer *layer = wrapper->layer; + + layer.rawLayer = NULL; +} + + /** An `MGLOpenGLStyleLayer` is a style layer that is rendered by OpenGL code that you provide. @@ -108,7 +138,9 @@ - (instancetype)initWithIdentifier:(NSString *)identifier { MGLPrepareCustomStyleLayer, MGLDrawCustomStyleLayer, MGLFinishCustomStyleLayer, + MGLDeallocateCustomStyleLayer, (__bridge void*)self); + return self = [super initWithPendingLayer:std::move(layer)]; } @@ -128,11 +160,21 @@ - (void)setStyle:(MGLStyle *)style { - (void)addToStyle:(MGLStyle *)style belowLayer:(MGLStyleLayer *)otherLayer { self.style = style; + + // We need to ensure that this layer is retained, so that any references from layer impl's + // e.g. contexts) are still valid + [style addToManagedLayers:self]; + [super addToStyle:style belowLayer:otherLayer]; } - (void)removeFromStyle:(MGLStyle *)style { [super removeFromStyle:style]; + + // We need to ensure that this layer is now released (however, if this layer is about to be + // used by the renderer then it will released once rendering is complete) + [style removeFromManagedLayers:self]; + self.style = nil; } diff --git a/platform/darwin/src/MGLStyle.mm b/platform/darwin/src/MGLStyle.mm index 695f8b55e7c..3d503d77aa9 100644 --- a/platform/darwin/src/MGLStyle.mm +++ b/platform/darwin/src/MGLStyle.mm @@ -84,10 +84,7 @@ @interface MGLStyle() @property (nonatomic, readonly) mbgl::style::Style *rawStyle; @property (readonly, copy, nullable) NSURL *URL; @property (nonatomic) NS_MUTABLE_DICTIONARY_OF(NSString *, NS_DICTIONARY_OF(NSObject *, MGLTextLanguage *) *) *localizedLayersByIdentifier; - -// Used for retain/release management -@property (nonatomic) NSMutableSet *layersForUpdating; -@property (nonatomic) NSSet *layersForRendering; +@property (nonatomic, readwrite) NSMutableSet *managedLayers; @end @@ -176,8 +173,7 @@ + (NSURL *)trafficNightStyleURLWithVersion:(NSInteger)version { - (instancetype)initWithRawStyle:(mbgl::style::Style *)rawStyle mapView:(MGLMapView *)mapView { if (self = [super init]) { - _layersForUpdating = [NSMutableSet set]; - + _managedLayers = [NSMutableSet set]; _mapView = mapView; _rawStyle = rawStyle; _localizedLayersByIdentifier = [NSMutableDictionary dictionary]; @@ -543,22 +539,13 @@ - (void)insertLayer:(MGLStyleLayer *)layer aboveLayer:(MGLStyleLayer *)sibling { #pragma mark - Layer retain/release management - (void)addToManagedLayers:(MGLStyleLayer*)layer { - [self.layersForUpdating addObject:layer]; + [self.managedLayers addObject:layer]; } - (void)removeFromManagedLayers:(MGLStyleLayer*)layer { - [self.layersForUpdating removeObject:layer]; -} - -- (void)retainLayersUsedDuringRendering { - self.layersForRendering = [self.layersForUpdating copy]; + [self.managedLayers removeObject:layer]; } -- (void)releaseLayersUsedDuringRendering { - self.layersForRendering = nil; -} - - #pragma mark Style classes - (NS_ARRAY_OF(NSString *) *)styleClasses diff --git a/platform/darwin/src/MGLStyleLayer.mm b/platform/darwin/src/MGLStyleLayer.mm index 444dbaeea30..f29142c0c72 100644 --- a/platform/darwin/src/MGLStyleLayer.mm +++ b/platform/darwin/src/MGLStyleLayer.mm @@ -5,9 +5,7 @@ #include @interface MGLStyleLayer () - -@property (nonatomic, readonly) mbgl::style::Layer *rawLayer; - +@property (nonatomic, readwrite, nullable) mbgl::style::Layer *rawLayer; @end @implementation MGLStyleLayer { @@ -38,10 +36,6 @@ - (void)addToStyle:(MGLStyle *)style belowLayer:(MGLStyleLayer *)otherLayer "to the style more than once is invalid.", self, style]; } - // We need to ensure that this layer is retained, so that any references from layer impl's - // e.g. contexts) are still valid - [style addToManagedLayers:self]; - if (otherLayer) { const mbgl::optional belowLayerId{otherLayer.identifier.UTF8String}; style.rawStyle->addLayer(std::move(_pendingLayer), belowLayerId); @@ -54,10 +48,6 @@ - (void)removeFromStyle:(MGLStyle *)style { if (self.rawLayer == style.rawStyle->getLayer(self.identifier.UTF8String)) { _pendingLayer = style.rawStyle->removeLayer(self.identifier.UTF8String); - - // We need to ensure that this layer is now released (however, if this layer is about to be - // used by the renderer then it will released once rendering is complete) - [style removeFromManagedLayers:self]; } } @@ -111,7 +101,14 @@ - (NSString *)description { return [NSString stringWithFormat:@"<%@: %p; identifier = %@; visible = %@>", NSStringFromClass([self class]), (void *)self, self.identifier, - self.visible ? @"YES" : @"NO"]; + self.rawLayer ? (self.visible ? @"YES" : @"NO") : @"(No raw layer)"]; +} + +#pragma mark - Debug methods + +- (void)debugResetRawLayerPeer { + MGLAssertStyleLayerIsValid(); + self.rawLayer->peer.reset(); } @end diff --git a/platform/darwin/src/MGLStyleLayerRetentionManager.mm b/platform/darwin/src/MGLStyleLayerRetentionManager.mm new file mode 100644 index 00000000000..71ebd574adb --- /dev/null +++ b/platform/darwin/src/MGLStyleLayerRetentionManager.mm @@ -0,0 +1,53 @@ +#import "MGLStyle.h" +#import "MGLStyleLayer.h" +#import "MGLStyleLayerRetentionManager_Private.h" +#include +#include + +static const NSUInteger MGLStyleLayerRetentionManagerCapacityHint = 100; + +@interface MGLStyleLayerRetentionManager () +@property (nonatomic) NSMapTable *retentionTable; +@end + +@implementation MGLStyleLayerRetentionManager + +- (instancetype)init { + if ((self = [super init])) { + _retentionTable = [[NSMapTable alloc] initWithKeyOptions:NSPointerFunctionsStrongMemory|NSPointerFunctionsObjectPointerPersonality + valueOptions:NSPointerFunctionsStrongMemory + capacity:MGLStyleLayerRetentionManagerCapacityHint]; + } + return self; +} + +- (BOOL)isManagedLayer:(nullable MGLStyleLayer*)styleLayer { + return [self.retentionTable objectForKey:styleLayer] != nil; +} + +- (void)updateRetainedLayers:(nonnull NSSet*)sourceObjects { + + // Add/update the objects from the source set, with a default lifetime + for (id layer in sourceObjects) { + [self.retentionTable setObject:@(MGLStyleLayerRetentionManagerDefaultLifetime) forKey:layer]; + } +} + +- (void)decrementLifetimes { + // Consider double-buffering the two tables, so we don't keep allocing/deallocing tables. + NSMapTable *retentionTable = [[NSMapTable alloc] initWithKeyOptions:NSPointerFunctionsStrongMemory|NSPointerFunctionsObjectPointerPersonality + valueOptions:NSPointerFunctionsStrongMemory + capacity:MGLStyleLayerRetentionManagerCapacityHint]; + + for (MGLStyleLayer *layer in self.retentionTable) { + NSInteger lifeTime = [[self.retentionTable objectForKey:layer] integerValue]; + + if (lifeTime > 0) { + [retentionTable setObject:@(lifeTime - 1) forKey:layer]; + } + } + + self.retentionTable = retentionTable; +} + +@end diff --git a/platform/darwin/src/MGLStyleLayerRetentionManager_Private.h b/platform/darwin/src/MGLStyleLayerRetentionManager_Private.h new file mode 100644 index 00000000000..0063d342a5d --- /dev/null +++ b/platform/darwin/src/MGLStyleLayerRetentionManager_Private.h @@ -0,0 +1,16 @@ +@class MGLStyle; + +static const NSInteger MGLStyleLayerRetentionManagerDefaultLifetime = 2; + +/** + Object use to manage the retain/release of `MGLStyleLayer`s (currently only `MGLOpenGLStyleLayer`. + Managed layers are given a "lifetime", and this is reset everytime `-updateRetainedLayers:` is called. + The lifetime is decremented with each call to `-decrementLifetimes`, when it reaches 0 the layer + is removed from the manager (potentially releasing it) + */ +@interface MGLStyleLayerRetentionManager : NSObject +- (BOOL)isManagedLayer:(nullable MGLStyleLayer*)styleLayer; +- (void)updateRetainedLayers:(nonnull NSSet*)sourceObjects; +- (void)decrementLifetimes; +@end + diff --git a/platform/darwin/src/MGLStyleLayer_Private.h b/platform/darwin/src/MGLStyleLayer_Private.h index 9bee013c3dc..9c93f4c0c97 100644 --- a/platform/darwin/src/MGLStyleLayer_Private.h +++ b/platform/darwin/src/MGLStyleLayer_Private.h @@ -59,7 +59,7 @@ struct LayerWrapper { pointer value stays even after ownership of the object is transferred via `mbgl::Map addLayer`. */ -@property (nonatomic, readonly) mbgl::style::Layer *rawLayer; +@property (nonatomic, readwrite, nullable) mbgl::style::Layer *rawLayer; /** Adds the mbgl style layer that this object represents to the mbgl map below the @@ -80,6 +80,12 @@ struct LayerWrapper { */ - (void)removeFromStyle:(MGLStyle *)style; + +/** + Debug method used for testing - it resets the peer object, essentially disconnecting the raw Layer + from the peer MGLStyleLayer. + */ +- (void)debugResetRawLayerPeer; @end NS_ASSUME_NONNULL_END diff --git a/platform/darwin/src/MGLStyle_Private.h b/platform/darwin/src/MGLStyle_Private.h index cfde78fd068..00985403f52 100644 --- a/platform/darwin/src/MGLStyle_Private.h +++ b/platform/darwin/src/MGLStyle_Private.h @@ -23,6 +23,7 @@ namespace mbgl { @property (nonatomic, readonly, weak) MGLMapView *mapView; @property (nonatomic, readonly) mbgl::style::Style *rawStyle; +@property (nonatomic, readonly) NSMutableSet *managedLayers; - (nullable NS_ARRAY_OF(MGLAttributionInfo *) *)attributionInfosWithFontSize:(CGFloat)fontSize linkColor:(nullable MGLColor *)linkColor; @@ -30,8 +31,6 @@ namespace mbgl { - (void)addToManagedLayers:(MGLStyleLayer*)layer; - (void)removeFromManagedLayers:(MGLStyleLayer*)layer; -- (void)retainLayersUsedDuringRendering; -- (void)releaseLayersUsedDuringRendering; @end @interface MGLStyle (MGLStreetsAdditions) diff --git a/platform/ios/Integration Tests/MBGLIntegrationTests.m b/platform/ios/Integration Tests/MBGLIntegrationTests.m deleted file mode 100644 index f79fac3dcd9..00000000000 --- a/platform/ios/Integration Tests/MBGLIntegrationTests.m +++ /dev/null @@ -1,247 +0,0 @@ -#import - -@import Mapbox; - -@interface MBGLIntegrationTests : XCTestCase - -@property (nonatomic) MGLMapView *mapView; -@property (nonatomic) MGLStyle *style; - -@end - -@implementation MBGLIntegrationTests { - XCTestExpectation *_styleLoadingExpectation; -} - -- (void)setUp { - [super setUp]; - - [MGLAccountManager setAccessToken:@"pk.feedcafedeadbeefbadebede"]; - NSURL *styleURL = [[NSBundle bundleForClass:[self class]] URLForResource:@"one-liner" withExtension:@"json"]; - self.mapView = [[MGLMapView alloc] initWithFrame:UIScreen.mainScreen.bounds styleURL:styleURL]; - self.mapView.delegate = self; - if (!self.mapView.style) { - _styleLoadingExpectation = [self expectationWithDescription:@"Map view should finish loading style."]; - [self waitForExpectationsWithTimeout:1 handler:nil]; - } - - UIView *superView = [[UIView alloc] initWithFrame:UIScreen.mainScreen.bounds]; - [superView addSubview:self.mapView]; - UIWindow *window = [[UIWindow alloc] initWithFrame:UIScreen.mainScreen.bounds]; - [window addSubview:superView]; - [window makeKeyAndVisible]; -} - -- (void)mapView:(MGLMapView *)mapView didFinishLoadingStyle:(MGLStyle *)style { - XCTAssertNotNil(mapView.style); - XCTAssertEqual(mapView.style, style); - - [_styleLoadingExpectation fulfill]; -} - -- (void)tearDown { - _styleLoadingExpectation = nil; - self.mapView = nil; - self.style = nil; - - [super tearDown]; -} - -- (MGLStyle *)style { - return self.mapView.style; -} - -- (void)testAddingRemovingOpenGLLayer { - XCTAssertNotNil(self.style); - - // Test fails with 0.1 (presumably because it's < one frame, ie. 1/60) - NSTimeInterval waitInterval = 0.02; - - void(^addRemoveGLLayer)(void) = ^{ - - MGLOpenGLStyleLayer *layer = nil; - __weak id retrievedLayer = nil; - - @autoreleasepool { - layer = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; - [self.style insertLayer:layer atIndex:0]; - layer = nil; - - [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; - - retrievedLayer = [self.style layerWithIdentifier:@"gl-layer"]; - XCTAssertNotNil(retrievedLayer); - [self.style removeLayer:retrievedLayer]; - - // We need to run the runloop for a little while so that the following assert will be correct - // this is because although the layer has been removed from the style, there's still a pending - // render (deinitialize) call, that will needs to be handled, which will finally release the - // layer (and then the layer will be dealloc'd when the autorelease pool drains) - [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; - } - - XCTAssertNil(retrievedLayer); - }; - - addRemoveGLLayer(); - addRemoveGLLayer(); - addRemoveGLLayer(); -} - -- (void)testAddingRemovingOpenGLLayerWithoutRendering { - XCTAssertNotNil(self.style); - - void(^addRemoveGLLayer)(void) = ^{ - MGLOpenGLStyleLayer *layer = nil; - __weak id weakLayer = nil; - - @autoreleasepool { - - layer = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; - [self.style insertLayer:layer atIndex:0]; - weakLayer = layer; - layer = nil; - [self.style removeLayer:weakLayer]; - } - - XCTAssertNil(weakLayer); - }; - - addRemoveGLLayer(); - addRemoveGLLayer(); - addRemoveGLLayer(); -} - -- (void)testReusingOpenGLLayer { - NSTimeInterval waitInterval = 0.02; - - MGLOpenGLStyleLayer *layer = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; - [self.style insertLayer:layer atIndex:0]; - - [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; - - [self.style removeLayer:layer]; - - [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; - - [self.style insertLayer:layer atIndex:0]; - - [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; - - [self.style removeLayer:layer]; - - [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; -} - -// This test does not strictly need to be in this test file/target. Including here for convenience. -- (void)testOpenGLLayerDoesNotLeakWhenCreatedAndDestroyedWithoutAddingToStyle { - MGLOpenGLStyleLayer *layer = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; - __weak id weakLayer = layer; - layer = nil; - - XCTAssertNil(weakLayer); -} - -- (void)testOpenGLLayerDoesNotLeakWhenMapViewDeallocs { - NSTimeInterval waitInterval = 0.02; - __weak id weakLayer; - - @autoreleasepool { - - NSURL *styleURL = [[NSBundle bundleForClass:[self class]] URLForResource:@"one-liner" withExtension:@"json"]; - MGLMapView *mapView2 = [[MGLMapView alloc] initWithFrame:UIScreen.mainScreen.bounds styleURL:styleURL]; - mapView2.delegate = self; - - XCTAssertNil(mapView2.style); - - _styleLoadingExpectation = [self expectationWithDescription:@"Map view should finish loading style."]; - [self waitForExpectationsWithTimeout:1 handler:nil]; - -// UIView *superView = [[UIView alloc] initWithFrame:UIScreen.mainScreen.bounds]; -// [superView addSubview:self.mapView]; -// UIWindow *window = [[UIWindow alloc] initWithFrame:UIScreen.mainScreen.bounds]; -// [window addSubview:superView]; -// [window makeKeyAndVisible]; - - MGLOpenGLStyleLayer *layer = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; - weakLayer = layer; - [mapView2.style insertLayer:layer atIndex:0]; - layer = nil; - - [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; - [self.mapView setNeedsDisplay]; - [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; - } - XCTAssertNil(weakLayer); -} - -- (void)testOpenGLLayerDoesNotLeakWhenRemovedFromStyle { - NSTimeInterval waitInterval = 0.02; - - MGLOpenGLStyleLayer *layer; - __weak id weakLayer; - @autoreleasepool { - layer = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; - weakLayer = layer; - [self.style insertLayer:layer atIndex:0]; - layer = nil; - - [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; - - [self.style removeLayer:[self.style layerWithIdentifier:@"gl-layer"]]; - [self.mapView setNeedsDisplay]; - - [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; - } - XCTAssertNil(weakLayer); -} - -- (void)testOpenGLLayerDoesNotLeakWhenStyleChanged { - NSTimeInterval waitInterval = 0.02; - __weak id weakLayer; - - @autoreleasepool { - MGLOpenGLStyleLayer *layer = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; - weakLayer = layer; - [self.style insertLayer:layer atIndex:0]; - layer = nil; - - [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; - - NSURL *styleURL = [[NSBundle bundleForClass:[self class]] URLForResource:@"one-liner" withExtension:@"json"]; - _styleLoadingExpectation = [self expectationWithDescription:@"Map view should finish loading style."]; - [self.mapView setStyleURL:styleURL]; - - [self waitForExpectationsWithTimeout:1 handler:nil]; - [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; - } - XCTAssertNil(weakLayer); -} - -- (void)testReusingOpenGLLayerIdentifier { - NSTimeInterval waitInterval = 0.02; - MGLOpenGLStyleLayer *layer1, *layer2; - @autoreleasepool { - layer1 = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; - [self.style insertLayer:layer1 atIndex:0]; - - [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; - - [self.style removeLayer:layer1]; - - layer2 = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; - [self.style insertLayer:layer2 atIndex:0]; - - [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; - - [self.style removeLayer:layer2]; - - [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; - } - XCTAssertNotNil(layer1); - XCTAssertNotNil(layer2); - XCTAssertNil([layer1 style]); - XCTAssertNil([layer2 style]); -} - -@end diff --git a/platform/ios/Integration Tests/MBGLIntegrationTests.mm b/platform/ios/Integration Tests/MBGLIntegrationTests.mm new file mode 100644 index 00000000000..5e390bb8e6f --- /dev/null +++ b/platform/ios/Integration Tests/MBGLIntegrationTests.mm @@ -0,0 +1,459 @@ +#import +#import +#import + +#import "src/MGLStyleLayerRetentionManager_Private.h" +#import "src/MGLStyleLayer_Private.h" +#import "../ios/src/MGLMapView_Private.h" + +@interface MBGLIntegrationTests : XCTestCase + +@property (nonatomic) MGLMapView *mapView; +@property (nonatomic) MGLStyle *style; + +@end + +@implementation MBGLIntegrationTests { + XCTestExpectation *_styleLoadingExpectation; + XCTestExpectation *_renderFinishedExpectation; +} + +#pragma mark - Setup/Teardown + +- (void)setUp { + [super setUp]; + + [MGLAccountManager setAccessToken:@"pk.feedcafedeadbeefbadebede"]; + NSURL *styleURL = [[NSBundle bundleForClass:[self class]] URLForResource:@"one-liner" withExtension:@"json"]; + self.mapView = [[MGLMapView alloc] initWithFrame:UIScreen.mainScreen.bounds styleURL:styleURL]; + self.mapView.delegate = self; + if (!self.mapView.style) { + _styleLoadingExpectation = [self expectationWithDescription:@"Map view should finish loading style."]; + [self waitForExpectationsWithTimeout:1 handler:nil]; + } + + UIView *superView = [[UIView alloc] initWithFrame:UIScreen.mainScreen.bounds]; + [superView addSubview:self.mapView]; + UIWindow *window = [[UIWindow alloc] initWithFrame:UIScreen.mainScreen.bounds]; + [window addSubview:superView]; + [window makeKeyAndVisible]; +} + +- (void)tearDown { + _styleLoadingExpectation = nil; + self.mapView = nil; + self.style = nil; + + [super tearDown]; +} + +#pragma mark - MGLMapViewDelegate + +- (void)mapView:(MGLMapView *)mapView didFinishLoadingStyle:(MGLStyle *)style { + XCTAssertNotNil(mapView.style); + XCTAssertEqual(mapView.style, style); + + [_styleLoadingExpectation fulfill]; +} + +- (void)mapViewDidFinishRenderingFrame:(MGLMapView *)mapView fullyRendered:(__unused BOOL)fullyRendered { + [_renderFinishedExpectation fulfill]; + _renderFinishedExpectation = nil; +} + +#pragma mark - Utilities + +- (void)waitForMapViewToBeRendered { + [self.mapView setNeedsGLDisplay]; + _renderFinishedExpectation = [self expectationWithDescription:@"Map view should be rendered"]; + [self waitForExpectations:@[_renderFinishedExpectation] timeout:1]; +} + +- (void)waitForManagedLayersToExpire { + NSInteger count = MGLStyleLayerRetentionManagerDefaultLifetime; + while (count--) { + [self waitForMapViewToBeRendered]; + } +} + +- (BOOL)isLayerBeingManaged:(MGLStyleLayer*)layer { + return [self.mapView debugIsStyleLayerBeingManaged:layer]; +} + +- (MGLStyle *)style { + return self.mapView.style; +} + +#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 + MGLStyleLayer *layer1 = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; + MGLStyleLayer *layer2 = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; + + XCTAssertNotEqual(layer1, layer2); // Test diff pointers, in case behind the scenes it tries to return the same object. + XCTAssertNotEqualObjects(layer1, layer2); // Although they have the same identifier, we DO NOT consider them equal. + NSMutableSet *set = [NSMutableSet set]; + [set addObject:layer1]; + [set addObject:layer2]; + XCTAssert(set.count == 2); +} + +- (void)testUsingMapTableWithLayersAsKeys { + __weak MGLStyleLayer *weakLayer = nil; + + @autoreleasepool { + NSMapTable *mapTable = [[NSMapTable alloc] initWithKeyOptions:NSPointerFunctionsStrongMemory|NSPointerFunctionsObjectPointerPersonality + valueOptions:NSPointerFunctionsStrongMemory + capacity:100]; + + MGLStyleLayer *layer1 = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; + + NSInteger (*retainCount)(id, SEL) = (NSInteger (*)(id, SEL))objc_msgSend; + SEL retainCountSelector = NSSelectorFromString(@"retainCount"); + + NSInteger retainCount1 = retainCount(layer1, retainCountSelector); + XCTAssert(retainCount1 == 1); + + [mapTable setObject:@(1) forKey:layer1]; + + NSInteger retainCount2 = retainCount(layer1, retainCountSelector); + XCTAssert(retainCount2 == 2); + + NSNumber *number1 = [mapTable objectForKey:layer1]; + XCTAssertNotNil(number1); + XCTAssert([number1 integerValue] == 1); + + // Try adding a second layer, and ensuring it's there... + MGLStyleLayer *layer2 = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; + [mapTable setObject:@(2) forKey:layer2]; + + NSNumber *number2 = [mapTable objectForKey:layer2]; + XCTAssertNotNil(number1); + XCTAssertNotEqual(number1, number2); + XCTAssertNotEqualObjects(number1, number2); + XCTAssert([number2 integerValue] == 2); + } + + XCTAssertNil(weakLayer); +} + +// This test does not strictly need to be in this test file/target. Including here for convenience. +- (void)testOpenGLLayerDoesNotLeakWhenCreatedAndDestroyedWithoutAddingToStyle { + MGLOpenGLStyleLayer *layer = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; + __weak id weakLayer = layer; + layer = nil; + + XCTAssertNil(weakLayer); +} + +- (void)testAddingRemovingOpenGLLayerWithoutRendering { + XCTAssertNotNil(self.style); + + void(^addRemoveGLLayer)(void) = ^{ + __weak id weakLayer = nil; + + @autoreleasepool { + MGLOpenGLStyleLayer *layer = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; + [self.style insertLayer:layer atIndex:0]; + weakLayer = layer; + + // Nil the layer prior to remove to ensure it's being retained + layer = nil; + [self.style removeLayer:weakLayer]; + } + + XCTAssertNil(weakLayer); + }; + + addRemoveGLLayer(); + addRemoveGLLayer(); + addRemoveGLLayer(); +} + +- (void)testReusingOpenGLLayerIdentifier { + __weak MGLOpenGLStyleLayer *weakLayer2; + + @autoreleasepool { + MGLOpenGLStyleLayer *layer1 = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; + [self.style insertLayer:layer1 atIndex:0]; + [self waitForMapViewToBeRendered]; + [self.style removeLayer:layer1]; + + MGLOpenGLStyleLayer *layer2 = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; + weakLayer2 = layer2; + + XCTAssertNotNil(layer2); + XCTAssert(layer1 != layer2); + + [self.style insertLayer:layer2 atIndex:0]; + [self waitForMapViewToBeRendered]; + [self.style removeLayer:layer2]; + + XCTAssertNil([layer1 style]); + XCTAssertNil([layer2 style]); + } + + // At this point, layer2 (and layer1) should still be around, since the render process needs to + // keep a reference to them. + XCTAssertNotNil(weakLayer2); + + // Let render loop run enough to release the layers + [self waitForManagedLayersToExpire]; + XCTAssertNil(weakLayer2); +} + +// Unlike MGLOpenGLStyleLayers other layers are NOT retained, and should be re-created when fetching +// with layerWithIdentifier. This test is to check that assumption (and should fire if we change how +// and if other layer types are retained) +- (void)testUserAddedLineLayersAreNotEqual { + NSString *filename = [[NSBundle mainBundle] pathForResource:@"polyline" ofType:@"geojson"]; + NSData *shapeData = [NSData dataWithContentsOfFile:filename]; + MGLShape *shape = [MGLShape shapeWithData:shapeData encoding:NSUTF8StringEncoding error:nil]; + MGLSource *source1 = [[MGLShapeSource alloc] initWithIdentifier:@"polyline" shape:shape options:nil]; + + [self.style addSource:source1]; + + MGLStyleLayer *lineLayer1 = [[MGLLineStyleLayer alloc] initWithIdentifier:@"line-layer" source:source1]; + [self.style insertLayer:lineLayer1 atIndex:0]; + + // Disconnect the raw layer from the MGLStyleLayer, so that -layerWithIdentifier: is forced to + // re-create the Obj-C layer. + [lineLayer1 debugResetRawLayerPeer]; + + MGLStyleLayer *lineLayer2 = [self.style layerWithIdentifier:@"line-layer"]; + XCTAssertNotNil(lineLayer2); + XCTAssert(lineLayer1 != lineLayer2); + XCTAssertEqualObjects(lineLayer1.identifier, lineLayer2.identifier); + XCTAssertNotEqualObjects(lineLayer1, lineLayer2); // Not currently considered equal. + + // Now fetch the source. This time we don't reset the peer ptr, so the following should return the + // original source object + MGLSource *source2 = [self.style sourceWithIdentifier:@"polyline"]; + XCTAssertNotNil(source2); + XCTAssert(source1 == source2); +} + + +- (void)testAddingRemovingOpenGLLayer { + XCTAssertNotNil(self.style); + + void(^addRemoveGLLayer)(void) = ^{ + + __weak id retrievedLayer = nil; + + @autoreleasepool { + MGLOpenGLStyleLayer *layer = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; + [self.style insertLayer:layer atIndex:0]; + + NSUInteger address1 = (NSUInteger)layer; + layer = nil; + + // We want to ensure the MGLOpenGLStyleLayer's raw Layer's Impl gets handed to a RenderLayer + // (which in turn has a raw context pointer back to the layer) + [self waitForMapViewToBeRendered]; + + retrievedLayer = [self.style layerWithIdentifier:@"gl-layer"]; + NSUInteger address2 = (NSUInteger)retrievedLayer; + + XCTAssertNotNil(retrievedLayer); + + // These two should be the same object in the case of MGLOpenGLStyleLayer + XCTAssert(address1 == address2); + + [self.style removeLayer:retrievedLayer]; + + // We need to run the runloop for a little while (2 trips through the renderer) to allow + // the managed layer to be released. This is because it takes 2 renders before the RenderLayer + // (and the associated Impl) are destroyed. + [self waitForManagedLayersToExpire]; + } + + XCTAssertNil(retrievedLayer); + }; + + addRemoveGLLayer(); + addRemoveGLLayer(); + addRemoveGLLayer(); +} + +- (void)testReusingOpenGLLayer { + NSTimeInterval waitInterval = 0.02; + + NSLog(@"testReusingOpenGLLayer - init and insert layer"); + MGLOpenGLStyleLayer *layer = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; + [self.style insertLayer:layer atIndex:0]; + + [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; + + NSLog(@"testReusingOpenGLLayer - remove layer %p", layer); + [self.style removeLayer:layer]; + + [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; + + NSLog(@"testReusingOpenGLLayer - re-insert layer %p", layer); + + [self.style insertLayer:layer atIndex:0]; + + [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; + + NSLog(@"testReusingOpenGLLayer - re-remove layer %p", layer); + + [self.style removeLayer:layer]; + + [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]]; +} + +- (void)testOpenGLLayerDoesNotLeakWhenRemovedFromStyle { + +// MGLOpenGLStyleLayer *layer; + __weak id weakLayer; + @autoreleasepool { + MGLOpenGLStyleLayer *layer = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; + weakLayer = layer; + [self.style insertLayer:layer atIndex:0]; + layer = nil; + + // 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"]]; + } + + MGLStyleLayer *layer2 = weakLayer; + + XCTAssertNotNil(weakLayer); + XCTAssert([self isLayerBeingManaged:weakLayer]); + [self waitForMapViewToBeRendered]; + + XCTAssertNotNil(weakLayer); + XCTAssert([self isLayerBeingManaged:weakLayer]); + [self waitForMapViewToBeRendered]; + + XCTAssertNotNil(weakLayer); + XCTAssert(![self isLayerBeingManaged:weakLayer]); + + layer2 = nil; + XCTAssertNil(weakLayer); +} + +- (void)testUserAddedLineLayerIsReleasedWhenStyleChanged { + __weak MGLStyleLayer* weakLayer1 = nil; + __weak MGLStyleLayer* weakLayer2 = nil; + + __unsafe_unretained MGLStyleLayer* unsafeLayer1 = nil; + __unsafe_unretained MGLStyleLayer* unsafeLayer2 = nil; + + // Add a line layer + @autoreleasepool { + NSString *filename = [[NSBundle mainBundle] pathForResource:@"polyline" ofType:@"geojson"]; + NSData *shapeData = [NSData dataWithContentsOfFile:filename]; + MGLShape *shape = [MGLShape shapeWithData:shapeData encoding:NSUTF8StringEncoding error:nil]; + MGLSource *source1 = [[MGLShapeSource alloc] initWithIdentifier:@"polyline" shape:shape options:nil]; + + [self.style addSource:source1]; + MGLStyleLayer *lineLayer1 = [[MGLLineStyleLayer alloc] initWithIdentifier:@"line-layer" source:source1]; + [self.style insertLayer:lineLayer1 atIndex:0]; + + unsafeLayer1 = lineLayer1; + weakLayer1 = lineLayer1; + } + XCTAssertNil(weakLayer1); + + // Look for the same layer by Id + @autoreleasepool { + MGLStyleLayer *lineLayer2 = [self.style layerWithIdentifier:@"line-layer"]; + MGLSource *source2 = [self.style sourceWithIdentifier:@"polyline"]; + + XCTAssertNotNil(lineLayer2); + XCTAssertNotNil(source2); + + unsafeLayer2 = lineLayer2; + weakLayer2 = lineLayer2; + } + XCTAssertNil(weakLayer2); + + // Since we only add MGLOpenGLStyleLayers to our retain/release management set, the above layers + // will have been released, but we want to check that the second one was re-created. So just + // check pointer values + XCTAssert(unsafeLayer1); + XCTAssert(unsafeLayer2); + XCTAssert(unsafeLayer1 != unsafeLayer2); + + // Swap style + NSURL *styleURL = [[NSBundle bundleForClass:[self class]] URLForResource:@"one-liner" withExtension:@"json"]; + _styleLoadingExpectation = [self expectationWithDescription:@"Map view should finish loading style."]; + [self.mapView setStyleURL:styleURL]; + [self waitForExpectations:@[_styleLoadingExpectation] timeout:10]; + + // Asking the style for the layer should return nil + { + MGLStyleLayer *lineLayer = [self.style layerWithIdentifier:@"line-layer"]; + MGLSource *lineSource = [self.style sourceWithIdentifier:@"polyline"]; + XCTAssertNil(lineLayer); + XCTAssertNil(lineSource); + } +} + +- (void)testOpenGLLayerDoesNotLeakWhenStyleChanged { + __weak id weakLayer; + + @autoreleasepool { + { + MGLOpenGLStyleLayer *layer = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; + weakLayer = layer; + [self.style insertLayer:layer atIndex:0]; + layer = nil; + } + // Unlike other MGLStyleLayers, MGLOpenGLStyleLayers are managed for retain/release purposes + XCTAssertNotNil(weakLayer); + + [self waitForMapViewToBeRendered]; + + NSURL *styleURL = [[NSBundle bundleForClass:[self class]] URLForResource:@"one-liner" withExtension:@"json"]; + _styleLoadingExpectation = [self expectationWithDescription:@"Map view should finish loading style."]; + [self.mapView setStyleURL:styleURL]; + [self waitForExpectations:@[_styleLoadingExpectation] timeout:10]; + } + + [self waitForManagedLayersToExpire]; + + // Asking the style for the layer should return nil + MGLStyleLayer *layer2 = [self.mapView.style layerWithIdentifier:@"gl-layer"]; + XCTAssertNil(layer2); + XCTAssertNil(weakLayer); +} + + +- (void)testOpenGLLayerDoesNotLeakWhenMapViewDeallocs { + __weak id weakLayer; + + @autoreleasepool { + + NSURL *styleURL = [[NSBundle bundleForClass:[self class]] URLForResource:@"one-liner" withExtension:@"json"]; + MGLMapView *mapView2 = [[MGLMapView alloc] initWithFrame:UIScreen.mainScreen.bounds styleURL:styleURL]; + mapView2.delegate = self; + + XCTAssertNil(mapView2.style); + + _styleLoadingExpectation = [self expectationWithDescription:@"Map view should finish loading style."]; + [self waitForExpectationsWithTimeout:1 handler:nil]; + + MGLOpenGLStyleLayer *layer = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; + weakLayer = layer; + [mapView2.style insertLayer:layer atIndex:0]; + layer = nil; + + // Let the render process fire, to ensure a RenderLayer gets created with a raw pointer back + // to the layer (and it should be retained). + [self waitForMapViewToBeRendered]; + + // We don't wait for the layers to be released here, we'll just let the MGLMapView dealloc + // so everything should be cleaned up. + } + XCTAssertNil(weakLayer); +} + +@end diff --git a/platform/ios/ios.xcodeproj/project.pbxproj b/platform/ios/ios.xcodeproj/project.pbxproj index af266543807..ff1d447f786 100644 --- a/platform/ios/ios.xcodeproj/project.pbxproj +++ b/platform/ios/ios.xcodeproj/project.pbxproj @@ -20,7 +20,7 @@ 07D947521F67488800E37934 /* MGLAbstractShapeSource.h in Headers */ = {isa = PBXBuildFile; fileRef = 07D9474F1F67487E00E37934 /* MGLAbstractShapeSource.h */; settings = {ATTRIBUTES = (Public, ); }; }; 07D947531F67488E00E37934 /* MGLAbstractShapeSource_Private.h in Headers */ = {isa = PBXBuildFile; fileRef = 07D9474E1F67487E00E37934 /* MGLAbstractShapeSource_Private.h */; }; 07D947541F67489200E37934 /* MGLAbstractShapeSource.mm in Sources */ = {isa = PBXBuildFile; fileRef = 07D947501F67487E00E37934 /* MGLAbstractShapeSource.mm */; }; - 16376B0A1FFD9DAF0000563E /* MBGLIntegrationTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 16376B091FFD9DAF0000563E /* MBGLIntegrationTests.m */; }; + 16376B0A1FFD9DAF0000563E /* MBGLIntegrationTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 16376B091FFD9DAF0000563E /* MBGLIntegrationTests.mm */; }; 16376B331FFDB4B40000563E /* AppDelegate.m in Sources */ = {isa = PBXBuildFile; fileRef = 16376B321FFDB4B40000563E /* AppDelegate.m */; }; 16376B3B1FFDB4B40000563E /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 16376B3A1FFDB4B40000563E /* Assets.xcassets */; }; 16376B3E1FFDB4B40000563E /* LaunchScreen.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = 16376B3C1FFDB4B40000563E /* LaunchScreen.storyboard */; }; @@ -308,6 +308,13 @@ AC518E04201BB56100EBC820 /* MGLTelemetryConfig.m in Sources */ = {isa = PBXBuildFile; fileRef = AC518DFE201BB55A00EBC820 /* MGLTelemetryConfig.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, ); }; }; + CA67C7FB204CFC9000C1CBB4 /* Mapbox.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = DA8847D21CBAF91600AB86E3 /* Mapbox.framework */; }; + CA67C7FD204CFCAA00C1CBB4 /* UIKit.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = CA67C7FC204CFCAA00C1CBB4 /* UIKit.framework */; }; + CA7F25182048841E00896957 /* polyline.geojson in Resources */ = {isa = PBXBuildFile; fileRef = DA1DC96D1CB6C6CE006E619F /* polyline.geojson */; }; + CA8F66E62049DD2900B5B9DC /* MGLStyleLayerRetentionManager_Private.h in Headers */ = {isa = PBXBuildFile; fileRef = CA8F66E42049DD2900B5B9DC /* MGLStyleLayerRetentionManager_Private.h */; }; + CA8F66E72049DD2900B5B9DC /* MGLStyleLayerRetentionManager_Private.h in Headers */ = {isa = PBXBuildFile; fileRef = CA8F66E42049DD2900B5B9DC /* MGLStyleLayerRetentionManager_Private.h */; }; + CA8F66E82049DD2900B5B9DC /* MGLStyleLayerRetentionManager.mm in Sources */ = {isa = PBXBuildFile; fileRef = CA8F66E52049DD2900B5B9DC /* MGLStyleLayerRetentionManager.mm */; }; + CA8F66E92049DD2900B5B9DC /* MGLStyleLayerRetentionManager.mm in Sources */ = {isa = PBXBuildFile; fileRef = CA8F66E52049DD2900B5B9DC /* MGLStyleLayerRetentionManager.mm */; }; CAAD2BD62041EF05003881EC /* Mapbox.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = DA4A26961CB6E795000B7809 /* Mapbox.framework */; }; CAAD2BD72041EF05003881EC /* Mapbox.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = DA4A26961CB6E795000B7809 /* Mapbox.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; }; DA00FC8E1D5EEB0D009AABC8 /* MGLAttributionInfo.h in Headers */ = {isa = PBXBuildFile; fileRef = DA00FC8C1D5EEB0D009AABC8 /* MGLAttributionInfo.h */; settings = {ATTRIBUTES = (Public, ); }; }; @@ -685,7 +692,7 @@ 07D9474F1F67487E00E37934 /* MGLAbstractShapeSource.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MGLAbstractShapeSource.h; sourceTree = ""; }; 07D947501F67487E00E37934 /* MGLAbstractShapeSource.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MGLAbstractShapeSource.mm; sourceTree = ""; }; 16376B071FFD9DAF0000563E /* integration.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = integration.xctest; sourceTree = BUILT_PRODUCTS_DIR; }; - 16376B091FFD9DAF0000563E /* MBGLIntegrationTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MBGLIntegrationTests.m; sourceTree = ""; }; + 16376B091FFD9DAF0000563E /* MBGLIntegrationTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = MBGLIntegrationTests.mm; sourceTree = ""; }; 16376B0B1FFD9DAF0000563E /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = ""; }; 16376B2F1FFDB4B40000563E /* Integration Test Harness.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = "Integration Test Harness.app"; sourceTree = BUILT_PRODUCTS_DIR; }; 16376B311FFDB4B40000563E /* AppDelegate.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = AppDelegate.h; sourceTree = ""; }; @@ -867,6 +874,9 @@ AC518DFD201BB55A00EBC820 /* MGLTelemetryConfig.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MGLTelemetryConfig.h; sourceTree = ""; }; AC518DFE201BB55A00EBC820 /* MGLTelemetryConfig.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MGLTelemetryConfig.m; sourceTree = ""; }; CA55CD3E202C16AA00CE7095 /* MGLCameraChangeReason.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MGLCameraChangeReason.h; sourceTree = ""; }; + CA67C7FC204CFCAA00C1CBB4 /* UIKit.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = UIKit.framework; path = System/Library/Frameworks/UIKit.framework; sourceTree = SDKROOT; }; + CA8F66E42049DD2900B5B9DC /* MGLStyleLayerRetentionManager_Private.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MGLStyleLayerRetentionManager_Private.h; sourceTree = ""; }; + CA8F66E52049DD2900B5B9DC /* MGLStyleLayerRetentionManager.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = MGLStyleLayerRetentionManager.mm; 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 = ""; }; @@ -1140,6 +1150,8 @@ isa = PBXFrameworksBuildPhase; buildActionMask = 2147483647; files = ( + CA67C7FD204CFCAA00C1CBB4 /* UIKit.framework in Frameworks */, + CA67C7FB204CFC9000C1CBB4 /* Mapbox.framework in Frameworks */, ); runOnlyForDeploymentPostprocessing = 0; }; @@ -1205,7 +1217,7 @@ 16376B081FFD9DAF0000563E /* Integration Tests */ = { isa = PBXGroup; children = ( - 16376B091FFD9DAF0000563E /* MBGLIntegrationTests.m */, + 16376B091FFD9DAF0000563E /* MBGLIntegrationTests.mm */, 16376B0B1FFD9DAF0000563E /* Info.plist */, ); path = "Integration Tests"; @@ -1496,6 +1508,7 @@ DA1DC9921CB6DF24006E619F /* Frameworks */ = { isa = PBXGroup; children = ( + CA67C7FC204CFCAA00C1CBB4 /* UIKit.framework */, 55D120AD1F791018004B6D81 /* libmbgl-loop-darwin.a */, 55D120AB1F791015004B6D81 /* libmbgl-filesource.a */, 55D120A91F79100C004B6D81 /* libmbgl-filesource.a */, @@ -1612,6 +1625,8 @@ DA8847EE1CBAFA5100AB86E3 /* MGLTypes.h */, DA8848111CBAFA6200AB86E3 /* MGLTypes.m */, 35E1A4D71D74336F007AA97F /* MGLValueEvaluator.h */, + CA8F66E42049DD2900B5B9DC /* MGLStyleLayerRetentionManager_Private.h */, + CA8F66E52049DD2900B5B9DC /* MGLStyleLayerRetentionManager.mm */, ); name = Foundation; path = ../darwin/src; @@ -1924,6 +1939,7 @@ 9654C1261FFC1AB900DB6A19 /* MGLPolyline_Private.h in Headers */, 40F887701D7A1E58008ECB67 /* MGLShapeSource_Private.h in Headers */, 350098DC1D484E60004B2AF0 /* NSValue+MGLStyleAttributeAdditions.h in Headers */, + CA8F66E62049DD2900B5B9DC /* MGLStyleLayerRetentionManager_Private.h in Headers */, DA8848231CBAFA6200AB86E3 /* MGLOfflineStorage_Private.h in Headers */, 404326891D5B9B27007111BD /* MGLAnnotationContainerView_Private.h in Headers */, CA55CD41202C16AA00CE7095 /* MGLCameraChangeReason.h in Headers */, @@ -2122,6 +2138,7 @@ DABFB8651CBE99E500D62B32 /* MGLOverlay.h in Headers */, 35E79F211D41266300957B9E /* MGLStyleLayer_Private.h in Headers */, 350098DD1D484E60004B2AF0 /* NSValue+MGLStyleAttributeAdditions.h in Headers */, + CA8F66E72049DD2900B5B9DC /* MGLStyleLayerRetentionManager_Private.h in Headers */, DABFB8681CBE99E500D62B32 /* MGLPolyline.h in Headers */, 96E516DF200054FB00A02306 /* MGLShape_Private.h in Headers */, DABFB86F1CBE9A0F00D62B32 /* MGLMapView.h in Headers */, @@ -2449,6 +2466,7 @@ buildActionMask = 2147483647; files = ( 16376B3E1FFDB4B40000563E /* LaunchScreen.storyboard in Resources */, + CA7F25182048841E00896957 /* polyline.geojson in Resources */, 16376B3B1FFDB4B40000563E /* Assets.xcassets in Resources */, ); runOnlyForDeploymentPostprocessing = 0; @@ -2546,7 +2564,7 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( - 16376B0A1FFD9DAF0000563E /* MBGLIntegrationTests.m in Sources */, + 16376B0A1FFD9DAF0000563E /* MBGLIntegrationTests.mm in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; @@ -2692,6 +2710,7 @@ 404C26E41D89B877000AA13D /* MGLTileSource.mm in Sources */, 07D947541F67489200E37934 /* MGLAbstractShapeSource.mm in Sources */, 355AE0011E9281DA00F3939D /* MGLScaleBar.mm in Sources */, + CA8F66E82049DD2900B5B9DC /* MGLStyleLayerRetentionManager.mm in Sources */, DA88481D1CBAFA6200AB86E3 /* MGLMapCamera.mm in Sources */, DACA86282019218600E9693A /* MGLRasterDEMSource.mm in Sources */, DA8848261CBAFA6200AB86E3 /* MGLPolygon.mm in Sources */, @@ -2785,6 +2804,7 @@ 355AE0021E9281DA00F3939D /* MGLScaleBar.mm in Sources */, 4018B1C81CDC287F00F666AF /* MGLAnnotationView.mm in Sources */, 07D8C6FB1F67560100381808 /* MGLComputedShapeSource.mm in Sources */, + CA8F66E92049DD2900B5B9DC /* MGLStyleLayerRetentionManager.mm in Sources */, DAA4E4341CBB730400178DFB /* MGLFaux3DUserLocationAnnotationView.m in Sources */, DACA86292019218600E9693A /* MGLRasterDEMSource.mm in Sources */, 35B82BFB1D6C5F8400B1B721 /* NSPredicate+MGLAdditions.mm in Sources */, @@ -3032,10 +3052,10 @@ /* Begin XCBuildConfiguration section */ 16376B0E1FFD9DAF0000563E /* Debug */ = { isa = XCBuildConfiguration; + baseConfigurationReference = 55D8C9941D0F133500F42F10 /* config.xcconfig */; buildSettings = { BUNDLE_LOADER = "$(TEST_HOST)"; CLANG_ANALYZER_NUMBER_OBJECT_CONVERSION = YES_AGGRESSIVE; - CLANG_CXX_LANGUAGE_STANDARD = "gnu++14"; CLANG_WARN_BLOCK_CAPTURE_AUTORELEASING = YES; CLANG_WARN_COMMA = YES; CLANG_WARN_NON_LITERAL_NULL_CONVERSION = YES; @@ -3046,9 +3066,11 @@ CODE_SIGN_IDENTITY = "iPhone Developer"; CODE_SIGN_STYLE = Automatic; GCC_C_LANGUAGE_STANDARD = gnu11; + HEADER_SEARCH_PATHS = "$(mbgl_core_INCLUDE_DIRECTORIES)"; INFOPLIST_FILE = "Integration Tests/Info.plist"; IPHONEOS_DEPLOYMENT_TARGET = 11.2; LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks @loader_path/Frameworks"; + OTHER_CFLAGS = "-fvisibility=hidden"; PRODUCT_BUNDLE_IDENTIFIER = "com.mapbox.integration-tests"; PRODUCT_NAME = "$(TARGET_NAME)"; TARGETED_DEVICE_FAMILY = "1,2"; @@ -3058,10 +3080,10 @@ }; 16376B0F1FFD9DAF0000563E /* Release */ = { isa = XCBuildConfiguration; + baseConfigurationReference = 55D8C9941D0F133500F42F10 /* config.xcconfig */; buildSettings = { BUNDLE_LOADER = "$(TEST_HOST)"; CLANG_ANALYZER_NUMBER_OBJECT_CONVERSION = YES_AGGRESSIVE; - CLANG_CXX_LANGUAGE_STANDARD = "gnu++14"; CLANG_WARN_BLOCK_CAPTURE_AUTORELEASING = YES; CLANG_WARN_COMMA = YES; CLANG_WARN_NON_LITERAL_NULL_CONVERSION = YES; @@ -3072,9 +3094,11 @@ CODE_SIGN_IDENTITY = "iPhone Developer"; CODE_SIGN_STYLE = Automatic; GCC_C_LANGUAGE_STANDARD = gnu11; + HEADER_SEARCH_PATHS = "$(mbgl_core_INCLUDE_DIRECTORIES)"; INFOPLIST_FILE = "Integration Tests/Info.plist"; IPHONEOS_DEPLOYMENT_TARGET = 11.2; LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks @loader_path/Frameworks"; + OTHER_CFLAGS = "-fvisibility=hidden"; PRODUCT_BUNDLE_IDENTIFIER = "com.mapbox.integration-tests"; PRODUCT_NAME = "$(TARGET_NAME)"; TARGETED_DEVICE_FAMILY = "1,2"; diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm index 92ee50ce3de..91121670a24 100644 --- a/platform/ios/src/MGLMapView.mm +++ b/platform/ios/src/MGLMapView.mm @@ -65,6 +65,7 @@ #import "MGLScaleBar.h" #import "MGLStyle_Private.h" #import "MGLStyleLayer_Private.h" +#import "MGLStyleLayerRetentionManager_Private.h" #import "MGLMapboxEvents.h" #import "MGLSDKUpdateChecker.h" #import "MGLCompactCalloutView.h" @@ -200,6 +201,7 @@ @interface MGLMapView () (layerID, init, render, contextLost, deinit, context)) { + : Layer(makeMutable(layerID, init, render, contextLost, deinit, context)), + deallocationFn(deallocation) { } CustomLayer::CustomLayer(const std::string& layerID, CustomLayerInitializeFunction init, CustomLayerRenderFunction render, CustomLayerDeinitializeFunction deinit, + CustomLayerDeallocationFunction deallocation, void* context) - : Layer(makeMutable(layerID, init, render, nullptr, deinit, context)) { + : Layer(makeMutable(layerID, init, render, nullptr, deinit, context)), + deallocationFn(deallocation) { } -CustomLayer::~CustomLayer() = default; +CustomLayer::~CustomLayer() { + if (deallocationFn) { + deallocationFn(&peer); + } +}; const CustomLayer::Impl& CustomLayer::impl() const { return static_cast(*baseImpl); From bcdf23028e93a8216fc6b877486a04ac5a579c58 Mon Sep 17 00:00:00 2001 From: Julian Rex Date: Mon, 5 Mar 2018 14:16:15 -0500 Subject: [PATCH 5/5] [macos] Updated MGLMapView with new layer manager. --- platform/macos/macos.xcodeproj/project.pbxproj | 8 ++++++++ platform/macos/src/MGLMapView.mm | 11 ++++++++--- test/api/custom_layer.test.cpp | 10 ++++++---- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/platform/macos/macos.xcodeproj/project.pbxproj b/platform/macos/macos.xcodeproj/project.pbxproj index 43276709114..903b34e7dd5 100644 --- a/platform/macos/macos.xcodeproj/project.pbxproj +++ b/platform/macos/macos.xcodeproj/project.pbxproj @@ -97,6 +97,8 @@ 9654C12B1FFC38E000DB6A19 /* MGLPolyline_Private.h in Headers */ = {isa = PBXBuildFile; fileRef = 9654C12A1FFC38E000DB6A19 /* MGLPolyline_Private.h */; }; 9654C12D1FFC394700DB6A19 /* MGLPolygon_Private.h in Headers */ = {isa = PBXBuildFile; fileRef = 9654C12C1FFC394700DB6A19 /* MGLPolygon_Private.h */; }; 96E027311E57C9A7004B8E66 /* Localizable.strings in Resources */ = {isa = PBXBuildFile; fileRef = 96E027331E57C9A7004B8E66 /* Localizable.strings */; }; + CA67C814204DCC0300C1CBB4 /* MGLStyleLayerRetentionManager_Private.h in Headers */ = {isa = PBXBuildFile; fileRef = CA67C805204DCBE800C1CBB4 /* MGLStyleLayerRetentionManager_Private.h */; }; + CA67C815204DCC0D00C1CBB4 /* MGLStyleLayerRetentionManager.mm in Sources */ = {isa = PBXBuildFile; fileRef = CA67C807204DCBE900C1CBB4 /* MGLStyleLayerRetentionManager.mm */; }; DA00FC8A1D5EEAC3009AABC8 /* MGLAttributionInfo.h in Headers */ = {isa = PBXBuildFile; fileRef = DA00FC881D5EEAC3009AABC8 /* MGLAttributionInfo.h */; settings = {ATTRIBUTES = (Public, ); }; }; DA00FC8B1D5EEAC3009AABC8 /* MGLAttributionInfo.mm in Sources */ = {isa = PBXBuildFile; fileRef = DA00FC891D5EEAC3009AABC8 /* MGLAttributionInfo.mm */; }; DA0CD58E1CF56F5800A5F5A5 /* MGLFeatureTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = DA0CD58D1CF56F5800A5F5A5 /* MGLFeatureTests.mm */; }; @@ -398,6 +400,8 @@ 96E027391E57C9B9004B8E66 /* sv */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = sv; path = sv.lproj/Localizable.strings; sourceTree = ""; }; 96E0273A1E57C9BB004B8E66 /* vi */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = vi; path = vi.lproj/Localizable.strings; sourceTree = ""; }; 96E0273B1E57C9BC004B8E66 /* pt-BR */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = "pt-BR"; path = "pt-BR.lproj/Localizable.strings"; sourceTree = ""; }; + CA67C805204DCBE800C1CBB4 /* MGLStyleLayerRetentionManager_Private.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MGLStyleLayerRetentionManager_Private.h; sourceTree = ""; }; + CA67C807204DCBE900C1CBB4 /* MGLStyleLayerRetentionManager.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MGLStyleLayerRetentionManager.mm; sourceTree = ""; }; DA00FC881D5EEAC3009AABC8 /* MGLAttributionInfo.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MGLAttributionInfo.h; sourceTree = ""; }; DA00FC891D5EEAC3009AABC8 /* MGLAttributionInfo.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MGLAttributionInfo.mm; sourceTree = ""; }; DA0CD58D1CF56F5800A5F5A5 /* MGLFeatureTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = MGLFeatureTests.mm; path = ../../darwin/test/MGLFeatureTests.mm; sourceTree = ""; }; @@ -1130,6 +1134,8 @@ 92F2C3EA1F0E3A1900268EC0 /* MGLRendererFrontend.h */, DAE6C3571CC31E0400DB3429 /* MGLStyle.h */, 3537CA731D3F93A600380318 /* MGLStyle_Private.h */, + CA67C805204DCBE800C1CBB4 /* MGLStyleLayerRetentionManager_Private.h */, + CA67C807204DCBE900C1CBB4 /* MGLStyleLayerRetentionManager.mm */, DAE6C37A1CC31E2A00DB3429 /* MGLStyle.mm */, DAE6C3591CC31E0400DB3429 /* MGLTypes.h */, DAE6C37C1CC31E2A00DB3429 /* MGLTypes.m */, @@ -1207,6 +1213,7 @@ DAE6C3971CC31E2A00DB3429 /* NSBundle+MGLAdditions.h in Headers */, DAED385F1D62CED700D7640F /* NSURL+MGLAdditions.h in Headers */, DAD165741CF4CD7A001FF4B9 /* MGLShapeCollection.h in Headers */, + CA67C814204DCC0300C1CBB4 /* MGLStyleLayerRetentionManager_Private.h in Headers */, DAE6C3631CC31E0400DB3429 /* MGLPointAnnotation.h in Headers */, DAC2ABC51CC6D343006D18C4 /* MGLAnnotationImage_Private.h in Headers */, DAE6C35F1CC31E0400DB3429 /* MGLOfflinePack.h in Headers */, @@ -1543,6 +1550,7 @@ DAE6C3911CC31E2A00DB3429 /* MGLPolygon.mm in Sources */, 35C6DF851E214C0400ACA483 /* MGLDistanceFormatter.m in Sources */, DAE6C39B1CC31E2A00DB3429 /* NSProcessInfo+MGLAdditions.m in Sources */, + CA67C815204DCC0D00C1CBB4 /* MGLStyleLayerRetentionManager.mm in Sources */, DAA998FC1E9F545C002E6EA6 /* MGLFillExtrusionStyleLayer.mm in Sources */, DAE6C38F1CC31E2A00DB3429 /* MGLOfflineStorage.mm in Sources */, DAED38601D62CED700D7640F /* NSURL+MGLAdditions.m in Sources */, diff --git a/platform/macos/src/MGLMapView.mm b/platform/macos/src/MGLMapView.mm index e54b0f195d8..8b9a5a982f1 100644 --- a/platform/macos/src/MGLMapView.mm +++ b/platform/macos/src/MGLMapView.mm @@ -15,6 +15,7 @@ #import "MGLMultiPoint_Private.h" #import "MGLOfflineStorage_Private.h" #import "MGLStyle_Private.h" +#import "MGLStyleLayerRetentionManager_Private.h" #import "MGLShape_Private.h" #import "MGLAccountManager.h" @@ -144,6 +145,7 @@ @interface MGLMapView () ( "custom", [] (void* context) { - reinterpret_cast(context)->initialize(); + reinterpret_cast(context)->initialize(); }, [] (void* context, const CustomLayerRenderParameters&) { - reinterpret_cast(context)->render(); + reinterpret_cast(context)->render(); }, [] (void* context) { - delete reinterpret_cast(context); - }, new TestLayer())); + delete reinterpret_cast(context); + }, + nullptr, + new TestLayer())); auto layer = std::make_unique("landcover", "mapbox"); layer->setSourceLayer("landcover");