From ca5f79c12d89005f3f2a582f50adc21512264498 Mon Sep 17 00:00:00 2001 From: Scott Hoyt Date: Fri, 1 Aug 2025 04:50:09 -0700 Subject: [PATCH] Fix for raster dem encoding override in style.json (#3570) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- CMakeLists.txt | 10 +- bazel/core.bzl | 10 +- .../style/conversion/raster_dem_options.hpp | 17 +++ .../mbgl/style/sources/raster_dem_source.hpp | 17 ++- include/mbgl/style/sources/raster_source.hpp | 28 +---- include/mbgl/style/sources/tile_source.hpp | 47 ++++++++ include/mbgl/style/sources/vector_source.hpp | 22 +--- src/mbgl/renderer/render_source.cpp | 6 +- .../sources/render_raster_dem_source.cpp | 6 +- .../sources/render_raster_dem_source.hpp | 6 +- .../renderer/sources/render_raster_source.cpp | 6 +- .../renderer/sources/render_raster_source.hpp | 6 +- .../renderer/sources/render_tile_source.hpp | 2 +- .../renderer/sources/render_vector_source.cpp | 4 +- .../renderer/sources/render_vector_source.hpp | 4 +- .../style/conversion/raster_dem_options.cpp | 34 ++++++ src/mbgl/style/conversion/source.cpp | 10 +- src/mbgl/style/conversion/tileset.cpp | 14 +-- src/mbgl/style/sources/raster_dem_source.cpp | 18 ++- src/mbgl/style/sources/raster_source.cpp | 78 +------------ src/mbgl/style/sources/tile_source.cpp | 104 ++++++++++++++++++ ...r_source_impl.cpp => tile_source_impl.cpp} | 10 +- ...r_source_impl.hpp => tile_source_impl.hpp} | 4 +- src/mbgl/style/sources/vector_source.cpp | 79 ++----------- src/mbgl/style/sources/vector_source_impl.cpp | 21 ---- src/mbgl/style/sources/vector_source_impl.hpp | 20 ---- test/CMakeLists.txt | 1 + .../conversion/raster_dem_options.test.cpp | 48 ++++++++ 28 files changed, 347 insertions(+), 285 deletions(-) create mode 100644 include/mbgl/style/conversion/raster_dem_options.hpp create mode 100644 include/mbgl/style/sources/tile_source.hpp create mode 100644 src/mbgl/style/conversion/raster_dem_options.cpp create mode 100644 src/mbgl/style/sources/tile_source.cpp rename src/mbgl/style/sources/{raster_source_impl.cpp => tile_source_impl.cpp} (51%) rename src/mbgl/style/sources/{raster_source_impl.hpp => tile_source_impl.hpp} (81%) delete mode 100644 src/mbgl/style/sources/vector_source_impl.cpp delete mode 100644 src/mbgl/style/sources/vector_source_impl.hpp create mode 100644 test/style/conversion/raster_dem_options.test.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index d8890d5bb..8f208a501 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -311,6 +311,7 @@ list(APPEND INCLUDE_FILES ${PROJECT_SOURCE_DIR}/include/mbgl/style/conversion/light.hpp ${PROJECT_SOURCE_DIR}/include/mbgl/style/conversion/position.hpp ${PROJECT_SOURCE_DIR}/include/mbgl/style/conversion/property_value.hpp + ${PROJECT_SOURCE_DIR}/include/mbgl/style/conversion/raster_dem_options.hpp ${PROJECT_SOURCE_DIR}/include/mbgl/style/conversion/rotation.hpp ${PROJECT_SOURCE_DIR}/include/mbgl/style/conversion/source.hpp ${PROJECT_SOURCE_DIR}/include/mbgl/style/conversion/tileset.hpp @@ -378,6 +379,7 @@ list(APPEND INCLUDE_FILES ${PROJECT_SOURCE_DIR}/include/mbgl/style/sources/geojson_source.hpp ${PROJECT_SOURCE_DIR}/include/mbgl/style/sources/image_source.hpp ${PROJECT_SOURCE_DIR}/include/mbgl/style/sources/raster_dem_source.hpp + ${PROJECT_SOURCE_DIR}/include/mbgl/style/sources/tile_source.hpp ${PROJECT_SOURCE_DIR}/include/mbgl/style/sources/raster_source.hpp ${PROJECT_SOURCE_DIR}/include/mbgl/style/sources/vector_source.hpp ${PROJECT_SOURCE_DIR}/include/mbgl/style/sprite.hpp @@ -690,6 +692,7 @@ list(APPEND SRC_FILES ${PROJECT_SOURCE_DIR}/src/mbgl/style/conversion/light.cpp ${PROJECT_SOURCE_DIR}/src/mbgl/style/conversion/position.cpp ${PROJECT_SOURCE_DIR}/src/mbgl/style/conversion/property_value.cpp + ${PROJECT_SOURCE_DIR}/src/mbgl/style/conversion/raster_dem_options.cpp ${PROJECT_SOURCE_DIR}/src/mbgl/style/conversion/rotation.cpp ${PROJECT_SOURCE_DIR}/src/mbgl/style/conversion/source.cpp ${PROJECT_SOURCE_DIR}/src/mbgl/style/conversion/stringify.hpp @@ -815,12 +818,11 @@ list(APPEND SRC_FILES ${PROJECT_SOURCE_DIR}/src/mbgl/style/sources/image_source_impl.cpp ${PROJECT_SOURCE_DIR}/src/mbgl/style/sources/image_source_impl.hpp ${PROJECT_SOURCE_DIR}/src/mbgl/style/sources/raster_dem_source.cpp + ${PROJECT_SOURCE_DIR}/src/mbgl/style/sources/tile_source.cpp ${PROJECT_SOURCE_DIR}/src/mbgl/style/sources/raster_source.cpp - ${PROJECT_SOURCE_DIR}/src/mbgl/style/sources/raster_source_impl.cpp - ${PROJECT_SOURCE_DIR}/src/mbgl/style/sources/raster_source_impl.hpp + ${PROJECT_SOURCE_DIR}/src/mbgl/style/sources/tile_source_impl.cpp + ${PROJECT_SOURCE_DIR}/src/mbgl/style/sources/tile_source_impl.hpp ${PROJECT_SOURCE_DIR}/src/mbgl/style/sources/vector_source.cpp - ${PROJECT_SOURCE_DIR}/src/mbgl/style/sources/vector_source_impl.cpp - ${PROJECT_SOURCE_DIR}/src/mbgl/style/sources/vector_source_impl.hpp ${PROJECT_SOURCE_DIR}/src/mbgl/style/style.cpp ${PROJECT_SOURCE_DIR}/src/mbgl/style/style_impl.cpp ${PROJECT_SOURCE_DIR}/src/mbgl/style/style_impl.hpp diff --git a/bazel/core.bzl b/bazel/core.bzl index 4ecc9a59b..31a59b642 100644 --- a/bazel/core.bzl +++ b/bazel/core.bzl @@ -345,6 +345,7 @@ MLN_CORE_SOURCE = [ "src/mbgl/style/conversion/light.cpp", "src/mbgl/style/conversion/position.cpp", "src/mbgl/style/conversion/property_value.cpp", + "src/mbgl/style/conversion/raster_dem_options.cpp", "src/mbgl/style/conversion/rotation.cpp", "src/mbgl/style/conversion/source.cpp", "src/mbgl/style/conversion/stringify.hpp", @@ -450,12 +451,11 @@ MLN_CORE_SOURCE = [ "src/mbgl/style/sources/image_source_impl.cpp", "src/mbgl/style/sources/image_source_impl.hpp", "src/mbgl/style/sources/raster_dem_source.cpp", + "src/mbgl/style/sources/tile_source.cpp", "src/mbgl/style/sources/raster_source.cpp", - "src/mbgl/style/sources/raster_source_impl.cpp", - "src/mbgl/style/sources/raster_source_impl.hpp", + "src/mbgl/style/sources/tile_source_impl.cpp", + "src/mbgl/style/sources/tile_source_impl.hpp", "src/mbgl/style/sources/vector_source.cpp", - "src/mbgl/style/sources/vector_source_impl.cpp", - "src/mbgl/style/sources/vector_source_impl.hpp", "src/mbgl/style/style.cpp", "src/mbgl/style/style_impl.cpp", "src/mbgl/style/style_impl.hpp", @@ -696,6 +696,7 @@ MLN_CORE_HEADERS = [ "include/mbgl/style/conversion/light.hpp", "include/mbgl/style/conversion/position.hpp", "include/mbgl/style/conversion/property_value.hpp", + "include/mbgl/style/conversion/raster_dem_options.hpp", "include/mbgl/style/conversion/rotation.hpp", "include/mbgl/style/conversion/source.hpp", "include/mbgl/style/conversion/tileset.hpp", @@ -755,6 +756,7 @@ MLN_CORE_HEADERS = [ "include/mbgl/style/sources/geojson_source.hpp", "include/mbgl/style/sources/image_source.hpp", "include/mbgl/style/sources/raster_dem_source.hpp", + "include/mbgl/style/sources/tile_source.hpp", "include/mbgl/style/sources/raster_source.hpp", "include/mbgl/style/sources/vector_source.hpp", "include/mbgl/style/sprite.hpp", diff --git a/include/mbgl/style/conversion/raster_dem_options.hpp b/include/mbgl/style/conversion/raster_dem_options.hpp new file mode 100644 index 000000000..5962dbba0 --- /dev/null +++ b/include/mbgl/style/conversion/raster_dem_options.hpp @@ -0,0 +1,17 @@ +#pragma once + +#include +#include + +namespace mbgl { +namespace style { +namespace conversion { + +template <> +struct Converter { + std::optional operator()(const Convertible& value, Error& error) const; +}; + +} // namespace conversion +} // namespace style +} // namespace mbgl diff --git a/include/mbgl/style/sources/raster_dem_source.hpp b/include/mbgl/style/sources/raster_dem_source.hpp index 42e27cd07..a7738d7f1 100644 --- a/include/mbgl/style/sources/raster_dem_source.hpp +++ b/include/mbgl/style/sources/raster_dem_source.hpp @@ -6,14 +6,25 @@ namespace mbgl { -class AsyncRequest; - namespace style { +struct RasterDEMOptions { + std::optional encoding = std::nullopt; +}; + class RasterDEMSource : public RasterSource { public: - RasterDEMSource(std::string id, variant urlOrTileset, uint16_t tileSize); + RasterDEMSource(std::string id, + variant urlOrTileset, + uint16_t tileSize, + std::optional options = std::nullopt); bool supportsLayerType(const mbgl::style::LayerTypeInfo*) const override; + +protected: + void setTilesetOverrides(Tileset& tileset) override; + +private: + std::optional options; }; template <> diff --git a/include/mbgl/style/sources/raster_source.hpp b/include/mbgl/style/sources/raster_source.hpp index 58002c284..97e2911e4 100644 --- a/include/mbgl/style/sources/raster_source.hpp +++ b/include/mbgl/style/sources/raster_source.hpp @@ -1,45 +1,21 @@ #pragma once -#include +#include #include #include namespace mbgl { -class AsyncRequest; - namespace style { -class RasterSource : public Source { +class RasterSource : public TileSource { public: RasterSource(std::string id, variant urlOrTileset, uint16_t tileSize, SourceType sourceType = SourceType::Raster); - ~RasterSource() override; - - const variant& getURLOrTileset() const; - std::optional getURL() const; - - uint16_t getTileSize() const; - - class Impl; - const Impl& impl() const; - - void loadDescription(FileSource&) final; bool supportsLayerType(const mbgl::style::LayerTypeInfo*) const override; - - mapbox::base::WeakPtr makeWeakPtr() final { return weakFactory.makeWeakPtr(); } - -protected: - Mutable createMutable() const noexcept final; - -private: - const variant urlOrTileset; - std::unique_ptr req; - mapbox::base::WeakPtrFactory weakFactory{this}; - // Do not add members here, see `WeakPtrFactory` }; template <> diff --git a/include/mbgl/style/sources/tile_source.hpp b/include/mbgl/style/sources/tile_source.hpp new file mode 100644 index 000000000..bbeb8de72 --- /dev/null +++ b/include/mbgl/style/sources/tile_source.hpp @@ -0,0 +1,47 @@ +#pragma once + +#include +#include +#include + +namespace mbgl { + +class AsyncRequest; + +namespace style { + +class TileSource : public Source { +public: + TileSource(std::string id, variant urlOrTileset, uint16_t tileSize, SourceType sourceType); + ~TileSource() override; + + const variant& getURLOrTileset() const; + std::optional getURL() const; + + uint16_t getTileSize() const; + + class Impl; + const Impl& impl() const; + + void loadDescription(FileSource&) final; + + mapbox::base::WeakPtr makeWeakPtr() final { return weakFactory.makeWeakPtr(); } + +protected: + Mutable createMutable() const noexcept final; + virtual void setTilesetOverrides(Tileset& tileset); + +private: + const variant urlOrTileset; + std::unique_ptr req; + mapbox::base::WeakPtrFactory weakFactory{this}; + // Do not add members here, see `WeakPtrFactory` +}; + +template <> +inline bool Source::is() const { + return getType() == SourceType::Raster; +} + +} // namespace style +} // namespace mbgl diff --git a/include/mbgl/style/sources/vector_source.hpp b/include/mbgl/style/sources/vector_source.hpp index 88ef1602a..6e5672c98 100644 --- a/include/mbgl/style/sources/vector_source.hpp +++ b/include/mbgl/style/sources/vector_source.hpp @@ -1,6 +1,6 @@ #pragma once -#include +#include #include #include @@ -10,21 +10,14 @@ class AsyncRequest; namespace style { -class VectorSource final : public Source { +class VectorSource final : public TileSource { public: VectorSource(std::string id, variant urlOrTileset, std::optional maxZoom = std::nullopt, std::optional minZoom = std::nullopt); - ~VectorSource() final; - const variant& getURLOrTileset() const; - std::optional getURL() const; - - class Impl; - const Impl& impl() const; - - void loadDescription(FileSource&) final; + void setTilesetOverrides(Tileset& tileset) override; /// @brief Gets the tile urls for this vector source. /// @return List of tile urls. @@ -36,18 +29,9 @@ public: bool supportsLayerType(const mbgl::style::LayerTypeInfo*) const override; - mapbox::base::WeakPtr makeWeakPtr() override { return weakFactory.makeWeakPtr(); } - -protected: - Mutable createMutable() const noexcept final; - private: - const variant urlOrTileset; - std::unique_ptr req; std::optional maxZoom; std::optional minZoom; - mapbox::base::WeakPtrFactory weakFactory{this}; - // Do not add members here, see `WeakPtrFactory` }; template <> diff --git a/src/mbgl/renderer/render_source.cpp b/src/mbgl/renderer/render_source.cpp index 46eddb69c..2188caf41 100644 --- a/src/mbgl/renderer/render_source.cpp +++ b/src/mbgl/renderer/render_source.cpp @@ -24,13 +24,13 @@ std::unique_ptr RenderSource::create(const Immutable const TaggedScheduler& threadPool_) { switch (impl->type) { case SourceType::Vector: - return std::make_unique(staticImmutableCast(impl), + return std::make_unique(staticImmutableCast(impl), std::move(threadPool_)); case SourceType::Raster: - return std::make_unique(staticImmutableCast(impl), + return std::make_unique(staticImmutableCast(impl), std::move(threadPool_)); case SourceType::RasterDEM: - return std::make_unique(staticImmutableCast(impl), + return std::make_unique(staticImmutableCast(impl), std::move(threadPool_)); case SourceType::GeoJSON: return std::make_unique(staticImmutableCast(impl), diff --git a/src/mbgl/renderer/sources/render_raster_dem_source.cpp b/src/mbgl/renderer/sources/render_raster_dem_source.cpp index ef95adbd3..753dc6a20 100644 --- a/src/mbgl/renderer/sources/render_raster_dem_source.cpp +++ b/src/mbgl/renderer/sources/render_raster_dem_source.cpp @@ -10,12 +10,12 @@ namespace mbgl { using namespace style; -RenderRasterDEMSource::RenderRasterDEMSource(Immutable impl_, +RenderRasterDEMSource::RenderRasterDEMSource(Immutable impl_, const TaggedScheduler& threadPool_) : RenderTileSetSource(std::move(impl_), threadPool_) {} -const style::RasterSource::Impl& RenderRasterDEMSource::impl() const { - return static_cast(*baseImpl); +const style::TileSource::Impl& RenderRasterDEMSource::impl() const { + return static_cast(*baseImpl); } const std::optional& RenderRasterDEMSource::getTileset() const { diff --git a/src/mbgl/renderer/sources/render_raster_dem_source.hpp b/src/mbgl/renderer/sources/render_raster_dem_source.hpp index 8531c6be8..6cacb6d47 100644 --- a/src/mbgl/renderer/sources/render_raster_dem_source.hpp +++ b/src/mbgl/renderer/sources/render_raster_dem_source.hpp @@ -1,13 +1,13 @@ #pragma once #include -#include +#include namespace mbgl { class RenderRasterDEMSource final : public RenderTileSetSource { public: - explicit RenderRasterDEMSource(Immutable, const TaggedScheduler&); + explicit RenderRasterDEMSource(Immutable, const TaggedScheduler&); std::unordered_map> queryRenderedFeatures( const ScreenLineString& geometry, @@ -27,7 +27,7 @@ private: const TileParameters&) override; const std::optional& getTileset() const override; - const style::RasterSource::Impl& impl() const; + const style::TileSource::Impl& impl() const; void onTileChanged(Tile&) override; }; diff --git a/src/mbgl/renderer/sources/render_raster_source.cpp b/src/mbgl/renderer/sources/render_raster_source.cpp index 0f32e51a4..072f709fb 100644 --- a/src/mbgl/renderer/sources/render_raster_source.cpp +++ b/src/mbgl/renderer/sources/render_raster_source.cpp @@ -8,11 +8,11 @@ namespace mbgl { using namespace style; -RenderRasterSource::RenderRasterSource(Immutable impl_, const TaggedScheduler& threadPool_) +RenderRasterSource::RenderRasterSource(Immutable impl_, const TaggedScheduler& threadPool_) : RenderTileSetSource(std::move(impl_), threadPool_) {} -inline const style::RasterSource::Impl& RenderRasterSource::impl() const { - return static_cast(*baseImpl); +inline const style::TileSource::Impl& RenderRasterSource::impl() const { + return static_cast(*baseImpl); } const std::optional& RenderRasterSource::getTileset() const { diff --git a/src/mbgl/renderer/sources/render_raster_source.hpp b/src/mbgl/renderer/sources/render_raster_source.hpp index d650e974e..09f0d7a8b 100644 --- a/src/mbgl/renderer/sources/render_raster_source.hpp +++ b/src/mbgl/renderer/sources/render_raster_source.hpp @@ -1,13 +1,13 @@ #pragma once #include -#include +#include namespace mbgl { class RenderRasterSource final : public RenderTileSetSource { public: - explicit RenderRasterSource(Immutable, const TaggedScheduler&); + explicit RenderRasterSource(Immutable, const TaggedScheduler&); private: void prepare(const SourcePrepareParameters&) final; @@ -29,7 +29,7 @@ private: const TileParameters&) override; const std::optional& getTileset() const override; - const style::RasterSource::Impl& impl() const; + const style::TileSource::Impl& impl() const; }; } // namespace mbgl diff --git a/src/mbgl/renderer/sources/render_tile_source.hpp b/src/mbgl/renderer/sources/render_tile_source.hpp index 3b09f2758..619d4d5c5 100644 --- a/src/mbgl/renderer/sources/render_tile_source.hpp +++ b/src/mbgl/renderer/sources/render_tile_source.hpp @@ -3,7 +3,7 @@ #include #include #include -#include +#include #include #include diff --git a/src/mbgl/renderer/sources/render_vector_source.cpp b/src/mbgl/renderer/sources/render_vector_source.cpp index 2ccaf5492..383de133a 100644 --- a/src/mbgl/renderer/sources/render_vector_source.cpp +++ b/src/mbgl/renderer/sources/render_vector_source.cpp @@ -8,11 +8,11 @@ namespace mbgl { using namespace style; -RenderVectorSource::RenderVectorSource(Immutable impl_, const TaggedScheduler& threadPool_) +RenderVectorSource::RenderVectorSource(Immutable impl_, const TaggedScheduler& threadPool_) : RenderTileSetSource(std::move(impl_), threadPool_) {} const std::optional& RenderVectorSource::getTileset() const { - return static_cast(*baseImpl).tileset; + return static_cast(*baseImpl).tileset; } void RenderVectorSource::updateInternal(const Tileset& tileset, diff --git a/src/mbgl/renderer/sources/render_vector_source.hpp b/src/mbgl/renderer/sources/render_vector_source.hpp index fff332c45..aa3d7c122 100644 --- a/src/mbgl/renderer/sources/render_vector_source.hpp +++ b/src/mbgl/renderer/sources/render_vector_source.hpp @@ -2,13 +2,13 @@ #include #include -#include +#include namespace mbgl { class RenderVectorSource final : public RenderTileSetSource { public: - explicit RenderVectorSource(Immutable, const TaggedScheduler&); + explicit RenderVectorSource(Immutable, const TaggedScheduler&); private: void updateInternal(const Tileset&, diff --git a/src/mbgl/style/conversion/raster_dem_options.cpp b/src/mbgl/style/conversion/raster_dem_options.cpp new file mode 100644 index 000000000..29f50dffc --- /dev/null +++ b/src/mbgl/style/conversion/raster_dem_options.cpp @@ -0,0 +1,34 @@ +#include +#include +#include + +#include + +namespace mbgl { +namespace style { +namespace conversion { + +std::optional Converter::operator()(const Convertible& value, Error& error) const { + RasterDEMOptions options; + + auto encodingValue = objectMember(value, "encoding"); + if (encodingValue) { + std::optional encoding = toString(*encodingValue); + if (encoding && *encoding == "terrarium") { + options.encoding = {Tileset::DEMEncoding::Terrarium}; + } else if (encoding && *encoding == "mapbox") { + options.encoding = {Tileset::DEMEncoding::Mapbox}; + } else { + error.message = + "invalid raster-dem encoding type - valid types are 'mapbox' " + "and 'terrarium'"; + return std::nullopt; + } + } + + return {std::move(options)}; +} + +} // namespace conversion +} // namespace style +} // namespace mbgl diff --git a/src/mbgl/style/conversion/source.cpp b/src/mbgl/style/conversion/source.cpp index 75c22c849..0ef4cbbf2 100644 --- a/src/mbgl/style/conversion/source.cpp +++ b/src/mbgl/style/conversion/source.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include #include #include @@ -76,7 +77,14 @@ static std::optional> convertRasterDEMSource(const std:: tileSize = static_cast(*size); } - return {std::make_unique(id, std::move(*urlOrTileset), tileSize)}; + /* + TODO: Refactor TileJSON parsing responsibilities. Currently `Tileset` handles full TileJSON parsing with optional + parameters. Since style.json can contain TileJSON parameter overrides or non-TileJSON spec parameters, a + separate mechanism is needed. Ideally these two pathways share parsing logic. + */ + std::optional options = convert(value, error); + + return {std::make_unique(id, std::move(*urlOrTileset), tileSize, options)}; } static std::optional> convertVectorSource(const std::string& id, diff --git a/src/mbgl/style/conversion/tileset.cpp b/src/mbgl/style/conversion/tileset.cpp index 417842f0f..c18a83339 100644 --- a/src/mbgl/style/conversion/tileset.cpp +++ b/src/mbgl/style/conversion/tileset.cpp @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -38,15 +39,10 @@ std::optional Converter::operator()(const Convertible& value, } } - auto encodingValue = objectMember(value, "encoding"); - if (encodingValue) { - std::optional encoding = toString(*encodingValue); - if (encoding && *encoding == "terrarium") { - result.encoding = Tileset::DEMEncoding::Terrarium; - } else if (encoding && *encoding != "mapbox") { - error.message = - "invalid raster-dem encoding type - valid types are 'mapbox' " - "and 'terrarium' "; + auto rasterDEMOptions = convert(value, error); + if (rasterDEMOptions) { + if (std::optional encoding = rasterDEMOptions.value().encoding) { + result.encoding = encoding.value(); } } diff --git a/src/mbgl/style/sources/raster_dem_source.cpp b/src/mbgl/style/sources/raster_dem_source.cpp index 98a83ee87..805984a7e 100644 --- a/src/mbgl/style/sources/raster_dem_source.cpp +++ b/src/mbgl/style/sources/raster_dem_source.cpp @@ -3,7 +3,7 @@ #include #include #include -#include +#include #include #include #include @@ -11,12 +11,24 @@ namespace mbgl { namespace style { -RasterDEMSource::RasterDEMSource(std::string id, variant urlOrTileset_, uint16_t tileSize) - : RasterSource(std::move(id), std::move(urlOrTileset_), tileSize, SourceType::RasterDEM) {} +RasterDEMSource::RasterDEMSource(std::string id, + variant urlOrTileset_, + uint16_t tileSize, + std::optional options_) + : RasterSource(std::move(id), std::move(urlOrTileset_), tileSize, SourceType::RasterDEM), + options(std::move(options_)) {} bool RasterDEMSource::supportsLayerType(const mbgl::style::LayerTypeInfo* info) const { return mbgl::underlying_type(Tile::Kind::RasterDEM) == mbgl::underlying_type(info->tileKind); } +void RasterDEMSource::setTilesetOverrides(Tileset& tileset) { + if (options) { + if (std::optional encoding = options.value().encoding) { + tileset.encoding = encoding.value(); + } + } +} + } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/sources/raster_source.cpp b/src/mbgl/style/sources/raster_source.cpp index e0b58480e..260fc30e1 100644 --- a/src/mbgl/style/sources/raster_source.cpp +++ b/src/mbgl/style/sources/raster_source.cpp @@ -4,7 +4,7 @@ #include #include #include -#include +#include #include #include #include @@ -17,85 +17,11 @@ RasterSource::RasterSource(std::string id, variant urlOrTileset_, uint16_t tileSize, SourceType sourceType) - : Source(makeMutable(sourceType, std::move(id), tileSize)), - urlOrTileset(std::move(urlOrTileset_)) {} - -RasterSource::~RasterSource() = default; - -const RasterSource::Impl& RasterSource::impl() const { - return static_cast(*baseImpl); -} - -const variant& RasterSource::getURLOrTileset() const { - return urlOrTileset; -} - -std::optional RasterSource::getURL() const { - if (urlOrTileset.is()) { - return {}; - } - - return urlOrTileset.get(); -} - -uint16_t RasterSource::getTileSize() const { - return impl().getTileSize(); -} - -void RasterSource::loadDescription(FileSource& fileSource) { - if (urlOrTileset.is()) { - baseImpl = makeMutable(impl(), urlOrTileset.get()); - loaded = true; - observer->onSourceLoaded(*this); - return; - } - - if (req) { - return; - } - - const auto& rawURL = urlOrTileset.get(); - const auto& url = util::mapbox::canonicalizeSourceURL(fileSource.getResourceOptions().tileServerOptions(), rawURL); - - req = fileSource.request(Resource::source(url), [this, url, &fileSource](const Response& res) { - if (res.error) { - observer->onSourceError(*this, std::make_exception_ptr(std::runtime_error(res.error->message))); - } else if (res.notModified) { - return; - } else if (res.noContent) { - observer->onSourceError(*this, std::make_exception_ptr(std::runtime_error("unexpectedly empty TileJSON"))); - } else { - conversion::Error error; - std::optional tileset = conversion::convertJSON(*res.data, error); - if (!tileset) { - observer->onSourceError(*this, std::make_exception_ptr(util::StyleParseException(error.message))); - return; - } - const auto& tileServerOptions = fileSource.getResourceOptions().tileServerOptions(); - if (tileServerOptions.uriSchemeAlias() == "mapbox") { - util::mapbox::canonicalizeTileset(tileServerOptions, *tileset, url, getType(), getTileSize()); - } - bool changed = impl().tileset != *tileset; - - baseImpl = makeMutable(impl(), *tileset); - loaded = true; - - observer->onSourceLoaded(*this); - - if (changed) { - observer->onSourceChanged(*this); - } - } - }); -} + : TileSource(id, urlOrTileset_, tileSize, sourceType) {} bool RasterSource::supportsLayerType(const mbgl::style::LayerTypeInfo* info) const { return mbgl::underlying_type(Tile::Kind::Raster) == mbgl::underlying_type(info->tileKind); } -Mutable RasterSource::createMutable() const noexcept { - return staticMutableCast(makeMutable(impl())); -} - } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/sources/tile_source.cpp b/src/mbgl/style/sources/tile_source.cpp new file mode 100644 index 000000000..5eb187220 --- /dev/null +++ b/src/mbgl/style/sources/tile_source.cpp @@ -0,0 +1,104 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace mbgl { +namespace style { + +TileSource::TileSource(std::string id, + variant urlOrTileset_, + uint16_t tileSize, + SourceType sourceType) + : Source(makeMutable(sourceType, std::move(id), tileSize)), + urlOrTileset(std::move(urlOrTileset_)) {} + +TileSource::~TileSource() = default; + +const TileSource::Impl& TileSource::impl() const { + return static_cast(*baseImpl); +} + +const variant& TileSource::getURLOrTileset() const { + return urlOrTileset; +} + +std::optional TileSource::getURL() const { + if (urlOrTileset.is()) { + return {}; + } + + return urlOrTileset.get(); +} + +uint16_t TileSource::getTileSize() const { + return impl().getTileSize(); +} + +void TileSource::loadDescription(FileSource& fileSource) { + if (urlOrTileset.is()) { + baseImpl = makeMutable(impl(), urlOrTileset.get()); + loaded = true; + observer->onSourceLoaded(*this); + return; + } + + if (req) { + return; + } + + const auto& rawURL = urlOrTileset.get(); + const auto& url = util::mapbox::canonicalizeSourceURL(fileSource.getResourceOptions().tileServerOptions(), rawURL); + + req = fileSource.request(Resource::source(url), [this, url, &fileSource](const Response& res) { + if (res.error) { + observer->onSourceError(*this, std::make_exception_ptr(std::runtime_error(res.error->message))); + } else if (res.notModified) { + return; + } else if (res.noContent) { + observer->onSourceError(*this, std::make_exception_ptr(std::runtime_error("unexpectedly empty TileJSON"))); + } else { + conversion::Error error; + std::optional tileset = conversion::convertJSON(*res.data, error); + if (!tileset) { + observer->onSourceError(*this, std::make_exception_ptr(util::StyleParseException(error.message))); + return; + } + const auto& tileServerOptions = fileSource.getResourceOptions().tileServerOptions(); + if (tileServerOptions.uriSchemeAlias() == "mapbox") { + util::mapbox::canonicalizeTileset(tileServerOptions, *tileset, url, getType(), getTileSize()); + } + + this->setTilesetOverrides(*tileset); + + bool changed = impl().tileset != *tileset; + + baseImpl = makeMutable(impl(), *tileset); + loaded = true; + + observer->onSourceLoaded(*this); + + if (changed) { + observer->onSourceChanged(*this); + } + } + }); +} + +void TileSource::setTilesetOverrides(Tileset&) { + // Default empty implementation, subclasses can override +} + +Mutable TileSource::createMutable() const noexcept { + return staticMutableCast(makeMutable(impl())); +} + +} // namespace style +} // namespace mbgl diff --git a/src/mbgl/style/sources/raster_source_impl.cpp b/src/mbgl/style/sources/tile_source_impl.cpp similarity index 51% rename from src/mbgl/style/sources/raster_source_impl.cpp rename to src/mbgl/style/sources/tile_source_impl.cpp index 683650792..f92c72ba2 100644 --- a/src/mbgl/style/sources/raster_source_impl.cpp +++ b/src/mbgl/style/sources/tile_source_impl.cpp @@ -1,22 +1,22 @@ -#include +#include namespace mbgl { namespace style { -RasterSource::Impl::Impl(SourceType sourceType, std::string id_, uint16_t tileSize_) +TileSource::Impl::Impl(SourceType sourceType, std::string id_, uint16_t tileSize_ = util::tileSize_I) : Source::Impl(sourceType, std::move(id_)), tileSize(tileSize_) {} -RasterSource::Impl::Impl(const Impl& other, Tileset tileset_) +TileSource::Impl::Impl(const Impl& other, Tileset tileset_) : Source::Impl(other), tileset(std::move(tileset_)), tileSize(other.tileSize) {} -uint16_t RasterSource::Impl::getTileSize() const { +uint16_t TileSource::Impl::getTileSize() const { return tileSize; } -std::optional RasterSource::Impl::getAttribution() const { +std::optional TileSource::Impl::getAttribution() const { if (!tileset) { return {}; } diff --git a/src/mbgl/style/sources/raster_source_impl.hpp b/src/mbgl/style/sources/tile_source_impl.hpp similarity index 81% rename from src/mbgl/style/sources/raster_source_impl.hpp rename to src/mbgl/style/sources/tile_source_impl.hpp index 23443bf27..277592980 100644 --- a/src/mbgl/style/sources/raster_source_impl.hpp +++ b/src/mbgl/style/sources/tile_source_impl.hpp @@ -1,12 +1,12 @@ #pragma once -#include +#include #include namespace mbgl { namespace style { -class RasterSource::Impl : public Source::Impl { +class TileSource::Impl : public Source::Impl { public: Impl(SourceType sourceType, std::string id, uint16_t tileSize); Impl(const Impl&, Tileset); diff --git a/src/mbgl/style/sources/vector_source.cpp b/src/mbgl/style/sources/vector_source.cpp index 47630bf3e..bd61f5ed2 100644 --- a/src/mbgl/style/sources/vector_source.cpp +++ b/src/mbgl/style/sources/vector_source.cpp @@ -4,7 +4,7 @@ #include #include #include -#include +#include #include #include #include @@ -18,78 +18,17 @@ VectorSource::VectorSource(std::string id, variant urlOrTileset_, std::optional maxZoom_, std::optional minZoom_) - : Source(makeMutable(std::move(id))), - urlOrTileset(std::move(urlOrTileset_)), + : TileSource(id, urlOrTileset_, util::tileSize_I, SourceType::Vector), maxZoom(std::move(maxZoom_)), minZoom(std::move(minZoom_)) {} -VectorSource::~VectorSource() = default; - -const VectorSource::Impl& VectorSource::impl() const { - return static_cast(*baseImpl); -} - -const variant& VectorSource::getURLOrTileset() const { - return urlOrTileset; -} - -std::optional VectorSource::getURL() const { - if (urlOrTileset.is()) { - return {}; +void VectorSource::setTilesetOverrides(Tileset& tileset) { + if (maxZoom) { + tileset.zoomRange.max = static_cast(*maxZoom); } - - return urlOrTileset.get(); -} - -void VectorSource::loadDescription(FileSource& fileSource) { - if (urlOrTileset.is()) { - baseImpl = makeMutable(impl(), urlOrTileset.get()); - loaded = true; - observer->onSourceLoaded(*this); - return; + if (minZoom) { + tileset.zoomRange.min = static_cast(*minZoom); } - - if (req) { - return; - } - - const auto& rawURL = urlOrTileset.get(); - const auto& url = util::mapbox::canonicalizeSourceURL(fileSource.getResourceOptions().tileServerOptions(), rawURL); - - req = fileSource.request(Resource::source(url), [this, url, &fileSource](const Response& res) { - if (res.error) { - observer->onSourceError(*this, std::make_exception_ptr(std::runtime_error(res.error->message))); - } else if (res.notModified) { - return; - } else if (res.noContent) { - observer->onSourceError(*this, std::make_exception_ptr(std::runtime_error("unexpectedly empty TileJSON"))); - } else { - conversion::Error error; - auto tileset = conversion::convertJSON(*res.data, error); - if (!tileset) { - observer->onSourceError(*this, std::make_exception_ptr(util::StyleParseException(error.message))); - return; - } - if (maxZoom) { - tileset->zoomRange.max = static_cast(*maxZoom); - } - if (minZoom) { - tileset->zoomRange.min = static_cast(*minZoom); - } - const TileServerOptions& tileServerOptions = fileSource.getResourceOptions().tileServerOptions(); - util::mapbox::canonicalizeTileset(tileServerOptions, *tileset, url, getType(), util::tileSize_I); - bool changed = impl().tileset != *tileset; - - baseImpl = makeMutable(impl(), *tileset); - loaded = true; - - observer->onSourceLoaded(*this); - - if (changed) { - observer->onSourceChanged(*this); - } - } - }); } const std::vector VectorSource::getTiles() const { @@ -115,9 +54,5 @@ bool VectorSource::supportsLayerType(const mbgl::style::LayerTypeInfo* info) con return mbgl::underlying_type(Tile::Kind::Geometry) == mbgl::underlying_type(info->tileKind); } -Mutable VectorSource::createMutable() const noexcept { - return staticMutableCast(makeMutable(impl())); -} - } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/sources/vector_source_impl.cpp b/src/mbgl/style/sources/vector_source_impl.cpp deleted file mode 100644 index 3f202d0e1..000000000 --- a/src/mbgl/style/sources/vector_source_impl.cpp +++ /dev/null @@ -1,21 +0,0 @@ -#include - -namespace mbgl { -namespace style { - -VectorSource::Impl::Impl(std::string id_) - : Source::Impl(SourceType::Vector, std::move(id_)) {} - -VectorSource::Impl::Impl(const Impl& other, Tileset tileset_) - : Source::Impl(other), - tileset(std::move(tileset_)) {} - -std::optional VectorSource::Impl::getAttribution() const { - if (!tileset) { - return {}; - } - return tileset->attribution; -} - -} // namespace style -} // namespace mbgl diff --git a/src/mbgl/style/sources/vector_source_impl.hpp b/src/mbgl/style/sources/vector_source_impl.hpp deleted file mode 100644 index 4d7b9d259..000000000 --- a/src/mbgl/style/sources/vector_source_impl.hpp +++ /dev/null @@ -1,20 +0,0 @@ -#pragma once - -#include -#include - -namespace mbgl { -namespace style { - -class VectorSource::Impl : public Source::Impl { -public: - Impl(std::string id); - Impl(const Impl&, Tileset); - - std::optional getAttribution() const final; - - const std::optional tileset; -}; - -} // namespace style -} // namespace mbgl diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 162caf3a7..1fa8fffcb 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -51,6 +51,7 @@ add_library( ${PROJECT_SOURCE_DIR}/test/style/conversion/light.test.cpp ${PROJECT_SOURCE_DIR}/test/style/conversion/padding.test.cpp ${PROJECT_SOURCE_DIR}/test/style/conversion/property_value.test.cpp + ${PROJECT_SOURCE_DIR}/test/style/conversion/raster_dem_options.test.cpp ${PROJECT_SOURCE_DIR}/test/style/conversion/stringify.test.cpp ${PROJECT_SOURCE_DIR}/test/style/conversion/tileset.test.cpp ${PROJECT_SOURCE_DIR}/test/style/expression/dependency.test.cpp diff --git a/test/style/conversion/raster_dem_options.test.cpp b/test/style/conversion/raster_dem_options.test.cpp new file mode 100644 index 000000000..416bd3f27 --- /dev/null +++ b/test/style/conversion/raster_dem_options.test.cpp @@ -0,0 +1,48 @@ +#include + +#include +#include +#include + +#include + +using namespace mbgl; +using namespace mbgl::style; +using namespace mbgl::style::conversion; + +TEST(RasterDEMOptions, Basic) { + Error error; + std::optional converted = convertJSON("{}", error); + ASSERT_TRUE((bool)converted); +} + +TEST(RasterDEMOptions, ErrorHandling) { + Error error; + std::optional converted = convertJSON( + R"JSON({ + "encoding": "this isn't a valid encoding" + })JSON", + error); + ASSERT_FALSE(converted); + ASSERT_EQ(error.message, "invalid raster-dem encoding type - valid types are 'mapbox' and 'terrarium'"); +} + +TEST(RasterDEMOptions, TerrariumEncodingParsed) { + Error error; + std::optional converted = convertJSON( + R"JSON({ + "encoding": "terrarium" + })JSON", + error); + ASSERT_EQ(converted.value().encoding, Tileset::DEMEncoding::Terrarium); +} + +TEST(RasterDEMOptions, MapboxEncodingParsed) { + Error error; + std::optional converted = convertJSON( + R"JSON({ + "encoding": "mapbox" + })JSON", + error); + ASSERT_EQ(converted.value().encoding, Tileset::DEMEncoding::Mapbox); +}