From aaaf1e4c148282d3371836f9fe7838e8fb09bb83 Mon Sep 17 00:00:00 2001 From: "Thiago Marcos P. Santos" Date: Mon, 11 May 2015 18:41:44 +0300 Subject: [PATCH] Only emit tile updated signals when the tile really changes We only emit signals when the number of buckets increase (i.e. the new data that arrived added a new bucket to the tile). Also, if we try to reparse a partial tile but it is not possible, we keep the flag up for trying to reparse it again on the next update, because the data might have arrived after the bucket that is waiting for the data was processed but before the end of the parsing. Fixes #1458 --- src/mbgl/map/resource_loader.cpp | 13 +++++++++--- src/mbgl/map/source.cpp | 34 +++++++++++++++++++++++-------- src/mbgl/map/source.hpp | 9 ++++++-- src/mbgl/map/tile_data.cpp | 5 +++-- src/mbgl/map/tile_data.hpp | 8 +++++++- src/mbgl/map/vector_tile_data.cpp | 6 ++++++ src/mbgl/map/vector_tile_data.hpp | 1 + 7 files changed, 59 insertions(+), 17 deletions(-) diff --git a/src/mbgl/map/resource_loader.cpp b/src/mbgl/map/resource_loader.cpp index 7b5ebd19d..2be86dc5e 100644 --- a/src/mbgl/map/resource_loader.cpp +++ b/src/mbgl/map/resource_loader.cpp @@ -83,12 +83,19 @@ void ResourceLoader::update(MapData& data, spriteAtlas.setSprite(sprite_); } + bool allTilesUpdated = true; for (const auto& source : style_->sources) { - source->update(data, transform, *style_, glyphAtlas, *glyphStore_, - spriteAtlas, sprite_, texturePool, shouldReparsePartialTiles_); + if (!source->update(data, transform, *style_, glyphAtlas, *glyphStore_, + spriteAtlas, sprite_, texturePool, shouldReparsePartialTiles_)) { + allTilesUpdated = false; + } } - shouldReparsePartialTiles_ = false; + // We can only stop updating "partial" tiles when all of them + // were notified of the arrival of the new resources. + if (allTilesUpdated) { + shouldReparsePartialTiles_ = false; + } } void ResourceLoader::onGlyphRangeLoaded() { diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp index e40cb0f16..e8dc3468d 100644 --- a/src/mbgl/map/source.cpp +++ b/src/mbgl/map/source.cpp @@ -219,21 +219,30 @@ TileData::State Source::hasTile(const TileID& id) { return TileData::State::invalid; } -void Source::handlePartialTile(const TileID& id, Worker& worker) { +bool Source::handlePartialTile(const TileID& id, Worker& worker) { const TileID normalized_id = id.normalized(); auto it = tile_data.find(normalized_id); if (it == tile_data.end()) { - return; + return true; } util::ptr data = it->second.lock(); if (!data) { - return; + return true; } - auto callback = std::bind(&Source::emitTileLoaded, this, false); - data->reparse(worker, callback); + // The signal is only emitted if there was an actual change on the tile. The + // tile can be in a "partial" state waiting for resources and get reparsed on + // the arrival of new resources that were needed by another tile. + size_t bucketCount = static_cast(data.get())->countBuckets(); + auto callback = [this, data, bucketCount]() { + if (static_cast(data.get())->countBuckets() > bucketCount) { + emitTileLoaded(false); + } + }; + + return data->reparse(worker, callback); } TileData::State Source::addTile(MapData& data, @@ -377,7 +386,7 @@ bool Source::findLoadedParent(const TileID& id, int32_t minCoveringZoom, std::fo return false; } -void Source::update(MapData& data, +bool Source::update(MapData& data, const TransformState& transformState, Style& style, GlyphAtlas& glyphAtlas, @@ -386,8 +395,10 @@ void Source::update(MapData& data, util::ptr sprite, TexturePool& texturePool, bool shouldReparsePartialTiles) { + bool allTilesUpdated = true; + if (!loaded || data.getAnimationTime() <= updated) { - return; + return allTilesUpdated; } int32_t zoom = std::floor(getZoom(transformState)); @@ -408,8 +419,11 @@ void Source::update(MapData& data, switch (state) { case TileData::State::partial: - if (shouldReparsePartialTiles) - handlePartialTile(id, style.workers); + if (shouldReparsePartialTiles) { + if (!handlePartialTile(id, style.workers)) { + allTilesUpdated = false; + } + } break; case TileData::State::invalid: state = addTile(data, transformState, style, glyphAtlas, glyphStore, @@ -484,6 +498,8 @@ void Source::update(MapData& data, updateTilePtrs(); updated = data.getAnimationTime(); + + return allTilesUpdated; } void Source::invalidateTiles(const std::vector& ids) { diff --git a/src/mbgl/map/source.hpp b/src/mbgl/map/source.hpp index fcb1b4183..9d3de81b7 100644 --- a/src/mbgl/map/source.hpp +++ b/src/mbgl/map/source.hpp @@ -68,7 +68,12 @@ public: bool isLoaded() const; void load(MapData&, Environment&, std::function callback); - void update(MapData&, + + // Request or parse all the tiles relevant for the "TransformState". This method + // will return true if all the tiles were scheduled for updating of false if + // they were not. shouldReparsePartialTiles must be set to "true" if there is + // new data available that a tile in the "partial" state might be interested at. + bool update(MapData&, const TransformState&, Style&, GlyphAtlas&, @@ -99,7 +104,7 @@ private: void emitSourceLoaded(); void emitTileLoaded(bool isNewTile); - void handlePartialTile(const TileID &id, Worker &worker); + bool handlePartialTile(const TileID &id, Worker &worker); 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; diff --git a/src/mbgl/map/tile_data.cpp b/src/mbgl/map/tile_data.cpp index aa66ceb80..e9084bdff 100644 --- a/src/mbgl/map/tile_data.cpp +++ b/src/mbgl/map/tile_data.cpp @@ -73,10 +73,11 @@ void TileData::endParsing() { parsing.clear(std::memory_order_release); } -void TileData::reparse(Worker& worker, std::function callback) { +bool TileData::reparse(Worker& worker, std::function callback) { if (!mayStartParsing()) { - return; + return false; } workRequest = worker.send([this] { parse(); endParsing(); }, callback); + return true; } diff --git a/src/mbgl/map/tile_data.hpp b/src/mbgl/map/tile_data.hpp index 7f16e7c41..3945a3214 100644 --- a/src/mbgl/map/tile_data.hpp +++ b/src/mbgl/map/tile_data.hpp @@ -45,7 +45,13 @@ public: ~TileData(); void request(Worker&, float pixelRatio, std::function callback); - void reparse(Worker&, std::function callback); + + // Schedule a tile reparse on a worker thread and call the callback on + // completion. It will return true if the work was schedule or false it was + // not, which can occur if the tile is already being parsed by another + // worker (see "mayStartParsing()"). + bool reparse(Worker&, std::function callback); + void cancel(); const std::string toString() const; diff --git a/src/mbgl/map/vector_tile_data.cpp b/src/mbgl/map/vector_tile_data.cpp index 263ad52b9..801d9a7c5 100644 --- a/src/mbgl/map/vector_tile_data.cpp +++ b/src/mbgl/map/vector_tile_data.cpp @@ -82,6 +82,12 @@ Bucket* VectorTileData::getBucket(StyleLayer const& layer) { return it->second.get(); } +size_t VectorTileData::countBuckets() const { + std::lock_guard lock(bucketsMutex); + + return buckets.size(); +} + void VectorTileData::setBucket(StyleLayer const& layer, std::unique_ptr bucket) { assert(layer.bucket); diff --git a/src/mbgl/map/vector_tile_data.hpp b/src/mbgl/map/vector_tile_data.hpp index 5fe351ca0..6549c106e 100644 --- a/src/mbgl/map/vector_tile_data.hpp +++ b/src/mbgl/map/vector_tile_data.hpp @@ -44,6 +44,7 @@ public: void parse() override; virtual Bucket* getBucket(StyleLayer const &layer_desc) override; + size_t countBuckets() const; void setBucket(StyleLayer const &layer_desc, std::unique_ptr bucket); void setState(const State& state) override;