From 7b041b123cb067d247a727bb3a4563bb2fc575eb Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 4 Apr 2017 11:33:22 -0700 Subject: [core, darwin] Object identity for MGLSource*, MGLStyleLayer* All `MGLSource` pointers referencing the same logical source will now be object identical; similarly for `MGLStyleLayer`. --- CMakeLists.txt | 1 + cmake/core.cmake | 1 + cmake/test.cmake | 1 + include/mbgl/style/layer.hpp | 6 +++++ include/mbgl/style/source.hpp | 6 +++++ include/mbgl/util/any.hpp | 10 +++++++ platform/darwin/src/MGLSource.mm | 1 + platform/darwin/src/MGLSource_Private.h | 8 ++++++ platform/darwin/src/MGLStyle.mm | 41 +++++++++++++++-------------- platform/darwin/src/MGLStyleLayer.mm | 1 + platform/darwin/src/MGLStyleLayer_Private.h | 8 ++++++ platform/darwin/test/MGLStyleTests.mm | 2 ++ 12 files changed, 66 insertions(+), 20 deletions(-) create mode 100644 include/mbgl/util/any.hpp diff --git a/CMakeLists.txt b/CMakeLists.txt index a6ed681bde..9e7ee314e7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -40,6 +40,7 @@ set_source_files_properties(src/mbgl/util/version.cpp PROPERTIES COMPILE_DEFINIT mason_use(geometry VERSION 0.9.0 HEADER_ONLY) mason_use(variant VERSION 1.1.4 HEADER_ONLY) +mason_use(any VERSION 8fef1e9 HEADER_ONLY) mason_use(unique_resource VERSION cba309e HEADER_ONLY) mason_use(rapidjson VERSION 1.1.0 HEADER_ONLY) mason_use(boost VERSION 1.62.0 HEADER_ONLY) diff --git a/cmake/core.cmake b/cmake/core.cmake index 616e43028c..531edc092d 100644 --- a/cmake/core.cmake +++ b/cmake/core.cmake @@ -14,6 +14,7 @@ target_include_directories(mbgl-core target_add_mason_package(mbgl-core PUBLIC geometry) target_add_mason_package(mbgl-core PUBLIC variant) +target_add_mason_package(mbgl-core PUBLIC any) target_add_mason_package(mbgl-core PRIVATE unique_resource) target_add_mason_package(mbgl-core PRIVATE rapidjson) target_add_mason_package(mbgl-core PRIVATE boost) diff --git a/cmake/test.cmake b/cmake/test.cmake index 5e77de2448..3c63f7bcf8 100644 --- a/cmake/test.cmake +++ b/cmake/test.cmake @@ -27,6 +27,7 @@ target_link_libraries(mbgl-test target_add_mason_package(mbgl-test PRIVATE geometry) target_add_mason_package(mbgl-test PRIVATE variant) +target_add_mason_package(mbgl-test PRIVATE any) target_add_mason_package(mbgl-test PRIVATE unique_resource) target_add_mason_package(mbgl-test PRIVATE rapidjson) target_add_mason_package(mbgl-test PRIVATE gtest) diff --git a/include/mbgl/style/layer.hpp b/include/mbgl/style/layer.hpp index 670faa8254..fa88b30dbc 100644 --- a/include/mbgl/style/layer.hpp +++ b/include/mbgl/style/layer.hpp @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include @@ -118,6 +119,11 @@ public: // Private implementation const std::unique_ptr baseImpl; + // For use in SDK bindings, which store a reference to a platform-native peer + // object here, so that separately-obtained references to this object share + // identical platform-native peers. + any peer; + friend std::string layoutKey(const Layer&); }; diff --git a/include/mbgl/style/source.hpp b/include/mbgl/style/source.hpp index 0e6e32b112..f2dfb6a896 100644 --- a/include/mbgl/style/source.hpp +++ b/include/mbgl/style/source.hpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include @@ -64,6 +65,11 @@ public: class Impl; const std::unique_ptr baseImpl; + // For use in SDK bindings, which store a reference to a platform-native peer + // object here, so that separately-obtained references to this object share + // identical platform-native peers. + any peer; + protected: const SourceType type; Source(SourceType, std::unique_ptr); diff --git a/include/mbgl/util/any.hpp b/include/mbgl/util/any.hpp new file mode 100644 index 0000000000..eea64b188a --- /dev/null +++ b/include/mbgl/util/any.hpp @@ -0,0 +1,10 @@ +#pragma once + +#include + +namespace mbgl { + +using linb::any; +using linb::any_cast; + +} // namespace mbgl diff --git a/platform/darwin/src/MGLSource.mm b/platform/darwin/src/MGLSource.mm index 1940db688c..59c2ae13e6 100644 --- a/platform/darwin/src/MGLSource.mm +++ b/platform/darwin/src/MGLSource.mm @@ -29,6 +29,7 @@ NSString *identifier = @(rawSource->getID().c_str()); if (self = [self initWithIdentifier:identifier]) { _rawSource = rawSource; + _rawSource->peer = SourceWrapper { self }; } return self; } diff --git a/platform/darwin/src/MGLSource_Private.h b/platform/darwin/src/MGLSource_Private.h index 2fc170b86b..6f86e4800b 100644 --- a/platform/darwin/src/MGLSource_Private.h +++ b/platform/darwin/src/MGLSource_Private.h @@ -10,6 +10,14 @@ namespace mbgl { } } +// A struct to be stored in the `peer` member of mbgl::style::Source, in order to implement +// object identity. We don't store a MGLSource pointer directly because that doesn't +// interoperate with ARC. The inner pointer is weak in order to avoid a reference cycle for +// "pending" MGLSources, which have a strong owning pointer to the mbgl::style::Source. +struct SourceWrapper { + __weak MGLSource *source; +}; + @class MGLMapView; @interface MGLSource (Private) diff --git a/platform/darwin/src/MGLStyle.mm b/platform/darwin/src/MGLStyle.mm index 83ff73e8f9..b01d9aa513 100644 --- a/platform/darwin/src/MGLStyle.mm +++ b/platform/darwin/src/MGLStyle.mm @@ -166,17 +166,21 @@ static NSURL *MGLStyleURL_emerald; return rawSource ? [self sourceFromMBGLSource:rawSource] : nil; } -- (MGLSource *)sourceFromMBGLSource:(mbgl::style::Source *)source { +- (MGLSource *)sourceFromMBGLSource:(mbgl::style::Source *)rawSource { + if (MGLSource *source = rawSource->peer.empty() ? nil : mbgl::any_cast(rawSource->peer).source) { + return source; + } + // TODO: Fill in options specific to the respective source classes // https://github.com/mapbox/mapbox-gl-native/issues/6584 - if (auto vectorSource = source->as()) { + if (auto vectorSource = rawSource->as()) { return [[MGLVectorSource alloc] initWithRawSource:vectorSource]; - } else if (auto geoJSONSource = source->as()) { + } else if (auto geoJSONSource = rawSource->as()) { return [[MGLShapeSource alloc] initWithRawSource:geoJSONSource]; - } else if (auto rasterSource = source->as()) { + } else if (auto rasterSource = rawSource->as()) { return [[MGLRasterSource alloc] initWithRawSource:rasterSource]; } else { - return [[MGLSource alloc] initWithRawSource:source]; + return [[MGLSource alloc] initWithRawSource:rawSource]; } } @@ -319,35 +323,32 @@ static NSURL *MGLStyleURL_emerald; [styleLayer removeFromMapView:self.mapView]; } -- (MGLStyleLayer *)layerFromMBGLLayer:(mbgl::style::Layer *)mbglLayer +- (MGLStyleLayer *)layerFromMBGLLayer:(mbgl::style::Layer *)rawLayer { - NSParameterAssert(mbglLayer); + NSParameterAssert(rawLayer); - NSString *identifier = @(mbglLayer->getID().c_str()); + if (MGLStyleLayer *layer = rawLayer->peer.empty() ? nil : mbgl::any_cast(rawLayer->peer).layer) { + return layer; + } - if (auto fillLayer = mbglLayer->as()) { + if (auto fillLayer = rawLayer->as()) { MGLSource *source = [self sourceWithIdentifier:@(fillLayer->getSourceID().c_str())]; return [[MGLFillStyleLayer alloc] initWithRawLayer:fillLayer source:source]; - } else if (auto lineLayer = mbglLayer->as()) { + } else if (auto lineLayer = rawLayer->as()) { MGLSource *source = [self sourceWithIdentifier:@(lineLayer->getSourceID().c_str())]; return [[MGLLineStyleLayer alloc] initWithRawLayer:lineLayer source:source]; - } else if (auto symbolLayer = mbglLayer->as()) { + } else if (auto symbolLayer = rawLayer->as()) { MGLSource *source = [self sourceWithIdentifier:@(symbolLayer->getSourceID().c_str())]; return [[MGLSymbolStyleLayer alloc] initWithRawLayer:symbolLayer source:source]; - } else if (auto rasterLayer = mbglLayer->as()) { + } else if (auto rasterLayer = rawLayer->as()) { MGLSource *source = [self sourceWithIdentifier:@(rasterLayer->getSourceID().c_str())]; return [[MGLRasterStyleLayer alloc] initWithRawLayer:rasterLayer source:source]; - } else if (auto circleLayer = mbglLayer->as()) { + } else if (auto circleLayer = rawLayer->as()) { MGLSource *source = [self sourceWithIdentifier:@(circleLayer->getSourceID().c_str())]; return [[MGLCircleStyleLayer alloc] initWithRawLayer:circleLayer source:source]; - } else if (auto backgroundLayer = mbglLayer->as()) { + } else if (auto backgroundLayer = rawLayer->as()) { return [[MGLBackgroundStyleLayer alloc] initWithRawLayer:backgroundLayer]; - } else if (auto customLayer = mbglLayer->as()) { - MGLStyleLayer *styleLayer = self.openGLLayers[identifier]; - if (styleLayer) { - NSAssert(styleLayer.rawLayer == customLayer, @"%@ wraps a CustomLayer that differs from the one associated with the underlying style.", styleLayer); - return styleLayer; - } + } else if (auto customLayer = rawLayer->as()) { return [[MGLOpenGLStyleLayer alloc] initWithRawLayer:customLayer]; } else { NSAssert(NO, @"Unrecognized layer type"); diff --git a/platform/darwin/src/MGLStyleLayer.mm b/platform/darwin/src/MGLStyleLayer.mm index c72541526f..4bfaea934b 100644 --- a/platform/darwin/src/MGLStyleLayer.mm +++ b/platform/darwin/src/MGLStyleLayer.mm @@ -18,6 +18,7 @@ if (self = [super init]) { _identifier = @(rawLayer->getID().c_str()); _rawLayer = rawLayer; + _rawLayer->peer = LayerWrapper { self }; } return self; } diff --git a/platform/darwin/src/MGLStyleLayer_Private.h b/platform/darwin/src/MGLStyleLayer_Private.h index d024a0bb13..ed8ec31755 100644 --- a/platform/darwin/src/MGLStyleLayer_Private.h +++ b/platform/darwin/src/MGLStyleLayer_Private.h @@ -7,6 +7,14 @@ NS_ASSUME_NONNULL_BEGIN +// A struct to be stored in the `peer` member of mbgl::style::Layer, in order to implement +// object identity. We don't store a MGLStyleLayer pointer directly because that doesn't +// interoperate with ARC. The inner pointer is weak in order to avoid a reference cycle for +// "pending" MGLStyleLayers, which have a strong owning pointer to the mbgl::style::Layer. +struct LayerWrapper { + __weak MGLStyleLayer *layer; +}; + /** Assert that the style layer is valid. diff --git a/platform/darwin/test/MGLStyleTests.mm b/platform/darwin/test/MGLStyleTests.mm index 36772e556d..f9598a143d 100644 --- a/platform/darwin/test/MGLStyleTests.mm +++ b/platform/darwin/test/MGLStyleTests.mm @@ -148,6 +148,7 @@ MGLShapeSource *shapeSource = [[MGLShapeSource alloc] initWithIdentifier:@"shapeSource" shape:nil options:nil]; [self.style addSource:shapeSource]; XCTAssertEqual(self.style.sources.count, initialSources.count + 1); + XCTAssertEqual(shapeSource, [self.style sourceWithIdentifier:@"shapeSource"]); [self.style removeSource:shapeSource]; XCTAssertEqual(self.style.sources.count, initialSources.count); } @@ -238,6 +239,7 @@ MGLFillStyleLayer *fillLayer = [[MGLFillStyleLayer alloc] initWithIdentifier:@"fillLayer" source:shapeSource]; [self.style addLayer:fillLayer]; XCTAssertEqual(self.style.layers.count, initialLayers.count + 1); + XCTAssertEqual(fillLayer, [self.style layerWithIdentifier:@"fillLayer"]); [self.style removeLayer:fillLayer]; XCTAssertEqual(self.style.layers.count, initialLayers.count); } -- cgit v1.2.1