diff options
Diffstat (limited to 'platform')
13 files changed, 323 insertions, 183 deletions
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/layers/CustomLayer.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/layers/CustomLayer.java index f77e7280f0..a0ba1e2159 100644 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/layers/CustomLayer.java +++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/layers/CustomLayer.java @@ -9,20 +9,8 @@ package com.mapbox.mapboxsdk.style.layers; public class CustomLayer extends Layer { public CustomLayer(String id, - long context, - long initializeFunction, - long renderFunction, - long deinitializeFunction) { - this(id, context, initializeFunction, renderFunction, 0L, deinitializeFunction); - } - - public CustomLayer(String id, - long context, - long initializeFunction, - long renderFunction, - long contextLostFunction, - long deinitializeFunction) { - initialize(id, initializeFunction, renderFunction, contextLostFunction, deinitializeFunction, context); + long host) { + initialize(id, host); } public CustomLayer(long nativePtr) { @@ -33,9 +21,7 @@ public class CustomLayer extends Layer { nativeUpdate(); } - protected native void initialize(String id, long initializeFunction, long renderFunction, - long contextLostFunction, long deinitializeFunction, - long context); + protected native void initialize(String id, long host); protected native void nativeUpdate(); diff --git a/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/customlayer/CustomLayerActivity.java b/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/customlayer/CustomLayerActivity.java index 4cad7593ef..7685dee840 100644 --- a/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/customlayer/CustomLayerActivity.java +++ b/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/customlayer/CustomLayerActivity.java @@ -58,11 +58,7 @@ public class CustomLayerActivity extends AppCompatActivity { fab.setImageResource(R.drawable.ic_layers); } else { customLayer = new CustomLayer("custom", - ExampleCustomLayer.createContext(), - ExampleCustomLayer.InitializeFunction, - ExampleCustomLayer.RenderFunction, - ExampleCustomLayer.ContextLostFunction, // Optional - ExampleCustomLayer.DeinitializeFunction); + ExampleCustomLayer.createContext()); mapboxMap.addLayerBelow(customLayer, "building"); fab.setImageResource(R.drawable.ic_layers_clear); } diff --git a/platform/android/src/example_custom_layer.cpp b/platform/android/src/example_custom_layer.cpp index f7b425c40a..e805532541 100644 --- a/platform/android/src/example_custom_layer.cpp +++ b/platform/android/src/example_custom_layer.cpp @@ -112,18 +112,9 @@ void checkCompileStatus(GLuint shader) { static const GLchar * vertexShaderSource = "attribute vec2 a_pos; void main() { gl_Position = vec4(a_pos, 0, 1); }"; static const GLchar * fragmentShaderSource = "uniform highp vec4 fill_color; void main() { gl_FragColor = fill_color; }"; -class ExampleCustomLayer { +class ExampleCustomLayer: mbgl::style::CustomLayerHost { public: ~ExampleCustomLayer() { - __android_log_write(ANDROID_LOG_INFO, LOG_TAG, "~ExampleCustomLayer"); - if (program) { - glDeleteBuffers(1, &buffer); - glDetachShader(program, vertexShader); - glDetachShader(program, fragmentShader); - glDeleteShader(vertexShader); - glDeleteShader(fragmentShader); - glDeleteProgram(program); - } } void initialize() { @@ -158,8 +149,15 @@ public: GL_CHECK_ERROR(glBufferData(GL_ARRAY_BUFFER, 8 * sizeof(GLfloat), background, GL_STATIC_DRAW)); } - void render() { + void render(const mbgl::style::CustomLayerRenderParameters&) { __android_log_write(ANDROID_LOG_INFO, LOG_TAG, "Render"); + glUseProgram(program); + glBindBuffer(GL_ARRAY_BUFFER, buffer); + glEnableVertexAttribArray(a_pos); + glVertexAttribPointer(a_pos, 2, GL_FLOAT, GL_FALSE, 0, NULL); + glDisable(GL_STENCIL_TEST); + glDisable(GL_DEPTH_TEST); + glUniform4fv(fill_color, 1, color); GL_CHECK_ERROR(glUseProgram(program)); GL_CHECK_ERROR(glBindBuffer(GL_ARRAY_BUFFER, buffer)); @@ -172,6 +170,23 @@ public: } + void contextLost() { + __android_log_write(ANDROID_LOG_INFO, LOG_TAG, "ContextLost"); + program = 0; + } + + void deinitialize() { + __android_log_write(ANDROID_LOG_INFO, LOG_TAG, "DeInitialize"); + if (program) { + glDeleteBuffers(1, &buffer); + glDetachShader(program, vertexShader); + glDetachShader(program, fragmentShader); + glDeleteShader(vertexShader); + glDeleteShader(fragmentShader); + glDeleteProgram(program); + } + } + GLuint program = 0; GLuint vertexShader = 0; GLuint fragmentShader = 0; @@ -186,7 +201,8 @@ GLfloat ExampleCustomLayer::color[] = { 0.0f, 1.0f, 0.0f, 1.0f }; jlong JNICALL nativeCreateContext(JNIEnv*, jobject) { __android_log_write(ANDROID_LOG_INFO, LOG_TAG, "nativeCreateContext"); - return reinterpret_cast<jlong>(new ExampleCustomLayer()); + auto exampleCustomLayer = std::make_unique<ExampleCustomLayer>(); + return reinterpret_cast<jlong>(exampleCustomLayer.release()); } void JNICALL nativeSetColor(JNIEnv*, jobject, jfloat red, jfloat green, jfloat blue, jfloat alpha) { @@ -197,25 +213,6 @@ void JNICALL nativeSetColor(JNIEnv*, jobject, jfloat red, jfloat green, jfloat b ExampleCustomLayer::color[3] = alpha; } -void nativeInitialize(void *context) { - __android_log_write(ANDROID_LOG_INFO, LOG_TAG, "nativeInitialize"); - reinterpret_cast<ExampleCustomLayer*>(context)->initialize(); -} - -void nativeRender(void *context, const mbgl::style::CustomLayerRenderParameters& /*parameters*/) { - __android_log_write(ANDROID_LOG_INFO, LOG_TAG, "nativeRender"); - reinterpret_cast<ExampleCustomLayer*>(context)->render(); -} - -void nativeContextLost(void */*context*/) { - __android_log_write(ANDROID_LOG_INFO, LOG_TAG, "nativeContextLost"); -} - -void nativeDeinitialize(void *context) { - __android_log_write(ANDROID_LOG_INFO, LOG_TAG, "nativeDeinitialize"); - delete reinterpret_cast<ExampleCustomLayer*>(context); -} - extern "C" JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM *vm, void *) { __android_log_write(ANDROID_LOG_INFO, LOG_TAG, "OnLoad"); @@ -234,22 +231,6 @@ extern "C" JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM *vm, void *) { return JNI_ERR; } - env->SetStaticLongField(customLayerClass, - env->GetStaticFieldID(customLayerClass, "InitializeFunction", "J"), - reinterpret_cast<jlong>(nativeInitialize)); - - env->SetStaticLongField(customLayerClass, - env->GetStaticFieldID(customLayerClass, "RenderFunction", "J"), - reinterpret_cast<jlong>(nativeRender)); - - env->SetStaticLongField(customLayerClass, - env->GetStaticFieldID(customLayerClass, "ContextLostFunction", "J"), - reinterpret_cast<jlong>(nativeContextLost)); - - env->SetStaticLongField(customLayerClass, - env->GetStaticFieldID(customLayerClass, "DeinitializeFunction", "J"), - reinterpret_cast<jlong>(nativeDeinitialize)); - return JNI_VERSION_1_6; } diff --git a/platform/android/src/style/layers/custom_layer.cpp b/platform/android/src/style/layers/custom_layer.cpp index 51a48520bf..61e74a9cf5 100644 --- a/platform/android/src/style/layers/custom_layer.cpp +++ b/platform/android/src/style/layers/custom_layer.cpp @@ -7,14 +7,10 @@ namespace mbgl { namespace android { - CustomLayer::CustomLayer(jni::JNIEnv& env, jni::String layerId, jni::jlong initializeFunction, jni::jlong renderFunction, jni::jlong contextLostFunction, jni::jlong deinitializeFunction, jni::jlong context) + CustomLayer::CustomLayer(jni::JNIEnv& env, jni::String layerId, jni::jlong host) : Layer(env, std::make_unique<mbgl::style::CustomLayer>( jni::Make<std::string>(env, layerId), - reinterpret_cast<mbgl::style::CustomLayerInitializeFunction>(initializeFunction), - reinterpret_cast<mbgl::style::CustomLayerRenderFunction>(renderFunction), - reinterpret_cast<mbgl::style::CustomLayerContextLostFunction>(contextLostFunction), - reinterpret_cast<mbgl::style::CustomLayerDeinitializeFunction>(deinitializeFunction), - reinterpret_cast<void*>(context)) + std::unique_ptr<mbgl::style::CustomLayerHost>(reinterpret_cast<mbgl::style::CustomLayerHost*>(host))) ) { } @@ -53,7 +49,7 @@ namespace android { // Register the peer jni::RegisterNativePeer<CustomLayer>( env, CustomLayer::javaClass, "nativePtr", - std::make_unique<CustomLayer, JNIEnv&, jni::String, jni::jlong, jni::jlong, jni::jlong, jni::jlong, jni::jlong>, + std::make_unique<CustomLayer, JNIEnv&, jni::String, jni::jlong>, "initialize", "finalize", METHOD(&CustomLayer::update, "nativeUpdate")); diff --git a/platform/android/src/style/layers/custom_layer.hpp b/platform/android/src/style/layers/custom_layer.hpp index 9e079c1288..7eb649d923 100644 --- a/platform/android/src/style/layers/custom_layer.hpp +++ b/platform/android/src/style/layers/custom_layer.hpp @@ -16,7 +16,7 @@ public: static void registerNative(jni::JNIEnv&); - CustomLayer(jni::JNIEnv&, jni::String, jni::jlong, jni::jlong, jni::jlong, jni::jlong, jni::jlong); + CustomLayer(jni::JNIEnv&, jni::String, jni::jlong); CustomLayer(mbgl::Map&, mbgl::style::CustomLayer&); diff --git a/platform/darwin/src/MGLOpenGLStyleLayer.mm b/platform/darwin/src/MGLOpenGLStyleLayer.mm index 8933a77382..bbd7422c74 100644 --- a/platform/darwin/src/MGLOpenGLStyleLayer.mm +++ b/platform/darwin/src/MGLOpenGLStyleLayer.mm @@ -7,49 +7,47 @@ #include <mbgl/style/layers/custom_layer.hpp> #include <mbgl/math/wrap.hpp> -/** - Runs the preparation handler block contained in the given context, which is - implicitly an instance of `MGLOpenGLStyleLayer`. +class MGLOpenGLLayerHost : public mbgl::style::CustomLayerHost { +public: + MGLOpenGLLayerHost(MGLOpenGLStyleLayer *styleLayer) { + layerRef = styleLayer; + layer = nil; + } - @param context An `MGLOpenGLStyleLayer` instance that was provided as context - when creating an OpenGL style layer. - */ -void MGLPrepareCustomStyleLayer(void *context) { - MGLOpenGLStyleLayer *layer = (__bridge MGLOpenGLStyleLayer *)context; - [layer didMoveToMapView:layer.style.mapView]; -} + void initialize() { + if (layerRef == nil) return; + else if (layer == nil) layer = layerRef; -/** - Runs the drawing handler block contained in the given context, which is - implicitly an instance of `MGLOpenGLStyleLayer`. + [layer didMoveToMapView:layer.style.mapView]; + } - @param context An `MGLOpenGLStyleLayer` instance that was provided as context - when creating an OpenGL style layer. - */ -void MGLDrawCustomStyleLayer(void *context, const mbgl::style::CustomLayerRenderParameters ¶ms) { - MGLOpenGLStyleLayer *layer = (__bridge MGLOpenGLStyleLayer *)context; - MGLStyleLayerDrawingContext drawingContext = { - .size = CGSizeMake(params.width, params.height), - .centerCoordinate = CLLocationCoordinate2DMake(params.latitude, params.longitude), - .zoomLevel = params.zoom, - .direction = mbgl::util::wrap(params.bearing, 0., 360.), - .pitch = static_cast<CGFloat>(params.pitch), - .fieldOfView = static_cast<CGFloat>(params.fieldOfView), - }; - [layer drawInMapView:layer.style.mapView withContext:drawingContext]; -} + void render(const mbgl::style::CustomLayerRenderParameters ¶ms) { + if(!layer) return; + + MGLStyleLayerDrawingContext drawingContext = { + .size = CGSizeMake(params.width, params.height), + .centerCoordinate = CLLocationCoordinate2DMake(params.latitude, params.longitude), + .zoomLevel = params.zoom, + .direction = mbgl::util::wrap(params.bearing, 0., 360.), + .pitch = static_cast<CGFloat>(params.pitch), + .fieldOfView = static_cast<CGFloat>(params.fieldOfView), + }; + [layer drawInMapView:layer.style.mapView withContext:drawingContext]; + } -/** - Runs the completion handler block contained in the given context, which is - implicitly an instance of `MGLOpenGLStyleLayer`. + void contextLost() {} - @param context An `MGLOpenGLStyleLayer` instance that was provided as context - when creating an OpenGL style layer. - */ -void MGLFinishCustomStyleLayer(void *context) { - MGLOpenGLStyleLayer *layer = (__bridge_transfer MGLOpenGLStyleLayer *)context; - [layer willMoveFromMapView:layer.style.mapView]; -} + void deinitialize() { + if (layer == nil) return; + + [layer willMoveFromMapView:layer.style.mapView]; + layerRef = layer; + layer = nil; + } +private: + __weak MGLOpenGLStyleLayer * layerRef; + MGLOpenGLStyleLayer * layer = nil; +}; /** An `MGLOpenGLStyleLayer` is a style layer that is rendered by OpenGL code that @@ -98,10 +96,7 @@ void MGLFinishCustomStyleLayer(void *context) { */ - (instancetype)initWithIdentifier:(NSString *)identifier { auto layer = std::make_unique<mbgl::style::CustomLayer>(identifier.UTF8String, - MGLPrepareCustomStyleLayer, - MGLDrawCustomStyleLayer, - MGLFinishCustomStyleLayer, - (__bridge_retained void *)self); + std::make_unique<MGLOpenGLLayerHost>(self)); return self = [super initWithPendingLayer:std::move(layer)]; } @@ -110,22 +105,15 @@ void MGLFinishCustomStyleLayer(void *context) { } #pragma mark - Adding to and removing from a map view - -- (void)setStyle:(MGLStyle *)style { - if (_style && style) { - [NSException raise:@"MGLLayerReuseException" - format:@"%@ cannot be added to more than one MGLStyle at a time.", self]; - } - _style = style; -} - - (void)addToStyle:(MGLStyle *)style belowLayer:(MGLStyleLayer *)otherLayer { self.style = style; + self.style.openGLLayers[self.identifier] = self; [super addToStyle:style belowLayer:otherLayer]; } - (void)removeFromStyle:(MGLStyle *)style { [super removeFromStyle:style]; + self.style.openGLLayers[self.identifier] = nil; self.style = nil; } diff --git a/platform/darwin/src/MGLStyle.mm b/platform/darwin/src/MGLStyle.mm index 66abf4391e..0162dbd354 100644 --- a/platform/darwin/src/MGLStyle.mm +++ b/platform/darwin/src/MGLStyle.mm @@ -82,6 +82,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) NS_MUTABLE_DICTIONARY_OF(NSString *, NS_DICTIONARY_OF(NSObject *, MGLTextLanguage *) *) *localizedLayersByIdentifier; @end @@ -124,6 +125,7 @@ static_assert(6 == mbgl::util::default_styles::numOrderedStyles, if (self = [super init]) { _mapView = mapView; _rawStyle = rawStyle; + _openGLLayers = [NSMutableDictionary dictionary]; _localizedLayersByIdentifier = [NSMutableDictionary dictionary]; } return self; diff --git a/platform/darwin/src/MGLStyle_Private.h b/platform/darwin/src/MGLStyle_Private.h index d7ad2975ef..24466b8018 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; - (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 db6cc13930..7b82f41b57 100644 --- a/platform/ios/Integration Tests/MBGLIntegrationTests.m +++ b/platform/ios/Integration Tests/MBGLIntegrationTests.m @@ -1,6 +1,6 @@ #import <XCTest/XCTest.h> - -@import Mapbox; +#import <objc/message.h> +#import <Mapbox/Mapbox.h> @interface MBGLIntegrationTests : XCTestCase <MGLMapViewDelegate> @@ -11,8 +11,11 @@ @implementation MBGLIntegrationTests { XCTestExpectation *_styleLoadingExpectation; + XCTestExpectation *_renderFinishedExpectation; } +#pragma mark - Setup/Teardown + - (void)setUp { [super setUp]; @@ -32,6 +35,16 @@ [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); @@ -39,30 +52,112 @@ [_styleLoadingExpectation fulfill]; } -- (void)tearDown { - _styleLoadingExpectation = nil; - self.mapView = nil; +- (void)mapViewDidFinishRenderingFrame:(MGLMapView *)mapView fullyRendered:(__unused BOOL)fullyRendered { + [_renderFinishedExpectation fulfill]; + _renderFinishedExpectation = nil; +} - [super tearDown]; +#pragma mark - Utilities + +- (void)waitForMapViewToBeRendered { + [self.mapView setNeedsDisplay]; + _renderFinishedExpectation = [self expectationWithDescription:@"Map view should be rendered"]; + [self waitForExpectations:@[_renderFinishedExpectation] timeout:1]; } - (MGLStyle *)style { return self.mapView.style; } +#pragma mark - Tests + +// 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 waitForMapViewToBeRendered]; + XCTAssertNil(weakLayer2); +} + - (void)testAddingRemovingOpenGLLayer { XCTAssertNotNil(self.style); void(^addRemoveGLLayer)(void) = ^{ - MGLOpenGLStyleLayer *layer = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; - [self.style insertLayer:layer atIndex:0]; - layer = nil; - [[NSRunLoop currentRunLoop] runUntilDate:[NSDate date]]; + __weak id retrievedLayer = nil; + + @autoreleasepool { + MGLOpenGLStyleLayer *layer = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; + [self.style insertLayer:layer atIndex:0]; + layer = nil; + + [self waitForMapViewToBeRendered]; - id retrievedLayer = [self.style layerWithIdentifier:@"gl-layer"]; - XCTAssertNotNil(retrievedLayer); - [self.style removeLayer:retrievedLayer]; + retrievedLayer = [self.style layerWithIdentifier:@"gl-layer"]; + XCTAssertNotNil(retrievedLayer); + + [self.style removeLayer:retrievedLayer]; + [self waitForMapViewToBeRendered]; + } + + XCTAssertNil(retrievedLayer); }; addRemoveGLLayer(); @@ -70,8 +165,98 @@ addRemoveGLLayer(); } -//- (void)testOpenGLLayerDoesNotLeakWhenCreatedAndDestroyedWithoutAddingToStyle { -// XCTFail(@"Not yet implemented"); -//} +- (void)testReusingOpenGLLayer { + MGLOpenGLStyleLayer *layer = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; + [self.style insertLayer:layer atIndex:0]; + [self waitForMapViewToBeRendered]; + + [self.style removeLayer:layer]; + [self waitForMapViewToBeRendered]; + + [self.style insertLayer:layer atIndex:0]; + [self waitForMapViewToBeRendered]; + + [self.style removeLayer:layer]; + [self waitForMapViewToBeRendered]; +} + +- (void)testOpenGLLayerDoesNotLeakWhenRemovedFromStyle { + __weak id weakLayer; + @autoreleasepool { + MGLOpenGLStyleLayer *layer = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; + weakLayer = layer; + [self.style insertLayer:layer atIndex:0]; + layer = nil; + + [self waitForMapViewToBeRendered]; + [self.style removeLayer:[self.style layerWithIdentifier:@"gl-layer"]]; + } + + MGLStyleLayer *layer2 = weakLayer; + + XCTAssertNotNil(weakLayer); + [self waitForMapViewToBeRendered]; + + layer2 = nil; + XCTAssertNil(weakLayer); +} + +- (void)testOpenGLLayerDoesNotLeakWhenStyleChanged { + __weak MGLOpenGLStyleLayer *weakLayer; + + @autoreleasepool { + { + MGLOpenGLStyleLayer *layer = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"]; + weakLayer = layer; + [self.style insertLayer:layer atIndex:0]; + layer = nil; + } + } + + XCTAssertNotNil(weakLayer); + + [self waitForMapViewToBeRendered]; + + MGLStyleLayer *layer2 = [self.mapView.style layerWithIdentifier:@"gl-layer"]; + + 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]; + + // At this point the C++ CustomLayer will have been destroyed, and the rawLayer pointer has been NULLed + XCTAssert(weakLayer == layer2); + XCTAssertNotNil(weakLayer); + + // Asking the style for the layer should return nil + MGLStyleLayer *layer3 = [self.mapView.style layerWithIdentifier:@"gl-layer"]; + XCTAssertNil(layer3); +} + + +- (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; + + [self waitForMapViewToBeRendered]; + } + XCTAssertNil(weakLayer); +} @end + diff --git a/platform/qt/include/qmapbox.hpp b/platform/qt/include/qmapbox.hpp index 5a5198108c..369890343f 100644 --- a/platform/qt/include/qmapbox.hpp +++ b/platform/qt/include/qmapbox.hpp @@ -112,12 +112,16 @@ struct Q_MAPBOXGL_EXPORT CustomLayerRenderParameters { double zoom; double bearing; double pitch; - double altitude; + double fieldOfView; }; -typedef void (*CustomLayerInitializeFunction)(void* context) ; -typedef void (*CustomLayerRenderFunction)(void* context, const CustomLayerRenderParameters&); -typedef void (*CustomLayerDeinitializeFunction)(void* context); +class Q_MAPBOXGL_EXPORT CustomLayerHostInterface { +public: + virtual ~CustomLayerHostInterface() = default; + virtual void initialize() = 0; + virtual void render(const CustomLayerRenderParameters&) = 0; + virtual void deinitialize() = 0; +}; } // namespace QMapbox diff --git a/platform/qt/include/qmapboxgl.hpp b/platform/qt/include/qmapboxgl.hpp index bc18eaba59..70fe270902 100644 --- a/platform/qt/include/qmapboxgl.hpp +++ b/platform/qt/include/qmapboxgl.hpp @@ -223,10 +223,7 @@ public: void removeImage(const QString &name); void addCustomLayer(const QString &id, - QMapbox::CustomLayerInitializeFunction, - QMapbox::CustomLayerRenderFunction, - QMapbox::CustomLayerDeinitializeFunction, - void* context, + QScopedPointer<QMapbox::CustomLayerHostInterface>& host, const QString& before = QString()); void addLayer(const QVariantMap ¶ms, const QString& before = QString()); bool layerExists(const QString &id); diff --git a/platform/qt/src/qmapbox.cpp b/platform/qt/src/qmapbox.cpp index ec76ebfe53..87a9358772 100644 --- a/platform/qt/src/qmapbox.cpp +++ b/platform/qt/src/qmapbox.cpp @@ -158,27 +158,9 @@ namespace QMapbox { */ /*! - \typedef QMapbox::CustomLayerDeinitializeFunction + \class QMapbox::CustomLayerHostInterface - Represents a callback to be called when destroying a custom layer. - - \warning This is used for delegating the rendering of a layer to the user of - this API and is not officially supported. Use at your own risk. -*/ - -/*! - \typedef QMapbox::CustomLayerInitializeFunction - - Represents a callback to be called when initializing a custom layer. - - \warning This is used for delegating the rendering of a layer to the user of - this API and is not officially supported. Use at your own risk. -*/ - -/*! - \typedef QMapbox::CustomLayerRenderFunction - - Represents a callback to be called on each render pass for a custom layer. + Represents a host interface to be implemented for rendering custom layers. \warning This is used for delegating the rendering of a layer to the user of this API and is not officially supported. Use at your own risk. diff --git a/platform/qt/src/qmapboxgl.cpp b/platform/qt/src/qmapboxgl.cpp index 414b65255c..2e736c0aa2 100644 --- a/platform/qt/src/qmapboxgl.cpp +++ b/platform/qt/src/qmapboxgl.cpp @@ -1343,20 +1343,43 @@ void QMapboxGL::removeSource(const QString& id) this API and is not officially supported. Use at your own risk. */ void QMapboxGL::addCustomLayer(const QString &id, - QMapbox::CustomLayerInitializeFunction initFn, - QMapbox::CustomLayerRenderFunction renderFn, - QMapbox::CustomLayerDeinitializeFunction deinitFn, - void *context, + QScopedPointer<QMapbox::CustomLayerHostInterface>& host, const QString& before) { + class HostWrapper : public mbgl::style::CustomLayerHost { + public: + QScopedPointer<QMapbox::CustomLayerHostInterface> ptr; + HostWrapper(QScopedPointer<QMapbox::CustomLayerHostInterface>& p) + : ptr(p.take()) { + } + + void initialize() { + ptr->initialize(); + } + + void render(const mbgl::style::CustomLayerRenderParameters& params) { + QMapbox::CustomLayerRenderParameters renderParams; + renderParams.width = params.width; + renderParams.height = params.height; + renderParams.latitude = params.latitude; + renderParams.longitude = params.longitude; + renderParams.zoom = params.zoom; + renderParams.bearing = params.bearing; + renderParams.pitch = params.pitch; + renderParams.fieldOfView = params.fieldOfView; + ptr->render(renderParams); + } + + void contextLost() { } + + void deinitialize() { + ptr->deinitialize(); + } + }; + d_ptr->mapObj->getStyle().addLayer(std::make_unique<mbgl::style::CustomLayer>( id.toStdString(), - reinterpret_cast<mbgl::style::CustomLayerInitializeFunction>(initFn), - // This cast is safe as long as both mbgl:: and QMapbox:: - // CustomLayerRenderParameters members remains the same. - (mbgl::style::CustomLayerRenderFunction)renderFn, - reinterpret_cast<mbgl::style::CustomLayerDeinitializeFunction>(deinitFn), - context), + std::make_unique<HostWrapper>(host)), before.isEmpty() ? mbgl::optional<std::string>() : mbgl::optional<std::string>(before.toStdString())); } |