[core] Immutable Impls

5 files changed, 55 insertions, 74 deletions

diff --git a/test/style/conversion/layer.test.cpp b/test/style/conversion/layer.test.cpp
index ae8d4058ab..8ca5ed7e2b 100644
--- a/test/style/conversion/layer.test.cpp
+++ b/test/style/conversion/layer.test.cpp
@@ -35,13 +35,13 @@ TEST(StyleConversion, LayerTransition) {
         }
     })JSON");
 
-    ASSERT_EQ(400ms, *layer->as<BackgroundLayer>()->impl->cascading
+    ASSERT_EQ(400ms, *layer->as<BackgroundLayer>()->impl().cascading
         .get<BackgroundColor>().getTransition({}).duration);
-    ASSERT_EQ(500ms, *layer->as<BackgroundLayer>()->impl->cascading
+    ASSERT_EQ(500ms, *layer->as<BackgroundLayer>()->impl().cascading
         .get<BackgroundColor>().getTransition({}).delay);
 
-    ASSERT_EQ(100ms, *layer->as<BackgroundLayer>()->impl->cascading
+    ASSERT_EQ(100ms, *layer->as<BackgroundLayer>()->impl().cascading
         .get<BackgroundColor>().getTransition({"class"}).duration);
-    ASSERT_FALSE(bool(layer->as<BackgroundLayer>()->impl->cascading
+    ASSERT_FALSE(bool(layer->as<BackgroundLayer>()->impl().cascading
         .get<BackgroundColor>().getTransition({"class"}).delay));
 }
diff --git a/test/style/source.test.cpp b/test/style/source.test.cpp
index 9c0d914543..ad28db7c19 100644
--- a/test/style/source.test.cpp
+++ b/test/style/source.test.cpp
@@ -95,8 +95,8 @@ TEST(Source, LoadingFail) {
     };
 
     VectorSource source("source", "url");
-    source.baseImpl->setObserver(&test.styleObserver);
-    source.baseImpl->loadDescription(test.fileSource);
+    source.setObserver(&test.styleObserver);
+    source.loadDescription(test.fileSource);
 
     test.run();
 }
@@ -118,8 +118,8 @@ TEST(Source, LoadingCorrupt) {
     };
 
     VectorSource source("source", "url");
-    source.baseImpl->setObserver(&test.styleObserver);
-    source.baseImpl->loadDescription(test.fileSource);
+    source.setObserver(&test.styleObserver);
+    source.loadDescription(test.fileSource);
 
     test.run();
 }
@@ -137,10 +137,10 @@ TEST(Source, RasterTileEmpty) {
     tileset.tiles = { "tiles" };
 
     RasterSource source("source", tileset, 512);
-    source.baseImpl->loadDescription(test.fileSource);
+    source.loadDescription(test.fileSource);
 
     test.renderSourceObserver.tileChanged = [&] (RenderSource& source_, const OverscaledTileID&) {
-        EXPECT_EQ("source", source_.baseImpl.id);
+        EXPECT_EQ("source", source_.baseImpl->id);
         test.end();
     };
 
@@ -148,9 +148,9 @@ TEST(Source, RasterTileEmpty) {
         FAIL() << "Should never be called";
     };
 
-    RenderRasterSource renderSource(*source.impl);
-    renderSource.setObserver(&test.renderSourceObserver);
-    renderSource.updateTiles(test.tileParameters);
+    auto renderSource = source.baseImpl->createRenderSource();
+    renderSource->setObserver(&test.renderSourceObserver);
+    renderSource->updateTiles(test.tileParameters);
 
     test.run();
 }
@@ -168,10 +168,10 @@ TEST(Source, VectorTileEmpty) {
     tileset.tiles = { "tiles" };
 
     VectorSource source("source", tileset);
-    source.baseImpl->loadDescription(test.fileSource);
+    source.loadDescription(test.fileSource);
 
     test.renderSourceObserver.tileChanged = [&] (RenderSource& source_, const OverscaledTileID&) {
-        EXPECT_EQ("source", source_.baseImpl.id);
+        EXPECT_EQ("source", source_.baseImpl->id);
         test.end();
     };
 
@@ -179,9 +179,9 @@ TEST(Source, VectorTileEmpty) {
         FAIL() << "Should never be called";
     };
 
-    RenderVectorSource renderSource(*source.impl);
-    renderSource.setObserver(&test.renderSourceObserver);
-    renderSource.updateTiles(test.tileParameters);
+    auto renderSource = source.baseImpl->createRenderSource();
+    renderSource->setObserver(&test.renderSourceObserver);
+    renderSource->updateTiles(test.tileParameters);
 
     test.run();
 }
@@ -201,18 +201,18 @@ TEST(Source, RasterTileFail) {
     tileset.tiles = { "tiles" };
 
     RasterSource source("source", tileset, 512);
-    source.baseImpl->loadDescription(test.fileSource);
+    source.loadDescription(test.fileSource);
 
     test.renderSourceObserver.tileError = [&] (RenderSource& source_, const OverscaledTileID& tileID, std::exception_ptr error) {
-        EXPECT_EQ(SourceType::Raster, source_.baseImpl.type);
+        EXPECT_EQ(SourceType::Raster, source_.baseImpl->type);
         EXPECT_EQ(OverscaledTileID(0, 0, 0), tileID);
         EXPECT_EQ("Failed by the test case", util::toString(error));
         test.end();
     };
 
-    RenderRasterSource renderSource(*source.impl);
-    renderSource.setObserver(&test.renderSourceObserver);
-    renderSource.updateTiles(test.tileParameters);
+    auto renderSource = source.baseImpl->createRenderSource();
+    renderSource->setObserver(&test.renderSourceObserver);
+    renderSource->updateTiles(test.tileParameters);
 
     test.run();
 }
@@ -232,18 +232,18 @@ TEST(Source, VectorTileFail) {
     tileset.tiles = { "tiles" };
 
     VectorSource source("source", tileset);
-    source.baseImpl->loadDescription(test.fileSource);
+    source.loadDescription(test.fileSource);
 
     test.renderSourceObserver.tileError = [&] (RenderSource& source_, const OverscaledTileID& tileID, std::exception_ptr error) {
-        EXPECT_EQ(SourceType::Vector, source_.baseImpl.type);
+        EXPECT_EQ(SourceType::Vector, source_.baseImpl->type);
         EXPECT_EQ(OverscaledTileID(0, 0, 0), tileID);
         EXPECT_EQ("Failed by the test case", util::toString(error));
         test.end();
     };
 
-    RenderVectorSource renderSource(*source.impl);
-    renderSource.setObserver(&test.renderSourceObserver);
-    renderSource.updateTiles(test.tileParameters);
+    auto renderSource = source.baseImpl->createRenderSource();
+    renderSource->setObserver(&test.renderSourceObserver);
+    renderSource->updateTiles(test.tileParameters);
 
     test.run();
 }
@@ -261,19 +261,19 @@ TEST(Source, RasterTileCorrupt) {
     tileset.tiles = { "tiles" };
 
     RasterSource source("source", tileset, 512);
-    source.baseImpl->loadDescription(test.fileSource);
+    source.loadDescription(test.fileSource);
 
     test.renderSourceObserver.tileError = [&] (RenderSource& source_, const OverscaledTileID& tileID, std::exception_ptr error) {
-        EXPECT_EQ(source_.baseImpl.type, SourceType::Raster);
+        EXPECT_EQ(source_.baseImpl->type, SourceType::Raster);
         EXPECT_EQ(OverscaledTileID(0, 0, 0), tileID);
         EXPECT_TRUE(bool(error));
         // Not asserting on platform-specific error text.
         test.end();
     };
 
-    RenderRasterSource renderSource(*source.impl);
-    renderSource.setObserver(&test.renderSourceObserver);
-    renderSource.updateTiles(test.tileParameters);
+    auto renderSource = source.baseImpl->createRenderSource();
+    renderSource->setObserver(&test.renderSourceObserver);
+    renderSource->updateTiles(test.tileParameters);
 
     test.run();
 }
@@ -296,18 +296,18 @@ TEST(Source, VectorTileCorrupt) {
     tileset.tiles = { "tiles" };
 
     VectorSource source("source", tileset);
-    source.baseImpl->loadDescription(test.fileSource);
+    source.loadDescription(test.fileSource);
 
     test.renderSourceObserver.tileError = [&] (RenderSource& source_, const OverscaledTileID& tileID, std::exception_ptr error) {
-        EXPECT_EQ(source_.baseImpl.type, SourceType::Vector);
+        EXPECT_EQ(source_.baseImpl->type, SourceType::Vector);
         EXPECT_EQ(OverscaledTileID(0, 0, 0), tileID);
         EXPECT_EQ(util::toString(error), "unknown pbf field type exception");
         test.end();
     };
 
-    RenderVectorSource renderSource(*source.impl);
-    renderSource.setObserver(&test.renderSourceObserver);
-    renderSource.updateTiles(test.tileParameters);
+    auto renderSource = source.baseImpl->createRenderSource();
+    renderSource->setObserver(&test.renderSourceObserver);
+    renderSource->updateTiles(test.tileParameters);
 
     test.run();
 }
@@ -324,7 +324,7 @@ TEST(Source, RasterTileCancel) {
     tileset.tiles = { "tiles" };
 
     RasterSource source("source", tileset, 512);
-    source.baseImpl->loadDescription(test.fileSource);
+    source.loadDescription(test.fileSource);
 
     test.renderSourceObserver.tileChanged = [&] (RenderSource&, const OverscaledTileID&) {
         FAIL() << "Should never be called";
@@ -334,9 +334,9 @@ TEST(Source, RasterTileCancel) {
         FAIL() << "Should never be called";
     };
 
-    RenderRasterSource renderSource(*source.impl);
-    renderSource.setObserver(&test.renderSourceObserver);
-    renderSource.updateTiles(test.tileParameters);
+    auto renderSource = source.baseImpl->createRenderSource();
+    renderSource->setObserver(&test.renderSourceObserver);
+    renderSource->updateTiles(test.tileParameters);
 
     test.run();
 }
@@ -353,7 +353,7 @@ TEST(Source, VectorTileCancel) {
     tileset.tiles = { "tiles" };
 
     VectorSource source("source", tileset);
-    source.baseImpl->loadDescription(test.fileSource);
+    source.loadDescription(test.fileSource);
 
     test.renderSourceObserver.tileChanged = [&] (RenderSource&, const OverscaledTileID&) {
         FAIL() << "Should never be called";
@@ -363,9 +363,9 @@ TEST(Source, VectorTileCancel) {
         FAIL() << "Should never be called";
     };
 
-    RenderVectorSource renderSource(*source.impl);
-    renderSource.setObserver(&test.renderSourceObserver);
-    renderSource.updateTiles(test.tileParameters);
+    auto renderSource = source.baseImpl->createRenderSource();
+    renderSource->setObserver(&test.renderSourceObserver);
+    renderSource->updateTiles(test.tileParameters);
 
     test.run();
 }
@@ -398,11 +398,11 @@ TEST(Source, RasterTileAttribution) {
     };
 
     RasterSource source("source", "url", 512);
-    source.baseImpl->setObserver(&test.styleObserver);
-    source.baseImpl->loadDescription(test.fileSource);
+    source.setObserver(&test.styleObserver);
+    source.loadDescription(test.fileSource);
 
-    RenderRasterSource renderSource(*source.impl);
-    renderSource.updateTiles(test.tileParameters);
+    auto renderSource = source.baseImpl->createRenderSource();
+    renderSource->updateTiles(test.tileParameters);
 
     test.run();
 }
@@ -423,10 +423,10 @@ TEST(Source, GeoJSonSourceUrlUpdate) {
     };
 
     GeoJSONSource source("source");
-    source.baseImpl->setObserver(&test.styleObserver);
+    source.setObserver(&test.styleObserver);
 
     // Load initial, so the source state will be loaded=true
-    source.baseImpl->loadDescription(test.fileSource);
+    source.loadDescription(test.fileSource);
 
     // Schedule an update
     test.loop.invoke([&] () {
diff --git a/test/style/style_layer.test.cpp b/test/style/style_layer.test.cpp
index 657dc24a70..2a8379bf20 100644
--- a/test/style/style_layer.test.cpp
+++ b/test/style/style_layer.test.cpp
@@ -28,15 +28,6 @@ using namespace mbgl::style;
 
 namespace {
 
-template <class T, class... Params> void testClone(Params... params) {
-    auto layer = std::make_unique<T>(std::forward<Params>(params)...);
-    auto clone = layer->baseImpl->clone();
-    EXPECT_NE(layer.get(), clone.get());
-    EXPECT_TRUE(reinterpret_cast<typename T::Impl*>(clone->baseImpl.get()));
-    layer->impl->id = "test";
-    EXPECT_EQ("test", layer->baseImpl->clone()->getID());
-}
-
 const auto color = Color { 1, 0, 0, 1 };
 const auto opacity = 1.0f;
 const auto radius = 1.0f;
@@ -61,16 +52,6 @@ const auto duration = 1.0f;
 
 } // namespace
 
-TEST(Layer, Clone) {
-    testClone<BackgroundLayer>("background");
-    testClone<CircleLayer>("circle", "source");
-    testClone<CustomLayer>("custom", [](void*){}, [](void*, const CustomLayerRenderParameters&){}, [](void*){}, nullptr),
-    testClone<FillLayer>("fill", "source");
-    testClone<LineLayer>("line", "source");
-    testClone<RasterLayer>("raster", "source");
-    testClone<SymbolLayer>("symbol", "source");
-}
-
 TEST(Layer, BackgroundProperties) {
     auto layer = std::make_unique<BackgroundLayer>("background");
     EXPECT_TRUE(layer->is<BackgroundLayer>());
@@ -222,7 +203,7 @@ TEST(Layer, RasterProperties) {
 TEST(Layer, Observer) {
     auto layer = std::make_unique<LineLayer>("line", "source");
     StubLayerObserver observer;
-    layer->baseImpl->setObserver(&observer);
+    layer->setObserver(&observer);
 
     // Notifies observer on filter change.
     bool filterChanged = false;
diff --git a/test/tile/geojson_tile.test.cpp b/test/tile/geojson_tile.test.cpp
index a0383f06c9..8669c02dfd 100644
--- a/test/tile/geojson_tile.test.cpp
+++ b/test/tile/geojson_tile.test.cpp
@@ -55,7 +55,7 @@ TEST(GeoJSONTile, Issue7648) {
     observer.tileChanged = [&] (const Tile&) {
         // Once present, the bucket should never "disappear", which would cause
         // flickering.
-        ASSERT_NE(nullptr, tile.getBucket(*test.style.getRenderLayer("circle")));
+        ASSERT_NE(nullptr, tile.getBucket(*test.style.getLayer("circle")->baseImpl));
     };
 
     tile.setObserver(&observer);
diff --git a/test/tile/vector_tile.test.cpp b/test/tile/vector_tile.test.cpp
index 37bfe8512d..a966f3b706 100644
--- a/test/tile/vector_tile.test.cpp
+++ b/test/tile/vector_tile.test.cpp
@@ -89,7 +89,7 @@ TEST(VectorTile, Issue7615) {
         0
     });
 
-    EXPECT_EQ(symbolBucket.get(), tile.getBucket(*symbolLayer.baseImpl->createRenderLayer()));
+    EXPECT_EQ(symbolBucket.get(), tile.getBucket(*symbolLayer.baseImpl));
 }
 
 TEST(VectorTile, Issue8542) {