From 4eff674c1d4caaf0ce624fe5db086f0d79d0d31f Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Thu, 18 Jun 2015 18:23:03 -0700 Subject: [PATCH 01/20] Revert "Do not hold a reference to the Style at the [Live|Vector]TileData" This reverts commit 044454417b61bdd102a376c1125ad6ee3a5eacd4. --- src/mbgl/map/live_tile_data.cpp | 7 +++---- src/mbgl/map/live_tile_data.hpp | 3 +-- src/mbgl/map/source.cpp | 4 ++-- src/mbgl/map/tile_parser.cpp | 7 ++++--- src/mbgl/map/tile_parser.hpp | 7 +++---- src/mbgl/map/vector_tile_data.cpp | 16 ++++++++-------- src/mbgl/map/vector_tile_data.hpp | 7 +++---- 7 files changed, 24 insertions(+), 27 deletions(-) diff --git a/src/mbgl/map/live_tile_data.cpp b/src/mbgl/map/live_tile_data.cpp index ccae542ee2c..4935c013451 100644 --- a/src/mbgl/map/live_tile_data.cpp +++ b/src/mbgl/map/live_tile_data.cpp @@ -10,8 +10,7 @@ using namespace mbgl; LiveTileData::LiveTileData(const TileID& id_, AnnotationManager& annotationManager_, - const std::vector>& layers_, - Worker& workers_, + Style& style_, GlyphAtlas& glyphAtlas_, GlyphStore& glyphStore_, SpriteAtlas& spriteAtlas_, @@ -19,7 +18,7 @@ LiveTileData::LiveTileData(const TileID& id_, const SourceInfo& source_, float angle_, bool collisionDebug_) - : VectorTileData::VectorTileData(id_, layers_, workers_, glyphAtlas_, glyphStore_, + : VectorTileData::VectorTileData(id_, style_, glyphAtlas_, glyphStore_, spriteAtlas_, sprite_, source_, angle_, collisionDebug_), annotationManager(annotationManager_) { // live features are always ready @@ -44,7 +43,7 @@ void LiveTileData::parse() { // Parsing creates state that is encapsulated in TileParser. While parsing, // the TileParser object writes results into this objects. All other state // is going to be discarded afterwards. - TileParser parser(*tile, *this, layers, glyphAtlas, glyphStore, spriteAtlas, sprite); + TileParser parser(*tile, *this, style, glyphAtlas, glyphStore, spriteAtlas, sprite); parser.parse(); } catch (const std::exception& ex) { Log::Error(Event::ParseTile, "Live-parsing [%d/%d/%d] failed: %s", id.z, id.x, id.y, ex.what()); diff --git a/src/mbgl/map/live_tile_data.hpp b/src/mbgl/map/live_tile_data.hpp index 57d2e8c0d3f..16206712301 100644 --- a/src/mbgl/map/live_tile_data.hpp +++ b/src/mbgl/map/live_tile_data.hpp @@ -11,8 +11,7 @@ class LiveTileData : public VectorTileData { public: LiveTileData(const TileID&, AnnotationManager&, - const std::vector>&, - Worker&, + Style&, GlyphAtlas&, GlyphStore&, SpriteAtlas&, diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp index b073321a775..7dd91804b57 100644 --- a/src/mbgl/map/source.cpp +++ b/src/mbgl/map/source.cpp @@ -295,7 +295,7 @@ TileData::State Source::addTile(MapData& data, // If we don't find working tile data, we're just going to load it. if (info.type == SourceType::Vector) { new_tile.data = - std::make_shared(normalized_id, style.layers, style.workers, glyphAtlas, + std::make_shared(normalized_id, style, glyphAtlas, glyphStore, spriteAtlas, sprite, info, transformState.getAngle(), data.getCollisionDebug()); new_tile.data->request(style.workers, transformState.getPixelRatio(), callback); @@ -305,7 +305,7 @@ TileData::State Source::addTile(MapData& data, style.workers, transformState.getPixelRatio(), callback); } else if (info.type == SourceType::Annotations) { new_tile.data = std::make_shared(normalized_id, data.annotationManager, - style.layers, style.workers, glyphAtlas, + style, glyphAtlas, glyphStore, spriteAtlas, sprite, info, transformState.getAngle(), data.getCollisionDebug()); new_tile.data->reparse(style.workers, callback); diff --git a/src/mbgl/map/tile_parser.cpp b/src/mbgl/map/tile_parser.cpp index 1ddf07ae9e9..61af227034b 100644 --- a/src/mbgl/map/tile_parser.cpp +++ b/src/mbgl/map/tile_parser.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include @@ -20,14 +21,14 @@ TileParser::~TileParser() = default; TileParser::TileParser(const GeometryTile& geometryTile_, VectorTileData& tile_, - const std::vector>& layers_, + const Style& style_, GlyphAtlas& glyphAtlas_, GlyphStore& glyphStore_, SpriteAtlas& spriteAtlas_, const util::ptr& sprite_) : geometryTile(geometryTile_), tile(tile_), - layers(layers_), + style(style_), glyphAtlas(glyphAtlas_), glyphStore(glyphStore_), spriteAtlas(spriteAtlas_), @@ -41,7 +42,7 @@ bool TileParser::obsolete() const { } void TileParser::parse() { - for (const auto& layer_desc : layers) { + for (const auto& layer_desc : style.layers) { // Cancel early when parsing. if (obsolete()) { return; diff --git a/src/mbgl/map/tile_parser.hpp b/src/mbgl/map/tile_parser.hpp index 7eadc45d463..88e56878045 100644 --- a/src/mbgl/map/tile_parser.hpp +++ b/src/mbgl/map/tile_parser.hpp @@ -6,7 +6,6 @@ #include #include #include -#include #include #include #include @@ -23,6 +22,7 @@ class GlyphAtlas; class GlyphStore; class SpriteAtlas; class Sprite; +class Style; class StyleBucket; class StyleLayoutFill; class StyleLayoutRaster; @@ -34,7 +34,7 @@ class TileParser : private util::noncopyable { public: TileParser(const GeometryTile& geometryTile, VectorTileData& tile, - const std::vector>& layers, + const Style& style, GlyphAtlas& glyphAtlas, GlyphStore& glyphStore, SpriteAtlas& spriteAtlas, @@ -61,9 +61,8 @@ class TileParser : private util::noncopyable { const GeometryTile& geometryTile; VectorTileData& tile; - std::vector> layers; - // Cross-thread shared data. + const Style& style; GlyphAtlas& glyphAtlas; GlyphStore& glyphStore; SpriteAtlas& spriteAtlas; diff --git a/src/mbgl/map/vector_tile_data.cpp b/src/mbgl/map/vector_tile_data.cpp index 017d3ecb7c5..6dc6e8e9b35 100644 --- a/src/mbgl/map/vector_tile_data.cpp +++ b/src/mbgl/map/vector_tile_data.cpp @@ -9,12 +9,12 @@ #include #include #include +#include using namespace mbgl; VectorTileData::VectorTileData(const TileID& id_, - const std::vector>& layers_, - Worker& workers_, + Style& style_, GlyphAtlas& glyphAtlas_, GlyphStore& glyphStore_, SpriteAtlas& spriteAtlas_, @@ -23,12 +23,11 @@ VectorTileData::VectorTileData(const TileID& id_, float angle, bool collisionDebug) : TileData(id_, source_), - layers(layers_), - workers(workers_), glyphAtlas(glyphAtlas_), glyphStore(glyphStore_), spriteAtlas(spriteAtlas_), sprite(sprite_), + style(style_), collision(std::make_unique(id_.z, 4096, source_.tile_size * id.overscaling, angle, collisionDebug)), lastAngle(angle), currentAngle(angle) { @@ -52,7 +51,7 @@ void VectorTileData::parse() { // is going to be discarded afterwards. VectorTile vectorTile(pbf((const uint8_t *)data.data(), data.size())); const VectorTile* vt = &vectorTile; - TileParser parser(*vt, *this, layers, glyphAtlas, glyphStore, spriteAtlas, sprite); + TileParser parser(*vt, *this, style, glyphAtlas, glyphStore, spriteAtlas, sprite); parser.parse(); if (getState() == State::obsolete) { @@ -129,14 +128,15 @@ void VectorTileData::redoPlacement(float angle, bool collisionDebug) { currentCollisionDebug = collisionDebug; auto callback = std::bind(&VectorTileData::endRedoPlacement, this); - workRequest = workers.send([this, angle, collisionDebug] { workerRedoPlacement(angle, collisionDebug); }, callback); + workRequest = style.workers.send([this, angle, collisionDebug] { workerRedoPlacement(angle, collisionDebug); }, callback); + } } void VectorTileData::workerRedoPlacement(float angle, bool collisionDebug) { collision->reset(angle, 0); collision->setDebug(collisionDebug); - for (const auto& layer_desc : layers) { + for (const auto& layer_desc : style.layers) { auto bucket = getBucket(*layer_desc); if (bucket) { bucket->placeFeatures(); @@ -145,7 +145,7 @@ void VectorTileData::workerRedoPlacement(float angle, bool collisionDebug) { } void VectorTileData::endRedoPlacement() { - for (const auto& layer_desc : layers) { + for (const auto& layer_desc : style.layers) { auto bucket = getBucket(*layer_desc); if (bucket) { bucket->swapRenderData(); diff --git a/src/mbgl/map/vector_tile_data.hpp b/src/mbgl/map/vector_tile_data.hpp index 51a627d7b73..5a6f448c69c 100644 --- a/src/mbgl/map/vector_tile_data.hpp +++ b/src/mbgl/map/vector_tile_data.hpp @@ -26,14 +26,14 @@ class GlyphAtlas; class GlyphStore; class SpriteAtlas; class Sprite; +class Style; class VectorTileData : public TileData { friend class TileParser; public: VectorTileData(const TileID&, - const std::vector>&, - Worker&, + Style&, GlyphAtlas&, GlyphStore&, SpriteAtlas&, @@ -66,12 +66,11 @@ class VectorTileData : public TileData { TriangleElementsBuffer triangleElementsBuffer; LineElementsBuffer lineElementsBuffer; - std::vector> layers; - Worker& workers; GlyphAtlas& glyphAtlas; GlyphStore& glyphStore; SpriteAtlas& spriteAtlas; util::ptr sprite; + Style& style; private: // Contains all the Bucket objects for the tile. Buckets are render From ee463785832343b60b2525126052d3ce65fea5a7 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Thu, 4 Jun 2015 15:19:06 -0700 Subject: [PATCH 02/20] Reduce number of arguments in Source::update pathway --- src/mbgl/map/live_tile_data.cpp | 9 ++------- src/mbgl/map/live_tile_data.hpp | 4 ---- src/mbgl/map/source.cpp | 17 +++-------------- src/mbgl/map/source.hpp | 12 ------------ src/mbgl/map/tile_parser.cpp | 16 ++++------------ src/mbgl/map/tile_parser.hpp | 18 ++---------------- src/mbgl/map/vector_tile_data.cpp | 12 ++---------- src/mbgl/map/vector_tile_data.hpp | 12 ------------ src/mbgl/style/style.cpp | 3 +-- 9 files changed, 14 insertions(+), 89 deletions(-) diff --git a/src/mbgl/map/live_tile_data.cpp b/src/mbgl/map/live_tile_data.cpp index 4935c013451..4a90ea786cd 100644 --- a/src/mbgl/map/live_tile_data.cpp +++ b/src/mbgl/map/live_tile_data.cpp @@ -11,15 +11,10 @@ using namespace mbgl; LiveTileData::LiveTileData(const TileID& id_, AnnotationManager& annotationManager_, Style& style_, - GlyphAtlas& glyphAtlas_, - GlyphStore& glyphStore_, - SpriteAtlas& spriteAtlas_, - util::ptr sprite_, const SourceInfo& source_, float angle_, bool collisionDebug_) - : VectorTileData::VectorTileData(id_, style_, glyphAtlas_, glyphStore_, - spriteAtlas_, sprite_, source_, angle_, collisionDebug_), + : VectorTileData::VectorTileData(id_, style_, source_, angle_, collisionDebug_), annotationManager(annotationManager_) { // live features are always ready setState(State::loaded); @@ -43,7 +38,7 @@ void LiveTileData::parse() { // Parsing creates state that is encapsulated in TileParser. While parsing, // the TileParser object writes results into this objects. All other state // is going to be discarded afterwards. - TileParser parser(*tile, *this, style, glyphAtlas, glyphStore, spriteAtlas, sprite); + TileParser parser(*tile, *this, style); parser.parse(); } catch (const std::exception& ex) { Log::Error(Event::ParseTile, "Live-parsing [%d/%d/%d] failed: %s", id.z, id.x, id.y, ex.what()); diff --git a/src/mbgl/map/live_tile_data.hpp b/src/mbgl/map/live_tile_data.hpp index 16206712301..f4f54120892 100644 --- a/src/mbgl/map/live_tile_data.hpp +++ b/src/mbgl/map/live_tile_data.hpp @@ -12,10 +12,6 @@ class LiveTileData : public VectorTileData { LiveTileData(const TileID&, AnnotationManager&, Style&, - GlyphAtlas&, - GlyphStore&, - SpriteAtlas&, - util::ptr, const SourceInfo&, float angle_, bool collisionDebug_); diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp index 7dd91804b57..0bac4c3c7fe 100644 --- a/src/mbgl/map/source.cpp +++ b/src/mbgl/map/source.cpp @@ -254,10 +254,6 @@ bool Source::handlePartialTile(const TileID& id, Worker& worker) { TileData::State Source::addTile(MapData& data, const TransformState& transformState, Style& style, - GlyphAtlas& glyphAtlas, - GlyphStore& glyphStore, - SpriteAtlas& spriteAtlas, - util::ptr sprite, TexturePool& texturePool, const TileID& id) { const TileData::State state = hasTile(id); @@ -295,8 +291,7 @@ TileData::State Source::addTile(MapData& data, // If we don't find working tile data, we're just going to load it. if (info.type == SourceType::Vector) { new_tile.data = - std::make_shared(normalized_id, style, glyphAtlas, - glyphStore, spriteAtlas, sprite, info, + std::make_shared(normalized_id, style, info, transformState.getAngle(), data.getCollisionDebug()); new_tile.data->request(style.workers, transformState.getPixelRatio(), callback); } else if (info.type == SourceType::Raster) { @@ -305,8 +300,7 @@ TileData::State Source::addTile(MapData& data, style.workers, transformState.getPixelRatio(), callback); } else if (info.type == SourceType::Annotations) { new_tile.data = std::make_shared(normalized_id, data.annotationManager, - style, glyphAtlas, - glyphStore, spriteAtlas, sprite, info, + style, info, transformState.getAngle(), data.getCollisionDebug()); new_tile.data->reparse(style.workers, callback); } else { @@ -405,10 +399,6 @@ bool Source::findLoadedParent(const TileID& id, int32_t minCoveringZoom, std::fo bool Source::update(MapData& data, const TransformState& transformState, Style& style, - GlyphAtlas& glyphAtlas, - GlyphStore& glyphStore, - SpriteAtlas& spriteAtlas, - util::ptr sprite, TexturePool& texturePool, bool shouldReparsePartialTiles) { bool allTilesUpdated = true; @@ -442,8 +432,7 @@ bool Source::update(MapData& data, } break; case TileData::State::invalid: - state = addTile(data, transformState, style, glyphAtlas, glyphStore, - spriteAtlas, sprite, texturePool, id); + state = addTile(data, transformState, style, texturePool, id); break; default: break; diff --git a/src/mbgl/map/source.hpp b/src/mbgl/map/source.hpp index 6b0b23f2665..336caa5b93d 100644 --- a/src/mbgl/map/source.hpp +++ b/src/mbgl/map/source.hpp @@ -22,10 +22,6 @@ namespace mbgl { class MapData; -class GlyphAtlas; -class GlyphStore; -class SpriteAtlas; -class Sprite; class TexturePool; class Style; class Painter; @@ -78,10 +74,6 @@ class Source : private util::noncopyable { bool update(MapData&, const TransformState&, Style&, - GlyphAtlas&, - GlyphStore&, - SpriteAtlas&, - util::ptr, TexturePool&, bool shouldReparsePartialTiles); @@ -119,10 +111,6 @@ class Source : private util::noncopyable { TileData::State addTile(MapData&, const TransformState&, Style&, - GlyphAtlas&, - GlyphStore&, - SpriteAtlas&, - util::ptr, TexturePool&, const TileID&); diff --git a/src/mbgl/map/tile_parser.cpp b/src/mbgl/map/tile_parser.cpp index 61af227034b..56b34acdd2a 100644 --- a/src/mbgl/map/tile_parser.cpp +++ b/src/mbgl/map/tile_parser.cpp @@ -21,20 +21,12 @@ TileParser::~TileParser() = default; TileParser::TileParser(const GeometryTile& geometryTile_, VectorTileData& tile_, - const Style& style_, - GlyphAtlas& glyphAtlas_, - GlyphStore& glyphStore_, - SpriteAtlas& spriteAtlas_, - const util::ptr& sprite_) + Style& style_) : geometryTile(geometryTile_), tile(tile_), style(style_), - glyphAtlas(glyphAtlas_), - glyphStore(glyphStore_), - spriteAtlas(spriteAtlas_), - sprite(sprite_), partialParse(false) { - assert(sprite); + assert(style.sprite); } bool TileParser::obsolete() const { @@ -222,7 +214,7 @@ std::unique_ptr TileParser::createSymbolBucket(const GeometryTileLayer& applyLayoutProperty(PropertyKey::TextOffset, bucket_desc.layout, layout.text.offset, z); applyLayoutProperty(PropertyKey::TextAllowOverlap, bucket_desc.layout, layout.text.allow_overlap, z); - if (bucket->needsDependencies(layer, bucket_desc.filter, glyphStore, *sprite)) { + if (bucket->needsDependencies(layer, bucket_desc.filter, *style.glyphStore, *style.sprite)) { partialParse = true; } @@ -235,7 +227,7 @@ std::unique_ptr TileParser::createSymbolBucket(const GeometryTileLayer& } bucket->addFeatures( - reinterpret_cast(&tile), spriteAtlas, *sprite, glyphAtlas, glyphStore); + reinterpret_cast(&tile), *style.spriteAtlas, *style.sprite, *style.glyphAtlas, *style.glyphStore); return bucket->hasData() ? std::move(bucket) : nullptr; } diff --git a/src/mbgl/map/tile_parser.hpp b/src/mbgl/map/tile_parser.hpp index 88e56878045..b716721a5d1 100644 --- a/src/mbgl/map/tile_parser.hpp +++ b/src/mbgl/map/tile_parser.hpp @@ -18,10 +18,6 @@ namespace mbgl { class Bucket; class FontStack; -class GlyphAtlas; -class GlyphStore; -class SpriteAtlas; -class Sprite; class Style; class StyleBucket; class StyleLayoutFill; @@ -32,13 +28,7 @@ class VectorTileData; class TileParser : private util::noncopyable { public: - TileParser(const GeometryTile& geometryTile, - VectorTileData& tile, - const Style& style, - GlyphAtlas& glyphAtlas, - GlyphStore& glyphStore, - SpriteAtlas& spriteAtlas, - const util::ptr& sprite); + TileParser(const GeometryTile&, VectorTileData&, Style&); ~TileParser(); public: @@ -62,11 +52,7 @@ class TileParser : private util::noncopyable { VectorTileData& tile; // Cross-thread shared data. - const Style& style; - GlyphAtlas& glyphAtlas; - GlyphStore& glyphStore; - SpriteAtlas& spriteAtlas; - util::ptr sprite; + Style& style; bool partialParse; }; diff --git a/src/mbgl/map/vector_tile_data.cpp b/src/mbgl/map/vector_tile_data.cpp index 6dc6e8e9b35..840e21b8ab3 100644 --- a/src/mbgl/map/vector_tile_data.cpp +++ b/src/mbgl/map/vector_tile_data.cpp @@ -15,18 +15,10 @@ using namespace mbgl; VectorTileData::VectorTileData(const TileID& id_, Style& style_, - GlyphAtlas& glyphAtlas_, - GlyphStore& glyphStore_, - SpriteAtlas& spriteAtlas_, - util::ptr sprite_, const SourceInfo& source_, float angle, bool collisionDebug) : TileData(id_, source_), - glyphAtlas(glyphAtlas_), - glyphStore(glyphStore_), - spriteAtlas(spriteAtlas_), - sprite(sprite_), style(style_), collision(std::make_unique(id_.z, 4096, source_.tile_size * id.overscaling, angle, collisionDebug)), lastAngle(angle), @@ -37,7 +29,7 @@ VectorTileData::~VectorTileData() { // Cancel in most derived class destructor so that worker tasks are joined before // any member data goes away. cancel(); - glyphAtlas.removeGlyphs(reinterpret_cast(this)); + style.glyphAtlas->removeGlyphs(reinterpret_cast(this)); } void VectorTileData::parse() { @@ -51,7 +43,7 @@ void VectorTileData::parse() { // is going to be discarded afterwards. VectorTile vectorTile(pbf((const uint8_t *)data.data(), data.size())); const VectorTile* vt = &vectorTile; - TileParser parser(*vt, *this, style, glyphAtlas, glyphStore, spriteAtlas, sprite); + TileParser parser(*vt, *this, style); parser.parse(); if (getState() == State::obsolete) { diff --git a/src/mbgl/map/vector_tile_data.hpp b/src/mbgl/map/vector_tile_data.hpp index 5a6f448c69c..697a054e094 100644 --- a/src/mbgl/map/vector_tile_data.hpp +++ b/src/mbgl/map/vector_tile_data.hpp @@ -22,10 +22,6 @@ class Painter; class SourceInfo; class StyleLayer; class TileParser; -class GlyphAtlas; -class GlyphStore; -class SpriteAtlas; -class Sprite; class Style; class VectorTileData : public TileData { @@ -34,10 +30,6 @@ class VectorTileData : public TileData { public: VectorTileData(const TileID&, Style&, - GlyphAtlas&, - GlyphStore&, - SpriteAtlas&, - util::ptr, const SourceInfo&, float angle_, bool collisionDebug_); @@ -66,10 +58,6 @@ class VectorTileData : public TileData { TriangleElementsBuffer triangleElementsBuffer; LineElementsBuffer lineElementsBuffer; - GlyphAtlas& glyphAtlas; - GlyphStore& glyphStore; - SpriteAtlas& spriteAtlas; - util::ptr sprite; Style& style; private: diff --git a/src/mbgl/style/style.cpp b/src/mbgl/style/style.cpp index 15dffd113b0..35a94adbc68 100644 --- a/src/mbgl/style/style.cpp +++ b/src/mbgl/style/style.cpp @@ -78,8 +78,7 @@ void Style::update(MapData& data, bool allTilesUpdated = true; for (const auto& source : sources) { - if (!source->update(data, transform, *this, *glyphAtlas, *glyphStore, - *spriteAtlas, sprite, texturePool, shouldReparsePartialTiles)) { + if (!source->update(data, transform, *this, texturePool, shouldReparsePartialTiles)) { allTilesUpdated = false; } } From 7e71145a89f0377736312c3b818a41ffb25bdf9a Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Thu, 4 Jun 2015 15:54:42 -0700 Subject: [PATCH 03/20] Make request and reparse pure virtual --- src/mbgl/map/raster_tile_data.cpp | 44 ++++++++++++++++++++++++++++++- src/mbgl/map/raster_tile_data.hpp | 10 ++++--- src/mbgl/map/tile_data.cpp | 42 ----------------------------- src/mbgl/map/tile_data.hpp | 10 ++++--- src/mbgl/map/vector_tile_data.cpp | 38 ++++++++++++++++++++++++++ src/mbgl/map/vector_tile_data.hpp | 8 +++++- 6 files changed, 101 insertions(+), 51 deletions(-) diff --git a/src/mbgl/map/raster_tile_data.cpp b/src/mbgl/map/raster_tile_data.cpp index 0f8efe3b483..ee53a6a2882 100644 --- a/src/mbgl/map/raster_tile_data.cpp +++ b/src/mbgl/map/raster_tile_data.cpp @@ -1,5 +1,12 @@ #include -#include +#include +#include +#include +#include +#include +#include + +#include using namespace mbgl; @@ -14,6 +21,41 @@ RasterTileData::~RasterTileData() { cancel(); } +void RasterTileData::request(Worker& worker, + float pixelRatio, + const std::function& callback) { + std::string url = source.tileURL(id, pixelRatio); + state = State::loading; + + FileSource* fs = util::ThreadContext::getFileSource(); + req = fs->request({ Resource::Kind::Tile, url }, util::RunLoop::current.get()->get(), [url, callback, &worker, this](const Response &res) { + req = nullptr; + + if (res.status != Response::Successful) { + std::stringstream message; + message << "Failed to load [" << url << "]: " << res.message; + setError(message.str()); + callback(); + return; + } + + state = State::loaded; + data = res.data; + + // Schedule tile parsing in another thread + reparse(worker, callback); + }); +} + +bool RasterTileData::reparse(Worker& worker, std::function callback) { + if (!mayStartParsing()) { + return false; + } + + workRequest = worker.send([this] { parse(); endParsing(); }, callback); + return true; +} + void RasterTileData::parse() { if (getState() != State::loaded) { return; diff --git a/src/mbgl/map/raster_tile_data.hpp b/src/mbgl/map/raster_tile_data.hpp index 45aee8f74c0..fb59482eff4 100644 --- a/src/mbgl/map/raster_tile_data.hpp +++ b/src/mbgl/map/raster_tile_data.hpp @@ -13,13 +13,17 @@ class StyleLayer; class TexturePool; class RasterTileData : public TileData { - friend class TileParser; - public: RasterTileData(const TileID&, TexturePool&, const SourceInfo&); ~RasterTileData(); - void parse() override; + void request(Worker&, + float pixelRatio, + const std::function& callback) override; + + bool reparse(Worker&, std::function callback) override; + + void parse(); Bucket* getBucket(StyleLayer const &layer_desc) override; protected: diff --git a/src/mbgl/map/tile_data.cpp b/src/mbgl/map/tile_data.cpp index f1c661300cc..4a30639cb29 100644 --- a/src/mbgl/map/tile_data.cpp +++ b/src/mbgl/map/tile_data.cpp @@ -1,13 +1,6 @@ #include - -#include -#include -#include #include #include -#include - -#include using namespace mbgl; @@ -35,32 +28,6 @@ void TileData::setState(const State& state_) { state = state_; } -void TileData::request(Worker& worker, - float pixelRatio, - const std::function& callback) { - std::string url = source.tileURL(id, pixelRatio); - state = State::loading; - - FileSource* fs = util::ThreadContext::getFileSource(); - req = fs->request({ Resource::Kind::Tile, url }, util::RunLoop::current.get()->get(), [url, callback, &worker, this](const Response &res) { - req = nullptr; - - if (res.status != Response::Successful) { - std::stringstream message; - message << "Failed to load [" << url << "]: " << res.message; - setError(message.str()); - callback(); - return; - } - - state = State::loaded; - data = res.data; - - // Schedule tile parsing in another thread - reparse(worker, callback); - }); -} - void TileData::cancel() { if (state != State::obsolete) { state = State::obsolete; @@ -80,15 +47,6 @@ void TileData::endParsing() { parsing.clear(std::memory_order_release); } -bool TileData::reparse(Worker& worker, std::function callback) { - if (!mayStartParsing()) { - return false; - } - - workRequest = worker.send([this] { parse(); endParsing(); }, callback); - return true; -} - void TileData::setError(const std::string& message) { error = message; setState(State::obsolete); diff --git a/src/mbgl/map/tile_data.hpp b/src/mbgl/map/tile_data.hpp index aa302a4fa2e..a3aaf1dfdba 100644 --- a/src/mbgl/map/tile_data.hpp +++ b/src/mbgl/map/tile_data.hpp @@ -77,13 +77,16 @@ class TileData : private util::noncopyable { TileData(const TileID&, const SourceInfo&); ~TileData(); - void request(Worker&, float pixelRatio, const std::function& callback); + virtual void request(Worker&, + float pixelRatio, + const std::function& callback) = 0; // 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); + virtual bool reparse(Worker&, + std::function callback) = 0; void cancel(); const std::string toString() const; @@ -116,7 +119,6 @@ class TileData : private util::noncopyable { } // Override this in the child class. - virtual void parse() = 0; virtual Bucket* getBucket(StyleLayer const &layer_desc) = 0; virtual void redoPlacement(float, bool) {} @@ -141,9 +143,9 @@ class TileData : private util::noncopyable { std::unique_ptr workRequest; -private: std::atomic state; +private: std::string error; protected: diff --git a/src/mbgl/map/vector_tile_data.cpp b/src/mbgl/map/vector_tile_data.cpp index 840e21b8ab3..34cfe4311f1 100644 --- a/src/mbgl/map/vector_tile_data.cpp +++ b/src/mbgl/map/vector_tile_data.cpp @@ -7,6 +7,9 @@ #include #include #include +#include +#include +#include #include #include #include @@ -32,6 +35,41 @@ VectorTileData::~VectorTileData() { style.glyphAtlas->removeGlyphs(reinterpret_cast(this)); } +void VectorTileData::request(Worker& worker, + float pixelRatio, + const std::function& callback) { + std::string url = source.tileURL(id, pixelRatio); + state = State::loading; + + FileSource* fs = util::ThreadContext::getFileSource(); + req = fs->request({ Resource::Kind::Tile, url }, util::RunLoop::current.get()->get(), [url, callback, &worker, this](const Response &res) { + req = nullptr; + + if (res.status != Response::Successful) { + std::stringstream message; + message << "Failed to load [" << url << "]: " << res.message; + setError(message.str()); + callback(); + return; + } + + state = State::loaded; + data = res.data; + + // Schedule tile parsing in another thread + reparse(worker, callback); + }); +} + +bool VectorTileData::reparse(Worker& worker, std::function callback) { + if (!mayStartParsing()) { + return false; + } + + workRequest = worker.send([this] { parse(); endParsing(); }, callback); + return true; +} + void VectorTileData::parse() { if (getState() != State::loaded && getState() != State::partial) { return; diff --git a/src/mbgl/map/vector_tile_data.hpp b/src/mbgl/map/vector_tile_data.hpp index 697a054e094..faddb1dc7dd 100644 --- a/src/mbgl/map/vector_tile_data.hpp +++ b/src/mbgl/map/vector_tile_data.hpp @@ -35,7 +35,7 @@ class VectorTileData : public TileData { bool collisionDebug_); ~VectorTileData(); - void parse() override; + virtual void parse(); void redoPlacement(float angle, bool collisionDebug) override; virtual Bucket* getBucket(StyleLayer const &layer_desc) override; @@ -48,6 +48,12 @@ class VectorTileData : public TileData { return collision.get(); } + void request(Worker&, + float pixelRatio, + const std::function& callback) override; + + bool reparse(Worker&, std::function callback) override; + protected: void redoPlacement(); From 29a2cd908f6ed3d62ecfd113457074d1524d4f49 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Thu, 4 Jun 2015 16:36:37 -0700 Subject: [PATCH 04/20] Introduce TileWorker --- src/mbgl/map/live_tile_data.cpp | 55 ++++++++------ src/mbgl/map/live_tile_data.hpp | 2 +- src/mbgl/map/tile_parser.cpp | 4 +- src/mbgl/map/tile_parser.hpp | 6 +- src/mbgl/map/tile_worker.cpp | 78 ++++++++++++++++++++ src/mbgl/map/tile_worker.hpp | 75 +++++++++++++++++++ src/mbgl/map/vector_tile_data.cpp | 116 +++++++++++------------------- src/mbgl/map/vector_tile_data.hpp | 50 ++----------- 8 files changed, 239 insertions(+), 147 deletions(-) create mode 100644 src/mbgl/map/tile_worker.cpp create mode 100644 src/mbgl/map/tile_worker.hpp diff --git a/src/mbgl/map/live_tile_data.cpp b/src/mbgl/map/live_tile_data.cpp index 4a90ea786cd..74d939f35ac 100644 --- a/src/mbgl/map/live_tile_data.cpp +++ b/src/mbgl/map/live_tile_data.cpp @@ -1,10 +1,10 @@ #include #include #include -#include -#include -#include -#include +#include +#include + +#include using namespace mbgl; @@ -26,28 +26,43 @@ LiveTileData::~LiveTileData() { cancel(); } -void LiveTileData::parse() { - if (getState() != State::loaded) { - return; +bool LiveTileData::reparse(Worker&, std::function callback) { + if (!mayStartParsing()) { + return false; } - const LiveTile* tile = annotationManager.getTile(id); + workRequest = worker.send([this] { + if (getState() != State::loaded) { + return; + } + + const LiveTile* tile = annotationManager.getTile(id); + + if (!tile) { + setState(State::parsed); + return; + } + + TileParseResult result; - if (tile) { try { - // Parsing creates state that is encapsulated in TileParser. While parsing, - // the TileParser object writes results into this objects. All other state - // is going to be discarded afterwards. - TileParser parser(*tile, *this, style); - parser.parse(); + result = workerData.parse(*tile); } catch (const std::exception& ex) { - Log::Error(Event::ParseTile, "Live-parsing [%d/%d/%d] failed: %s", id.z, id.x, id.y, ex.what()); - setState(State::obsolete); + std::stringstream message; + message << "Failed to parse [" << int(id.sourceZ) << "/" << id.x << "/" << id.y << "]: " << ex.what(); + result = message.str(); + } + + if (getState() == TileData::State::obsolete) { return; + } else if (result.is()) { + setState(result.get()); + } else { + setError(result.get()); } - } - if (getState() != State::obsolete) { - setState(State::parsed); - } + endParsing(); + }, callback); + + return true; } diff --git a/src/mbgl/map/live_tile_data.hpp b/src/mbgl/map/live_tile_data.hpp index f4f54120892..c837057e82d 100644 --- a/src/mbgl/map/live_tile_data.hpp +++ b/src/mbgl/map/live_tile_data.hpp @@ -17,7 +17,7 @@ class LiveTileData : public VectorTileData { bool collisionDebug_); ~LiveTileData(); - void parse() override; + bool reparse(Worker&, std::function callback) override; private: AnnotationManager& annotationManager; diff --git a/src/mbgl/map/tile_parser.cpp b/src/mbgl/map/tile_parser.cpp index 56b34acdd2a..a88084391f3 100644 --- a/src/mbgl/map/tile_parser.cpp +++ b/src/mbgl/map/tile_parser.cpp @@ -20,7 +20,7 @@ namespace mbgl { TileParser::~TileParser() = default; TileParser::TileParser(const GeometryTile& geometryTile_, - VectorTileData& tile_, + TileWorker& tile_, Style& style_) : geometryTile(geometryTile_), tile(tile_), @@ -98,7 +98,7 @@ void applyLayoutProperty(PropertyKey key, const ClassProperties &classProperties std::unique_ptr TileParser::createBucket(const StyleBucket &bucketDesc) { // Skip this bucket if we are to not render this - if (tile.id.z < std::floor(bucketDesc.min_zoom) && std::floor(bucketDesc.min_zoom) < tile.source.max_zoom) return nullptr; + if (tile.id.z < std::floor(bucketDesc.min_zoom) && std::floor(bucketDesc.min_zoom) < tile.maxZoom) return nullptr; if (tile.id.z >= std::ceil(bucketDesc.max_zoom)) return nullptr; if (bucketDesc.visibility == mbgl::VisibilityType::None) return nullptr; diff --git a/src/mbgl/map/tile_parser.hpp b/src/mbgl/map/tile_parser.hpp index b716721a5d1..63c1fd72aac 100644 --- a/src/mbgl/map/tile_parser.hpp +++ b/src/mbgl/map/tile_parser.hpp @@ -24,11 +24,11 @@ class StyleLayoutFill; class StyleLayoutRaster; class StyleLayoutLine; class StyleLayoutSymbol; -class VectorTileData; +class TileWorker; class TileParser : private util::noncopyable { public: - TileParser(const GeometryTile&, VectorTileData&, Style&); + TileParser(const GeometryTile&, TileWorker&, Style&); ~TileParser(); public: @@ -49,7 +49,7 @@ class TileParser : private util::noncopyable { void addBucketGeometries(Bucket&, const GeometryTileLayer&, const FilterExpression&); const GeometryTile& geometryTile; - VectorTileData& tile; + TileWorker& tile; // Cross-thread shared data. Style& style; diff --git a/src/mbgl/map/tile_worker.cpp b/src/mbgl/map/tile_worker.cpp new file mode 100644 index 00000000000..1991ab1885e --- /dev/null +++ b/src/mbgl/map/tile_worker.cpp @@ -0,0 +1,78 @@ +#include +#include +#include +#include +#include +#include + +using namespace mbgl; + +TileWorker::TileWorker(const TileID& id_, + Style& style_, + const uint16_t maxZoom_, + const std::atomic& state_, + std::unique_ptr collision_) + : id(id_), + style(style_), + maxZoom(maxZoom_), + state(state_), + collision(std::move(collision_)) { +} + +TileWorker::~TileWorker() { + style.glyphAtlas->removeGlyphs(reinterpret_cast(this)); +} + +void TileWorker::setBucket(const StyleLayer& layer, std::unique_ptr bucket) { + assert(layer.bucket); + + std::lock_guard lock(bucketsMutex); + + if (buckets.find(layer.bucket->name) != buckets.end()) { + return; + } + + buckets[layer.bucket->name] = std::move(bucket); +} + +Bucket* TileWorker::getBucket(const StyleLayer& layer) const { + std::lock_guard lock(bucketsMutex); + + const auto it = buckets.find(layer.bucket->name); + if (it == buckets.end()) { + return nullptr; + } + + assert(it->second); + return it->second.get(); +} + +size_t TileWorker::countBuckets() const { + std::lock_guard lock(bucketsMutex); + return buckets.size(); +} + +TileParseResult TileWorker::parse(const GeometryTile& geometryTile) { + // Parsing creates state that is encapsulated in TileParser. While parsing, + // the TileParser object writes results into this objects. All other state + // is going to be discarded afterwards. + TileParser parser(geometryTile, *this, style); + parser.parse(); + + if (parser.isPartialParse()) { + return TileData::State::partial; + } else { + return TileData::State::parsed; + } +} + +void TileWorker::redoPlacement(float angle, bool collisionDebug) { + collision->reset(angle, 0); + collision->setDebug(collisionDebug); + for (const auto& layer_desc : style.layers) { + auto bucket = getBucket(*layer_desc); + if (bucket) { + bucket->placeFeatures(); + } + } +} diff --git a/src/mbgl/map/tile_worker.hpp b/src/mbgl/map/tile_worker.hpp new file mode 100644 index 00000000000..420b0bf9f22 --- /dev/null +++ b/src/mbgl/map/tile_worker.hpp @@ -0,0 +1,75 @@ +#ifndef MBGL_MAP_TILE_WORKER +#define MBGL_MAP_TILE_WORKER + +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +namespace mbgl { + +class CollisionTile; +class GeometryTile; +class Style; +class Bucket; + +using TileParseResult = mapbox::util::variant< + TileData::State, // success + std::string>; // error + +class TileWorker : public util::noncopyable { +public: + TileWorker(const TileID&, + Style&, + const uint16_t maxZoom, + const std::atomic&, + std::unique_ptr); + ~TileWorker(); + + void setBucket(const StyleLayer&, std::unique_ptr); + Bucket* getBucket(const StyleLayer&) const; + size_t countBuckets() const; + + inline TileData::State getState() const { + return state; + } + + inline CollisionTile* getCollision() const { + return collision.get(); + } + + TileParseResult parse(const GeometryTile&); + void redoPlacement(float angle, bool collisionDebug); + + const TileID id; + Style& style; + const uint16_t maxZoom; + const std::atomic& state; + + FillVertexBuffer fillVertexBuffer; + LineVertexBuffer lineVertexBuffer; + + TriangleElementsBuffer triangleElementsBuffer; + LineElementsBuffer lineElementsBuffer; + + std::unique_ptr collision; + + // Contains all the Bucket objects for the tile. Buckets are render + // objects and they get added to this std::map<> by the workers doing + // the actual tile parsing as they get processed. Tiles partially + // parsed can get new buckets at any moment but are also fit for + // rendering. That said, access to this list needs locking unless + // the tile is completely parsed. + std::unordered_map> buckets; + mutable std::mutex bucketsMutex; +}; + +} + +#endif diff --git a/src/mbgl/map/vector_tile_data.cpp b/src/mbgl/map/vector_tile_data.cpp index 34cfe4311f1..f485a249585 100644 --- a/src/mbgl/map/vector_tile_data.cpp +++ b/src/mbgl/map/vector_tile_data.cpp @@ -1,10 +1,8 @@ #include -#include #include #include #include -#include -#include +#include #include #include #include @@ -22,8 +20,14 @@ VectorTileData::VectorTileData(const TileID& id_, float angle, bool collisionDebug) : TileData(id_, source_), - style(style_), - collision(std::make_unique(id_.z, 4096, source_.tile_size * id.overscaling, angle, collisionDebug)), + worker(style_.workers), + workerData(id_, + style_, + source.max_zoom, + state, + std::make_unique(id_.z, 4096, + source_.tile_size * id.overscaling, + angle, collisionDebug)), lastAngle(angle), currentAngle(angle) { } @@ -32,17 +36,16 @@ VectorTileData::~VectorTileData() { // Cancel in most derived class destructor so that worker tasks are joined before // any member data goes away. cancel(); - style.glyphAtlas->removeGlyphs(reinterpret_cast(this)); } -void VectorTileData::request(Worker& worker, +void VectorTileData::request(Worker&, float pixelRatio, const std::function& callback) { std::string url = source.tileURL(id, pixelRatio); state = State::loading; FileSource* fs = util::ThreadContext::getFileSource(); - req = fs->request({ Resource::Kind::Tile, url }, util::RunLoop::current.get()->get(), [url, callback, &worker, this](const Response &res) { + req = fs->request({ Resource::Kind::Tile, url }, util::RunLoop::current.get()->get(), [url, callback, this](const Response &res) { req = nullptr; if (res.status != Response::Successful) { @@ -56,89 +59,62 @@ void VectorTileData::request(Worker& worker, state = State::loaded; data = res.data; - // Schedule tile parsing in another thread reparse(worker, callback); }); } -bool VectorTileData::reparse(Worker& worker, std::function callback) { +bool VectorTileData::reparse(Worker&, std::function callback) { if (!mayStartParsing()) { return false; } - workRequest = worker.send([this] { parse(); endParsing(); }, callback); - return true; -} - -void VectorTileData::parse() { - if (getState() != State::loaded && getState() != State::partial) { - return; - } + workRequest = worker.send([this] { + if (getState() != TileData::State::loaded && getState() != TileData::State::partial) { + return; + } - try { - // Parsing creates state that is encapsulated in TileParser. While parsing, - // the TileParser object writes results into this objects. All other state - // is going to be discarded afterwards. - VectorTile vectorTile(pbf((const uint8_t *)data.data(), data.size())); - const VectorTile* vt = &vectorTile; - TileParser parser(*vt, *this, style); - parser.parse(); + TileParseResult result; - if (getState() == State::obsolete) { - return; + try { + VectorTile vectorTile(pbf((const uint8_t *)data.data(), data.size())); + result = workerData.parse(vectorTile); + } catch (const std::exception& ex) { + std::stringstream message; + message << "Failed to parse [" << int(id.sourceZ) << "/" << id.x << "/" << id.y << "]: " << ex.what(); + result = message.str(); } - if (parser.isPartialParse()) { - setState(State::partial); + if (getState() == TileData::State::obsolete) { + return; + } else if (result.is()) { + setState(result.get()); } else { - setState(State::parsed); + setError(result.get()); } - } catch (const std::exception& ex) { - std::stringstream message; - message << "Failed to parse [" << int(id.sourceZ) << "/" << id.x << "/" << id.y << "]: " << ex.what(); - setError(message.str()); - } -} -Bucket* VectorTileData::getBucket(StyleLayer const& layer) { - if (!isReady() || !layer.bucket) { - return nullptr; - } + endParsing(); + }, callback); - std::lock_guard lock(bucketsMutex); + return true; +} - const auto it = buckets.find(layer.bucket->name); - if (it == buckets.end()) { +Bucket* VectorTileData::getBucket(const StyleLayer& layer) { + if (!isReady() || !layer.bucket) { return nullptr; } - assert(it->second); - return it->second.get(); + return workerData.getBucket(layer); } 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); - - std::lock_guard lock(bucketsMutex); - - if (buckets.find(layer.bucket->name) != buckets.end()) { - return; - } - - buckets[layer.bucket->name] = std::move(bucket); + return workerData.countBuckets(); } void VectorTileData::setState(const State& state_) { TileData::setState(state_); if (isImmutable()) { - collision->reset(0, 0); + workerData.collision->reset(0, 0); } } @@ -158,24 +134,12 @@ void VectorTileData::redoPlacement(float angle, bool collisionDebug) { currentCollisionDebug = collisionDebug; auto callback = std::bind(&VectorTileData::endRedoPlacement, this); - workRequest = style.workers.send([this, angle, collisionDebug] { workerRedoPlacement(angle, collisionDebug); }, callback); - - } -} - -void VectorTileData::workerRedoPlacement(float angle, bool collisionDebug) { - collision->reset(angle, 0); - collision->setDebug(collisionDebug); - for (const auto& layer_desc : style.layers) { - auto bucket = getBucket(*layer_desc); - if (bucket) { - bucket->placeFeatures(); - } + workRequest = worker.send([this, angle, collisionDebug] { workerData.redoPlacement(angle, collisionDebug); }, callback); } } void VectorTileData::endRedoPlacement() { - for (const auto& layer_desc : style.layers) { + for (const auto& layer_desc : workerData.style.layers) { auto bucket = getBucket(*layer_desc); if (bucket) { bucket->swapRenderData(); diff --git a/src/mbgl/map/vector_tile_data.hpp b/src/mbgl/map/vector_tile_data.hpp index faddb1dc7dd..4cfe4e12aed 100644 --- a/src/mbgl/map/vector_tile_data.hpp +++ b/src/mbgl/map/vector_tile_data.hpp @@ -2,31 +2,16 @@ #define MBGL_MAP_VECTOR_TILE_DATA #include -#include -#include -#include -#include -#include -#include - -#include -#include -#include -#include +#include namespace mbgl { class Bucket; -class CollisionTile; -class Painter; class SourceInfo; class StyleLayer; -class TileParser; class Style; class VectorTileData : public TileData { - friend class TileParser; - public: VectorTileData(const TileID&, Style&, @@ -35,18 +20,11 @@ class VectorTileData : public TileData { bool collisionDebug_); ~VectorTileData(); - virtual void parse(); void redoPlacement(float angle, bool collisionDebug) 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; - inline CollisionTile* getCollision() const { - return collision.get(); - } + Bucket* getBucket(const StyleLayer&) override; + size_t countBuckets() const; void request(Worker&, float pixelRatio, @@ -57,34 +35,16 @@ class VectorTileData : public TileData { protected: void redoPlacement(); - // Holds the actual geometries in this tile. - FillVertexBuffer fillVertexBuffer; - LineVertexBuffer lineVertexBuffer; - - TriangleElementsBuffer triangleElementsBuffer; - LineElementsBuffer lineElementsBuffer; - - Style& style; + Worker& worker; + TileWorker workerData; private: - // Contains all the Bucket objects for the tile. Buckets are render - // objects and they get added to this std::map<> by the workers doing - // the actual tile parsing as they get processed. Tiles partially - // parsed can get new buckets at any moment but are also fit for - // rendering. That said, access to this list needs locking unless - // the tile is completely parsed. - std::unordered_map> buckets; - mutable std::mutex bucketsMutex; - - std::unique_ptr collision; - float lastAngle = 0; float currentAngle; bool lastCollisionDebug = 0; bool currentCollisionDebug = 0; bool redoingPlacement = false; void endRedoPlacement(); - void workerRedoPlacement(float angle, bool collisionDebug); }; } From 217c376291967aecd7c8cc26e485e3a4fe09ee60 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Thu, 4 Jun 2015 17:22:47 -0700 Subject: [PATCH 05/20] Push TileMembers members down hierarchy --- src/mbgl/map/raster_tile_data.cpp | 56 ++++++++++++----------- src/mbgl/map/raster_tile_data.hpp | 15 +++++-- src/mbgl/map/tile_data.cpp | 50 ++------------------- src/mbgl/map/tile_data.hpp | 75 +++++++------------------------ src/mbgl/map/vector_tile_data.cpp | 31 ++++++++++++- src/mbgl/map/vector_tile_data.hpp | 40 ++++++++++++++++- 6 files changed, 130 insertions(+), 137 deletions(-) diff --git a/src/mbgl/map/raster_tile_data.cpp b/src/mbgl/map/raster_tile_data.cpp index ee53a6a2882..ead33dcb078 100644 --- a/src/mbgl/map/raster_tile_data.cpp +++ b/src/mbgl/map/raster_tile_data.cpp @@ -10,14 +10,15 @@ using namespace mbgl; -RasterTileData::RasterTileData(const TileID& id_, TexturePool &texturePool, +RasterTileData::RasterTileData(const TileID& id_, + TexturePool &texturePool, const SourceInfo &source_) - : TileData(id_, source_), bucket(texturePool, layout) { + : TileData(id_), + source(source_), + bucket(texturePool, layout) { } RasterTileData::~RasterTileData() { - // Cancel in most derived class destructor so that worker tasks are joined before - // any member data goes away. cancel(); } @@ -34,7 +35,8 @@ void RasterTileData::request(Worker& worker, if (res.status != Response::Successful) { std::stringstream message; message << "Failed to load [" << url << "]: " << res.message; - setError(message.str()); + error = message.str(); + state = State::obsolete; callback(); return; } @@ -42,32 +44,36 @@ void RasterTileData::request(Worker& worker, state = State::loaded; data = res.data; - // Schedule tile parsing in another thread - reparse(worker, callback); + workRequest = worker.send([this] { + if (getState() != State::loaded) { + return; + } + + if (bucket.setImage(data)) { + state = State::parsed; + } else { + state = State::invalid; + } + }, callback); }); } -bool RasterTileData::reparse(Worker& worker, std::function callback) { - if (!mayStartParsing()) { - return false; - } +bool RasterTileData::reparse(Worker&, std::function) { + assert(false); + return false; +} - workRequest = worker.send([this] { parse(); endParsing(); }, callback); - return true; +Bucket* RasterTileData::getBucket(StyleLayer const&) { + return &bucket; } -void RasterTileData::parse() { - if (getState() != State::loaded) { - return; +void RasterTileData::cancel() { + if (state != State::obsolete) { + state = State::obsolete; } - - if (bucket.setImage(data)) { - setState(State::parsed); - } else { - setState(State::invalid); + if (req) { + util::ThreadContext::getFileSource()->cancel(req); + req = nullptr; } -} - -Bucket* RasterTileData::getBucket(StyleLayer const&) { - return &bucket; + workRequest.reset(); } diff --git a/src/mbgl/map/raster_tile_data.hpp b/src/mbgl/map/raster_tile_data.hpp index fb59482eff4..195698865d2 100644 --- a/src/mbgl/map/raster_tile_data.hpp +++ b/src/mbgl/map/raster_tile_data.hpp @@ -7,10 +7,11 @@ namespace mbgl { -class Painter; class SourceInfo; +class Request; class StyleLayer; class TexturePool; +class WorkRequest; class RasterTileData : public TileData { public: @@ -23,12 +24,20 @@ class RasterTileData : public TileData { bool reparse(Worker&, std::function callback) override; - void parse(); + void cancel() override; + Bucket* getBucket(StyleLayer const &layer_desc) override; -protected: +private: + const SourceInfo& source; + + Request *req = nullptr; + std::string data; + StyleLayoutRaster layout; RasterBucket bucket; + + std::unique_ptr workRequest; }; } diff --git a/src/mbgl/map/tile_data.cpp b/src/mbgl/map/tile_data.cpp index 4a30639cb29..453233211ce 100644 --- a/src/mbgl/map/tile_data.cpp +++ b/src/mbgl/map/tile_data.cpp @@ -1,53 +1,11 @@ #include -#include -#include using namespace mbgl; -TileData::TileData(const TileID& id_, const SourceInfo& source_) +TileData::TileData(const TileID& id_) : id(id_), - name(id), - source(source_), - state(State::initial), - debugBucket(debugFontBuffer) { + debugBucket(debugFontBuffer), + state(State::initial) { // Initialize tile debug coordinates - debugFontBuffer.addText(name.c_str(), 50, 200, 5); -} - -TileData::~TileData() { - cancel(); -} - -const std::string TileData::toString() const { - return std::string { "[tile " } + name + "]"; -} - -void TileData::setState(const State& state_) { - assert(!isImmutable()); - - state = state_; -} - -void TileData::cancel() { - if (state != State::obsolete) { - state = State::obsolete; - } - if (req) { - util::ThreadContext::getFileSource()->cancel(req); - req = nullptr; - } - workRequest.reset(); -} - -bool TileData::mayStartParsing() { - return !parsing.test_and_set(std::memory_order_acquire); -} - -void TileData::endParsing() { - parsing.clear(std::memory_order_release); -} - -void TileData::setError(const std::string& message) { - error = message; - setState(State::obsolete); + debugFontBuffer.addText(std::string(id).c_str(), 50, 200, 5); } diff --git a/src/mbgl/map/tile_data.hpp b/src/mbgl/map/tile_data.hpp index a3aaf1dfdba..9006eb6ad8b 100644 --- a/src/mbgl/map/tile_data.hpp +++ b/src/mbgl/map/tile_data.hpp @@ -1,26 +1,19 @@ #ifndef MBGL_MAP_TILE_DATA #define MBGL_MAP_TILE_DATA +#include #include #include #include -#include -#include - #include #include #include namespace mbgl { -class Painter; -class SourceInfo; class StyleLayer; -class Request; class Worker; -class WorkRequest; -class TransformState; class TileData : private util::noncopyable { public: @@ -74,8 +67,8 @@ class TileData : private util::noncopyable { return state == State::partial || state == State::parsed; } - TileData(const TileID&, const SourceInfo&); - ~TileData(); + TileData(const TileID&); + virtual ~TileData() = default; virtual void request(Worker&, float pixelRatio, @@ -88,72 +81,34 @@ class TileData : private util::noncopyable { virtual bool reparse(Worker&, std::function callback) = 0; - void cancel(); - const std::string toString() const; + // Mark this tile as no longer needed and cancel any pending work. + virtual void cancel() = 0; - inline bool isReady() const { - return isReadyState(state); - } + virtual Bucket* getBucket(const StyleLayer&) = 0; + + virtual void redoPlacement(float, bool) {} - // Returns true if the TileData is in a final state and we cannot - // make changes to it anymore. - inline bool isImmutable() const { - return state == State::parsed || state == State::obsolete; + bool isReady() const { + return isReadyState(state); } - // We let subclasses override setState() so they - // can intercept the state change and react accordingly. - virtual void setState(const State& state); - inline State getState() const { + State getState() const { return state; } - void endParsing(); - - // Error message to be set in case of request - // and parsing errors. - void setError(const std::string& message); - std::string getError() const { return error; } - // Override this in the child class. - virtual Bucket* getBucket(StyleLayer const &layer_desc) = 0; - - virtual void redoPlacement(float, bool) {} - const TileID id; - const std::string name; - std::atomic_flag parsing = ATOMIC_FLAG_INIT; - -protected: - // Set the internal parsing state to true so we prevent - // multiple workers to parse the same tile in parallel, - // which can happen if the tile is in the "partial" state. - // It will return true if is possible to start pasing the - // tile or false if not (so some other worker is already - // parsing the tile). - bool mayStartParsing(); - const SourceInfo& source; - - Request *req = nullptr; - std::string data; - - std::unique_ptr workRequest; - - std::atomic state; - -private: - std::string error; - -protected: // Contains the tile ID string for painting debug information. + DebugBucket debugBucket; DebugFontBuffer debugFontBuffer; -public: - DebugBucket debugBucket; +protected: + std::atomic state; + std::string error; }; } diff --git a/src/mbgl/map/vector_tile_data.cpp b/src/mbgl/map/vector_tile_data.cpp index f485a249585..0b241d722a8 100644 --- a/src/mbgl/map/vector_tile_data.cpp +++ b/src/mbgl/map/vector_tile_data.cpp @@ -19,7 +19,8 @@ VectorTileData::VectorTileData(const TileID& id_, const SourceInfo& source_, float angle, bool collisionDebug) - : TileData(id_, source_), + : TileData(id_), + source(source_), worker(style_.workers), workerData(id_, style_, @@ -111,7 +112,9 @@ size_t VectorTileData::countBuckets() const { } void VectorTileData::setState(const State& state_) { - TileData::setState(state_); + assert(!isImmutable()); + + state = state_; if (isImmutable()) { workerData.collision->reset(0, 0); @@ -148,3 +151,27 @@ void VectorTileData::endRedoPlacement() { redoingPlacement = false; redoPlacement(); } + +void VectorTileData::cancel() { + if (state != State::obsolete) { + state = State::obsolete; + } + if (req) { + util::ThreadContext::getFileSource()->cancel(req); + req = nullptr; + } + workRequest.reset(); +} + +bool VectorTileData::mayStartParsing() { + return !parsing.test_and_set(std::memory_order_acquire); +} + +void VectorTileData::endParsing() { + parsing.clear(std::memory_order_release); +} + +void VectorTileData::setError(const std::string& message) { + error = message; + setState(State::obsolete); +} diff --git a/src/mbgl/map/vector_tile_data.hpp b/src/mbgl/map/vector_tile_data.hpp index 4cfe4e12aed..bb0e316c18a 100644 --- a/src/mbgl/map/vector_tile_data.hpp +++ b/src/mbgl/map/vector_tile_data.hpp @@ -4,12 +4,17 @@ #include #include +#include + namespace mbgl { +class SourceInfo; +class Request; class Bucket; class SourceInfo; class StyleLayer; class Style; +class WorkRequest; class VectorTileData : public TileData { public: @@ -21,7 +26,6 @@ class VectorTileData : public TileData { ~VectorTileData(); void redoPlacement(float angle, bool collisionDebug) override; - void setState(const State& state) override; Bucket* getBucket(const StyleLayer&) override; size_t countBuckets() const; @@ -32,13 +36,47 @@ class VectorTileData : public TileData { bool reparse(Worker&, std::function callback) override; + void cancel() override; + protected: + // We let subclasses override setState() so they + // can intercept the state change and react accordingly. + void setState(const State&); + + // Set the internal parsing state to true so we prevent + // multiple workers to parse the same tile in parallel, + // which can happen if the tile is in the "partial" state. + // It will return true if is possible to start pasing the + // tile or false if not (so some other worker is already + // parsing the tile). + bool mayStartParsing(); + + void endParsing(); + + // Error message to be set in case of request + // and parsing errors. + void setError(const std::string& message); + void redoPlacement(); + const SourceInfo& source; + + Request *req = nullptr; + std::string data; + Worker& worker; TileWorker workerData; + std::unique_ptr workRequest; + std::atomic_flag parsing = ATOMIC_FLAG_INIT; + private: + // Returns true if the TileData is in a final state and we cannot + // make changes to it anymore. + inline bool isImmutable() const { + return state == State::parsed || state == State::obsolete; + } + float lastAngle = 0; float currentAngle; bool lastCollisionDebug = 0; From 875ef979be0140332ab219e422022ccb38b99a1e Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Thu, 4 Jun 2015 19:07:48 -0700 Subject: [PATCH 06/20] Remove indirection --- src/mbgl/map/live_tile_data.cpp | 19 ++++++++-------- src/mbgl/map/tile_data.hpp | 2 +- src/mbgl/map/vector_tile_data.cpp | 37 ++++++------------------------- src/mbgl/map/vector_tile_data.hpp | 27 ---------------------- 4 files changed, 17 insertions(+), 68 deletions(-) diff --git a/src/mbgl/map/live_tile_data.cpp b/src/mbgl/map/live_tile_data.cpp index 74d939f35ac..ab6f5d983ff 100644 --- a/src/mbgl/map/live_tile_data.cpp +++ b/src/mbgl/map/live_tile_data.cpp @@ -14,20 +14,18 @@ LiveTileData::LiveTileData(const TileID& id_, const SourceInfo& source_, float angle_, bool collisionDebug_) - : VectorTileData::VectorTileData(id_, style_, source_, angle_, collisionDebug_), + : VectorTileData(id_, style_, source_, angle_, collisionDebug_), annotationManager(annotationManager_) { - // live features are always ready - setState(State::loaded); + // live features are always loaded + state = State::loaded; } LiveTileData::~LiveTileData() { - // Cancel in most derived class destructor so that worker tasks are joined before - // any member data goes away. cancel(); } bool LiveTileData::reparse(Worker&, std::function callback) { - if (!mayStartParsing()) { + if (parsing.test_and_set(std::memory_order_acquire)) { return false; } @@ -39,7 +37,7 @@ bool LiveTileData::reparse(Worker&, std::function callback) { const LiveTile* tile = annotationManager.getTile(id); if (!tile) { - setState(State::parsed); + state = State::parsed; return; } @@ -56,12 +54,13 @@ bool LiveTileData::reparse(Worker&, std::function callback) { if (getState() == TileData::State::obsolete) { return; } else if (result.is()) { - setState(result.get()); + state = result.get(); } else { - setError(result.get()); + error = result.get(); + state = State::obsolete; } - endParsing(); + parsing.clear(std::memory_order_release); }, callback); return true; diff --git a/src/mbgl/map/tile_data.hpp b/src/mbgl/map/tile_data.hpp index 9006eb6ad8b..5ed4ef4fd4b 100644 --- a/src/mbgl/map/tile_data.hpp +++ b/src/mbgl/map/tile_data.hpp @@ -77,7 +77,7 @@ class TileData : private util::noncopyable { // 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()"). + // worker. virtual bool reparse(Worker&, std::function callback) = 0; diff --git a/src/mbgl/map/vector_tile_data.cpp b/src/mbgl/map/vector_tile_data.cpp index 0b241d722a8..171fc7cf7a3 100644 --- a/src/mbgl/map/vector_tile_data.cpp +++ b/src/mbgl/map/vector_tile_data.cpp @@ -34,8 +34,6 @@ VectorTileData::VectorTileData(const TileID& id_, } VectorTileData::~VectorTileData() { - // Cancel in most derived class destructor so that worker tasks are joined before - // any member data goes away. cancel(); } @@ -52,7 +50,8 @@ void VectorTileData::request(Worker&, if (res.status != Response::Successful) { std::stringstream message; message << "Failed to load [" << url << "]: " << res.message; - setError(message.str()); + error = message.str(); + state = State::obsolete; callback(); return; } @@ -65,7 +64,7 @@ void VectorTileData::request(Worker&, } bool VectorTileData::reparse(Worker&, std::function callback) { - if (!mayStartParsing()) { + if (parsing.test_and_set(std::memory_order_acquire)) { return false; } @@ -88,12 +87,13 @@ bool VectorTileData::reparse(Worker&, std::function callback) { if (getState() == TileData::State::obsolete) { return; } else if (result.is()) { - setState(result.get()); + state = result.get(); } else { - setError(result.get()); + error = result.get(); + state = State::obsolete; } - endParsing(); + parsing.clear(std::memory_order_release); }, callback); return true; @@ -111,16 +111,6 @@ size_t VectorTileData::countBuckets() const { return workerData.countBuckets(); } -void VectorTileData::setState(const State& state_) { - assert(!isImmutable()); - - state = state_; - - if (isImmutable()) { - workerData.collision->reset(0, 0); - } -} - void VectorTileData::redoPlacement() { redoPlacement(lastAngle, lastCollisionDebug); } @@ -162,16 +152,3 @@ void VectorTileData::cancel() { } workRequest.reset(); } - -bool VectorTileData::mayStartParsing() { - return !parsing.test_and_set(std::memory_order_acquire); -} - -void VectorTileData::endParsing() { - parsing.clear(std::memory_order_release); -} - -void VectorTileData::setError(const std::string& message) { - error = message; - setState(State::obsolete); -} diff --git a/src/mbgl/map/vector_tile_data.hpp b/src/mbgl/map/vector_tile_data.hpp index bb0e316c18a..4157a19d922 100644 --- a/src/mbgl/map/vector_tile_data.hpp +++ b/src/mbgl/map/vector_tile_data.hpp @@ -39,44 +39,17 @@ class VectorTileData : public TileData { void cancel() override; protected: - // We let subclasses override setState() so they - // can intercept the state change and react accordingly. - void setState(const State&); - - // Set the internal parsing state to true so we prevent - // multiple workers to parse the same tile in parallel, - // which can happen if the tile is in the "partial" state. - // It will return true if is possible to start pasing the - // tile or false if not (so some other worker is already - // parsing the tile). - bool mayStartParsing(); - - void endParsing(); - - // Error message to be set in case of request - // and parsing errors. - void setError(const std::string& message); - void redoPlacement(); const SourceInfo& source; - Request *req = nullptr; std::string data; - Worker& worker; TileWorker workerData; - std::unique_ptr workRequest; std::atomic_flag parsing = ATOMIC_FLAG_INIT; private: - // Returns true if the TileData is in a final state and we cannot - // make changes to it anymore. - inline bool isImmutable() const { - return state == State::parsed || state == State::obsolete; - } - float lastAngle = 0; float currentAngle; bool lastCollisionDebug = 0; From 4676553a3e31bef3094e8be5d6f9dce2809ef5ae Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Thu, 4 Jun 2015 19:32:07 -0700 Subject: [PATCH 07/20] Merge TileParser into TileWorker --- src/mbgl/map/tile_parser.cpp | 235 ----------------------------------- src/mbgl/map/tile_parser.hpp | 62 --------- src/mbgl/map/tile_worker.cpp | 218 ++++++++++++++++++++++++++++++-- src/mbgl/map/tile_worker.hpp | 24 +++- 4 files changed, 232 insertions(+), 307 deletions(-) delete mode 100644 src/mbgl/map/tile_parser.cpp delete mode 100644 src/mbgl/map/tile_parser.hpp diff --git a/src/mbgl/map/tile_parser.cpp b/src/mbgl/map/tile_parser.cpp deleted file mode 100644 index a88084391f3..00000000000 --- a/src/mbgl/map/tile_parser.cpp +++ /dev/null @@ -1,235 +0,0 @@ -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include - -namespace mbgl { - -// Note: This destructor is seemingly empty, but we need to declare it anyway -// because this object has a std::unique_ptr<> of a forward-declare type in -// its header file. -TileParser::~TileParser() = default; - -TileParser::TileParser(const GeometryTile& geometryTile_, - TileWorker& tile_, - Style& style_) - : geometryTile(geometryTile_), - tile(tile_), - style(style_), - partialParse(false) { - assert(style.sprite); -} - -bool TileParser::obsolete() const { - return tile.getState() == TileData::State::obsolete; -} - -void TileParser::parse() { - for (const auto& layer_desc : style.layers) { - // Cancel early when parsing. - if (obsolete()) { - return; - } - - if (layer_desc->isBackground()) { - // background is a special, fake bucket - continue; - } - - if (layer_desc->bucket) { - // This is a singular layer. Check if this bucket already exists. If not, - // parse this bucket. - if (tile.getBucket(*layer_desc)) { - continue; - } - - std::unique_ptr bucket = createBucket(*layer_desc->bucket); - if (bucket) { - // Bucket creation might fail because the data tile may not - // contain any data that falls into this bucket. - tile.setBucket(*layer_desc, std::move(bucket)); - } - } else { - Log::Warning(Event::ParseTile, "layer '%s' does not have buckets", layer_desc->id.c_str()); - } - } -} - -template -struct PropertyEvaluator { - typedef T result_type; - PropertyEvaluator(float z_) : z(z_) {} - - template ::value, int>::type = 0> - T operator()(const P &value) const { - return value; - } - - T operator()(const Function &value) const { - return mapbox::util::apply_visitor(FunctionEvaluator(z), value); - } - - template ::value, int>::type = 0> - T operator()(const P &) const { - return T(); - } - -private: - const float z; -}; - -template -void applyLayoutProperty(PropertyKey key, const ClassProperties &classProperties, T &target, const float z) { - auto it = classProperties.properties.find(key); - if (it != classProperties.properties.end()) { - const PropertyEvaluator evaluator(z); - target = mapbox::util::apply_visitor(evaluator, it->second); - } -} - -std::unique_ptr TileParser::createBucket(const StyleBucket &bucketDesc) { - // Skip this bucket if we are to not render this - if (tile.id.z < std::floor(bucketDesc.min_zoom) && std::floor(bucketDesc.min_zoom) < tile.maxZoom) return nullptr; - if (tile.id.z >= std::ceil(bucketDesc.max_zoom)) return nullptr; - if (bucketDesc.visibility == mbgl::VisibilityType::None) return nullptr; - - auto layer = geometryTile.getLayer(bucketDesc.source_layer); - if (layer) { - if (bucketDesc.type == StyleLayerType::Fill) { - return createFillBucket(*layer, bucketDesc); - } else if (bucketDesc.type == StyleLayerType::Line) { - return createLineBucket(*layer, bucketDesc); - } else if (bucketDesc.type == StyleLayerType::Symbol) { - return createSymbolBucket(*layer, bucketDesc); - } else if (bucketDesc.type == StyleLayerType::Raster) { - return nullptr; - } else { - Log::Warning(Event::ParseTile, "unknown bucket render type for layer '%s' (source layer '%s')", - bucketDesc.name.c_str(), bucketDesc.source_layer.c_str()); - } - } else { - // The layer specified in the bucket does not exist. Do nothing. - if (debug::tileParseWarnings) { - Log::Warning(Event::ParseTile, "layer '%s' does not exist in tile %d/%d/%d", - bucketDesc.source_layer.c_str(), tile.id.z, tile.id.x, tile.id.y); - } - } - - return nullptr; -} - -template -void TileParser::addBucketGeometries(Bucket& bucket, const GeometryTileLayer& layer, const FilterExpression &filter) { - for (std::size_t i = 0; i < layer.featureCount(); i++) { - auto feature = layer.getFeature(i); - - if (obsolete()) - return; - - GeometryTileFeatureExtractor extractor(*feature); - if (!evaluate(filter, extractor)) - continue; - - bucket->addGeometry(feature->getGeometries()); - } -} - -std::unique_ptr TileParser::createFillBucket(const GeometryTileLayer& layer, - const StyleBucket& bucket_desc) { - auto bucket = std::make_unique(tile.fillVertexBuffer, - tile.triangleElementsBuffer, - tile.lineElementsBuffer); - addBucketGeometries(bucket, layer, bucket_desc.filter); - return bucket->hasData() ? std::move(bucket) : nullptr; -} - -std::unique_ptr TileParser::createLineBucket(const GeometryTileLayer& layer, - const StyleBucket& bucket_desc) { - auto bucket = std::make_unique(tile.lineVertexBuffer, - tile.triangleElementsBuffer); - - const float z = tile.id.z; - auto& layout = bucket->layout; - - applyLayoutProperty(PropertyKey::LineCap, bucket_desc.layout, layout.cap, z); - applyLayoutProperty(PropertyKey::LineJoin, bucket_desc.layout, layout.join, z); - applyLayoutProperty(PropertyKey::LineMiterLimit, bucket_desc.layout, layout.miter_limit, z); - applyLayoutProperty(PropertyKey::LineRoundLimit, bucket_desc.layout, layout.round_limit, z); - - addBucketGeometries(bucket, layer, bucket_desc.filter); - return bucket->hasData() ? std::move(bucket) : nullptr; -} - -std::unique_ptr TileParser::createSymbolBucket(const GeometryTileLayer& layer, - const StyleBucket& bucket_desc) { - auto bucket = std::make_unique(*tile.getCollision(), tile.id.overscaling); - - const float z = tile.id.z; - auto& layout = bucket->layout; - - applyLayoutProperty(PropertyKey::SymbolPlacement, bucket_desc.layout, layout.placement, z); - if (layout.placement == PlacementType::Line) { - layout.icon.rotation_alignment = RotationAlignmentType::Map; - layout.text.rotation_alignment = RotationAlignmentType::Map; - }; - applyLayoutProperty(PropertyKey::SymbolMinDistance, bucket_desc.layout, layout.min_distance, z); - applyLayoutProperty(PropertyKey::SymbolAvoidEdges, bucket_desc.layout, layout.avoid_edges, z); - - applyLayoutProperty(PropertyKey::IconAllowOverlap, bucket_desc.layout, layout.icon.allow_overlap, z); - applyLayoutProperty(PropertyKey::IconIgnorePlacement, bucket_desc.layout, layout.icon.ignore_placement, z); - applyLayoutProperty(PropertyKey::IconOptional, bucket_desc.layout, layout.icon.optional, z); - applyLayoutProperty(PropertyKey::IconRotationAlignment, bucket_desc.layout, layout.icon.rotation_alignment, z); - applyLayoutProperty(PropertyKey::IconMaxSize, bucket_desc.layout, layout.icon.max_size, z); - applyLayoutProperty(PropertyKey::IconImage, bucket_desc.layout, layout.icon.image, z); - applyLayoutProperty(PropertyKey::IconPadding, bucket_desc.layout, layout.icon.padding, z); - applyLayoutProperty(PropertyKey::IconRotate, bucket_desc.layout, layout.icon.rotate, z); - applyLayoutProperty(PropertyKey::IconKeepUpright, bucket_desc.layout, layout.icon.keep_upright, z); - applyLayoutProperty(PropertyKey::IconOffset, bucket_desc.layout, layout.icon.offset, z); - - applyLayoutProperty(PropertyKey::TextRotationAlignment, bucket_desc.layout, layout.text.rotation_alignment, z); - applyLayoutProperty(PropertyKey::TextField, bucket_desc.layout, layout.text.field, z); - applyLayoutProperty(PropertyKey::TextFont, bucket_desc.layout, layout.text.font, z); - applyLayoutProperty(PropertyKey::TextMaxSize, bucket_desc.layout, layout.text.max_size, z); - applyLayoutProperty(PropertyKey::TextMaxWidth, bucket_desc.layout, layout.text.max_width, z); - applyLayoutProperty(PropertyKey::TextLineHeight, bucket_desc.layout, layout.text.line_height, z); - applyLayoutProperty(PropertyKey::TextLetterSpacing, bucket_desc.layout, layout.text.letter_spacing, z); - applyLayoutProperty(PropertyKey::TextMaxAngle, bucket_desc.layout, layout.text.max_angle, z); - applyLayoutProperty(PropertyKey::TextRotate, bucket_desc.layout, layout.text.rotate, z); - applyLayoutProperty(PropertyKey::TextPadding, bucket_desc.layout, layout.text.padding, z); - applyLayoutProperty(PropertyKey::TextIgnorePlacement, bucket_desc.layout, layout.text.ignore_placement, z); - applyLayoutProperty(PropertyKey::TextOptional, bucket_desc.layout, layout.text.optional, z); - applyLayoutProperty(PropertyKey::TextJustify, bucket_desc.layout, layout.text.justify, z); - applyLayoutProperty(PropertyKey::TextAnchor, bucket_desc.layout, layout.text.anchor, z); - applyLayoutProperty(PropertyKey::TextKeepUpright, bucket_desc.layout, layout.text.keep_upright, z); - applyLayoutProperty(PropertyKey::TextTransform, bucket_desc.layout, layout.text.transform, z); - applyLayoutProperty(PropertyKey::TextOffset, bucket_desc.layout, layout.text.offset, z); - applyLayoutProperty(PropertyKey::TextAllowOverlap, bucket_desc.layout, layout.text.allow_overlap, z); - - if (bucket->needsDependencies(layer, bucket_desc.filter, *style.glyphStore, *style.sprite)) { - partialParse = true; - } - - // We do not proceed if the parser is in a "partial" state because - // the layer ordering needs to be respected when calculating text - // collisions. Although, at this point, we requested all the resources - // needed by this tile. - if (partialParse) { - return nullptr; - } - - bucket->addFeatures( - reinterpret_cast(&tile), *style.spriteAtlas, *style.sprite, *style.glyphAtlas, *style.glyphStore); - - return bucket->hasData() ? std::move(bucket) : nullptr; -} - -} diff --git a/src/mbgl/map/tile_parser.hpp b/src/mbgl/map/tile_parser.hpp deleted file mode 100644 index 63c1fd72aac..00000000000 --- a/src/mbgl/map/tile_parser.hpp +++ /dev/null @@ -1,62 +0,0 @@ -#ifndef MBGL_MAP_TILE_PARSER -#define MBGL_MAP_TILE_PARSER - -#include -#include -#include -#include -#include -#include -#include -#include - -#include -#include -#include - -namespace mbgl { - -class Bucket; -class FontStack; -class Style; -class StyleBucket; -class StyleLayoutFill; -class StyleLayoutRaster; -class StyleLayoutLine; -class StyleLayoutSymbol; -class TileWorker; - -class TileParser : private util::noncopyable { -public: - TileParser(const GeometryTile&, TileWorker&, Style&); - ~TileParser(); - -public: - void parse(); - inline bool isPartialParse() const { - return partialParse; - } - -private: - bool obsolete() const; - - std::unique_ptr createBucket(const StyleBucket&); - std::unique_ptr createFillBucket(const GeometryTileLayer&, const StyleBucket&); - std::unique_ptr createLineBucket(const GeometryTileLayer&, const StyleBucket&); - std::unique_ptr createSymbolBucket(const GeometryTileLayer&, const StyleBucket&); - - template - void addBucketGeometries(Bucket&, const GeometryTileLayer&, const FilterExpression&); - - const GeometryTile& geometryTile; - TileWorker& tile; - - // Cross-thread shared data. - Style& style; - - bool partialParse; -}; - -} - -#endif diff --git a/src/mbgl/map/tile_worker.cpp b/src/mbgl/map/tile_worker.cpp index 1991ab1885e..a70e5a772f2 100644 --- a/src/mbgl/map/tile_worker.cpp +++ b/src/mbgl/map/tile_worker.cpp @@ -1,9 +1,13 @@ #include #include -#include #include #include #include +#include +#include +#include +#include +#include using namespace mbgl; @@ -12,11 +16,12 @@ TileWorker::TileWorker(const TileID& id_, const uint16_t maxZoom_, const std::atomic& state_, std::unique_ptr collision_) - : id(id_), - style(style_), + : style(style_), + id(id_), maxZoom(maxZoom_), state(state_), collision(std::move(collision_)) { + assert(style.sprite); } TileWorker::~TileWorker() { @@ -53,13 +58,36 @@ size_t TileWorker::countBuckets() const { } TileParseResult TileWorker::parse(const GeometryTile& geometryTile) { - // Parsing creates state that is encapsulated in TileParser. While parsing, - // the TileParser object writes results into this objects. All other state - // is going to be discarded afterwards. - TileParser parser(geometryTile, *this, style); - parser.parse(); + for (const auto& layer_desc : style.layers) { + // Cancel early when parsing. + if (obsolete()) { + return TileData::State::obsolete; + } + + if (layer_desc->isBackground()) { + // background is a special, fake bucket + continue; + } + + if (layer_desc->bucket) { + // This is a singular layer. Check if this bucket already exists. If not, + // parse this bucket. + if (getBucket(*layer_desc)) { + continue; + } + + std::unique_ptr bucket = createBucket(*layer_desc->bucket, geometryTile); + if (bucket) { + // Bucket creation might fail because the data tile may not + // contain any data that falls into this bucket. + setBucket(*layer_desc, std::move(bucket)); + } + } else { + Log::Warning(Event::ParseTile, "layer '%s' does not have buckets", layer_desc->id.c_str()); + } + } - if (parser.isPartialParse()) { + if (isPartialParse()) { return TileData::State::partial; } else { return TileData::State::parsed; @@ -76,3 +104,175 @@ void TileWorker::redoPlacement(float angle, bool collisionDebug) { } } } + +bool TileWorker::obsolete() const { + return getState() == TileData::State::obsolete; +} + +template +struct PropertyEvaluator { + typedef T result_type; + PropertyEvaluator(float z_) : z(z_) {} + + template ::value, int>::type = 0> + T operator()(const P &value) const { + return value; + } + + T operator()(const Function &value) const { + return mapbox::util::apply_visitor(FunctionEvaluator(z), value); + } + + template ::value, int>::type = 0> + T operator()(const P &) const { + return T(); + } + +private: + const float z; +}; + +template +void applyLayoutProperty(PropertyKey key, const ClassProperties &classProperties, T &target, const float z) { + auto it = classProperties.properties.find(key); + if (it != classProperties.properties.end()) { + const PropertyEvaluator evaluator(z); + target = mapbox::util::apply_visitor(evaluator, it->second); + } +} + +std::unique_ptr TileWorker::createBucket(const StyleBucket& bucketDesc, const GeometryTile& geometryTile) { + // Skip this bucket if we are to not render this + if (id.z < std::floor(bucketDesc.min_zoom) && std::floor(bucketDesc.min_zoom) < maxZoom) return nullptr; + if (id.z >= std::ceil(bucketDesc.max_zoom)) return nullptr; + if (bucketDesc.visibility == mbgl::VisibilityType::None) return nullptr; + + auto layer = geometryTile.getLayer(bucketDesc.source_layer); + if (layer) { + if (bucketDesc.type == StyleLayerType::Fill) { + return createFillBucket(*layer, bucketDesc); + } else if (bucketDesc.type == StyleLayerType::Line) { + return createLineBucket(*layer, bucketDesc); + } else if (bucketDesc.type == StyleLayerType::Symbol) { + return createSymbolBucket(*layer, bucketDesc); + } else if (bucketDesc.type == StyleLayerType::Raster) { + return nullptr; + } else { + Log::Warning(Event::ParseTile, "unknown bucket render type for layer '%s' (source layer '%s')", + bucketDesc.name.c_str(), bucketDesc.source_layer.c_str()); + } + } else { + // The layer specified in the bucket does not exist. Do nothing. + if (debug::tileParseWarnings) { + Log::Warning(Event::ParseTile, "layer '%s' does not exist in tile %d/%d/%d", + bucketDesc.source_layer.c_str(), id.z, id.x, id.y); + } + } + + return nullptr; +} + +template +void TileWorker::addBucketGeometries(Bucket& bucket, const GeometryTileLayer& layer, const FilterExpression &filter) { + for (std::size_t i = 0; i < layer.featureCount(); i++) { + auto feature = layer.getFeature(i); + + if (obsolete()) + return; + + GeometryTileFeatureExtractor extractor(*feature); + if (!evaluate(filter, extractor)) + continue; + + bucket->addGeometry(feature->getGeometries()); + } +} + +std::unique_ptr TileWorker::createFillBucket(const GeometryTileLayer& layer, + const StyleBucket& bucket_desc) { + auto bucket = std::make_unique(fillVertexBuffer, + triangleElementsBuffer, + lineElementsBuffer); + addBucketGeometries(bucket, layer, bucket_desc.filter); + return bucket->hasData() ? std::move(bucket) : nullptr; +} + +std::unique_ptr TileWorker::createLineBucket(const GeometryTileLayer& layer, + const StyleBucket& bucket_desc) { + auto bucket = std::make_unique(lineVertexBuffer, + triangleElementsBuffer); + + const float z = id.z; + auto& layout = bucket->layout; + + applyLayoutProperty(PropertyKey::LineCap, bucket_desc.layout, layout.cap, z); + applyLayoutProperty(PropertyKey::LineJoin, bucket_desc.layout, layout.join, z); + applyLayoutProperty(PropertyKey::LineMiterLimit, bucket_desc.layout, layout.miter_limit, z); + applyLayoutProperty(PropertyKey::LineRoundLimit, bucket_desc.layout, layout.round_limit, z); + + addBucketGeometries(bucket, layer, bucket_desc.filter); + return bucket->hasData() ? std::move(bucket) : nullptr; +} + +std::unique_ptr TileWorker::createSymbolBucket(const GeometryTileLayer& layer, + const StyleBucket& bucket_desc) { + auto bucket = std::make_unique(*getCollision(), id.overscaling); + + const float z = id.z; + auto& layout = bucket->layout; + + applyLayoutProperty(PropertyKey::SymbolPlacement, bucket_desc.layout, layout.placement, z); + if (layout.placement == PlacementType::Line) { + layout.icon.rotation_alignment = RotationAlignmentType::Map; + layout.text.rotation_alignment = RotationAlignmentType::Map; + }; + applyLayoutProperty(PropertyKey::SymbolMinDistance, bucket_desc.layout, layout.min_distance, z); + applyLayoutProperty(PropertyKey::SymbolAvoidEdges, bucket_desc.layout, layout.avoid_edges, z); + + applyLayoutProperty(PropertyKey::IconAllowOverlap, bucket_desc.layout, layout.icon.allow_overlap, z); + applyLayoutProperty(PropertyKey::IconIgnorePlacement, bucket_desc.layout, layout.icon.ignore_placement, z); + applyLayoutProperty(PropertyKey::IconOptional, bucket_desc.layout, layout.icon.optional, z); + applyLayoutProperty(PropertyKey::IconRotationAlignment, bucket_desc.layout, layout.icon.rotation_alignment, z); + applyLayoutProperty(PropertyKey::IconMaxSize, bucket_desc.layout, layout.icon.max_size, z); + applyLayoutProperty(PropertyKey::IconImage, bucket_desc.layout, layout.icon.image, z); + applyLayoutProperty(PropertyKey::IconPadding, bucket_desc.layout, layout.icon.padding, z); + applyLayoutProperty(PropertyKey::IconRotate, bucket_desc.layout, layout.icon.rotate, z); + applyLayoutProperty(PropertyKey::IconKeepUpright, bucket_desc.layout, layout.icon.keep_upright, z); + applyLayoutProperty(PropertyKey::IconOffset, bucket_desc.layout, layout.icon.offset, z); + + applyLayoutProperty(PropertyKey::TextRotationAlignment, bucket_desc.layout, layout.text.rotation_alignment, z); + applyLayoutProperty(PropertyKey::TextField, bucket_desc.layout, layout.text.field, z); + applyLayoutProperty(PropertyKey::TextFont, bucket_desc.layout, layout.text.font, z); + applyLayoutProperty(PropertyKey::TextMaxSize, bucket_desc.layout, layout.text.max_size, z); + applyLayoutProperty(PropertyKey::TextMaxWidth, bucket_desc.layout, layout.text.max_width, z); + applyLayoutProperty(PropertyKey::TextLineHeight, bucket_desc.layout, layout.text.line_height, z); + applyLayoutProperty(PropertyKey::TextLetterSpacing, bucket_desc.layout, layout.text.letter_spacing, z); + applyLayoutProperty(PropertyKey::TextMaxAngle, bucket_desc.layout, layout.text.max_angle, z); + applyLayoutProperty(PropertyKey::TextRotate, bucket_desc.layout, layout.text.rotate, z); + applyLayoutProperty(PropertyKey::TextPadding, bucket_desc.layout, layout.text.padding, z); + applyLayoutProperty(PropertyKey::TextIgnorePlacement, bucket_desc.layout, layout.text.ignore_placement, z); + applyLayoutProperty(PropertyKey::TextOptional, bucket_desc.layout, layout.text.optional, z); + applyLayoutProperty(PropertyKey::TextJustify, bucket_desc.layout, layout.text.justify, z); + applyLayoutProperty(PropertyKey::TextAnchor, bucket_desc.layout, layout.text.anchor, z); + applyLayoutProperty(PropertyKey::TextKeepUpright, bucket_desc.layout, layout.text.keep_upright, z); + applyLayoutProperty(PropertyKey::TextTransform, bucket_desc.layout, layout.text.transform, z); + applyLayoutProperty(PropertyKey::TextOffset, bucket_desc.layout, layout.text.offset, z); + applyLayoutProperty(PropertyKey::TextAllowOverlap, bucket_desc.layout, layout.text.allow_overlap, z); + + if (bucket->needsDependencies(layer, bucket_desc.filter, *style.glyphStore, *style.sprite)) { + partialParse = true; + } + + // We do not proceed if the parser is in a "partial" state because + // the layer ordering needs to be respected when calculating text + // collisions. Although, at this point, we requested all the resources + // needed by this tile. + if (partialParse) { + return nullptr; + } + + bucket->addFeatures( + reinterpret_cast(this), *style.spriteAtlas, *style.sprite, *style.glyphAtlas, *style.glyphStore); + + return bucket->hasData() ? std::move(bucket) : nullptr; +} diff --git a/src/mbgl/map/tile_worker.hpp b/src/mbgl/map/tile_worker.hpp index 420b0bf9f22..c3f131ccd48 100644 --- a/src/mbgl/map/tile_worker.hpp +++ b/src/mbgl/map/tile_worker.hpp @@ -7,7 +7,9 @@ #include #include #include +#include +#include #include #include #include @@ -18,6 +20,8 @@ class CollisionTile; class GeometryTile; class Style; class Bucket; +class StyleBucket; +class GeometryTileLayer; using TileParseResult = mapbox::util::variant< TileData::State, // success @@ -44,14 +48,32 @@ class TileWorker : public util::noncopyable { return collision.get(); } + inline bool isPartialParse() const { + return partialParse; + } + TileParseResult parse(const GeometryTile&); void redoPlacement(float angle, bool collisionDebug); - const TileID id; Style& style; + +private: + bool obsolete() const; + + std::unique_ptr createBucket(const StyleBucket&, const GeometryTile&); + std::unique_ptr createFillBucket(const GeometryTileLayer&, const StyleBucket&); + std::unique_ptr createLineBucket(const GeometryTileLayer&, const StyleBucket&); + std::unique_ptr createSymbolBucket(const GeometryTileLayer&, const StyleBucket&); + + template + void addBucketGeometries(Bucket&, const GeometryTileLayer&, const FilterExpression&); + + const TileID id; const uint16_t maxZoom; const std::atomic& state; + bool partialParse = false; + FillVertexBuffer fillVertexBuffer; LineVertexBuffer lineVertexBuffer; From 9e0b241ddefa8e22b1b5c115b3ffdc1a19e9d624 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Wed, 10 Jun 2015 11:34:38 -0400 Subject: [PATCH 08/20] reset partialParse when starting a new parsing run --- src/mbgl/map/tile_worker.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/mbgl/map/tile_worker.cpp b/src/mbgl/map/tile_worker.cpp index a70e5a772f2..de6c71898af 100644 --- a/src/mbgl/map/tile_worker.cpp +++ b/src/mbgl/map/tile_worker.cpp @@ -58,6 +58,8 @@ size_t TileWorker::countBuckets() const { } TileParseResult TileWorker::parse(const GeometryTile& geometryTile) { + partialParse = false; + for (const auto& layer_desc : style.layers) { // Cancel early when parsing. if (obsolete()) { From 24f50fdd4f888b5524b6a95dbe9d920404a6bd64 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Thu, 4 Jun 2015 19:33:53 -0700 Subject: [PATCH 09/20] Remove indirection --- src/mbgl/map/tile_worker.cpp | 12 ++++-------- src/mbgl/map/tile_worker.hpp | 14 -------------- 2 files changed, 4 insertions(+), 22 deletions(-) diff --git a/src/mbgl/map/tile_worker.cpp b/src/mbgl/map/tile_worker.cpp index de6c71898af..ae291b04e4d 100644 --- a/src/mbgl/map/tile_worker.cpp +++ b/src/mbgl/map/tile_worker.cpp @@ -62,7 +62,7 @@ TileParseResult TileWorker::parse(const GeometryTile& geometryTile) { for (const auto& layer_desc : style.layers) { // Cancel early when parsing. - if (obsolete()) { + if (state == TileData::State::obsolete) { return TileData::State::obsolete; } @@ -89,7 +89,7 @@ TileParseResult TileWorker::parse(const GeometryTile& geometryTile) { } } - if (isPartialParse()) { + if (partialParse) { return TileData::State::partial; } else { return TileData::State::parsed; @@ -107,10 +107,6 @@ void TileWorker::redoPlacement(float angle, bool collisionDebug) { } } -bool TileWorker::obsolete() const { - return getState() == TileData::State::obsolete; -} - template struct PropertyEvaluator { typedef T result_type; @@ -179,7 +175,7 @@ void TileWorker::addBucketGeometries(Bucket& bucket, const GeometryTileLayer& la for (std::size_t i = 0; i < layer.featureCount(); i++) { auto feature = layer.getFeature(i); - if (obsolete()) + if (state == TileData::State::obsolete) return; GeometryTileFeatureExtractor extractor(*feature); @@ -218,7 +214,7 @@ std::unique_ptr TileWorker::createLineBucket(const GeometryTileLayer& la std::unique_ptr TileWorker::createSymbolBucket(const GeometryTileLayer& layer, const StyleBucket& bucket_desc) { - auto bucket = std::make_unique(*getCollision(), id.overscaling); + auto bucket = std::make_unique(*collision, id.overscaling); const float z = id.z; auto& layout = bucket->layout; diff --git a/src/mbgl/map/tile_worker.hpp b/src/mbgl/map/tile_worker.hpp index c3f131ccd48..ae97e08cbb1 100644 --- a/src/mbgl/map/tile_worker.hpp +++ b/src/mbgl/map/tile_worker.hpp @@ -40,26 +40,12 @@ class TileWorker : public util::noncopyable { Bucket* getBucket(const StyleLayer&) const; size_t countBuckets() const; - inline TileData::State getState() const { - return state; - } - - inline CollisionTile* getCollision() const { - return collision.get(); - } - - inline bool isPartialParse() const { - return partialParse; - } - TileParseResult parse(const GeometryTile&); void redoPlacement(float angle, bool collisionDebug); Style& style; private: - bool obsolete() const; - std::unique_ptr createBucket(const StyleBucket&, const GeometryTile&); std::unique_ptr createFillBucket(const GeometryTileLayer&, const StyleBucket&); std::unique_ptr createLineBucket(const GeometryTileLayer&, const StyleBucket&); From d596d9dc381170013563348b7af46812f6fcf790 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Thu, 4 Jun 2015 19:58:43 -0700 Subject: [PATCH 10/20] Clean up bucket creation loop --- src/mbgl/map/tile_worker.cpp | 122 ++++++++++++++++------------------- src/mbgl/map/tile_worker.hpp | 13 ++-- 2 files changed, 62 insertions(+), 73 deletions(-) diff --git a/src/mbgl/map/tile_worker.cpp b/src/mbgl/map/tile_worker.cpp index ae291b04e4d..b69698689ad 100644 --- a/src/mbgl/map/tile_worker.cpp +++ b/src/mbgl/map/tile_worker.cpp @@ -28,18 +28,6 @@ TileWorker::~TileWorker() { style.glyphAtlas->removeGlyphs(reinterpret_cast(this)); } -void TileWorker::setBucket(const StyleLayer& layer, std::unique_ptr bucket) { - assert(layer.bucket); - - std::lock_guard lock(bucketsMutex); - - if (buckets.find(layer.bucket->name) != buckets.end()) { - return; - } - - buckets[layer.bucket->name] = std::move(bucket); -} - Bucket* TileWorker::getBucket(const StyleLayer& layer) const { std::lock_guard lock(bucketsMutex); @@ -60,40 +48,11 @@ size_t TileWorker::countBuckets() const { TileParseResult TileWorker::parse(const GeometryTile& geometryTile) { partialParse = false; - for (const auto& layer_desc : style.layers) { - // Cancel early when parsing. - if (state == TileData::State::obsolete) { - return TileData::State::obsolete; - } - - if (layer_desc->isBackground()) { - // background is a special, fake bucket - continue; - } - - if (layer_desc->bucket) { - // This is a singular layer. Check if this bucket already exists. If not, - // parse this bucket. - if (getBucket(*layer_desc)) { - continue; - } - - std::unique_ptr bucket = createBucket(*layer_desc->bucket, geometryTile); - if (bucket) { - // Bucket creation might fail because the data tile may not - // contain any data that falls into this bucket. - setBucket(*layer_desc, std::move(bucket)); - } - } else { - Log::Warning(Event::ParseTile, "layer '%s' does not have buckets", layer_desc->id.c_str()); - } + for (const auto& layer : style.layers) { + parseLayer(*layer, geometryTile); } - if (partialParse) { - return TileData::State::partial; - } else { - return TileData::State::parsed; - } + return partialParse ? TileData::State::partial : TileData::State::parsed; } void TileWorker::redoPlacement(float angle, bool collisionDebug) { @@ -139,35 +98,66 @@ void applyLayoutProperty(PropertyKey key, const ClassProperties &classProperties } } -std::unique_ptr TileWorker::createBucket(const StyleBucket& bucketDesc, const GeometryTile& geometryTile) { +void TileWorker::parseLayer(const StyleLayer& layer, const GeometryTile& geometryTile) { + // Cancel early when parsing. + if (state == TileData::State::obsolete) + return; + + // Background is a special case. + if (layer.isBackground()) + return; + + if (!layer.bucket) { + Log::Warning(Event::ParseTile, "layer '%s' does not have buckets", layer.id.c_str()); + return; + } + + // This is a singular layer. Check if this bucket already exists. + if (getBucket(layer)) + return; + + const StyleBucket& styleBucket = *layer.bucket; + // Skip this bucket if we are to not render this - if (id.z < std::floor(bucketDesc.min_zoom) && std::floor(bucketDesc.min_zoom) < maxZoom) return nullptr; - if (id.z >= std::ceil(bucketDesc.max_zoom)) return nullptr; - if (bucketDesc.visibility == mbgl::VisibilityType::None) return nullptr; - - auto layer = geometryTile.getLayer(bucketDesc.source_layer); - if (layer) { - if (bucketDesc.type == StyleLayerType::Fill) { - return createFillBucket(*layer, bucketDesc); - } else if (bucketDesc.type == StyleLayerType::Line) { - return createLineBucket(*layer, bucketDesc); - } else if (bucketDesc.type == StyleLayerType::Symbol) { - return createSymbolBucket(*layer, bucketDesc); - } else if (bucketDesc.type == StyleLayerType::Raster) { - return nullptr; - } else { - Log::Warning(Event::ParseTile, "unknown bucket render type for layer '%s' (source layer '%s')", - bucketDesc.name.c_str(), bucketDesc.source_layer.c_str()); - } - } else { + if (id.z < std::floor(styleBucket.min_zoom) && std::floor(styleBucket.min_zoom) < maxZoom) + return; + if (id.z >= std::ceil(styleBucket.max_zoom)) + return; + if (styleBucket.visibility == mbgl::VisibilityType::None) + return; + + auto geometryLayer = geometryTile.getLayer(styleBucket.source_layer); + if (!geometryLayer) { // The layer specified in the bucket does not exist. Do nothing. if (debug::tileParseWarnings) { Log::Warning(Event::ParseTile, "layer '%s' does not exist in tile %d/%d/%d", - bucketDesc.source_layer.c_str(), id.z, id.x, id.y); + styleBucket.source_layer.c_str(), id.z, id.x, id.y); } + return; + } + + std::unique_ptr bucket; + + if (styleBucket.type == StyleLayerType::Fill) { + bucket = createFillBucket(*geometryLayer, styleBucket); + } else if (styleBucket.type == StyleLayerType::Line) { + bucket = createLineBucket(*geometryLayer, styleBucket); + } else if (styleBucket.type == StyleLayerType::Symbol) { + bucket = createSymbolBucket(*geometryLayer, styleBucket); + } else if (styleBucket.type == StyleLayerType::Raster) { + return; + } else { + Log::Warning(Event::ParseTile, "unknown bucket render type for layer '%s' (source layer '%s')", + styleBucket.name.c_str(), styleBucket.source_layer.c_str()); } - return nullptr; + // Bucket creation might fail because the data tile may not + // contain any data that falls into this bucket. + if (!bucket) + return; + + std::lock_guard lock(bucketsMutex); + buckets[styleBucket.name] = std::move(bucket); } template diff --git a/src/mbgl/map/tile_worker.hpp b/src/mbgl/map/tile_worker.hpp index ae97e08cbb1..7fb474a5eed 100644 --- a/src/mbgl/map/tile_worker.hpp +++ b/src/mbgl/map/tile_worker.hpp @@ -36,7 +36,6 @@ class TileWorker : public util::noncopyable { std::unique_ptr); ~TileWorker(); - void setBucket(const StyleLayer&, std::unique_ptr); Bucket* getBucket(const StyleLayer&) const; size_t countBuckets() const; @@ -46,7 +45,8 @@ class TileWorker : public util::noncopyable { Style& style; private: - std::unique_ptr createBucket(const StyleBucket&, const GeometryTile&); + void parseLayer(const StyleLayer&, const GeometryTile&); + std::unique_ptr createFillBucket(const GeometryTileLayer&, const StyleBucket&); std::unique_ptr createLineBucket(const GeometryTileLayer&, const StyleBucket&); std::unique_ptr createSymbolBucket(const GeometryTileLayer&, const StyleBucket&); @@ -69,11 +69,10 @@ class TileWorker : public util::noncopyable { std::unique_ptr collision; // Contains all the Bucket objects for the tile. Buckets are render - // objects and they get added to this std::map<> by the workers doing - // the actual tile parsing as they get processed. Tiles partially - // parsed can get new buckets at any moment but are also fit for - // rendering. That said, access to this list needs locking unless - // the tile is completely parsed. + // objects and they get added to this map as they get processed. + // Tiles partially parsed can get new buckets at any moment but are + // also fit for rendering. That said, access to this list needs locking + // unless the tile is completely parsed. std::unordered_map> buckets; mutable std::mutex bucketsMutex; }; From 5fadda53e5cf7044ead3a3e688c557c7af290453 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Mon, 8 Jun 2015 18:40:11 -0400 Subject: [PATCH 11/20] Move WorkerTask functionality into RunLoop core As a byproduct, this makes FileCache get requests properly cancelable. --- include/mbgl/storage/file_cache.hpp | 3 +- include/mbgl/storage/sqlite_cache.hpp | 2 +- platform/default/sqlite_cache.cpp | 4 +- src/mbgl/storage/default_file_source.cpp | 37 ++-- src/mbgl/storage/default_file_source_impl.hpp | 8 +- src/mbgl/util/run_loop.hpp | 163 ++++++++++++++---- src/mbgl/util/thread.hpp | 16 +- src/mbgl/util/work_task.cpp | 37 ---- src/mbgl/util/work_task.hpp | 20 +-- src/mbgl/util/worker.cpp | 30 +--- test/miscellaneous/thread.cpp | 31 ++-- 11 files changed, 200 insertions(+), 151 deletions(-) diff --git a/include/mbgl/storage/file_cache.hpp b/include/mbgl/storage/file_cache.hpp index f815d5b8c2c..a9071b1b672 100644 --- a/include/mbgl/storage/file_cache.hpp +++ b/include/mbgl/storage/file_cache.hpp @@ -10,6 +10,7 @@ namespace mbgl { struct Resource; class Response; +class WorkRequest; class FileCache : private util::noncopyable { public: @@ -18,7 +19,7 @@ class FileCache : private util::noncopyable { enum class Hint : uint8_t { Full, Refresh, No }; using Callback = std::function)>; - virtual void get(const Resource &resource, Callback callback) = 0; + virtual std::unique_ptr get(const Resource &resource, Callback callback) = 0; virtual void put(const Resource &resource, std::shared_ptr response, Hint hint) = 0; }; diff --git a/include/mbgl/storage/sqlite_cache.hpp b/include/mbgl/storage/sqlite_cache.hpp index fd67d48bb56..65328652f40 100644 --- a/include/mbgl/storage/sqlite_cache.hpp +++ b/include/mbgl/storage/sqlite_cache.hpp @@ -17,7 +17,7 @@ class SQLiteCache : public FileCache { ~SQLiteCache() override; // FileCache API - void get(const Resource &resource, Callback callback) override; + std::unique_ptr get(const Resource &resource, Callback callback) override; void put(const Resource &resource, std::shared_ptr response, Hint hint) override; class Impl; diff --git a/platform/default/sqlite_cache.cpp b/platform/default/sqlite_cache.cpp index a2d2c6815df..3c23d167a6b 100644 --- a/platform/default/sqlite_cache.cpp +++ b/platform/default/sqlite_cache.cpp @@ -126,12 +126,12 @@ void SQLiteCache::Impl::createSchema() { } } -void SQLiteCache::get(const Resource &resource, Callback callback) { +std::unique_ptr SQLiteCache::get(const Resource &resource, Callback callback) { // Can be called from any thread, but most likely from the file source thread. // Will try to load the URL from the SQLite database and call the callback when done. // Note that the callback is probably going to invoked from another thread, so the caller // must make sure that it can run in that thread. - thread->invokeWithResult(&Impl::get, std::move(callback), resource); + return thread->invokeWithResult(&Impl::get, std::move(callback), resource); } std::unique_ptr SQLiteCache::Impl::get(const Resource &resource) { diff --git a/src/mbgl/storage/default_file_source.cpp b/src/mbgl/storage/default_file_source.cpp index dcd0090da15..5f6bdcb6ace 100644 --- a/src/mbgl/storage/default_file_source.cpp +++ b/src/mbgl/storage/default_file_source.cpp @@ -100,26 +100,16 @@ void DefaultFileSource::Impl::add(Request* req) { request->observers.insert(req); if (cache) { - startCacheRequest(resource); + startCacheRequest(request); } else { - startRealRequest(resource); + startRealRequest(request); } } -void DefaultFileSource::Impl::startCacheRequest(const Resource& resource) { +void DefaultFileSource::Impl::startCacheRequest(DefaultFileRequest* request) { // Check the cache for existing data so that we can potentially // revalidate the information without having to redownload everything. - cache->get(resource, [this, resource](std::unique_ptr response) { - DefaultFileRequest* request = find(resource); - - if (!request || request->request) { - // There is no request for this URL anymore. Likely, the request was canceled - // before we got around to process the cache result. - // The second possibility is that a request has already been started by another cache - // request. In this case, we don't have to do anything either. - return; - } - + request->cacheRequest = cache->get(request->resource, [this, request](std::unique_ptr response) { auto expired = [&response] { const int64_t now = std::chrono::duration_cast( SystemClock::now().time_since_epoch()).count(); @@ -128,7 +118,7 @@ void DefaultFileSource::Impl::startCacheRequest(const Resource& resource) { if (!response || expired()) { // No response or stale cache. Run the real request. - startRealRequest(resource, std::move(response)); + startRealRequest(request, std::move(response)); } else { // The response is fresh. We're good to notify the caller. notify(request, std::move(response), FileCache::Hint::No); @@ -136,17 +126,15 @@ void DefaultFileSource::Impl::startCacheRequest(const Resource& resource) { }); } -void DefaultFileSource::Impl::startRealRequest(const Resource& resource, std::shared_ptr response) { - DefaultFileRequest* request = find(resource); - +void DefaultFileSource::Impl::startRealRequest(DefaultFileRequest* request, std::shared_ptr response) { auto callback = [request, this] (std::shared_ptr res, FileCache::Hint hint) { notify(request, res, hint); }; - if (algo::starts_with(resource.url, "asset://")) { - request->request = assetContext->createRequest(resource, callback, loop, assetRoot); + if (algo::starts_with(request->resource.url, "asset://")) { + request->realRequest = assetContext->createRequest(request->resource, callback, loop, assetRoot); } else { - request->request = httpContext->createRequest(resource, callback, loop, response); + request->realRequest = httpContext->createRequest(request->resource, callback, loop, response); } } @@ -158,8 +146,11 @@ void DefaultFileSource::Impl::cancel(Request* req) { // cancel the request and remove it from the pending list. request->observers.erase(req); if (request->observers.empty()) { - if (request->request) { - request->request->cancel(); + if (request->cacheRequest) { + request->cacheRequest.reset(); + } + if (request->realRequest) { + request->realRequest->cancel(); } pending.erase(request->resource); } diff --git a/src/mbgl/storage/default_file_source_impl.hpp b/src/mbgl/storage/default_file_source_impl.hpp index 3845014e971..22ca5b6d151 100644 --- a/src/mbgl/storage/default_file_source_impl.hpp +++ b/src/mbgl/storage/default_file_source_impl.hpp @@ -17,7 +17,9 @@ class RequestBase; struct DefaultFileRequest { const Resource resource; std::set observers; - RequestBase* request = nullptr; + + std::unique_ptr cacheRequest; + RequestBase* realRequest = nullptr; inline DefaultFileRequest(const Resource& resource_) : resource(resource_) {} @@ -39,8 +41,8 @@ class DefaultFileSource::Impl { private: DefaultFileRequest* find(const Resource&); - void startCacheRequest(const Resource&); - void startRealRequest(const Resource&, std::shared_ptr = nullptr); + void startCacheRequest(DefaultFileRequest*); + void startRealRequest(DefaultFileRequest*, std::shared_ptr = nullptr); void notify(DefaultFileRequest*, std::shared_ptr, FileCache::Hint); std::unordered_map pending; diff --git a/src/mbgl/util/run_loop.hpp b/src/mbgl/util/run_loop.hpp index 1c92847b695..3b753aca889 100644 --- a/src/mbgl/util/run_loop.hpp +++ b/src/mbgl/util/run_loop.hpp @@ -2,6 +2,8 @@ #define MBGL_UTIL_RUN_LOOP #include +#include +#include #include #include @@ -23,34 +25,54 @@ class RunLoop : private util::noncopyable { template void invoke(Fn&& fn, Args&&... args) { auto tuple = std::make_tuple(std::move(args)...); - auto invokable = std::make_unique>(std::move(fn), std::move(tuple)); - withMutex([&] { queue.push(std::move(invokable)); }); + auto task = std::make_shared>( + std::move(fn), + std::move(tuple)); + + withMutex([&] { queue.push(task); }); async.send(); } - // Return a function that invokes the given function on this RunLoop. - template - auto bind(std::function fn) { - return [this, fn = std::move(fn)] (Args&&... args) { - invoke(std::move(fn), std::move(args)...); - }; + // Invoke fn(args...) on this RunLoop, then invoke callback() on the current RunLoop. + template + std::unique_ptr + invokeWithResult(Fn&& fn, std::function callback, Args&&... args) { + auto tuple = std::make_tuple(std::move(args)...); + auto task = std::make_shared>( + std::move(fn), + std::move(tuple)); + + task->bind(callback); + + withMutex([&] { queue.push(task); }); + async.send(); + + return std::make_unique(task); } // Invoke fn(args...) on this RunLoop, then invoke callback(result) on the current RunLoop. template - void invokeWithResult(Fn&& fn, std::function callback, Args&&... args) { - invoke([fn = std::move(fn), callback = current.get()->bind(callback)] (Args&&... a) mutable { - callback(fn(std::forward(a)...)); - }, std::forward(args)...); + std::unique_ptr + invokeWithResult(Fn&& fn, std::function callback, Args&&... args) { + auto tuple = std::make_tuple(std::move(args)...); + auto task = std::make_shared>( + std::move(fn), + std::move(tuple)); + + task->bind(callback); + + withMutex([&] { queue.push(task); }); + async.send(); + + return std::make_unique(task); } - // Invoke fn(args...) on this RunLoop, then invoke callback() on the current RunLoop. - template - void invokeWithResult(Fn&& fn, std::function callback, Args&&... args) { - invoke([fn = std::move(fn), callback = current.get()->bind(callback)] (Args&&... a) mutable { - fn(std::forward(a)...); - callback(); - }, std::forward(args)...); + // Return a function that invokes the given function on this RunLoop. + template + auto bind(std::function fn) { + return [this, fn = std::move(fn)] (Args&&... args) { + invoke(std::move(fn), std::move(args)...); + }; } uv_loop_t* get() { return async.get()->loop; } @@ -58,35 +80,114 @@ class RunLoop : private util::noncopyable { static uv::tls current; private: - // A movable type-erasing invokable entity wrapper. This allows to store arbitrary invokable - // things (like std::function<>, or the result of a movable-only std::bind()) in the queue. - // Source: http://stackoverflow.com/a/29642072/331379 - struct Message { - virtual void operator()() = 0; - virtual ~Message() = default; - }; - template - struct Invoker : Message { + class Invoker : public WorkTask, public std::enable_shared_from_this> { + public: Invoker(F&& f, P&& p) : func(std::move(f)), params(std::move(p)) { } + using C = std::function; + + void bind(C after) { + auto task = this->shared_from_this(); + callback = RunLoop::current.get()->bind(C([task, after] { + if (!task->canceled) { + after(); + } + })); + } + + void operator()() override { + // We are only running the task when there's an after callback to call. This means that an + // empty after callback will be treated as a cancelled request. The mutex will be locked while + // processing so that the cancel() callback will block. + std::lock_guard lock(mutex); + if (!canceled) { + invoke(std::index_sequence_for{}); + } + } + + void cancel() override { + // Remove the after callback to indicate that this callback has been canceled. The mutex will + // block when the task is currently in progres. When the task has not begun yet, the runTask() + // method will not do anything. When the task has been completed already, and the after callback + // was run as well, this will also do nothing. + std::lock_guard lock(mutex); + canceled = true; + } + + private: + template + void invoke(std::index_sequence) { + func(std::forward(std::get(params))...); + if (callback) { + callback(); + } + } + + std::mutex mutex; + bool canceled = false; + + F func; + P params; + C callback; + }; + + template + class InvokerWithResult : public WorkTask, public std::enable_shared_from_this> { + public: + InvokerWithResult(F&& f, P&& p) + : func(std::move(f)), + params(std::move(p)) { + } + + using C = std::function; + + void bind(C after) { + auto task = this->shared_from_this(); + callback = RunLoop::current.get()->bind(C([task, after] (R result) { + if (!task->canceled) { + after(std::move(result)); + } + })); + } + void operator()() override { - invoke(std::index_sequence_for{}); + // We are only running the task when there's an after callback to call. This means that an + // empty after callback will be treated as a cancelled request. The mutex will be locked while + // processing so that the cancel() callback will block. + std::lock_guard lock(mutex); + if (!canceled) { + invoke(std::index_sequence_for{}); + } } + void cancel() override { + // Remove the after callback to indicate that this callback has been canceled. The mutex will + // block when the task is currently in progres. When the task has not begun yet, the runTask() + // method will not do anything. When the task has been completed already, and the after callback + // was run as well, this will also do nothing. + std::lock_guard lock(mutex); + canceled = true; + } + + private: template void invoke(std::index_sequence) { - func(std::forward(std::get(params))...); + callback(func(std::forward(std::get(params))...)); } + std::mutex mutex; + bool canceled = false; + F func; P params; + C callback; }; - using Queue = std::queue>; + using Queue = std::queue>; void withMutex(std::function&&); void process(); diff --git a/src/mbgl/util/thread.hpp b/src/mbgl/util/thread.hpp index 5bd856cbbcf..16bb81db1a8 100644 --- a/src/mbgl/util/thread.hpp +++ b/src/mbgl/util/thread.hpp @@ -36,15 +36,17 @@ class Thread { } // Invoke object->fn(args...) in the runloop thread, then invoke callback(result) in the current thread. - template - void invokeWithResult(Fn fn, std::function callback, Args&&... args) { - loop->invokeWithResult(bind(fn), callback, std::forward(args)...); + template + std::unique_ptr + invokeWithResult(Fn fn, std::function callback, Args&&... args) { + return loop->invokeWithResult(bind(fn), callback, std::forward(args)...); } - // Invoke object->fn(args...) in the runloop thread, then invoke callback() in the current thread. - template - void invokeWithResult(Fn fn, std::function callback, Args&&... args) { - loop->invokeWithResult(bind(fn), callback, std::forward(args)...); + // Invoke object->fn(args...) in the runloop thread, then invoke callback(result) in the current thread. + template + std::unique_ptr + invokeWithResult(Fn fn, std::function callback, Args&&... args) { + return loop->invokeWithResult(bind(fn), callback, std::forward(args)...); } // Invoke object->fn(args...) in the runloop thread, and wait for the result. diff --git a/src/mbgl/util/work_task.cpp b/src/mbgl/util/work_task.cpp index ebec420dec4..e69de29bb2d 100644 --- a/src/mbgl/util/work_task.cpp +++ b/src/mbgl/util/work_task.cpp @@ -1,37 +0,0 @@ -#include - -#include - -namespace mbgl { - -WorkTask::WorkTask(std::function task_, std::function after_) - : task(task_), after(after_) { - assert(after); -} - -void WorkTask::runTask() { - // We are only running the task when there's an after callback to call. This means that an - // empty after callback will be treated as a cancelled request. The mutex will be locked while - // processing so that the cancel() callback will block. - std::lock_guard lock(mutex); - if (after) { - task(); - } -} - -void WorkTask::runAfter() { - if (after) { - after(); - } -} - -void WorkTask::cancel() { - // Remove the after callback to indicate that this callback has been canceled. The mutex will - // block when the task is currently in progres. When the task has not begun yet, the runTask() - // method will not do anything. When the task has been completed already, and the after callback - // was run as well, this will also do nothing. - std::lock_guard lock(mutex); - after = {}; -} - -} // end namespace mbgl diff --git a/src/mbgl/util/work_task.hpp b/src/mbgl/util/work_task.hpp index f730a31c56f..2224b211c42 100644 --- a/src/mbgl/util/work_task.hpp +++ b/src/mbgl/util/work_task.hpp @@ -3,25 +3,19 @@ #include -#include -#include - namespace mbgl { +// A movable type-erasing function wrapper. This allows to store arbitrary invokable +// things (like std::function<>, or the result of a movable-only std::bind()) in the queue. +// Source: http://stackoverflow.com/a/29642072/331379 class WorkTask : private util::noncopyable { public: - WorkTask(std::function task, std::function after); - - void runTask(); - void runAfter(); - void cancel(); + virtual ~WorkTask() = default; -private: - const std::function task; - std::function after; - std::mutex mutex; + virtual void operator()() = 0; + virtual void cancel() = 0; }; -} // end namespace mbgl +} #endif diff --git a/src/mbgl/util/worker.cpp b/src/mbgl/util/worker.cpp index 4c56057347a..331fea19b54 100644 --- a/src/mbgl/util/worker.cpp +++ b/src/mbgl/util/worker.cpp @@ -10,41 +10,29 @@ namespace mbgl { class Worker::Impl { public: - Impl(uv_loop_t*) {} + Impl(uv_loop_t*, FileSource* fs) { + // FIXME: Workers should not access the FileSource but it + // is currently needed because of GlyphsPBF. See #1664. + util::ThreadContext::setFileSource(fs); + } - void doWork(std::shared_ptr& task) { - task->runTask(); + void doWork(Fn work) { + work(); } }; Worker::Worker(std::size_t count) { util::ThreadContext context = {"Worker", util::ThreadType::Worker, util::ThreadPriority::Low}; for (std::size_t i = 0; i < count; i++) { - std::unique_ptr> worker(new util::Thread(context)); - - // FIXME: Workers should not access the FileSource but it - // is currently needed because of GlyphsPBF. See #1664. - auto task = std::make_shared([fs = util::ThreadContext::getFileSource()]{ - util::ThreadContext::setFileSource(fs); - }, []{}); - - worker->invoke(&Worker::Impl::doWork, task); - threads.emplace_back(std::move(worker)); + threads.emplace_back(std::make_unique>(context, util::ThreadContext::getFileSource())); } } Worker::~Worker() = default; std::unique_ptr Worker::send(Fn work, Fn after) { - auto task = std::make_shared(work, after); - auto request = std::make_unique(task); - - threads[current]->invokeWithResult(&Worker::Impl::doWork, [task] { - task->runAfter(); - }, task); - current = (current + 1) % threads.size(); - return request; + return threads[current]->invokeWithResult(&Worker::Impl::doWork, after, work); } } // end namespace mbgl diff --git a/test/miscellaneous/thread.cpp b/test/miscellaneous/thread.cpp index f8d91bf97f4..1206afc149c 100644 --- a/test/miscellaneous/thread.cpp +++ b/test/miscellaneous/thread.cpp @@ -67,40 +67,46 @@ TEST(Thread, invoke) { const std::thread::id tid = std::this_thread::get_id(); RunLoop loop(uv_default_loop()); + std::vector> requests; loop.invoke([&] { EXPECT_EQ(tid, std::this_thread::get_id()); Thread thread({"Test", ThreadType::Map, ThreadPriority::Regular}, tid); thread.invoke(&TestObject::fn1, 1); - thread.invokeWithResult(&TestObject::fn2, [&] (int result) { + requests.push_back(thread.invokeWithResult(&TestObject::fn2, [&] (int result) { EXPECT_EQ(tid, std::this_thread::get_id()); EXPECT_EQ(result, 1); - }); + })); thread.invoke(&TestObject::transferIn, std::make_unique(1)); - thread.invokeWithResult>(&TestObject::transferOut, [&] (std::unique_ptr result) { + requests.push_back(thread.invokeWithResult>(&TestObject::transferOut, [&] (std::unique_ptr result) { EXPECT_EQ(tid, std::this_thread::get_id()); EXPECT_EQ(*result, 1); - }); + })); - thread.invokeWithResult>(&TestObject::transferInOut, [&] (std::unique_ptr result) { + requests.push_back(thread.invokeWithResult>(&TestObject::transferInOut, [&] (std::unique_ptr result) { EXPECT_EQ(tid, std::this_thread::get_id()); EXPECT_EQ(*result, 1); - }, std::make_unique(1)); + }, std::make_unique(1))); thread.invoke(&TestObject::transferInShared, std::make_shared(1)); - thread.invokeWithResult>(&TestObject::transferOutShared, [&] (std::shared_ptr result) { + requests.push_back(thread.invokeWithResult>(&TestObject::transferOutShared, [&] (std::shared_ptr result) { EXPECT_EQ(tid, std::this_thread::get_id()); EXPECT_EQ(*result, 1); - }); + })); + + // Cancelled request + thread.invokeWithResult(&TestObject::fn1, [&] { + ADD_FAILURE(); + }, 1); std::string test("test"); - thread.invokeWithResult(&TestObject::transferString, [&] (std::string result){ + requests.push_back(thread.invokeWithResult(&TestObject::transferString, [&] (std::string result){ EXPECT_EQ(tid, std::this_thread::get_id()); EXPECT_EQ(result, "test"); loop.stop(); - }, test); + }, test)); test.clear(); }); @@ -117,14 +123,15 @@ TEST(Thread, context) { const std::thread::id tid = std::this_thread::get_id(); RunLoop loop(uv_default_loop()); + std::vector> requests; loop.invoke([&] { Thread thread({"Test", ThreadType::Worker, ThreadPriority::Low}, tid); - thread.invokeWithResult(&TestObject::checkContext, [&] (bool inTestThreadContext) { + requests.push_back(thread.invokeWithResult(&TestObject::checkContext, [&] (bool inTestThreadContext) { EXPECT_EQ(inTestThreadContext, true); loop.stop(); - }); + })); }); uv_run(uv_default_loop(), UV_RUN_DEFAULT); From f14e4c67fdea1796f1cce105256f181e54d8473f Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Mon, 8 Jun 2015 19:19:48 -0400 Subject: [PATCH 12/20] Add comment about a subtlety --- src/mbgl/util/run_loop.hpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/mbgl/util/run_loop.hpp b/src/mbgl/util/run_loop.hpp index 3b753aca889..14021ef42fb 100644 --- a/src/mbgl/util/run_loop.hpp +++ b/src/mbgl/util/run_loop.hpp @@ -42,6 +42,17 @@ class RunLoop : private util::noncopyable { std::move(fn), std::move(tuple)); + // `task` is a shared pointer with ownership in the following three places: + // 1. In the `queue` of pending invocations. + // 2. In the `WorkRequest` result. + // 3. In the lambda binding of the callback to be executed on the invoking + // RunLoop. This last shared ownership is necessary in the case where + // callback execution has been scheduled (queued on the invoking RunLoop), + // but the other two places have released ownership -- i.e. the task was + // cancelled after the work is completed, but before the callback is + // executed. In this case, the lambda binding checks the cancellation flag + // and does not execute the original callback. + task->bind(callback); withMutex([&] { queue.push(task); }); From eee02425fa4a38b51067ea8d325340ec699e47fa Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Mon, 8 Jun 2015 20:08:48 -0400 Subject: [PATCH 13/20] Single-purpose Worker functions --- src/mbgl/map/live_tile_data.cpp | 39 ++++++--------- src/mbgl/map/raster_tile_data.cpp | 11 +++-- src/mbgl/map/raster_tile_data.hpp | 4 +- src/mbgl/map/vector_tile_data.cpp | 80 ++++++++++++------------------- src/mbgl/map/vector_tile_data.hpp | 12 ++--- src/mbgl/util/worker.cpp | 40 ++++++++++++++-- src/mbgl/util/worker.hpp | 34 ++++++++++--- 7 files changed, 122 insertions(+), 98 deletions(-) diff --git a/src/mbgl/map/live_tile_data.cpp b/src/mbgl/map/live_tile_data.cpp index ab6f5d983ff..a760e33fc32 100644 --- a/src/mbgl/map/live_tile_data.cpp +++ b/src/mbgl/map/live_tile_data.cpp @@ -24,44 +24,35 @@ LiveTileData::~LiveTileData() { cancel(); } -bool LiveTileData::reparse(Worker&, std::function callback) { - if (parsing.test_and_set(std::memory_order_acquire)) { +bool LiveTileData::reparse(Worker& worker, std::function callback) { + if (parsing || (state != State::loaded && state != State::partial)) { return false; } - workRequest = worker.send([this] { - if (getState() != State::loaded) { - return; - } - - const LiveTile* tile = annotationManager.getTile(id); + const LiveTile* tile = annotationManager.getTile(id); + if (!tile) { + state = State::parsed; + return true; + } - if (!tile) { - state = State::parsed; - return; - } + parsing = true; - TileParseResult result; + workRequest = worker.parseLiveTile(workerData, *tile, [this, callback] (TileParseResult result) { + parsing = false; - try { - result = workerData.parse(*tile); - } catch (const std::exception& ex) { - std::stringstream message; - message << "Failed to parse [" << int(id.sourceZ) << "/" << id.x << "/" << id.y << "]: " << ex.what(); - result = message.str(); + if (state == State::obsolete) { + return; } - if (getState() == TileData::State::obsolete) { - return; - } else if (result.is()) { + if (result.is()) { state = result.get(); } else { error = result.get(); state = State::obsolete; } - parsing.clear(std::memory_order_release); - }, callback); + callback(); + }); return true; } diff --git a/src/mbgl/map/raster_tile_data.cpp b/src/mbgl/map/raster_tile_data.cpp index ead33dcb078..e5a26260351 100644 --- a/src/mbgl/map/raster_tile_data.cpp +++ b/src/mbgl/map/raster_tile_data.cpp @@ -42,19 +42,20 @@ void RasterTileData::request(Worker& worker, } state = State::loaded; - data = res.data; - workRequest = worker.send([this] { - if (getState() != State::loaded) { + workRequest = worker.parseRasterTile(bucket, res.data, [this, callback] (bool result) { + if (state != State::loaded) { return; } - if (bucket.setImage(data)) { + if (result) { state = State::parsed; } else { state = State::invalid; } - }, callback); + + callback(); + }); }); } diff --git a/src/mbgl/map/raster_tile_data.hpp b/src/mbgl/map/raster_tile_data.hpp index 195698865d2..e519874c033 100644 --- a/src/mbgl/map/raster_tile_data.hpp +++ b/src/mbgl/map/raster_tile_data.hpp @@ -30,9 +30,7 @@ class RasterTileData : public TileData { private: const SourceInfo& source; - - Request *req = nullptr; - std::string data; + Request* req = nullptr; StyleLayoutRaster layout; RasterBucket bucket; diff --git a/src/mbgl/map/vector_tile_data.cpp b/src/mbgl/map/vector_tile_data.cpp index 171fc7cf7a3..be264084920 100644 --- a/src/mbgl/map/vector_tile_data.cpp +++ b/src/mbgl/map/vector_tile_data.cpp @@ -2,9 +2,7 @@ #include #include #include -#include #include -#include #include #include #include @@ -20,15 +18,15 @@ VectorTileData::VectorTileData(const TileID& id_, float angle, bool collisionDebug) : TileData(id_), - source(source_), - worker(style_.workers), workerData(id_, style_, - source.max_zoom, + source_.max_zoom, state, std::make_unique(id_.z, 4096, source_.tile_size * id.overscaling, angle, collisionDebug)), + source(source_), + worker(style_.workers), lastAngle(angle), currentAngle(angle) { } @@ -37,9 +35,7 @@ VectorTileData::~VectorTileData() { cancel(); } -void VectorTileData::request(Worker&, - float pixelRatio, - const std::function& callback) { +void VectorTileData::request(Worker&, float pixelRatio, const std::function& callback) { std::string url = source.tileURL(id, pixelRatio); state = State::loading; @@ -64,37 +60,28 @@ void VectorTileData::request(Worker&, } bool VectorTileData::reparse(Worker&, std::function callback) { - if (parsing.test_and_set(std::memory_order_acquire)) { + if (parsing || (state != State::loaded && state != State::partial)) { return false; } - workRequest = worker.send([this] { - if (getState() != TileData::State::loaded && getState() != TileData::State::partial) { - return; - } + parsing = true; - TileParseResult result; + workRequest = worker.parseVectorTile(workerData, data, [this, callback] (TileParseResult result) { + parsing = false; - try { - VectorTile vectorTile(pbf((const uint8_t *)data.data(), data.size())); - result = workerData.parse(vectorTile); - } catch (const std::exception& ex) { - std::stringstream message; - message << "Failed to parse [" << int(id.sourceZ) << "/" << id.x << "/" << id.y << "]: " << ex.what(); - result = message.str(); + if (state == State::obsolete) { + return; } - if (getState() == TileData::State::obsolete) { - return; - } else if (result.is()) { + if (result.is()) { state = result.get(); } else { error = result.get(); state = State::obsolete; } - parsing.clear(std::memory_order_release); - }, callback); + callback(); + }); return true; } @@ -111,35 +98,30 @@ size_t VectorTileData::countBuckets() const { return workerData.countBuckets(); } -void VectorTileData::redoPlacement() { - redoPlacement(lastAngle, lastCollisionDebug); -} - void VectorTileData::redoPlacement(float angle, bool collisionDebug) { - if (angle != currentAngle || collisionDebug != currentCollisionDebug) { - lastAngle = angle; - lastCollisionDebug = collisionDebug; + if (angle == currentAngle && collisionDebug == currentCollisionDebug) + return; - if (getState() != State::parsed || redoingPlacement) return; + lastAngle = angle; + lastCollisionDebug = collisionDebug; - redoingPlacement = true; - currentAngle = angle; - currentCollisionDebug = collisionDebug; + if (state != State::parsed || redoingPlacement) + return; - auto callback = std::bind(&VectorTileData::endRedoPlacement, this); - workRequest = worker.send([this, angle, collisionDebug] { workerData.redoPlacement(angle, collisionDebug); }, callback); - } -} + redoingPlacement = true; + currentAngle = angle; + currentCollisionDebug = collisionDebug; -void VectorTileData::endRedoPlacement() { - for (const auto& layer_desc : workerData.style.layers) { - auto bucket = getBucket(*layer_desc); - if (bucket) { - bucket->swapRenderData(); + workRequest = worker.redoPlacement(workerData, angle, collisionDebug, [this] { + for (const auto& layer : workerData.style.layers) { + auto bucket = getBucket(*layer); + if (bucket) { + bucket->swapRenderData(); + } } - } - redoingPlacement = false; - redoPlacement(); + redoingPlacement = false; + redoPlacement(lastAngle, lastCollisionDebug); + }); } void VectorTileData::cancel() { diff --git a/src/mbgl/map/vector_tile_data.hpp b/src/mbgl/map/vector_tile_data.hpp index 4157a19d922..e12554d85d8 100644 --- a/src/mbgl/map/vector_tile_data.hpp +++ b/src/mbgl/map/vector_tile_data.hpp @@ -39,23 +39,21 @@ class VectorTileData : public TileData { void cancel() override; protected: - void redoPlacement(); + TileWorker workerData; + std::unique_ptr workRequest; + bool parsing = false; +private: const SourceInfo& source; - Request *req = nullptr; + Request* req = nullptr; std::string data; Worker& worker; - TileWorker workerData; - std::unique_ptr workRequest; - std::atomic_flag parsing = ATOMIC_FLAG_INIT; -private: float lastAngle = 0; float currentAngle; bool lastCollisionDebug = 0; bool currentCollisionDebug = 0; bool redoingPlacement = false; - void endRedoPlacement(); }; } diff --git a/src/mbgl/util/worker.cpp b/src/mbgl/util/worker.cpp index 331fea19b54..74408a7daf4 100644 --- a/src/mbgl/util/worker.cpp +++ b/src/mbgl/util/worker.cpp @@ -2,6 +2,10 @@ #include #include #include +#include +#include +#include +#include #include #include @@ -16,8 +20,21 @@ class Worker::Impl { util::ThreadContext::setFileSource(fs); } - void doWork(Fn work) { - work(); + bool parseRasterTile(RasterBucket* bucket, std::string data) { + return bucket->setImage(data); + } + + TileParseResult parseVectorTile(TileWorker* worker, std::string data) { + VectorTile vectorTile(pbf(reinterpret_cast(data.data()), data.size())); + return worker->parse(vectorTile); + } + + TileParseResult parseLiveTile(TileWorker* worker, const LiveTile* tile) { + return worker->parse(*tile); + } + + void redoPlacement(TileWorker* worker, float angle, bool collisionDebug) { + worker->redoPlacement(angle, collisionDebug); } }; @@ -30,9 +47,24 @@ Worker::Worker(std::size_t count) { Worker::~Worker() = default; -std::unique_ptr Worker::send(Fn work, Fn after) { +std::unique_ptr Worker::parseRasterTile(RasterBucket& bucket, std::string data, std::function callback) { + current = (current + 1) % threads.size(); + return threads[current]->invokeWithResult(&Worker::Impl::parseRasterTile, callback, &bucket, data); +} + +std::unique_ptr Worker::parseVectorTile(TileWorker& worker, std::string data, std::function callback) { + current = (current + 1) % threads.size(); + return threads[current]->invokeWithResult(&Worker::Impl::parseVectorTile, callback, &worker, data); +} + +std::unique_ptr Worker::parseLiveTile(TileWorker& worker, const LiveTile& tile, std::function callback) { + current = (current + 1) % threads.size(); + return threads[current]->invokeWithResult(&Worker::Impl::parseLiveTile, callback, &worker, &tile); +} + +std::unique_ptr Worker::redoPlacement(TileWorker& worker, float angle, bool collisionDebug, std::function callback) { current = (current + 1) % threads.size(); - return threads[current]->invokeWithResult(&Worker::Impl::doWork, after, work); + return threads[current]->invokeWithResult(&Worker::Impl::redoPlacement, callback, &worker, angle, collisionDebug); } } // end namespace mbgl diff --git a/src/mbgl/util/worker.hpp b/src/mbgl/util/worker.hpp index 0c001e419b9..1c302d58501 100644 --- a/src/mbgl/util/worker.hpp +++ b/src/mbgl/util/worker.hpp @@ -3,6 +3,7 @@ #include #include +#include #include #include @@ -10,17 +11,16 @@ namespace mbgl { class WorkRequest; +class RasterBucket; +class LiveTile; class Worker : public mbgl::util::noncopyable { public: - using Fn = std::function; - Worker(std::size_t count); ~Worker(); - // Request work be done on a thread pool. The optional after callback is - // executed on the invoking thread, which must have a run loop, after the - // work is complete. + // Request work be done on a thread pool. Callbacks are executed on the invoking + // thread, which must have a run loop, after the work is complete. // // The return value represents the request to perform the work asynchronously. // Its destructor guarantees that the work function has finished executing, and @@ -28,7 +28,29 @@ class Worker : public mbgl::util::noncopyable { // Together, this means that an object may make a work request with lambdas which // bind references to itself, and if and when those lambdas execute, the references // will still be valid. - std::unique_ptr send(Fn work, Fn after); + + using Request = std::unique_ptr; + + Request parseRasterTile( + RasterBucket&, + std::string data, + std::function callback); + + Request parseVectorTile( + TileWorker&, + std::string data, + std::function callback); + + Request parseLiveTile( + TileWorker&, + const LiveTile&, + std::function callback); + + Request redoPlacement( + TileWorker&, + float angle, + bool collisionDebug, + std::function callback); private: class Impl; From 3670a78569bd9f7f92b768a9c761b4d8d4b278e3 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 9 Jun 2015 10:45:51 -0400 Subject: [PATCH 14/20] Convert Worker tests to Thread tests --- test/miscellaneous/thread.cpp | 83 +++++++++++++++++++++++++++ test/miscellaneous/worker.cpp | 104 ---------------------------------- test/test.gypi | 1 - 3 files changed, 83 insertions(+), 105 deletions(-) delete mode 100644 test/miscellaneous/worker.cpp diff --git a/test/miscellaneous/thread.cpp b/test/miscellaneous/thread.cpp index 1206afc149c..68f8ddccdf4 100644 --- a/test/miscellaneous/thread.cpp +++ b/test/miscellaneous/thread.cpp @@ -136,3 +136,86 @@ TEST(Thread, context) { uv_run(uv_default_loop(), UV_RUN_DEFAULT); } + +class TestWorker { +public: + TestWorker(uv_loop_t*) {} + void send(std::function fn) { fn(); } +}; + +TEST(Thread, ExecutesAfter) { + RunLoop loop(uv_default_loop()); + Thread thread({"Test", ThreadType::Map, ThreadPriority::Regular}); + + bool didWork = false; + bool didAfter = false; + + auto request = thread.invokeWithResult(&TestWorker::send, [&] { + didAfter = true; + loop.stop(); + }, [&] { + didWork = true; + }); + + uv_run(uv_default_loop(), UV_RUN_DEFAULT); + + EXPECT_TRUE(didWork); + EXPECT_TRUE(didAfter); +} + +TEST(Thread, WorkRequestDeletionWaitsForWorkToComplete) { + RunLoop loop(uv_default_loop()); + Thread thread({"Test", ThreadType::Map, ThreadPriority::Regular}); + + std::promise started; + bool didWork = false; + + auto request = thread.invokeWithResult(&TestWorker::send, [&] {}, [&] { + started.set_value(); + usleep(10000); + didWork = true; + }); + + started.get_future().get(); + request.reset(); + EXPECT_TRUE(didWork); +} + +TEST(Thread, WorkRequestDeletionCancelsAfter) { + RunLoop loop(uv_default_loop()); + Thread thread({"Test", ThreadType::Map, ThreadPriority::Regular}); + + std::promise started; + bool didAfter = false; + + auto request = thread.invokeWithResult(&TestWorker::send, [&] { + didAfter = true; + }, [&] { + started.set_value(); + }); + + started.get_future().get(); + request.reset(); + uv_run(uv_default_loop(), UV_RUN_ONCE); + EXPECT_FALSE(didAfter); +} + +TEST(Thread, WorkRequestDeletionCancelsImmediately) { + RunLoop loop(uv_default_loop()); + Thread thread({"Test", ThreadType::Map, ThreadPriority::Regular}); + + std::promise started; + + auto request1 = thread.invokeWithResult(&TestWorker::send, [&] {}, [&] { + usleep(10000); + started.set_value(); + }); + + auto request2 = thread.invokeWithResult(&TestWorker::send, [&] {}, [&] { + ADD_FAILURE() << "Second work item should not be invoked"; + }); + request2.reset(); + + started.get_future().get(); + request1.reset(); +} diff --git a/test/miscellaneous/worker.cpp b/test/miscellaneous/worker.cpp deleted file mode 100644 index 3d3c781b8cb..00000000000 --- a/test/miscellaneous/worker.cpp +++ /dev/null @@ -1,104 +0,0 @@ -#include "../fixtures/util.hpp" - -#include -#include -#include - -using namespace mbgl; -using namespace mbgl::util; - -TEST(Worker, ExecutesWorkAndAfter) { - RunLoop loop(uv_default_loop()); - - Worker worker(1); - std::unique_ptr request; - - bool didWork = false; - bool didAfter = false; - - loop.invoke([&] { - request = worker.send([&] { - didWork = true; - }, [&] { - didAfter = true; - loop.stop(); - }); - }); - - uv_run(uv_default_loop(), UV_RUN_DEFAULT); - EXPECT_TRUE(didWork); - EXPECT_TRUE(didAfter); -} - -TEST(Worker, WorkRequestDeletionWaitsForWorkToComplete) { - RunLoop loop(uv_default_loop()); - - Worker worker(1); - std::promise started; - bool didWork = false; - - loop.invoke([&] { - auto request = worker.send([&] { - started.set_value(); - usleep(10000); - didWork = true; - }, [&] {}); - started.get_future().get(); - - request.reset(); - EXPECT_TRUE(didWork); - loop.stop(); - }); - - uv_run(uv_default_loop(), UV_RUN_DEFAULT); -} - -TEST(Worker, WorkRequestJoinCancelsAfter) { - RunLoop loop(uv_default_loop()); - - Worker worker(1); - std::promise started; - bool didAfter = false; - - loop.invoke([&] { - auto request = worker.send([&] { - started.set_value(); - }, [&] { - didAfter = true; - }); - started.get_future().get(); - - request.reset(); - loop.stop(); - }); - - uv_run(uv_default_loop(), UV_RUN_DEFAULT); - EXPECT_FALSE(didAfter); -} - -TEST(Worker, WorkRequestCancelsImmediately) { - RunLoop loop(uv_default_loop()); - - Worker worker(1); - - loop.invoke([&] { - std::promise started; - // First worker item. - auto request1 = worker.send([&] { - usleep(10000); - started.set_value(); - }, [&] {}); - - auto request2 = worker.send([&] { - ADD_FAILURE() << "Second work item should not be invoked"; - }, [&] {}); - request2.reset(); - - started.get_future().get(); - request1.reset(); - - loop.stop(); - }); - - uv_run(uv_default_loop(), UV_RUN_DEFAULT); -} diff --git a/test/test.gypi b/test/test.gypi index f3d6976dd22..ddbc5324565 100644 --- a/test/test.gypi +++ b/test/test.gypi @@ -59,7 +59,6 @@ 'miscellaneous/tile.cpp', 'miscellaneous/transform.cpp', 'miscellaneous/variant.cpp', - 'miscellaneous/worker.cpp', 'storage/storage.hpp', 'storage/storage.cpp', From d30fce5f7dbc42b46e020a9d7b45347473097d6d Mon Sep 17 00:00:00 2001 From: "Thiago Marcos P. Santos" Date: Wed, 10 Jun 2015 21:50:35 +0300 Subject: [PATCH 15/20] Catch exceptions when parsing tile PBFs Also improved the error message and updated the test case. --- src/mbgl/map/vector_tile_data.cpp | 4 +++- src/mbgl/util/worker.cpp | 8 ++++++-- test/style/resource_loading.cpp | 2 +- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/mbgl/map/vector_tile_data.cpp b/src/mbgl/map/vector_tile_data.cpp index be264084920..bf4c0f5814d 100644 --- a/src/mbgl/map/vector_tile_data.cpp +++ b/src/mbgl/map/vector_tile_data.cpp @@ -76,7 +76,9 @@ bool VectorTileData::reparse(Worker&, std::function callback) { if (result.is()) { state = result.get(); } else { - error = result.get(); + std::stringstream message; + message << "Failed to parse [" << std::string(id) << "]: " << result.get(); + error = message.str(); state = State::obsolete; } diff --git a/src/mbgl/util/worker.cpp b/src/mbgl/util/worker.cpp index 74408a7daf4..0d83bc49f4b 100644 --- a/src/mbgl/util/worker.cpp +++ b/src/mbgl/util/worker.cpp @@ -25,8 +25,12 @@ class Worker::Impl { } TileParseResult parseVectorTile(TileWorker* worker, std::string data) { - VectorTile vectorTile(pbf(reinterpret_cast(data.data()), data.size())); - return worker->parse(vectorTile); + try { + pbf tilePBF(reinterpret_cast(data.data()), data.size()); + return worker->parse(VectorTile(tilePBF)); + } catch (const std::exception& ex) { + return TileParseResult(ex.what()); + } } TileParseResult parseLiveTile(TileWorker* worker, const LiveTile* tile) { diff --git a/test/style/resource_loading.cpp b/test/style/resource_loading.cpp index 92d479b512f..c37e17d7131 100644 --- a/test/style/resource_loading.cpp +++ b/test/style/resource_loading.cpp @@ -159,7 +159,7 @@ TEST_P(ResourceLoading, RequestWithCorruptedData) { message << "Failed to parse "; if (param == "vector.pbf") { - message << "\\[15\\/1638(3|4)\\/1638(3|4)\\]\\: pbf unknown field type exception"; + message << "\\[1(5|6)\\/1638(3|4)\\/1638(3|4)\\]\\: pbf unknown field type exception"; } else { message << "\\[test\\/fixtures\\/resources\\/" << param << "\\]"; } From abdf205510b1f2d7b3e0c9b432f17c8c4d71e895 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Mon, 8 Jun 2015 20:16:40 -0400 Subject: [PATCH 16/20] =?UTF-8?q?Rename=20workerData=20=E2=87=A2=20tileWor?= =?UTF-8?q?ker?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/mbgl/map/live_tile_data.cpp | 2 +- src/mbgl/map/vector_tile_data.cpp | 12 ++++++------ src/mbgl/map/vector_tile_data.hpp | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/mbgl/map/live_tile_data.cpp b/src/mbgl/map/live_tile_data.cpp index a760e33fc32..22f9a54b4a2 100644 --- a/src/mbgl/map/live_tile_data.cpp +++ b/src/mbgl/map/live_tile_data.cpp @@ -37,7 +37,7 @@ bool LiveTileData::reparse(Worker& worker, std::function callback) { parsing = true; - workRequest = worker.parseLiveTile(workerData, *tile, [this, callback] (TileParseResult result) { + workRequest = worker.parseLiveTile(tileWorker, *tile, [this, callback] (TileParseResult result) { parsing = false; if (state == State::obsolete) { diff --git a/src/mbgl/map/vector_tile_data.cpp b/src/mbgl/map/vector_tile_data.cpp index bf4c0f5814d..d0f4b131f98 100644 --- a/src/mbgl/map/vector_tile_data.cpp +++ b/src/mbgl/map/vector_tile_data.cpp @@ -18,7 +18,7 @@ VectorTileData::VectorTileData(const TileID& id_, float angle, bool collisionDebug) : TileData(id_), - workerData(id_, + tileWorker(id_, style_, source_.max_zoom, state, @@ -66,7 +66,7 @@ bool VectorTileData::reparse(Worker&, std::function callback) { parsing = true; - workRequest = worker.parseVectorTile(workerData, data, [this, callback] (TileParseResult result) { + workRequest = worker.parseVectorTile(tileWorker, data, [this, callback] (TileParseResult result) { parsing = false; if (state == State::obsolete) { @@ -93,11 +93,11 @@ Bucket* VectorTileData::getBucket(const StyleLayer& layer) { return nullptr; } - return workerData.getBucket(layer); + return tileWorker.getBucket(layer); } size_t VectorTileData::countBuckets() const { - return workerData.countBuckets(); + return tileWorker.countBuckets(); } void VectorTileData::redoPlacement(float angle, bool collisionDebug) { @@ -114,8 +114,8 @@ void VectorTileData::redoPlacement(float angle, bool collisionDebug) { currentAngle = angle; currentCollisionDebug = collisionDebug; - workRequest = worker.redoPlacement(workerData, angle, collisionDebug, [this] { - for (const auto& layer : workerData.style.layers) { + workRequest = worker.redoPlacement(tileWorker, angle, collisionDebug, [this] { + for (const auto& layer : tileWorker.style.layers) { auto bucket = getBucket(*layer); if (bucket) { bucket->swapRenderData(); diff --git a/src/mbgl/map/vector_tile_data.hpp b/src/mbgl/map/vector_tile_data.hpp index e12554d85d8..6b866de376b 100644 --- a/src/mbgl/map/vector_tile_data.hpp +++ b/src/mbgl/map/vector_tile_data.hpp @@ -39,7 +39,7 @@ class VectorTileData : public TileData { void cancel() override; protected: - TileWorker workerData; + TileWorker tileWorker; std::unique_ptr workRequest; bool parsing = false; From db1202e8e335be540ded48029216ad9884fc5bb9 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 9 Jun 2015 10:08:20 -0400 Subject: [PATCH 17/20] Don't pass Worker& around so much --- src/mbgl/map/live_tile_data.cpp | 2 +- src/mbgl/map/live_tile_data.hpp | 2 +- src/mbgl/map/raster_tile_data.cpp | 13 +++++++------ src/mbgl/map/raster_tile_data.hpp | 8 ++++---- src/mbgl/map/source.cpp | 13 ++++++------- src/mbgl/map/tile_data.hpp | 6 ++---- src/mbgl/map/vector_tile_data.cpp | 8 ++++---- src/mbgl/map/vector_tile_data.hpp | 11 +++++------ 8 files changed, 30 insertions(+), 33 deletions(-) diff --git a/src/mbgl/map/live_tile_data.cpp b/src/mbgl/map/live_tile_data.cpp index 22f9a54b4a2..ad28c6c628d 100644 --- a/src/mbgl/map/live_tile_data.cpp +++ b/src/mbgl/map/live_tile_data.cpp @@ -24,7 +24,7 @@ LiveTileData::~LiveTileData() { cancel(); } -bool LiveTileData::reparse(Worker& worker, std::function callback) { +bool LiveTileData::reparse(std::function callback) { if (parsing || (state != State::loaded && state != State::partial)) { return false; } diff --git a/src/mbgl/map/live_tile_data.hpp b/src/mbgl/map/live_tile_data.hpp index c837057e82d..2d6b0106142 100644 --- a/src/mbgl/map/live_tile_data.hpp +++ b/src/mbgl/map/live_tile_data.hpp @@ -17,7 +17,7 @@ class LiveTileData : public VectorTileData { bool collisionDebug_); ~LiveTileData(); - bool reparse(Worker&, std::function callback) override; + bool reparse(std::function callback) override; private: AnnotationManager& annotationManager; diff --git a/src/mbgl/map/raster_tile_data.cpp b/src/mbgl/map/raster_tile_data.cpp index e5a26260351..837f4c0f1ba 100644 --- a/src/mbgl/map/raster_tile_data.cpp +++ b/src/mbgl/map/raster_tile_data.cpp @@ -12,9 +12,11 @@ using namespace mbgl; RasterTileData::RasterTileData(const TileID& id_, TexturePool &texturePool, - const SourceInfo &source_) + const SourceInfo &source_, + Worker& worker_) : TileData(id_), source(source_), + worker(worker_), bucket(texturePool, layout) { } @@ -22,14 +24,13 @@ RasterTileData::~RasterTileData() { cancel(); } -void RasterTileData::request(Worker& worker, - float pixelRatio, - const std::function& callback) { +void RasterTileData::request(float pixelRatio, + const std::function& callback) { std::string url = source.tileURL(id, pixelRatio); state = State::loading; FileSource* fs = util::ThreadContext::getFileSource(); - req = fs->request({ Resource::Kind::Tile, url }, util::RunLoop::current.get()->get(), [url, callback, &worker, this](const Response &res) { + req = fs->request({ Resource::Kind::Tile, url }, util::RunLoop::current.get()->get(), [url, callback, this](const Response &res) { req = nullptr; if (res.status != Response::Successful) { @@ -59,7 +60,7 @@ void RasterTileData::request(Worker& worker, }); } -bool RasterTileData::reparse(Worker&, std::function) { +bool RasterTileData::reparse(std::function) { assert(false); return false; } diff --git a/src/mbgl/map/raster_tile_data.hpp b/src/mbgl/map/raster_tile_data.hpp index e519874c033..ce0286f5a03 100644 --- a/src/mbgl/map/raster_tile_data.hpp +++ b/src/mbgl/map/raster_tile_data.hpp @@ -15,14 +15,13 @@ class WorkRequest; class RasterTileData : public TileData { public: - RasterTileData(const TileID&, TexturePool&, const SourceInfo&); + RasterTileData(const TileID&, TexturePool&, const SourceInfo&, Worker&); ~RasterTileData(); - void request(Worker&, - float pixelRatio, + void request(float pixelRatio, const std::function& callback) override; - bool reparse(Worker&, std::function callback) override; + bool reparse(std::function callback) override; void cancel() override; @@ -30,6 +29,7 @@ class RasterTileData : public TileData { private: const SourceInfo& source; + Worker& worker; Request* req = nullptr; StyleLayoutRaster layout; diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp index 0bac4c3c7fe..55761eb26df 100644 --- a/src/mbgl/map/source.cpp +++ b/src/mbgl/map/source.cpp @@ -225,7 +225,7 @@ TileData::State Source::hasTile(const TileID& id) { return TileData::State::invalid; } -bool Source::handlePartialTile(const TileID& id, Worker& worker) { +bool Source::handlePartialTile(const TileID& id, Worker&) { const TileID normalized_id = id.normalized(); auto it = tile_data.find(normalized_id); @@ -248,7 +248,7 @@ bool Source::handlePartialTile(const TileID& id, Worker& worker) { } }; - return data->reparse(worker, callback); + return data->reparse(callback); } TileData::State Source::addTile(MapData& data, @@ -293,16 +293,15 @@ TileData::State Source::addTile(MapData& data, new_tile.data = std::make_shared(normalized_id, style, info, transformState.getAngle(), data.getCollisionDebug()); - new_tile.data->request(style.workers, transformState.getPixelRatio(), callback); + new_tile.data->request(transformState.getPixelRatio(), callback); } else if (info.type == SourceType::Raster) { - new_tile.data = std::make_shared(normalized_id, texturePool, info); - new_tile.data->request( - style.workers, transformState.getPixelRatio(), callback); + new_tile.data = std::make_shared(normalized_id, texturePool, info, style.workers); + new_tile.data->request(transformState.getPixelRatio(), callback); } else if (info.type == SourceType::Annotations) { new_tile.data = std::make_shared(normalized_id, data.annotationManager, style, info, transformState.getAngle(), data.getCollisionDebug()); - new_tile.data->reparse(style.workers, callback); + new_tile.data->reparse(callback); } else { throw std::runtime_error("source type not implemented"); } diff --git a/src/mbgl/map/tile_data.hpp b/src/mbgl/map/tile_data.hpp index 5ed4ef4fd4b..5525256957c 100644 --- a/src/mbgl/map/tile_data.hpp +++ b/src/mbgl/map/tile_data.hpp @@ -70,16 +70,14 @@ class TileData : private util::noncopyable { TileData(const TileID&); virtual ~TileData() = default; - virtual void request(Worker&, - float pixelRatio, + virtual void request(float pixelRatio, const std::function& callback) = 0; // 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. - virtual bool reparse(Worker&, - std::function callback) = 0; + virtual bool reparse(std::function callback) = 0; // Mark this tile as no longer needed and cancel any pending work. virtual void cancel() = 0; diff --git a/src/mbgl/map/vector_tile_data.cpp b/src/mbgl/map/vector_tile_data.cpp index d0f4b131f98..9b04ab371a8 100644 --- a/src/mbgl/map/vector_tile_data.cpp +++ b/src/mbgl/map/vector_tile_data.cpp @@ -18,6 +18,7 @@ VectorTileData::VectorTileData(const TileID& id_, float angle, bool collisionDebug) : TileData(id_), + worker(style_.workers), tileWorker(id_, style_, source_.max_zoom, @@ -26,7 +27,6 @@ VectorTileData::VectorTileData(const TileID& id_, source_.tile_size * id.overscaling, angle, collisionDebug)), source(source_), - worker(style_.workers), lastAngle(angle), currentAngle(angle) { } @@ -35,7 +35,7 @@ VectorTileData::~VectorTileData() { cancel(); } -void VectorTileData::request(Worker&, float pixelRatio, const std::function& callback) { +void VectorTileData::request(float pixelRatio, const std::function& callback) { std::string url = source.tileURL(id, pixelRatio); state = State::loading; @@ -55,11 +55,11 @@ void VectorTileData::request(Worker&, float pixelRatio, const std::function callback) { +bool VectorTileData::reparse(std::function callback) { if (parsing || (state != State::loaded && state != State::partial)) { return false; } diff --git a/src/mbgl/map/vector_tile_data.hpp b/src/mbgl/map/vector_tile_data.hpp index 6b866de376b..a035015f4d9 100644 --- a/src/mbgl/map/vector_tile_data.hpp +++ b/src/mbgl/map/vector_tile_data.hpp @@ -25,20 +25,20 @@ class VectorTileData : public TileData { bool collisionDebug_); ~VectorTileData(); - void redoPlacement(float angle, bool collisionDebug) override; - Bucket* getBucket(const StyleLayer&) override; size_t countBuckets() const; - void request(Worker&, - float pixelRatio, + void request(float pixelRatio, const std::function& callback) override; - bool reparse(Worker&, std::function callback) override; + bool reparse(std::function callback) override; + + void redoPlacement(float angle, bool collisionDebug) override; void cancel() override; protected: + Worker& worker; TileWorker tileWorker; std::unique_ptr workRequest; bool parsing = false; @@ -47,7 +47,6 @@ class VectorTileData : public TileData { const SourceInfo& source; Request* req = nullptr; std::string data; - Worker& worker; float lastAngle = 0; float currentAngle; From 3ab682e627fa70d926908632ffbdb53694adc0d5 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 9 Jun 2015 11:37:00 -0400 Subject: [PATCH 18/20] Fix name shadowing --- src/mbgl/text/collision_tile.cpp | 4 ++-- src/mbgl/text/collision_tile.hpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/mbgl/text/collision_tile.cpp b/src/mbgl/text/collision_tile.cpp index 0d1b25af54d..3c2d22a5b28 100644 --- a/src/mbgl/text/collision_tile.cpp +++ b/src/mbgl/text/collision_tile.cpp @@ -91,11 +91,11 @@ void CollisionTile::insertFeature(CollisionFeature &feature, const float minPlac Box CollisionTile::getTreeBox(const vec2 &anchor, const CollisionBox &box) { return Box{ - Point{ + CollisionPoint{ anchor.x + box.x1, anchor.y + box.y1 * yStretch }, - Point{ + CollisionPoint{ anchor.x + box.x2, anchor.y + box.y2 * yStretch } diff --git a/src/mbgl/text/collision_tile.hpp b/src/mbgl/text/collision_tile.hpp index 882f347b469..aed9b59f321 100644 --- a/src/mbgl/text/collision_tile.hpp +++ b/src/mbgl/text/collision_tile.hpp @@ -27,8 +27,8 @@ namespace mbgl { namespace bg = boost::geometry; namespace bgm = bg::model; namespace bgi = bg::index; - typedef bgm::point Point; - typedef bgm::box Box; + typedef bgm::point CollisionPoint; + typedef bgm::box Box; typedef std::pair CollisionTreeBox; typedef bgi::rtree> Tree; From fc64e97bce3e082b0396e68bd6493457defe4ec7 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Thu, 11 Jun 2015 19:03:19 -0400 Subject: [PATCH 19/20] Simplify RunLoop Instead of transforming between return value and callback, pass the wrapped callback on to the invoked function as the last argument. This eliminates the need for multiple invokeWithArgument overloads and permits invoked functions to be asynchronous. --- platform/default/sqlite_cache.cpp | 10 +- platform/default/sqlite_cache_impl.hpp | 2 +- src/mbgl/map/source.cpp | 14 +-- src/mbgl/util/run_loop.hpp | 161 ++++++------------------- src/mbgl/util/thread.hpp | 21 ++-- src/mbgl/util/worker.cpp | 29 +++-- test/miscellaneous/thread.cpp | 61 +++++----- test/storage/database.cpp | 63 ++++++---- 8 files changed, 147 insertions(+), 214 deletions(-) diff --git a/platform/default/sqlite_cache.cpp b/platform/default/sqlite_cache.cpp index 3c23d167a6b..c1b1daf2344 100644 --- a/platform/default/sqlite_cache.cpp +++ b/platform/default/sqlite_cache.cpp @@ -131,10 +131,10 @@ std::unique_ptr SQLiteCache::get(const Resource &resource, Callback // Will try to load the URL from the SQLite database and call the callback when done. // Note that the callback is probably going to invoked from another thread, so the caller // must make sure that it can run in that thread. - return thread->invokeWithResult(&Impl::get, std::move(callback), resource); + return thread->invokeWithCallback(&Impl::get, callback, resource); } -std::unique_ptr SQLiteCache::Impl::get(const Resource &resource) { +void SQLiteCache::Impl::get(const Resource &resource, Callback callback) { try { // This is called in the SQLite event loop. if (!db) { @@ -167,14 +167,14 @@ std::unique_ptr SQLiteCache::Impl::get(const Resource &resource) { if (getStmt->get(5)) { // == compressed response->data = util::decompress(response->data); } - return std::move(response); + callback(std::move(response)); } else { // There is no data. - return nullptr; + callback(nullptr); } } catch (mapbox::sqlite::Exception& ex) { Log::Error(Event::Database, ex.code, ex.what()); - return nullptr; + callback(nullptr); } } diff --git a/platform/default/sqlite_cache_impl.hpp b/platform/default/sqlite_cache_impl.hpp index e01400a5894..86c7c07bdea 100644 --- a/platform/default/sqlite_cache_impl.hpp +++ b/platform/default/sqlite_cache_impl.hpp @@ -19,7 +19,7 @@ class SQLiteCache::Impl { Impl(uv_loop_t*, const std::string &path = ":memory:"); ~Impl(); - std::unique_ptr get(const Resource&); + void get(const Resource&, Callback); void put(const Resource& resource, std::shared_ptr response); void refresh(const Resource& resource, int64_t expires); diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp index 55761eb26df..f468ab33ed7 100644 --- a/src/mbgl/map/source.cpp +++ b/src/mbgl/map/source.cpp @@ -233,7 +233,9 @@ bool Source::handlePartialTile(const TileID& id, Worker&) { return true; } - util::ptr data = it->second.lock(); + // Note: this uses a raw pointer; we don't want the callback binding to have a + // shared pointer. + VectorTileData* data = static_cast(it->second.lock().get()); if (!data) { return true; } @@ -241,14 +243,12 @@ bool Source::handlePartialTile(const TileID& id, Worker&) { // 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) { + size_t bucketCount = data->countBuckets(); + return data->reparse([this, data, bucketCount]() { + if (data->countBuckets() > bucketCount) { emitTileLoaded(false); } - }; - - return data->reparse(callback); + }); } TileData::State Source::addTile(MapData& data, diff --git a/src/mbgl/util/run_loop.hpp b/src/mbgl/util/run_loop.hpp index 14021ef42fb..c0a56e956c9 100644 --- a/src/mbgl/util/run_loop.hpp +++ b/src/mbgl/util/run_loop.hpp @@ -25,7 +25,7 @@ class RunLoop : private util::noncopyable { template void invoke(Fn&& fn, Args&&... args) { auto tuple = std::make_tuple(std::move(args)...); - auto task = std::make_shared>( + auto task = std::make_shared>( std::move(fn), std::move(tuple)); @@ -33,44 +33,24 @@ class RunLoop : private util::noncopyable { async.send(); } - // Invoke fn(args...) on this RunLoop, then invoke callback() on the current RunLoop. - template + // Invoke fn(args...) on this RunLoop, then invoke callback(results...) on the current RunLoop. + template std::unique_ptr - invokeWithResult(Fn&& fn, std::function callback, Args&&... args) { - auto tuple = std::make_tuple(std::move(args)...); - auto task = std::make_shared>( - std::move(fn), - std::move(tuple)); - - // `task` is a shared pointer with ownership in the following three places: - // 1. In the `queue` of pending invocations. - // 2. In the `WorkRequest` result. - // 3. In the lambda binding of the callback to be executed on the invoking - // RunLoop. This last shared ownership is necessary in the case where - // callback execution has been scheduled (queued on the invoking RunLoop), - // but the other two places have released ownership -- i.e. the task was - // cancelled after the work is completed, but before the callback is - // executed. In this case, the lambda binding checks the cancellation flag - // and does not execute the original callback. - - task->bind(callback); + invokeWithCallback(Fn&& fn, Cb&& callback, Args&&... args) { + auto flag = std::make_shared(); + *flag = false; - withMutex([&] { queue.push(task); }); - async.send(); - - return std::make_unique(task); - } + auto after = RunLoop::current.get()->bind([flag, callback] (auto&&... results) { + if (!*flag) { + callback(std::move(results)...); + } + }); - // Invoke fn(args...) on this RunLoop, then invoke callback(result) on the current RunLoop. - template - std::unique_ptr - invokeWithResult(Fn&& fn, std::function callback, Args&&... args) { - auto tuple = std::make_tuple(std::move(args)...); - auto task = std::make_shared>( + auto tuple = std::make_tuple(std::move(args)..., after); + auto task = std::make_shared>( std::move(fn), - std::move(tuple)); - - task->bind(callback); + std::move(tuple), + flag); withMutex([&] { queue.push(task); }); async.send(); @@ -79,10 +59,11 @@ class RunLoop : private util::noncopyable { } // Return a function that invokes the given function on this RunLoop. - template - auto bind(std::function fn) { - return [this, fn = std::move(fn)] (Args&&... args) { - invoke(std::move(fn), std::move(args)...); + template + auto bind(Fn&& fn) { + return [this, fn = std::move(fn)] (auto&&... args) { + // `this->` is a workaround for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61636 + this->invoke(std::move(fn), std::move(args)...); }; } @@ -91,111 +72,47 @@ class RunLoop : private util::noncopyable { static uv::tls current; private: - template - class Invoker : public WorkTask, public std::enable_shared_from_this> { + template + class Invoker : public WorkTask { public: - Invoker(F&& f, P&& p) - : func(std::move(f)), + Invoker(F&& f, P&& p, std::shared_ptr canceled_ = nullptr) + : canceled(canceled_), + func(std::move(f)), params(std::move(p)) { } - using C = std::function; - - void bind(C after) { - auto task = this->shared_from_this(); - callback = RunLoop::current.get()->bind(C([task, after] { - if (!task->canceled) { - after(); - } - })); - } - - void operator()() override { - // We are only running the task when there's an after callback to call. This means that an - // empty after callback will be treated as a cancelled request. The mutex will be locked while - // processing so that the cancel() callback will block. - std::lock_guard lock(mutex); - if (!canceled) { - invoke(std::index_sequence_for{}); - } - } - - void cancel() override { - // Remove the after callback to indicate that this callback has been canceled. The mutex will - // block when the task is currently in progres. When the task has not begun yet, the runTask() - // method will not do anything. When the task has been completed already, and the after callback - // was run as well, this will also do nothing. - std::lock_guard lock(mutex); - canceled = true; - } - - private: - template - void invoke(std::index_sequence) { - func(std::forward(std::get(params))...); - if (callback) { - callback(); - } - } - - std::mutex mutex; - bool canceled = false; - - F func; - P params; - C callback; - }; - - template - class InvokerWithResult : public WorkTask, public std::enable_shared_from_this> { - public: - InvokerWithResult(F&& f, P&& p) - : func(std::move(f)), - params(std::move(p)) { - } - - using C = std::function; - - void bind(C after) { - auto task = this->shared_from_this(); - callback = RunLoop::current.get()->bind(C([task, after] (R result) { - if (!task->canceled) { - after(std::move(result)); - } - })); - } - void operator()() override { - // We are only running the task when there's an after callback to call. This means that an - // empty after callback will be treated as a cancelled request. The mutex will be locked while - // processing so that the cancel() callback will block. + // Lock the mutex while processing so that cancel() will block. std::lock_guard lock(mutex); - if (!canceled) { - invoke(std::index_sequence_for{}); + if (!canceled || !*canceled) { + invoke(std::make_index_sequence::value>{}); } } + // If the task has not yet begun, this will cancel it. + // If the task is in progress, this will block until it completed. (Currently + // necessary because of shared state, but should be removed.) It will also + // cancel the after callback. + // If the task has completed, but the after callback has not executed, this + // will cancel the after callback. + // If the task has completed and the after callback has executed, this will + // do nothing. void cancel() override { - // Remove the after callback to indicate that this callback has been canceled. The mutex will - // block when the task is currently in progres. When the task has not begun yet, the runTask() - // method will not do anything. When the task has been completed already, and the after callback - // was run as well, this will also do nothing. std::lock_guard lock(mutex); - canceled = true; + *canceled = true; } private: template void invoke(std::index_sequence) { - callback(func(std::forward(std::get(params))...)); + func(std::get(std::forward

(params))...); } std::mutex mutex; - bool canceled = false; + std::shared_ptr canceled; F func; P params; - C callback; }; using Queue = std::queue>; diff --git a/src/mbgl/util/thread.hpp b/src/mbgl/util/thread.hpp index 16bb81db1a8..88a709cfa64 100644 --- a/src/mbgl/util/thread.hpp +++ b/src/mbgl/util/thread.hpp @@ -32,21 +32,14 @@ class Thread { // Invoke object->fn(args...) in the runloop thread. template void invoke(Fn fn, Args&&... args) { - loop->invoke(bind(fn), std::forward(args)...); + loop->invoke(bind(fn), std::forward(args)...); } // Invoke object->fn(args...) in the runloop thread, then invoke callback(result) in the current thread. - template + template std::unique_ptr - invokeWithResult(Fn fn, std::function callback, Args&&... args) { - return loop->invokeWithResult(bind(fn), callback, std::forward(args)...); - } - - // Invoke object->fn(args...) in the runloop thread, then invoke callback(result) in the current thread. - template - std::unique_ptr - invokeWithResult(Fn fn, std::function callback, Args&&... args) { - return loop->invokeWithResult(bind(fn), callback, std::forward(args)...); + invokeWithCallback(Fn fn, Cb&& callback, Args&&... args) { + return loop->invokeWithCallback(bind(fn), callback, std::forward(args)...); } // Invoke object->fn(args...) in the runloop thread, and wait for the result. @@ -73,9 +66,11 @@ class Thread { Thread& operator=(const Thread&) = delete; Thread& operator=(Thread&&) = delete; - template + template auto bind(Fn fn) { - return [fn, this] (Args&&... a) { return (object->*fn)(std::forward(a)...); }; + return [fn, this] (auto &&... args) { + return (object->*fn)(std::forward(args)...); + }; } template diff --git a/src/mbgl/util/worker.cpp b/src/mbgl/util/worker.cpp index 0d83bc49f4b..cc9fa156672 100644 --- a/src/mbgl/util/worker.cpp +++ b/src/mbgl/util/worker.cpp @@ -20,25 +20,30 @@ class Worker::Impl { util::ThreadContext::setFileSource(fs); } - bool parseRasterTile(RasterBucket* bucket, std::string data) { - return bucket->setImage(data); + void parseRasterTile(RasterBucket* bucket, std::string data, std::function callback) { + callback(bucket->setImage(data)); } - TileParseResult parseVectorTile(TileWorker* worker, std::string data) { + void parseVectorTile(TileWorker* worker, std::string data, std::function callback) { try { pbf tilePBF(reinterpret_cast(data.data()), data.size()); - return worker->parse(VectorTile(tilePBF)); + callback(worker->parse(VectorTile(tilePBF))); } catch (const std::exception& ex) { - return TileParseResult(ex.what()); + callback(TileParseResult(ex.what())); } } - TileParseResult parseLiveTile(TileWorker* worker, const LiveTile* tile) { - return worker->parse(*tile); + void parseLiveTile(TileWorker* worker, const LiveTile* tile, std::function callback) { + try { + callback(worker->parse(*tile)); + } catch (const std::exception& ex) { + callback(TileParseResult(ex.what())); + } } - void redoPlacement(TileWorker* worker, float angle, bool collisionDebug) { + void redoPlacement(TileWorker* worker, float angle, bool collisionDebug, std::function callback) { worker->redoPlacement(angle, collisionDebug); + callback(); } }; @@ -53,22 +58,22 @@ Worker::~Worker() = default; std::unique_ptr Worker::parseRasterTile(RasterBucket& bucket, std::string data, std::function callback) { current = (current + 1) % threads.size(); - return threads[current]->invokeWithResult(&Worker::Impl::parseRasterTile, callback, &bucket, data); + return threads[current]->invokeWithCallback(&Worker::Impl::parseRasterTile, callback, &bucket, data); } std::unique_ptr Worker::parseVectorTile(TileWorker& worker, std::string data, std::function callback) { current = (current + 1) % threads.size(); - return threads[current]->invokeWithResult(&Worker::Impl::parseVectorTile, callback, &worker, data); + return threads[current]->invokeWithCallback(&Worker::Impl::parseVectorTile, callback, &worker, data); } std::unique_ptr Worker::parseLiveTile(TileWorker& worker, const LiveTile& tile, std::function callback) { current = (current + 1) % threads.size(); - return threads[current]->invokeWithResult(&Worker::Impl::parseLiveTile, callback, &worker, &tile); + return threads[current]->invokeWithCallback(&Worker::Impl::parseLiveTile, callback, &worker, &tile); } std::unique_ptr Worker::redoPlacement(TileWorker& worker, float angle, bool collisionDebug, std::function callback) { current = (current + 1) % threads.size(); - return threads[current]->invokeWithResult(&Worker::Impl::redoPlacement, callback, &worker, angle, collisionDebug); + return threads[current]->invokeWithCallback(&Worker::Impl::redoPlacement, callback, &worker, angle, collisionDebug); } } // end namespace mbgl diff --git a/test/miscellaneous/thread.cpp b/test/miscellaneous/thread.cpp index 68f8ddccdf4..78e6ffee6bd 100644 --- a/test/miscellaneous/thread.cpp +++ b/test/miscellaneous/thread.cpp @@ -17,9 +17,9 @@ class TestObject { EXPECT_EQ(val, 1); } - int fn2() { + void fn2(std::function cb) { EXPECT_EQ(tid, std::this_thread::get_id()); - return 1; + cb(1); } void transferIn(std::unique_ptr val) { @@ -27,15 +27,15 @@ class TestObject { EXPECT_EQ(*val, 1); } - std::unique_ptr transferOut() { + void transferOut(std::function)> cb) { EXPECT_EQ(tid, std::this_thread::get_id()); - return std::make_unique(1); + cb(std::make_unique(1)); } - std::unique_ptr transferInOut(std::unique_ptr val) { + void transferInOut(std::unique_ptr val, std::function)> cb) { EXPECT_EQ(tid, std::this_thread::get_id()); EXPECT_EQ(*val, 1); - return std::move(val); + cb(std::move(val)); } void transferInShared(std::shared_ptr val) { @@ -43,21 +43,21 @@ class TestObject { EXPECT_EQ(*val, 1); } - std::shared_ptr transferOutShared() { + void transferOutShared(std::function)> cb) { EXPECT_EQ(tid, std::this_thread::get_id()); - return std::make_shared(1); + cb(std::make_shared(1)); } - std::string transferString(const std::string& string) { + void transferString(const std::string& string, std::function cb) { EXPECT_EQ(tid, std::this_thread::get_id()); EXPECT_EQ(string, "test"); - return string; + cb(string); } - bool checkContext() const { - return ThreadContext::currentlyOn(ThreadType::Worker) + void checkContext(std::function cb) const { + cb(ThreadContext::currentlyOn(ThreadType::Worker) && ThreadContext::getName() == "Test" - && ThreadContext::getPriority() == ThreadPriority::Low; + && ThreadContext::getPriority() == ThreadPriority::Low); } const std::thread::id tid; @@ -74,35 +74,35 @@ TEST(Thread, invoke) { Thread thread({"Test", ThreadType::Map, ThreadPriority::Regular}, tid); thread.invoke(&TestObject::fn1, 1); - requests.push_back(thread.invokeWithResult(&TestObject::fn2, [&] (int result) { + requests.push_back(thread.invokeWithCallback(&TestObject::fn2, [&] (int result) { EXPECT_EQ(tid, std::this_thread::get_id()); EXPECT_EQ(result, 1); })); thread.invoke(&TestObject::transferIn, std::make_unique(1)); - requests.push_back(thread.invokeWithResult>(&TestObject::transferOut, [&] (std::unique_ptr result) { + requests.push_back(thread.invokeWithCallback(&TestObject::transferOut, [&] (std::unique_ptr result) { EXPECT_EQ(tid, std::this_thread::get_id()); EXPECT_EQ(*result, 1); })); - requests.push_back(thread.invokeWithResult>(&TestObject::transferInOut, [&] (std::unique_ptr result) { + requests.push_back(thread.invokeWithCallback(&TestObject::transferInOut, [&] (std::unique_ptr result) { EXPECT_EQ(tid, std::this_thread::get_id()); EXPECT_EQ(*result, 1); }, std::make_unique(1))); thread.invoke(&TestObject::transferInShared, std::make_shared(1)); - requests.push_back(thread.invokeWithResult>(&TestObject::transferOutShared, [&] (std::shared_ptr result) { + requests.push_back(thread.invokeWithCallback(&TestObject::transferOutShared, [&] (std::shared_ptr result) { EXPECT_EQ(tid, std::this_thread::get_id()); EXPECT_EQ(*result, 1); })); // Cancelled request - thread.invokeWithResult(&TestObject::fn1, [&] { + thread.invokeWithCallback(&TestObject::fn2, [&] (int) { ADD_FAILURE(); - }, 1); + }); std::string test("test"); - requests.push_back(thread.invokeWithResult(&TestObject::transferString, [&] (std::string result){ + requests.push_back(thread.invokeWithCallback(&TestObject::transferString, [&] (std::string result){ EXPECT_EQ(tid, std::this_thread::get_id()); EXPECT_EQ(result, "test"); loop.stop(); @@ -128,7 +128,7 @@ TEST(Thread, context) { loop.invoke([&] { Thread thread({"Test", ThreadType::Worker, ThreadPriority::Low}, tid); - requests.push_back(thread.invokeWithResult(&TestObject::checkContext, [&] (bool inTestThreadContext) { + requests.push_back(thread.invokeWithCallback(&TestObject::checkContext, [&] (bool inTestThreadContext) { EXPECT_EQ(inTestThreadContext, true); loop.stop(); })); @@ -139,8 +139,13 @@ TEST(Thread, context) { class TestWorker { public: - TestWorker(uv_loop_t*) {} - void send(std::function fn) { fn(); } + TestWorker(uv_loop_t*) { + } + + void send(std::function fn, std::function cb) { + fn(); + cb(); + } }; TEST(Thread, ExecutesAfter) { @@ -150,7 +155,7 @@ TEST(Thread, ExecutesAfter) { bool didWork = false; bool didAfter = false; - auto request = thread.invokeWithResult(&TestWorker::send, [&] { + auto request = thread.invokeWithCallback(&TestWorker::send, [&] { didAfter = true; loop.stop(); }, [&] { @@ -170,7 +175,7 @@ TEST(Thread, WorkRequestDeletionWaitsForWorkToComplete) { std::promise started; bool didWork = false; - auto request = thread.invokeWithResult(&TestWorker::send, [&] {}, [&] { + auto request = thread.invokeWithCallback(&TestWorker::send, [&] {}, [&] { started.set_value(); usleep(10000); didWork = true; @@ -188,7 +193,7 @@ TEST(Thread, WorkRequestDeletionCancelsAfter) { std::promise started; bool didAfter = false; - auto request = thread.invokeWithResult(&TestWorker::send, [&] { + auto request = thread.invokeWithCallback(&TestWorker::send, [&] { didAfter = true; }, [&] { started.set_value(); @@ -206,12 +211,12 @@ TEST(Thread, WorkRequestDeletionCancelsImmediately) { std::promise started; - auto request1 = thread.invokeWithResult(&TestWorker::send, [&] {}, [&] { + auto request1 = thread.invokeWithCallback(&TestWorker::send, [&] {}, [&] { usleep(10000); started.set_value(); }); - auto request2 = thread.invokeWithResult(&TestWorker::send, [&] {}, [&] { + auto request2 = thread.invokeWithCallback(&TestWorker::send, [&] {}, [&] { ADD_FAILURE() << "Second work item should not be invoked"; }); request2.reset(); diff --git a/test/storage/database.cpp b/test/storage/database.cpp index b54b69a1333..a7560262fec 100644 --- a/test/storage/database.cpp +++ b/test/storage/database.cpp @@ -15,8 +15,9 @@ TEST_F(Storage, DatabaseDoesNotExist) { SQLiteCache::Impl cache(nullptr, "test/fixtures/404/cache.db"); - std::unique_ptr res = cache.get({ Resource::Unknown, "mapbox://test" }); - EXPECT_EQ(nullptr, res.get()); + cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr res) { + EXPECT_EQ(nullptr, res.get()); + }); auto observer = Log::removeObserver(); EXPECT_EQ(1ul, dynamic_cast(observer.get())->count({ EventSeverity::Error, Event::Database, 14, "unable to open database file" })); @@ -56,8 +57,9 @@ TEST_F(Storage, DatabaseCreate) { SQLiteCache::Impl cache(nullptr, "test/fixtures/database/cache.db"); - std::unique_ptr res = cache.get({ Resource::Unknown, "mapbox://test" }); - EXPECT_EQ(nullptr, res.get()); + cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr res) { + EXPECT_EQ(nullptr, res.get()); + }); Log::removeObserver(); } @@ -115,8 +117,9 @@ TEST_F(Storage, DatabaseLockedRead) { // First request should fail. Log::setObserver(std::make_unique()); - std::unique_ptr res = cache.get({ Resource::Unknown, "mapbox://test" }); - EXPECT_EQ(nullptr, res.get()); + cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr res) { + EXPECT_EQ(nullptr, res.get()); + }); // Make sure that we got a few "database locked" errors auto observer = Log::removeObserver(); @@ -131,8 +134,9 @@ TEST_F(Storage, DatabaseLockedRead) { // First, try getting a file (the cache value should not exist). Log::setObserver(std::make_unique()); - std::unique_ptr res = cache.get({ Resource::Unknown, "mapbox://test" }); - EXPECT_EQ(nullptr, res.get()); + cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr res) { + EXPECT_EQ(nullptr, res.get()); + }); // Make sure that we got a no errors Log::removeObserver(); @@ -157,8 +161,9 @@ TEST_F(Storage, DatabaseLockedWrite) { auto response = std::make_shared(); cache.put({ Resource::Unknown, "mapbox://test" }, response); - std::unique_ptr res = cache.get({ Resource::Unknown, "mapbox://test" }); - EXPECT_EQ(nullptr, res.get()); + cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr res) { + EXPECT_EQ(nullptr, res.get()); + }); auto observer = Log::removeObserver(); auto flo = dynamic_cast(observer.get()); @@ -175,9 +180,10 @@ TEST_F(Storage, DatabaseLockedWrite) { auto response = std::make_shared(); response->data = "Demo"; cache.put({ Resource::Unknown, "mapbox://test" }, response); - std::unique_ptr res = cache.get({ Resource::Unknown, "mapbox://test" }); - EXPECT_NE(nullptr, res.get()); - EXPECT_EQ("Demo", res->data); + cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr res) { + EXPECT_NE(nullptr, res.get()); + EXPECT_EQ("Demo", res->data); + }); // Make sure that we got a no errors Log::removeObserver(); @@ -206,8 +212,9 @@ TEST_F(Storage, DatabaseLockedRefresh) { auto response = std::make_shared(); response->data = "Demo"; cache.put({ Resource::Unknown, "mapbox://test" }, response); - std::unique_ptr res = cache.get({ Resource::Unknown, "mapbox://test" }); - EXPECT_EQ(nullptr, res.get()); + cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr res) { + EXPECT_EQ(nullptr, res.get()); + }); auto observer = Log::removeObserver(); auto flo = dynamic_cast(observer.get()); @@ -221,8 +228,9 @@ TEST_F(Storage, DatabaseLockedRefresh) { auto response = std::make_shared(); response->data = "Demo"; cache.refresh({ Resource::Unknown, "mapbox://test" }, response->expires); - std::unique_ptr res = cache.get({ Resource::Unknown, "mapbox://test" }); - EXPECT_EQ(nullptr, res.get()); + cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr res) { + EXPECT_EQ(nullptr, res.get()); + }); // Make sure that we got the right errors. auto observer = Log::removeObserver(); @@ -249,9 +257,10 @@ TEST_F(Storage, DatabaseDeleted) { auto response = std::make_shared(); response->data = "Demo"; cache.put({ Resource::Unknown, "mapbox://test" }, response); - std::unique_ptr res = cache.get({ Resource::Unknown, "mapbox://test" }); - EXPECT_NE(nullptr, res.get()); - EXPECT_EQ("Demo", res->data); + cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr res) { + EXPECT_NE(nullptr, res.get()); + EXPECT_EQ("Demo", res->data); + }); Log::removeObserver(); } @@ -265,9 +274,10 @@ TEST_F(Storage, DatabaseDeleted) { auto response = std::make_shared(); response->data = "Demo"; cache.put({ Resource::Unknown, "mapbox://test" }, response); - std::unique_ptr res = cache.get({ Resource::Unknown, "mapbox://test" }); - EXPECT_NE(nullptr, res.get()); - EXPECT_EQ("Demo", res->data); + cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr res) { + EXPECT_NE(nullptr, res.get()); + EXPECT_EQ("Demo", res->data); + }); auto observer = Log::removeObserver(); auto flo = dynamic_cast(observer.get()); @@ -294,9 +304,10 @@ TEST_F(Storage, DatabaseInvalid) { auto response = std::make_shared(); response->data = "Demo"; cache.put({ Resource::Unknown, "mapbox://test" }, response); - std::unique_ptr res = cache.get({ Resource::Unknown, "mapbox://test" }); - EXPECT_NE(nullptr, res.get()); - EXPECT_EQ("Demo", res->data); + cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr res) { + EXPECT_NE(nullptr, res.get()); + EXPECT_EQ("Demo", res->data); + }); auto observer = Log::removeObserver(); auto flo = dynamic_cast(observer.get()); From c0af683f2eaa8963103721284dbf0a08b3d10f77 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Thu, 18 Jun 2015 18:33:29 -0700 Subject: [PATCH 20/20] Reapply style layer copy technique --- src/mbgl/map/tile_worker.cpp | 8 +++++--- src/mbgl/map/tile_worker.hpp | 7 ++++++- src/mbgl/map/vector_tile_data.cpp | 3 ++- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/mbgl/map/tile_worker.cpp b/src/mbgl/map/tile_worker.cpp index b69698689ad..790a34cf5c6 100644 --- a/src/mbgl/map/tile_worker.cpp +++ b/src/mbgl/map/tile_worker.cpp @@ -13,10 +13,12 @@ using namespace mbgl; TileWorker::TileWorker(const TileID& id_, Style& style_, + std::vector> layers_, const uint16_t maxZoom_, const std::atomic& state_, std::unique_ptr collision_) - : style(style_), + : layers(std::move(layers_)), + style(style_), id(id_), maxZoom(maxZoom_), state(state_), @@ -48,7 +50,7 @@ size_t TileWorker::countBuckets() const { TileParseResult TileWorker::parse(const GeometryTile& geometryTile) { partialParse = false; - for (const auto& layer : style.layers) { + for (const auto& layer : layers) { parseLayer(*layer, geometryTile); } @@ -58,7 +60,7 @@ TileParseResult TileWorker::parse(const GeometryTile& geometryTile) { void TileWorker::redoPlacement(float angle, bool collisionDebug) { collision->reset(angle, 0); collision->setDebug(collisionDebug); - for (const auto& layer_desc : style.layers) { + for (const auto& layer_desc : layers) { auto bucket = getBucket(*layer_desc); if (bucket) { bucket->placeFeatures(); diff --git a/src/mbgl/map/tile_worker.hpp b/src/mbgl/map/tile_worker.hpp index 7fb474a5eed..d8c8aa3e00d 100644 --- a/src/mbgl/map/tile_worker.hpp +++ b/src/mbgl/map/tile_worker.hpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -20,6 +21,7 @@ class CollisionTile; class GeometryTile; class Style; class Bucket; +class StyleLayer; class StyleBucket; class GeometryTileLayer; @@ -31,6 +33,7 @@ class TileWorker : public util::noncopyable { public: TileWorker(const TileID&, Style&, + std::vector>, const uint16_t maxZoom, const std::atomic&, std::unique_ptr); @@ -42,7 +45,7 @@ class TileWorker : public util::noncopyable { TileParseResult parse(const GeometryTile&); void redoPlacement(float angle, bool collisionDebug); - Style& style; + std::vector> layers; private: void parseLayer(const StyleLayer&, const GeometryTile&); @@ -54,6 +57,8 @@ class TileWorker : public util::noncopyable { template void addBucketGeometries(Bucket&, const GeometryTileLayer&, const FilterExpression&); + Style& style; + const TileID id; const uint16_t maxZoom; const std::atomic& state; diff --git a/src/mbgl/map/vector_tile_data.cpp b/src/mbgl/map/vector_tile_data.cpp index 9b04ab371a8..04ef339c436 100644 --- a/src/mbgl/map/vector_tile_data.cpp +++ b/src/mbgl/map/vector_tile_data.cpp @@ -21,6 +21,7 @@ VectorTileData::VectorTileData(const TileID& id_, worker(style_.workers), tileWorker(id_, style_, + style_.layers, source_.max_zoom, state, std::make_unique(id_.z, 4096, @@ -115,7 +116,7 @@ void VectorTileData::redoPlacement(float angle, bool collisionDebug) { currentCollisionDebug = collisionDebug; workRequest = worker.redoPlacement(tileWorker, angle, collisionDebug, [this] { - for (const auto& layer : tileWorker.style.layers) { + for (const auto& layer : tileWorker.layers) { auto bucket = getBucket(*layer); if (bucket) { bucket->swapRenderData();