From f4baa0ba3eaef86b42b6307f37ea7315827f9c2a Mon Sep 17 00:00:00 2001 From: "Thiago Marcos P. Santos" Date: Tue, 21 Apr 2015 14:33:14 +0200 Subject: [PATCH] Notify data changes using Observer pattern This pattern, in contrast with callbacks, allows us to clearly see the dependency and hierarchy between the objects. It will also help on writing unit tests as we can count how many times a notification was triggered. --- src/mbgl/map/map_context.cpp | 5 +---- src/mbgl/map/resource_loader.cpp | 16 +++++++++++++-- src/mbgl/map/resource_loader.hpp | 7 ++++++- src/mbgl/map/source.cpp | 34 ++++++++++++++++++++++---------- src/mbgl/map/source.hpp | 26 +++++++++++++++++------- 5 files changed, 64 insertions(+), 24 deletions(-) diff --git a/src/mbgl/map/map_context.cpp b/src/mbgl/map/map_context.cpp index c14cedb03..146d7ec4e 100644 --- a/src/mbgl/map/map_context.cpp +++ b/src/mbgl/map/map_context.cpp @@ -168,10 +168,7 @@ void MapContext::updateTiles() { if (!style) return; for (const auto& source : style->sources) { source->update(data, transformState, *style, *glyphAtlas, *glyphStore, *spriteAtlas, - getSprite(), *texturePool, [this]() { - assert(Environment::currentlyOn(ThreadType::Map)); - triggerUpdate(); - }); + getSprite(), *texturePool); } } diff --git a/src/mbgl/map/resource_loader.cpp b/src/mbgl/map/resource_loader.cpp index 5186c665f..91f183d69 100644 --- a/src/mbgl/map/resource_loader.cpp +++ b/src/mbgl/map/resource_loader.cpp @@ -5,7 +5,6 @@ #include #include -#include namespace mbgl { @@ -15,6 +14,10 @@ ResourceLoader::ResourceLoader() { ResourceLoader::~ResourceLoader() { assert(Environment::currentlyOn(ThreadType::Map)); + + for (const auto& source : style_->sources) { + source->setObserver(nullptr); + } } void ResourceLoader::setObserver(Observer* observer) { @@ -29,7 +32,8 @@ void ResourceLoader::setStyle(Style* style) { Environment& env = Environment::Get(); for (const auto& source : style->sources) { - source->load(accessToken_, env, std::bind(&ResourceLoader::emitTileDataChanged, this)); + source->setObserver(this); + source->load(accessToken_, env); } } @@ -37,6 +41,14 @@ void ResourceLoader::setAccessToken(const std::string& accessToken) { accessToken_ = accessToken; } +void ResourceLoader::onSourceLoaded() { + emitTileDataChanged(); +} + +void ResourceLoader::onTileLoaded() { + emitTileDataChanged(); +} + void ResourceLoader::emitTileDataChanged() { assert(Environment::currentlyOn(ThreadType::Map)); diff --git a/src/mbgl/map/resource_loader.hpp b/src/mbgl/map/resource_loader.hpp index 6d2089e3a..f03ba791d 100644 --- a/src/mbgl/map/resource_loader.hpp +++ b/src/mbgl/map/resource_loader.hpp @@ -1,6 +1,7 @@ #ifndef MBGL_MAP_RESOURCE_LOADER #define MBGL_MAP_RESOURCE_LOADER +#include #include #include @@ -13,7 +14,7 @@ class Style; // by the Style. The Source object currently owns all the tiles, thus this // class will notify its observers of any change on these tiles which will // ultimately cause a new rendering to be triggered. -class ResourceLoader : private util::noncopyable { +class ResourceLoader : public Source::Observer, private util::noncopyable { public: class Observer { public: @@ -34,6 +35,10 @@ public: // Set the access token to be used for loading the tile data. void setAccessToken(const std::string& accessToken); + // Source::Observer implementation. + void onSourceLoaded() override; + void onTileLoaded() override; + private: void emitTileDataChanged(); diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp index 708ff6b27..c63cd026d 100644 --- a/src/mbgl/map/source.cpp +++ b/src/mbgl/map/source.cpp @@ -136,9 +136,7 @@ bool Source::isLoaded() const { // Note: This is a separate function that must be called exactly once after creation // The reason this isn't part of the constructor is that calling shared_from_this() in // the constructor fails. -void Source::load(const std::string& accessToken, - Environment& env, - std::function callback) { +void Source::load(const std::string& accessToken, Environment& env) { if (info.url.empty()) { loaded = true; return; @@ -147,7 +145,7 @@ void Source::load(const std::string& accessToken, util::ptr source = shared_from_this(); const std::string url = util::mapbox::normalizeSourceURL(info.url, accessToken); - env.request({ Resource::Kind::JSON, url }, [source, callback](const Response &res) { + env.request({ Resource::Kind::JSON, url }, [source](const Response &res) { if (res.status != Response::Successful) { Log::Warning(Event::General, "Failed to load source TileJSON: %s", res.message.c_str()); return; @@ -164,7 +162,7 @@ void Source::load(const std::string& accessToken, source->info.parseTileJSONProperties(d); source->loaded = true; - callback(); + source->emitSourceLoaded(); }); } @@ -226,8 +224,7 @@ TileData::State Source::addTile(MapData& data, SpriteAtlas& spriteAtlas, util::ptr sprite, TexturePool& texturePool, - const TileID& id, - std::function callback) { + const TileID& id) { const TileData::State state = hasTile(id); if (state != TileData::State::invalid) { @@ -256,6 +253,8 @@ TileData::State Source::addTile(MapData& data, new_tile.data = cache.get(normalized_id.to_uint64()); } + auto callback = std::bind(&Source::emitTileLoaded, this); + if (!new_tile.data) { // If we don't find working tile data, we're just going to load it. if (info.type == SourceType::Vector) { @@ -366,8 +365,7 @@ void Source::update(MapData& data, GlyphStore& glyphStore, SpriteAtlas& spriteAtlas, util::ptr sprite, - TexturePool& texturePool, - std::function callback) { + TexturePool& texturePool) { if (!loaded || data.getAnimationTime() <= updated) { return; } @@ -387,7 +385,7 @@ void Source::update(MapData& data, // Add existing child/parent tiles if the actual tile is not yet loaded for (const auto& id : required) { const TileData::State state = addTile(data, transformState, style, glyphAtlas, glyphStore, - spriteAtlas, sprite, texturePool, id, callback); + spriteAtlas, sprite, texturePool, id); if (state != TileData::State::parsed) { // The tile we require is not yet loaded. Try to find a parent or @@ -477,4 +475,20 @@ void Source::onLowMemory() { cache.clear(); } +void Source::setObserver(Observer* observer) { + observer_ = observer; +} + +void Source::emitSourceLoaded() { + if (observer_) { + observer_->onSourceLoaded(); + } +} + +void Source::emitTileLoaded() { + if (observer_) { + observer_->onTileLoaded(); + } +} + } diff --git a/src/mbgl/map/source.hpp b/src/mbgl/map/source.hpp index b15de0e64..255e97208 100644 --- a/src/mbgl/map/source.hpp +++ b/src/mbgl/map/source.hpp @@ -52,12 +52,18 @@ public: class Source : public std::enable_shared_from_this, private util::noncopyable { public: + class Observer { + public: + virtual ~Observer() = default; + + virtual void onSourceLoaded() = 0; + virtual void onTileLoaded() = 0; + }; + Source(); ~Source(); - void load(const std::string& accessToken, - Environment&, - std::function callback); + void load(const std::string& accessToken, Environment&); bool isLoaded() const; void load(MapData&, Environment&, std::function callback); @@ -68,8 +74,7 @@ public: GlyphStore&, SpriteAtlas&, util::ptr, - TexturePool&, - std::function callback); + TexturePool&); void invalidateTiles(const std::vector&); @@ -83,10 +88,15 @@ public: void setCacheSize(size_t); void onLowMemory(); + void setObserver(Observer* observer); + SourceInfo info; bool enabled; private: + void emitSourceLoaded(); + void emitTileLoaded(); + bool findLoadedChildren(const TileID& id, int32_t maxCoveringZoom, std::forward_list& retain); bool findLoadedParent(const TileID& id, int32_t minCoveringZoom, std::forward_list& retain); int32_t coveringZoomLevel(const TransformState&) const; @@ -100,8 +110,7 @@ private: SpriteAtlas&, util::ptr, TexturePool&, - const TileID&, - std::function callback); + const TileID&); TileData::State hasTile(const TileID& id); void updateTilePtrs(); @@ -117,6 +126,9 @@ private: std::vector tilePtrs; std::map> tile_data; TileCache cache; + + + Observer* observer_ = nullptr; }; }