Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[core][ios][Android] Check layer compatibility with source #15644

Merged
merged 8 commits into from
Sep 18, 2019
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions include/mbgl/style/layer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ struct LayerTypeInfo {
* requires cross-tile indexing and placement. Contains \c CrossTileIndex::NotRequired otherwise.
*/
const enum class CrossTileIndex { Required, NotRequired } crossTileIndex;

/**
* @brief contains the Id of the supported tile type. Used for internal checks.
* The contained values correspond to \c Tile::Kind enum.
*/
const enum class TileKind : uint8_t { Geometry, Raster, RasterDEM, NotRequired } tileKind;
};

/**
Expand Down
3 changes: 3 additions & 0 deletions include/mbgl/style/source.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class RasterSource;
class RasterDEMSource;
class GeoJSONSource;
class SourceObserver;
struct LayerTypeInfo;

/**
* The runtime representation of a [source](https://www.mapbox.com/mapbox-gl-style-spec/#sources) from the Mapbox Style
Expand Down Expand Up @@ -74,6 +75,8 @@ class Source : public mbgl::util::noncopyable {
virtual void loadDescription(FileSource&) = 0;
void dumpDebugLogs() const;

virtual bool supportsLayerType(const mbgl::style::LayerTypeInfo*) const = 0;

bool loaded = false;

// For use in SDK bindings, which store a reference to a platform-native peer
Expand Down
1 change: 1 addition & 0 deletions include/mbgl/style/sources/custom_geometry_source.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class CustomGeometrySource final : public Source {
// Private implementation
class Impl;
const Impl& impl() const;
bool supportsLayerType(const mbgl::style::LayerTypeInfo*) const override;
mapbox::base::WeakPtr<Source> makeWeakPtr() override {
return weakFactory.makeWeakPtr();
}
Expand Down
2 changes: 2 additions & 0 deletions include/mbgl/style/sources/geojson_source.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ class GeoJSONSource final : public Source {

void loadDescription(FileSource&) final;

bool supportsLayerType(const mbgl::style::LayerTypeInfo*) const override;

mapbox::base::WeakPtr<Source> makeWeakPtr() override {
return weakFactory.makeWeakPtr();
}
Expand Down
2 changes: 2 additions & 0 deletions include/mbgl/style/sources/image_source.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ class ImageSource final : public Source {

void loadDescription(FileSource&) final;

bool supportsLayerType(const mbgl::style::LayerTypeInfo*) const override;

mapbox::base::WeakPtr<Source> makeWeakPtr() override {
return weakFactory.makeWeakPtr();
}
Expand Down
2 changes: 1 addition & 1 deletion include/mbgl/style/sources/raster_dem_source.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace style {
class RasterDEMSource : public RasterSource {
public:
RasterDEMSource(std::string id, variant<std::string, Tileset> urlOrTileset, uint16_t tileSize);

bool supportsLayerType(const mbgl::style::LayerTypeInfo*) const override;
};

template <>
Expand Down
2 changes: 2 additions & 0 deletions include/mbgl/style/sources/raster_source.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ class RasterSource : public Source {

void loadDescription(FileSource&) final;

bool supportsLayerType(const mbgl::style::LayerTypeInfo*) const override;

mapbox::base::WeakPtr<Source> makeWeakPtr() final {
return weakFactory.makeWeakPtr();
}
Expand Down
2 changes: 2 additions & 0 deletions include/mbgl/style/sources/vector_source.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ class VectorSource final : public Source {

void loadDescription(FileSource&) final;

bool supportsLayerType(const mbgl::style::LayerTypeInfo*) const override;

mapbox::base::WeakPtr<Source> makeWeakPtr() override {
return weakFactory.makeWeakPtr();
}
Expand Down
11 changes: 5 additions & 6 deletions platform/darwin/src/MGLRasterDEMSource.mm
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@

@implementation MGLRasterDEMSource

- (std::unique_ptr<mbgl::style::RasterSource>)pendingSourceWithIdentifier:(NSString *)identifier configurationURL:(NSURL *)configurationURL tileSize:(CGFloat)tileSize {
NSString *configurationURLString = configurationURL.mgl_URLByStandardizingScheme.absoluteString;
return std::make_unique<mbgl::style::RasterDEMSource>(identifier.UTF8String,
configurationURLString.UTF8String,
uint16_t(round(tileSize)));
- (std::unique_ptr<mbgl::style::RasterSource>)pendingSourceWithIdentifier:(NSString *)identifier urlOrTileset:(mbgl::variant<std::string, mbgl::Tileset>)urlOrTileset tileSize:(uint16_t)tileSize {
auto source = std::make_unique<mbgl::style::RasterDEMSource>(identifier.UTF8String,
urlOrTileset,
tileSize);
return source;
}

@end
15 changes: 8 additions & 7 deletions platform/darwin/src/MGLRasterTileSource.mm
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,16 @@ - (instancetype)initWithIdentifier:(NSString *)identifier configurationURL:(NSUR
}

- (instancetype)initWithIdentifier:(NSString *)identifier configurationURL:(NSURL *)configurationURL tileSize:(CGFloat)tileSize {
auto source = [self pendingSourceWithIdentifier:identifier configurationURL:configurationURL tileSize:tileSize];
NSString *configurationURLString = configurationURL.mgl_URLByStandardizingScheme.absoluteString;
auto source = [self pendingSourceWithIdentifier:identifier urlOrTileset:configurationURLString.UTF8String tileSize:tileSize];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should tileSize be rounded?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good spot. That was there before, I'll re-add.

return self = [super initWithPendingSource:std::move(source)];
}

- (std::unique_ptr<mbgl::style::RasterSource>)pendingSourceWithIdentifier:(NSString *)identifier configurationURL:(NSURL *)configurationURL tileSize:(CGFloat)tileSize {
NSString *configurationURLString = configurationURL.mgl_URLByStandardizingScheme.absoluteString;
return std::make_unique<mbgl::style::RasterSource>(identifier.UTF8String,
configurationURLString.UTF8String,
uint16_t(round(tileSize)));
- (std::unique_ptr<mbgl::style::RasterSource>)pendingSourceWithIdentifier:(NSString *)identifier urlOrTileset:(mbgl::variant<std::string, mbgl::Tileset>)urlOrTileset tileSize:(uint16_t)tileSize {
auto source = std::make_unique<mbgl::style::RasterSource>(identifier.UTF8String,
urlOrTileset,
tileSize);
return source;
}

- (instancetype)initWithIdentifier:(NSString *)identifier tileURLTemplates:(NSArray<NSString *> *)tileURLTemplates options:(nullable NSDictionary<MGLTileSourceOption, id> *)options {
Expand All @@ -56,7 +57,7 @@ - (instancetype)initWithIdentifier:(NSString *)identifier tileURLTemplates:(NSAr
tileSize = static_cast<uint16_t>(round(tileSizeNumber.doubleValue));
}

auto source = std::make_unique<mbgl::style::RasterSource>(identifier.UTF8String, tileSet, tileSize);
auto source = [self pendingSourceWithIdentifier:identifier urlOrTileset:tileSet tileSize:tileSize];
return self = [super initWithPendingSource:std::move(source)];
}

Expand Down
4 changes: 3 additions & 1 deletion platform/darwin/src/MGLRasterTileSource_Private.h
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
#import "MGLRasterTileSource.h"

#include <memory>
#include <mbgl/util/variant.hpp>

namespace mbgl {
class Tileset;
namespace style {
class RasterSource;
}
Expand All @@ -14,7 +16,7 @@ NS_ASSUME_NONNULL_BEGIN

@property (nonatomic, readonly, nullable) mbgl::style::RasterSource *rawSource;

- (std::unique_ptr<mbgl::style::RasterSource>)pendingSourceWithIdentifier:(NSString *)identifier configurationURL:(NSURL *)configurationURL tileSize:(CGFloat)tileSize;
- (std::unique_ptr<mbgl::style::RasterSource>)pendingSourceWithIdentifier:(NSString *)identifier urlOrTileset:(mbgl::variant<std::string, mbgl::Tileset>)urlOrTileset tileSize:(uint16_t)tileSize;

@end

Expand Down
95 changes: 51 additions & 44 deletions platform/darwin/test/MGLStyleTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -229,23 +229,23 @@ - (void)testAddingSourceOfTypeABeforeSourceOfTypeBWithSameIdentifier {

- (void)testRemovingSourceInUse {
// Add a raster tile source
MGLRasterTileSource *rasterTileSource = [[MGLRasterTileSource alloc] initWithIdentifier:@"some-identifier" tileURLTemplates:@[] options:nil];
[self.style addSource:rasterTileSource];
MGLVectorTileSource *vectorTileSource = [[MGLVectorTileSource alloc] initWithIdentifier:@"some-identifier" tileURLTemplates:@[] options:nil];
[self.style addSource:vectorTileSource];

// Add a layer using it
MGLFillStyleLayer *fillLayer = [[MGLFillStyleLayer alloc] initWithIdentifier:@"fillLayer" source:rasterTileSource];
MGLFillStyleLayer *fillLayer = [[MGLFillStyleLayer alloc] initWithIdentifier:@"fillLayer" source:vectorTileSource];
[self.style addLayer:fillLayer];

// Attempt to remove the raster tile source
NSError *error;
BOOL result = [self.style removeSource:rasterTileSource error:&error];
BOOL result = [self.style removeSource:vectorTileSource error:&error];

XCTAssertFalse(result);
XCTAssertEqualObjects(error.domain, MGLErrorDomain);
XCTAssertEqual(error.code, MGLErrorCodeSourceIsInUseCannotRemove);

// Ensure it is still there
XCTAssertTrue([[self.style sourceWithIdentifier:rasterTileSource.identifier] isMemberOfClass:[MGLRasterTileSource class]]);
XCTAssertTrue([[self.style sourceWithIdentifier:vectorTileSource.identifier] isMemberOfClass:[MGLVectorTileSource class]]);
}

- (void)testLayers {
Expand Down Expand Up @@ -311,54 +311,61 @@ - (void)testAddingLayersWithDuplicateIdentifiers {
}

- (void)testRemovingLayerBeforeAddingSameLayer {
MGLShapeSource *source = [[MGLShapeSource alloc] initWithIdentifier:@"shape-source-removing-before-adding" shape:nil options:nil];

// Attempting to find a layer with identifier will trigger an exception if the source associated with the layer is not added
[self.style addSource:source];

MGLFillStyleLayer *fillLayer = [[MGLFillStyleLayer alloc] initWithIdentifier:@"fill-layer" source:source];
[self.style removeLayer:fillLayer];
[self.style addLayer:fillLayer];
XCTAssertNotNil([self.style layerWithIdentifier:fillLayer.identifier]);

MGLRasterStyleLayer *rasterLayer = [[MGLRasterStyleLayer alloc] initWithIdentifier:@"raster-layer" source:source];
[self.style removeLayer:rasterLayer];
[self.style addLayer:rasterLayer];
XCTAssertNotNil([self.style layerWithIdentifier:rasterLayer.identifier]);

MGLSymbolStyleLayer *symbolLayer = [[MGLSymbolStyleLayer alloc] initWithIdentifier:@"symbol-layer" source:source];
[self.style removeLayer:symbolLayer];
[self.style addLayer:symbolLayer];
XCTAssertNotNil([self.style layerWithIdentifier:symbolLayer.identifier]);

MGLLineStyleLayer *lineLayer = [[MGLLineStyleLayer alloc] initWithIdentifier:@"line-layer" source:source];
[self.style removeLayer:lineLayer];
[self.style addLayer:lineLayer];
XCTAssertNotNil([self.style layerWithIdentifier:lineLayer.identifier]);

MGLCircleStyleLayer *circleLayer = [[MGLCircleStyleLayer alloc] initWithIdentifier:@"circle-layer" source:source];
[self.style removeLayer:circleLayer];
[self.style addLayer:circleLayer];
XCTAssertNotNil([self.style layerWithIdentifier:circleLayer.identifier]);

MGLBackgroundStyleLayer *backgroundLayer = [[MGLBackgroundStyleLayer alloc] initWithIdentifier:@"background-layer"];
[self.style removeLayer:backgroundLayer];
[self.style addLayer:backgroundLayer];
XCTAssertNotNil([self.style layerWithIdentifier:backgroundLayer.identifier]);
{
MGLShapeSource *source = [[MGLShapeSource alloc] initWithIdentifier:@"shape-source-removing-before-adding" shape:nil options:nil];

// Attempting to find a layer with identifier will trigger an exception if the source associated with the layer is not added
[self.style addSource:source];

MGLFillStyleLayer *fillLayer = [[MGLFillStyleLayer alloc] initWithIdentifier:@"fill-layer" source:source];
[self.style removeLayer:fillLayer];
[self.style addLayer:fillLayer];
XCTAssertNotNil([self.style layerWithIdentifier:fillLayer.identifier]);

MGLSymbolStyleLayer *symbolLayer = [[MGLSymbolStyleLayer alloc] initWithIdentifier:@"symbol-layer" source:source];
[self.style removeLayer:symbolLayer];
[self.style addLayer:symbolLayer];
XCTAssertNotNil([self.style layerWithIdentifier:symbolLayer.identifier]);

MGLLineStyleLayer *lineLayer = [[MGLLineStyleLayer alloc] initWithIdentifier:@"line-layer" source:source];
[self.style removeLayer:lineLayer];
[self.style addLayer:lineLayer];
XCTAssertNotNil([self.style layerWithIdentifier:lineLayer.identifier]);

MGLCircleStyleLayer *circleLayer = [[MGLCircleStyleLayer alloc] initWithIdentifier:@"circle-layer" source:source];
[self.style removeLayer:circleLayer];
[self.style addLayer:circleLayer];
XCTAssertNotNil([self.style layerWithIdentifier:circleLayer.identifier]);

MGLBackgroundStyleLayer *backgroundLayer = [[MGLBackgroundStyleLayer alloc] initWithIdentifier:@"background-layer"];
[self.style removeLayer:backgroundLayer];
[self.style addLayer:backgroundLayer];
XCTAssertNotNil([self.style layerWithIdentifier:backgroundLayer.identifier]);
}

{
MGLRasterTileSource *rasterSource = [[MGLRasterTileSource alloc] initWithIdentifier:@"raster-tile-source" tileURLTemplates:@[] options:nil];
[self.style addSource:rasterSource];

MGLRasterStyleLayer *rasterLayer = [[MGLRasterStyleLayer alloc] initWithIdentifier:@"raster-layer" source:rasterSource];
[self.style removeLayer:rasterLayer];
[self.style addLayer:rasterLayer];
XCTAssertNotNil([self.style layerWithIdentifier:rasterLayer.identifier]);
}
}

- (void)testAddingLayerOfTypeABeforeRemovingLayerOfTypeBWithSameIdentifier {
MGLShapeSource *source = [[MGLShapeSource alloc] initWithIdentifier:@"shape-source-identifier" shape:nil options:nil];
[self.style addSource:source];

// Add a fill layer
MGLFillStyleLayer *fillLayer = [[MGLFillStyleLayer alloc] initWithIdentifier:@"some-identifier" source:source];
[self.style addLayer:fillLayer];

// Attempt to remove a line layer with the same identifier as the fill layer
MGLLineStyleLayer *lineLayer = [[MGLLineStyleLayer alloc] initWithIdentifier:fillLayer.identifier source:source];
[self.style removeLayer:lineLayer];

XCTAssertTrue([[self.style layerWithIdentifier:fillLayer.identifier] isMemberOfClass:[MGLFillStyleLayer class]]);
}

Expand All @@ -382,10 +389,10 @@ - (void)testImages {
MGLImage *image = [[NSBundle bundleForClass:[self class]] imageForResource:imageName];
#endif
XCTAssertNotNil(image);

[self.style setImage:image forName:imageName];
MGLImage *styleImage = [self.style imageForName:imageName];

XCTAssertNotNil(styleImage);
XCTAssertEqual(image.size.width, styleImage.size.width);
XCTAssertEqual(image.size.height, styleImage.size.height);
Expand Down
16 changes: 8 additions & 8 deletions platform/ios/Integration Tests/MGLStyleLayerIntegrationTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -82,31 +82,31 @@ - (void)testForRaisingExceptionsOnStaleLayerObject {

// Testing generated layers
MGLLineStyleLayer *lineLayer = [[MGLLineStyleLayer alloc] initWithIdentifier:@"lineLayerID" source:source];
MGLRasterStyleLayer *rasterLayer = [[MGLRasterStyleLayer alloc] initWithIdentifier:@"rasterLayerID" source:source];
MGLCircleStyleLayer *circleLayer = [[MGLCircleStyleLayer alloc] initWithIdentifier:@"circleLayerID" source:source];

[self.mapView.style addSource:source];
[self.mapView.style addLayer:lineLayer];
[self.mapView.style addLayer:rasterLayer];
[self.mapView.style addLayer:circleLayer];

XCTAssertNoThrow(lineLayer.isVisible);
XCTAssertNoThrow(rasterLayer.isVisible);
XCTAssertNoThrow(circleLayer.isVisible);

XCTAssert(![source.description containsString:@"<unknown>"]);
XCTAssert(![lineLayer.description containsString:@"<unknown>"]);
XCTAssert(![rasterLayer.description containsString:@"<unknown>"]);
XCTAssert(![circleLayer.description containsString:@"<unknown>"]);

self.styleLoadingExpectation = nil;
[self.mapView setStyleURL:[[NSBundle bundleForClass:[self class]] URLForResource:@"one-liner" withExtension:@"json"]];
[self waitForMapViewToFinishLoadingStyleWithTimeout:10];

XCTAssert([source.description containsString:@"<unknown>"]);
XCTAssert([lineLayer.description containsString:@"<unknown>"]);
XCTAssert([rasterLayer.description containsString:@"<unknown>"]);
XCTAssert([circleLayer.description containsString:@"<unknown>"]);

XCTAssertThrowsSpecificNamed(lineLayer.isVisible, NSException, MGLInvalidStyleLayerException, @"Layer should raise an exception if its core peer got invalidated");
XCTAssertThrowsSpecificNamed(rasterLayer.isVisible, NSException, MGLInvalidStyleLayerException, @"Layer should raise an exception if its core peer got invalidated");
XCTAssertThrowsSpecificNamed(circleLayer.isVisible, NSException, MGLInvalidStyleLayerException, @"Layer should raise an exception if its core peer got invalidated");

XCTAssertThrowsSpecificNamed([self.mapView.style removeLayer:lineLayer], NSException, NSInvalidArgumentException, @"Style should raise an exception when attempting to remove an invalid layer (e.g. if its core peer got invalidated)");
XCTAssertThrowsSpecificNamed([self.mapView.style removeLayer:rasterLayer], NSException, NSInvalidArgumentException, @"Style should raise an exception when attempting to remove an invalid layer (e.g. if its core peer got invalidated)");
XCTAssertThrowsSpecificNamed([self.mapView.style removeLayer:circleLayer], NSException, NSInvalidArgumentException, @"Style should raise an exception when attempting to remove an invalid layer (e.g. if its core peer got invalidated)");
}
@end
7 changes: 6 additions & 1 deletion src/mbgl/annotation/annotation_source.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include <mbgl/annotation/annotation_source.hpp>
#include <mbgl/annotation/annotation_manager.hpp>
#include <mbgl/annotation/annotation_source.hpp>
#include <mbgl/style/layer.hpp>

namespace mbgl {

Expand All @@ -21,4 +22,8 @@ optional<std::string> AnnotationSource::Impl::getAttribution() const {
return {};
}

bool AnnotationSource::supportsLayerType(const mbgl::style::LayerTypeInfo* info) const {
return !std::strcmp(info->type, "line") || !std::strcmp(info->type, "symbol") || !std::strcmp(info->type, "fill");
}

} // namespace mbgl
7 changes: 3 additions & 4 deletions src/mbgl/annotation/annotation_source.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,12 @@ class AnnotationSource final : public style::Source {
class Impl;
const Impl& impl() const;

private:
void loadDescription(FileSource&) final;
bool supportsLayerType(const mbgl::style::LayerTypeInfo*) const override;
mapbox::base::WeakPtr<Source> makeWeakPtr() override {
return weakFactory.makeWeakPtr();
}

private:
void loadDescription(FileSource&) final;

Mutable<Impl> mutableImpl() const;
mapbox::base::WeakPtrFactory<Source> weakFactory {this};
};
Expand Down
14 changes: 11 additions & 3 deletions src/mbgl/style/layer.cpp
Original file line number Diff line number Diff line change
@@ -1,15 +1,23 @@
#include <mbgl/style/layer.hpp>
#include <mbgl/style/layer_impl.hpp>
#include <mbgl/style/layer_observer.hpp>
#include <mbgl/style/conversion/constant.hpp>
#include <mbgl/style/conversion/filter.hpp>
#include <mbgl/style/conversion_impl.hpp>
#include <mbgl/style/layer.hpp>
#include <mbgl/style/layer_impl.hpp>
#include <mbgl/style/layer_observer.hpp>
#include <mbgl/tile/tile.hpp>

#include <mbgl/renderer/render_layer.hpp>

namespace mbgl {
namespace style {

static_assert(mbgl::underlying_type(Tile::Kind::Geometry) == mbgl::underlying_type(LayerTypeInfo::TileKind::Geometry),
"tile kind error");
static_assert(mbgl::underlying_type(Tile::Kind::Raster) == mbgl::underlying_type(LayerTypeInfo::TileKind::Raster),
"tile kind error");
static_assert(mbgl::underlying_type(Tile::Kind::RasterDEM) == mbgl::underlying_type(LayerTypeInfo::TileKind::RasterDEM),
"tile kind error");

static LayerObserver nullObserver;

Layer::Layer(Immutable<Impl> impl)
Expand Down
Loading