From 59820fb63d9970c1e2cda9cf950b819b79b1e7f7 Mon Sep 17 00:00:00 2001 From: Jesse Bounds Date: Mon, 20 Feb 2017 16:39:23 -0800 Subject: [PATCH] [ios, macos] Simplify implementation of layer removal from map view This reverts a previous change that recreated the pending and raw layer pointers if an identifier match caused a layer to be removed but the removed layer was of a different type than the layer triggering the removal This refactors the pointer replacement to use a simpler solution that returns early if the layer does not have a raw pointer loaded in the mbgl map instance. --- platform/darwin/src/MGLBackgroundStyleLayer.mm | 7 ++++--- platform/darwin/src/MGLCircleStyleLayer.mm | 8 ++++---- platform/darwin/src/MGLFillStyleLayer.mm | 8 ++++---- platform/darwin/src/MGLLineStyleLayer.mm | 8 ++++---- platform/darwin/src/MGLRasterStyleLayer.mm | 8 ++++---- platform/darwin/src/MGLStyleLayer.mm.ejs | 11 ++++------- platform/darwin/src/MGLSymbolStyleLayer.mm | 8 ++++---- platform/darwin/test/MGLStyleTests.mm | 17 ++++++++++++++++- 8 files changed, 44 insertions(+), 31 deletions(-) diff --git a/platform/darwin/src/MGLBackgroundStyleLayer.mm b/platform/darwin/src/MGLBackgroundStyleLayer.mm index f6a697336..bcad0aa11 100644 --- a/platform/darwin/src/MGLBackgroundStyleLayer.mm +++ b/platform/darwin/src/MGLBackgroundStyleLayer.mm @@ -62,13 +62,14 @@ - (void)removeFromMapView:(MGLMapView *)mapView { + if (self.rawLayer != mapView.mbglMap->getLayer(self.identifier.UTF8String)) { + return; + } + auto removedLayer = mapView.mbglMap->removeLayer(self.identifier.UTF8String); if (!removedLayer) { return; } - auto originalLayer = std::make_unique(self.identifier.UTF8String); - _pendingLayer = std::move(originalLayer); - self.rawLayer = _pendingLayer.get(); mbgl::style::BackgroundLayer *layer = dynamic_cast(removedLayer.get()); if (!layer) { diff --git a/platform/darwin/src/MGLCircleStyleLayer.mm b/platform/darwin/src/MGLCircleStyleLayer.mm index ad087c5d0..808e00bc3 100644 --- a/platform/darwin/src/MGLCircleStyleLayer.mm +++ b/platform/darwin/src/MGLCircleStyleLayer.mm @@ -112,14 +112,14 @@ namespace mbgl { - (void)removeFromMapView:(MGLMapView *)mapView { + if (self.rawLayer != mapView.mbglMap->getLayer(self.identifier.UTF8String)) { + return; + } + auto removedLayer = mapView.mbglMap->removeLayer(self.identifier.UTF8String); if (!removedLayer) { return; } - - auto originalLayer = std::make_unique(self.identifier.UTF8String, self.sourceIdentifier.UTF8String); - _pendingLayer = std::move(originalLayer); - self.rawLayer = _pendingLayer.get(); mbgl::style::CircleLayer *layer = dynamic_cast(removedLayer.get()); if (!layer) { diff --git a/platform/darwin/src/MGLFillStyleLayer.mm b/platform/darwin/src/MGLFillStyleLayer.mm index ffd55884c..6716e0efb 100644 --- a/platform/darwin/src/MGLFillStyleLayer.mm +++ b/platform/darwin/src/MGLFillStyleLayer.mm @@ -107,14 +107,14 @@ namespace mbgl { - (void)removeFromMapView:(MGLMapView *)mapView { + if (self.rawLayer != mapView.mbglMap->getLayer(self.identifier.UTF8String)) { + return; + } + auto removedLayer = mapView.mbglMap->removeLayer(self.identifier.UTF8String); if (!removedLayer) { return; } - - auto originalLayer = std::make_unique(self.identifier.UTF8String, self.sourceIdentifier.UTF8String); - _pendingLayer = std::move(originalLayer); - self.rawLayer = _pendingLayer.get(); mbgl::style::FillLayer *layer = dynamic_cast(removedLayer.get()); if (!layer) { diff --git a/platform/darwin/src/MGLLineStyleLayer.mm b/platform/darwin/src/MGLLineStyleLayer.mm index fc220650f..80b1e907e 100644 --- a/platform/darwin/src/MGLLineStyleLayer.mm +++ b/platform/darwin/src/MGLLineStyleLayer.mm @@ -119,14 +119,14 @@ namespace mbgl { - (void)removeFromMapView:(MGLMapView *)mapView { + if (self.rawLayer != mapView.mbglMap->getLayer(self.identifier.UTF8String)) { + return; + } + auto removedLayer = mapView.mbglMap->removeLayer(self.identifier.UTF8String); if (!removedLayer) { return; } - - auto originalLayer = std::make_unique(self.identifier.UTF8String, self.sourceIdentifier.UTF8String); - _pendingLayer = std::move(originalLayer); - self.rawLayer = _pendingLayer.get(); mbgl::style::LineLayer *layer = dynamic_cast(removedLayer.get()); if (!layer) { diff --git a/platform/darwin/src/MGLRasterStyleLayer.mm b/platform/darwin/src/MGLRasterStyleLayer.mm index 0ddb6baf0..2108a5a0c 100644 --- a/platform/darwin/src/MGLRasterStyleLayer.mm +++ b/platform/darwin/src/MGLRasterStyleLayer.mm @@ -69,14 +69,14 @@ - (void)removeFromMapView:(MGLMapView *)mapView { + if (self.rawLayer != mapView.mbglMap->getLayer(self.identifier.UTF8String)) { + return; + } + auto removedLayer = mapView.mbglMap->removeLayer(self.identifier.UTF8String); if (!removedLayer) { return; } - - auto originalLayer = std::make_unique(self.identifier.UTF8String, self.sourceIdentifier.UTF8String); - _pendingLayer = std::move(originalLayer); - self.rawLayer = _pendingLayer.get(); mbgl::style::RasterLayer *layer = dynamic_cast(removedLayer.get()); if (!layer) { diff --git a/platform/darwin/src/MGLStyleLayer.mm.ejs b/platform/darwin/src/MGLStyleLayer.mm.ejs index 7f44a7b4a..24aff7fca 100644 --- a/platform/darwin/src/MGLStyleLayer.mm.ejs +++ b/platform/darwin/src/MGLStyleLayer.mm.ejs @@ -150,17 +150,14 @@ namespace mbgl { - (void)removeFromMapView:(MGLMapView *)mapView { + if (self.rawLayer != mapView.mbglMap->getLayer(self.identifier.UTF8String)) { + return; + } + auto removedLayer = mapView.mbglMap->removeLayer(self.identifier.UTF8String); if (!removedLayer) { return; } -<% if (type == 'background') { -%> - auto originalLayer = std::make_uniqueLayer>(self.identifier.UTF8String); -<% } else { -%> - auto originalLayer = std::make_uniqueLayer>(self.identifier.UTF8String, self.sourceIdentifier.UTF8String); -<% } -%> - _pendingLayer = std::move(originalLayer); - self.rawLayer = _pendingLayer.get(); mbgl::style::<%- camelize(type) %>Layer *layer = dynamic_castLayer *>(removedLayer.get()); if (!layer) { diff --git a/platform/darwin/src/MGLSymbolStyleLayer.mm b/platform/darwin/src/MGLSymbolStyleLayer.mm index ae0ac2d6f..0b065cd7b 100644 --- a/platform/darwin/src/MGLSymbolStyleLayer.mm +++ b/platform/darwin/src/MGLSymbolStyleLayer.mm @@ -166,14 +166,14 @@ namespace mbgl { - (void)removeFromMapView:(MGLMapView *)mapView { + if (self.rawLayer != mapView.mbglMap->getLayer(self.identifier.UTF8String)) { + return; + } + auto removedLayer = mapView.mbglMap->removeLayer(self.identifier.UTF8String); if (!removedLayer) { return; } - - auto originalLayer = std::make_unique(self.identifier.UTF8String, self.sourceIdentifier.UTF8String); - _pendingLayer = std::move(originalLayer); - self.rawLayer = _pendingLayer.get(); mbgl::style::SymbolLayer *layer = dynamic_cast(removedLayer.get()); if (!layer) { diff --git a/platform/darwin/test/MGLStyleTests.mm b/platform/darwin/test/MGLStyleTests.mm index 86cf11d4a..4f6f5f0db 100644 --- a/platform/darwin/test/MGLStyleTests.mm +++ b/platform/darwin/test/MGLStyleTests.mm @@ -284,12 +284,27 @@ [self.style addLayer:circleLayer]; XCTAssertNotNil([self.style layerWithIdentifier:circleLayer.identifier]); - MGLBackgroundStyleLayer *backgroundLayer = [[MGLBackgroundStyleLayer alloc] initWithIdentifier:@"circle-layer"]; + MGLBackgroundStyleLayer *backgroundLayer = [[MGLBackgroundStyleLayer alloc] initWithIdentifier:@"background-layer"]; [self.style removeLayer:backgroundLayer]; [self.style addLayer:backgroundLayer]; XCTAssertNotNil([self.style layerWithIdentifier:backgroundLayer.identifier]); } +- (void)testAddingLayerOfTypeABeforeRemovingLayerOfTypeBWithSameIdentifier { + MGLShapeSource *source = [[MGLShapeSource alloc] initWithIdentifier:@"shape-source-removing-before-adding" shape:nil options:nil]; + [self.style addSource:source]; + + // Add a fill layer with an identifer + MGLFillStyleLayer *fillLayer = [[MGLFillStyleLayer alloc] initWithIdentifier:@"some-identifier" source:source]; + [self.style addLayer:fillLayer]; + + // Remove a line layer (before adding it) with the same identifier as the fill layer previously added + MGLLineStyleLayer *lineLayer = [[MGLLineStyleLayer alloc] initWithIdentifier:fillLayer.identifier source:source]; + [self.style removeLayer:lineLayer]; + + XCTAssertTrue([[self.style layerWithIdentifier:fillLayer.identifier] isMemberOfClass:[MGLFillStyleLayer class]]); +} + - (NSString *)stringWithContentsOfStyleHeader { NSURL *styleHeaderURL = [[[NSBundle mgl_frameworkBundle].bundleURL URLByAppendingPathComponent:@"Headers" isDirectory:YES]