From 5a26cd47f8f2bffce7963ebddde4ac030494379e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Minh=20Nguye=CC=82=CC=83n?= Date: Sat, 13 Aug 2016 11:30:15 -0700 Subject: [PATCH] [core] Source-driven attribution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implemented observer callbacks so the style knows when the source’s attribution changes and the map knows when the style’s attribution changes. Also implemented a getter for a tile source’s attribution. Fixes #2723. --- include/mbgl/map/map.hpp | 1 + platform/qt/include/qmapboxgl.hpp | 3 +- platform/qt/src/qmapboxgl.cpp | 1 + src/mbgl/map/change.hpp | 3 +- src/mbgl/map/map.cpp | 9 +++++ src/mbgl/style/source_observer.hpp | 1 + src/mbgl/style/style.cpp | 28 ++++++++++++++++ src/mbgl/style/style.hpp | 2 ++ src/mbgl/style/tile_source_impl.cpp | 12 +++++-- src/mbgl/style/tile_source_impl.hpp | 2 ++ test/src/mbgl/test/stub_style_observer.hpp | 7 +++- test/style/source.cpp | 39 ++++++++++++++++++++++ 12 files changed, 103 insertions(+), 5 deletions(-) diff --git a/include/mbgl/map/map.hpp b/include/mbgl/map/map.hpp index 6c478c17f01..fb39f37c1b7 100644 --- a/include/mbgl/map/map.hpp +++ b/include/mbgl/map/map.hpp @@ -155,6 +155,7 @@ class Map : private util::noncopyable { style::Source* getSource(const std::string& sourceID); void addSource(std::unique_ptr); void removeSource(const std::string& sourceID); + std::vector getAttributions() const; // Layers style::Layer* getLayer(const std::string& layerID); diff --git a/platform/qt/include/qmapboxgl.hpp b/platform/qt/include/qmapboxgl.hpp index bab46fd096c..490f48aa791 100644 --- a/platform/qt/include/qmapboxgl.hpp +++ b/platform/qt/include/qmapboxgl.hpp @@ -111,7 +111,8 @@ class Q_DECL_EXPORT QMapboxGL : public QObject MapChangeDidFinishRenderingFrameFullyRendered, MapChangeWillStartRenderingMap, MapChangeDidFinishRenderingMap, - MapChangeDidFinishRenderingMapFullyRendered + MapChangeDidFinishRenderingMapFullyRendered, + MapChangeSourceAttributionDidChange }; QMapboxGL(QObject *parent = 0, const QMapboxGLSettings& = QMapboxGLSettings()); diff --git a/platform/qt/src/qmapboxgl.cpp b/platform/qt/src/qmapboxgl.cpp index 6dd53d628c2..615a27aaea2 100644 --- a/platform/qt/src/qmapboxgl.cpp +++ b/platform/qt/src/qmapboxgl.cpp @@ -76,6 +76,7 @@ static_assert(mbgl::underlying_type(QMapboxGL::MapChangeDidFinishRenderingFrameF static_assert(mbgl::underlying_type(QMapboxGL::MapChangeWillStartRenderingMap) == mbgl::underlying_type(mbgl::MapChangeWillStartRenderingMap), "error"); static_assert(mbgl::underlying_type(QMapboxGL::MapChangeDidFinishRenderingMap) == mbgl::underlying_type(mbgl::MapChangeDidFinishRenderingMap), "error"); static_assert(mbgl::underlying_type(QMapboxGL::MapChangeDidFinishRenderingMapFullyRendered) == mbgl::underlying_type(mbgl::MapChangeDidFinishRenderingMapFullyRendered), "error"); +static_assert(mbgl::underlying_type(QMapboxGL::MapChangeSourceAttributionDidChange) == mbgl::underlying_type(mbgl::MapChangeSourceAttributionDidChange), "error"); namespace { diff --git a/src/mbgl/map/change.hpp b/src/mbgl/map/change.hpp index 343d7d8e4f5..50079af7100 100644 --- a/src/mbgl/map/change.hpp +++ b/src/mbgl/map/change.hpp @@ -19,7 +19,8 @@ enum MapChange : uint8_t { MapChangeWillStartRenderingMap = 11, MapChangeDidFinishRenderingMap = 12, MapChangeDidFinishRenderingMapFullyRendered = 13, - MapChangeDidFinishLoadingStyle = 14 + MapChangeDidFinishLoadingStyle = 14, + MapChangeSourceAttributionDidChange = 15 }; } // namespace mbgl diff --git a/src/mbgl/map/map.cpp b/src/mbgl/map/map.cpp index 1c376e40aa6..edf3a7593ed 100644 --- a/src/mbgl/map/map.cpp +++ b/src/mbgl/map/map.cpp @@ -38,6 +38,7 @@ class Map::Impl : public style::Observer { public: Impl(View&, FileSource&, MapMode, GLContextMode, ConstrainMode, ViewportMode); + void onSourceAttributionChanged(style::Source&, std::string) override; void onUpdate(Update) override; void onStyleLoaded() override; void onStyleError() override; @@ -785,6 +786,10 @@ void Map::removeSource(const std::string& sourceID) { } } +std::vector Map::getAttributions() const { + return impl->style ? impl->style->getAttributions() : std::vector(); +} + style::Layer* Map::getLayer(const std::string& layerID) { if (impl->style) { impl->styleMutated = true; @@ -958,6 +963,10 @@ void Map::onLowMemory() { impl->view.invalidate(); } +void Map::Impl::onSourceAttributionChanged(style::Source&, std::string) { + view.notifyMapChange(MapChangeSourceAttributionDidChange); +} + void Map::Impl::onUpdate(Update flags) { if (flags & Update::Dimensions) { transform.resize(view.getSize()); diff --git a/src/mbgl/style/source_observer.hpp b/src/mbgl/style/source_observer.hpp index 26b25b78da6..dfb0eb391ed 100644 --- a/src/mbgl/style/source_observer.hpp +++ b/src/mbgl/style/source_observer.hpp @@ -17,6 +17,7 @@ class SourceObserver { virtual ~SourceObserver() = default; virtual void onSourceLoaded(Source&) {} + virtual void onSourceAttributionChanged(Source&, std::string) {} virtual void onSourceError(Source&, std::exception_ptr) {} virtual void onTileChanged(Source&, const OverscaledTileID&) {} diff --git a/src/mbgl/style/style.cpp b/src/mbgl/style/style.cpp index a82cdec5727..805bf8987a0 100644 --- a/src/mbgl/style/style.cpp +++ b/src/mbgl/style/style.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -25,6 +26,7 @@ #include #include #include +#include #include #include @@ -293,6 +295,28 @@ Source* Style::getSource(const std::string& id) const { return it != sources.end() ? it->get() : nullptr; } +std::vector Style::getAttributions() const { + std::vector result; + for (const auto& source : sources) { + switch (source->baseImpl->type) { + case SourceType::Vector: + case SourceType::Raster: { + style::TileSourceImpl* tileSource = static_cast(source->baseImpl.get()); + auto attribution = tileSource->getAttribution(); + if (!attribution.empty()) { + result.push_back(attribution); + } + } + + case SourceType::GeoJSON: + case SourceType::Video: + case SourceType::Annotations: + break; + } + } + return result; +} + bool Style::hasTransitions() const { return hasPendingTransitions; } @@ -465,6 +489,10 @@ void Style::onSourceLoaded(Source& source) { observer->onUpdate(Update::Repaint); } +void Style::onSourceAttributionChanged(Source& source, std::string attribution) { + observer->onSourceAttributionChanged(source, attribution); +} + void Style::onSourceError(Source& source, std::exception_ptr error) { lastError = error; Log::Error(Event::Style, "Failed to load source %s: %s", diff --git a/src/mbgl/style/style.hpp b/src/mbgl/style/style.hpp index a3c51a45e7a..580e9f60096 100644 --- a/src/mbgl/style/style.hpp +++ b/src/mbgl/style/style.hpp @@ -67,6 +67,7 @@ class Style : public GlyphAtlasObserver, Source* getSource(const std::string& id) const; void addSource(std::unique_ptr); void removeSource(const std::string& sourceID); + std::vector getAttributions() const; std::vector getLayers() const; Layer* getLayer(const std::string& id) const; @@ -132,6 +133,7 @@ class Style : public GlyphAtlasObserver, // SourceObserver implementation. void onSourceLoaded(Source&) override; + void onSourceAttributionChanged(Source&, std::string) override; void onSourceError(Source&, std::exception_ptr) override; void onTileChanged(Source&, const OverscaledTileID&) override; void onTileError(Source&, const OverscaledTileID&, std::exception_ptr) override; diff --git a/src/mbgl/style/tile_source_impl.cpp b/src/mbgl/style/tile_source_impl.cpp index b201045d033..a318fdd7553 100644 --- a/src/mbgl/style/tile_source_impl.cpp +++ b/src/mbgl/style/tile_source_impl.cpp @@ -82,6 +82,7 @@ void TileSourceImpl::loadDescription(FileSource& fileSource) { } // Check whether previous information specifies different tile + bool attributionChanged = false; if (tileset.tiles != newTileset.tiles) { // Tile URLs changed: force tiles to be reloaded. invalidateTiles(); @@ -95,8 +96,8 @@ void TileSourceImpl::loadDescription(FileSource& fileSource) { // This is done automatically when we trigger the onSourceLoaded observer below. // Attribution changed: We need to notify the embedding application that this - // changed. See https://github.com/mapbox/mapbox-gl-native/issues/2723 - // This is not yet implemented. + // changed. + attributionChanged = true; // Center/bounds changed: We're not using these values currently } @@ -105,6 +106,9 @@ void TileSourceImpl::loadDescription(FileSource& fileSource) { loaded = true; observer->onSourceLoaded(base); + if (attributionChanged) { + observer->onSourceAttributionChanged(base, newTileset.attribution); + } } }); } @@ -114,5 +118,9 @@ Range TileSourceImpl::getZoomRange() { return tileset.zoomRange; } +std::string TileSourceImpl::getAttribution() const { + return loaded ? tileset.attribution : ""; +} + } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/tile_source_impl.hpp b/src/mbgl/style/tile_source_impl.hpp index d0b4b3d8ee1..4ce1eac2231 100644 --- a/src/mbgl/style/tile_source_impl.hpp +++ b/src/mbgl/style/tile_source_impl.hpp @@ -33,6 +33,8 @@ class TileSourceImpl : public Source::Impl { const variant& getURLOrTileset() const { return urlOrTileset; } + + std::string getAttribution() const; protected: Range getZoomRange() final; diff --git a/test/src/mbgl/test/stub_style_observer.hpp b/test/src/mbgl/test/stub_style_observer.hpp index aa780121f56..c185435fdf1 100644 --- a/test/src/mbgl/test/stub_style_observer.hpp +++ b/test/src/mbgl/test/stub_style_observer.hpp @@ -6,7 +6,7 @@ using namespace mbgl; using namespace mbgl::style; /** - * An implementation of style::Observer that forwards all methods to dynamically-settable lambas. + * An implementation of style::Observer that forwards all methods to dynamically-settable lambdas. */ class StubStyleObserver : public style::Observer { public: @@ -30,6 +30,10 @@ class StubStyleObserver : public style::Observer { if (sourceLoaded) sourceLoaded(source); } + void onSourceAttributionChanged(Source& source, std::string attribution) override { + if (sourceAttributionChanged) sourceAttributionChanged(source, attribution); + } + void onSourceError(Source& source, std::exception_ptr error) override { if (sourceError) sourceError(source, error); } @@ -52,6 +56,7 @@ class StubStyleObserver : public style::Observer { std::function spriteLoaded; std::function spriteError; std::function sourceLoaded; + std::function sourceAttributionChanged; std::function sourceError; std::function tileChanged; std::function tileError; diff --git a/test/style/source.cpp b/test/style/source.cpp index 519ca9288e4..91369c03c9a 100644 --- a/test/style/source.cpp +++ b/test/style/source.cpp @@ -342,3 +342,42 @@ TEST(Source, VectorTileCancel) { test.run(); } + +TEST(Source, RasterTileAttribution) { + SourceTest test; + + std::string mapboxOSM = ("© Mapbox " + "©️ OpenStreetMap"); + + test.fileSource.tileResponse = [&] (const Resource&) { + Response response; + response.noContent = true; + return response; + }; + + test.fileSource.sourceResponse = [&] (const Resource& resource) { + EXPECT_EQ("url", resource.url); + Response response; + response.data = std::make_unique(R"TILEJSON({ "tilejson": "2.1.0", "attribution": ")TILEJSON" + + mapboxOSM + + R"TILEJSON(", "tiles": [ "tiles" ] })TILEJSON"); + return response; + }; + + test.observer.sourceAttributionChanged = [&] (Source&, std::string attribution) { + EXPECT_EQ(mapboxOSM, attribution); + EXPECT_FALSE(mapboxOSM.find("©️ OpenStreetMap") == std::string::npos); + test.end(); + }; + + test.observer.tileError = [&] (Source&, const OverscaledTileID&, std::exception_ptr) { + FAIL() << "Should never be called"; + }; + + RasterSource source("source", "url", 512); + source.baseImpl->setObserver(&test.observer); + source.baseImpl->load(test.fileSource); + source.baseImpl->update(test.updateParameters); + + test.run(); +}