From ac1dcecef6bb6eab52ef157a086bdbf20d70acc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Wed, 13 May 2015 10:38:43 +0200 Subject: Merge pull request #1532 from mapbox/1465-text_collision Calculate text collision correctly --- src/mbgl/map/live_tile_data.cpp | 10 +++++----- src/mbgl/map/raster_tile_data.cpp | 6 +++--- src/mbgl/map/source.cpp | 22 +++++++++++----------- src/mbgl/map/tile_cache.cpp | 2 +- src/mbgl/map/tile_data.cpp | 9 +++++++-- src/mbgl/map/tile_data.hpp | 23 ++++++++++++++++++++--- src/mbgl/map/tile_parser.cpp | 13 ++++++++----- src/mbgl/map/tile_parser.hpp | 2 -- src/mbgl/map/vector_tile_data.cpp | 24 +++++++++++++++++------- src/mbgl/map/vector_tile_data.hpp | 12 ++++++++++-- src/mbgl/renderer/painter.cpp | 2 +- 11 files changed, 83 insertions(+), 42 deletions(-) diff --git a/src/mbgl/map/live_tile_data.cpp b/src/mbgl/map/live_tile_data.cpp index 096e899d8f..59f7b4de3d 100644 --- a/src/mbgl/map/live_tile_data.cpp +++ b/src/mbgl/map/live_tile_data.cpp @@ -21,7 +21,7 @@ LiveTileData::LiveTileData(const TileID& id_, spriteAtlas_, sprite_, source_), annotationManager(annotationManager_) { // live features are always ready - state = State::loaded; + setState(State::loaded); } LiveTileData::~LiveTileData() { @@ -31,7 +31,7 @@ LiveTileData::~LiveTileData() { } void LiveTileData::parse() { - if (state != State::loaded) { + if (getState() != State::loaded) { return; } @@ -46,12 +46,12 @@ void LiveTileData::parse() { 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()); - state = State::obsolete; + setState(State::obsolete); return; } } - if (state != State::obsolete) { - state = State::parsed; + if (getState() != State::obsolete) { + setState(State::parsed); } } diff --git a/src/mbgl/map/raster_tile_data.cpp b/src/mbgl/map/raster_tile_data.cpp index 20c07b573d..0f8efe3b48 100644 --- a/src/mbgl/map/raster_tile_data.cpp +++ b/src/mbgl/map/raster_tile_data.cpp @@ -15,14 +15,14 @@ RasterTileData::~RasterTileData() { } void RasterTileData::parse() { - if (state != State::loaded) { + if (getState() != State::loaded) { return; } if (bucket.setImage(data)) { - state = State::parsed; + setState(State::parsed); } else { - state = State::invalid; + setState(State::invalid); } } diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp index 8b0bf905ff..e663ef3d58 100644 --- a/src/mbgl/map/source.cpp +++ b/src/mbgl/map/source.cpp @@ -127,7 +127,7 @@ bool Source::isLoaded() const { } for (const auto& tile : tiles) { - if (tile.second->data->state != TileData::State::parsed) { + if (tile.second->data->getState() != TileData::State::parsed) { return false; } } @@ -193,11 +193,11 @@ void Source::finishRender(Painter &painter) { } } -std::forward_list Source::getLoadedTiles() const { - std::forward_list ptrs; +std::forward_list Source::getLoadedTiles() const { + std::forward_list ptrs; auto it = ptrs.before_begin(); - for (const auto &pair : tiles) { - if (pair.second->data->ready()) { + for (const auto& pair : tiles) { + if (pair.second->data->isReady()) { it = ptrs.insert_after(it, pair.second.get()); } } @@ -211,16 +211,16 @@ const std::vector& Source::getTiles() const { TileData::State Source::hasTile(const TileID& id) { auto it = tiles.find(id); if (it != tiles.end()) { - Tile &tile = *it->second; + Tile& tile = *it->second; if (tile.id == id && tile.data) { - return tile.data->state; + return tile.data->getState(); } } return TileData::State::invalid; } -void Source::handlePartialTile(const TileID &id, Worker &worker) { +void Source::handlePartialTile(const TileID& id, Worker& worker) { const TileID normalized_id = id.normalized(); auto it = tile_data.find(normalized_id); @@ -265,7 +265,7 @@ TileData::State Source::addTile(MapData& data, new_tile.data = it->second.lock(); } - if (new_tile.data && new_tile.data->state == TileData::State::obsolete) { + if (new_tile.data && new_tile.data->getState() == TileData::State::obsolete) { // Do not consider the tile if it's already obsolete. new_tile.data.reset(); } @@ -296,7 +296,7 @@ TileData::State Source::addTile(MapData& data, tile_data.emplace(new_tile.data->id, new_tile.data); } - return new_tile.data->state; + return new_tile.data->getState(); } double Source::getZoom(const TransformState& state) const { @@ -453,7 +453,7 @@ void Source::update(MapData& data, bool obsolete = std::find(retain.begin(), retain.end(), tile.id) == retain.end(); if (!obsolete) { retain_data.insert(tile.data->id); - } else if (type != SourceType::Raster && tile.data->state == TileData::State::parsed) { + } else if (type != SourceType::Raster && tile.data->getState() == TileData::State::parsed) { // Partially parsed tiles are never added to the cache because otherwise // they never get updated if the go out from the viewport and the pending // resources arrive. diff --git a/src/mbgl/map/tile_cache.cpp b/src/mbgl/map/tile_cache.cpp index 25aed6d15e..37acf628a0 100644 --- a/src/mbgl/map/tile_cache.cpp +++ b/src/mbgl/map/tile_cache.cpp @@ -46,7 +46,7 @@ std::shared_ptr TileCache::get(uint64_t key) { data = it->second; tiles.erase(it); orderedKeys.remove(key); - assert(data->ready()); + assert(data->isReady()); } return data; diff --git a/src/mbgl/map/tile_data.cpp b/src/mbgl/map/tile_data.cpp index c6d87c7bde..aa66ceb80b 100644 --- a/src/mbgl/map/tile_data.cpp +++ b/src/mbgl/map/tile_data.cpp @@ -12,10 +12,9 @@ using namespace mbgl; TileData::TileData(const TileID& id_, const SourceInfo& source_) : id(id_), name(id), - state(State::initial), - parsing(ATOMIC_FLAG_INIT), source(source_), env(Environment::Get()), + state(State::initial), debugBucket(debugFontBuffer) { // Initialize tile debug coordinates debugFontBuffer.addText(name.c_str(), 50, 200, 5); @@ -29,6 +28,12 @@ const std::string TileData::toString() const { return std::string { "[tile " } + name + "]"; } +void TileData::setState(const State& state_) { + assert(!isImmutable()); + + state = state_; +} + void TileData::request(Worker& worker, float pixelRatio, std::function callback) { std::string url = source.tileURL(id, pixelRatio); state = State::loading; diff --git a/src/mbgl/map/tile_data.hpp b/src/mbgl/map/tile_data.hpp index ecc53a6b5e..7f16e7c413 100644 --- a/src/mbgl/map/tile_data.hpp +++ b/src/mbgl/map/tile_data.hpp @@ -49,10 +49,24 @@ public: void cancel(); const std::string toString() const; - inline bool ready() const { + inline bool isReady() const { return isReadyState(state); } + // 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; + } + + // 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 { + return state; + } + void endParsing(); // Override this in the child class. @@ -61,8 +75,7 @@ public: const TileID id; const std::string name; - std::atomic state; - std::atomic_flag parsing; + std::atomic_flag parsing = ATOMIC_FLAG_INIT; protected: // Set the internal parsing state to true so we prevent @@ -81,6 +94,10 @@ protected: std::unique_ptr workRequest; +private: + std::atomic state; + +protected: // Contains the tile ID string for painting debug information. DebugFontBuffer debugFontBuffer; diff --git a/src/mbgl/map/tile_parser.cpp b/src/mbgl/map/tile_parser.cpp index 65b445a6cf..388950491c 100644 --- a/src/mbgl/map/tile_parser.cpp +++ b/src/mbgl/map/tile_parser.cpp @@ -8,7 +8,6 @@ #include #include #include -#include #include #include @@ -35,13 +34,13 @@ TileParser::TileParser(const GeometryTile& geometryTile_, glyphStore(glyphStore_), spriteAtlas(spriteAtlas_), sprite(sprite_), - collision(util::make_unique(tile.id.z, 4096, tile.source.tile_size, tile.depth)), partialParse(false) { assert(sprite); - assert(collision); } -bool TileParser::obsolete() const { return tile.state == TileData::State::obsolete; } +bool TileParser::obsolete() const { + return tile.getState() == TileData::State::obsolete; +} void TileParser::parse() { for (const auto& layer_desc : style.layers) { @@ -119,6 +118,10 @@ std::unique_ptr TileParser::createBucket(const StyleBucket &bucketDesc) } else if (bucketDesc.type == StyleLayerType::Line) { return createLineBucket(*layer, bucketDesc); } else if (bucketDesc.type == StyleLayerType::Symbol) { + if (partialParse) { + return nullptr; + } + bool needsResources = false; auto symbolBucket = createSymbolBucket(*layer, bucketDesc, needsResources); if (needsResources) { @@ -194,7 +197,7 @@ std::unique_ptr TileParser::createSymbolBucket(const GeometryTileLayer& return nullptr; } - auto bucket = util::make_unique(*collision); + auto bucket = util::make_unique(*tile.getCollision()); const float z = tile.id.z; auto& layout = bucket->layout; diff --git a/src/mbgl/map/tile_parser.hpp b/src/mbgl/map/tile_parser.hpp index 18184e27a3..67150bcd4d 100644 --- a/src/mbgl/map/tile_parser.hpp +++ b/src/mbgl/map/tile_parser.hpp @@ -29,7 +29,6 @@ class StyleLayoutRaster; class StyleLayoutLine; class StyleLayoutSymbol; class VectorTileData; -class Collision; class TileParser : private util::noncopyable { public: @@ -69,7 +68,6 @@ private: SpriteAtlas& spriteAtlas; util::ptr sprite; - std::unique_ptr collision; bool partialParse; }; diff --git a/src/mbgl/map/vector_tile_data.cpp b/src/mbgl/map/vector_tile_data.cpp index 7560168ed7..263ad52b9d 100644 --- a/src/mbgl/map/vector_tile_data.cpp +++ b/src/mbgl/map/vector_tile_data.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include using namespace mbgl; @@ -19,12 +20,13 @@ VectorTileData::VectorTileData(const TileID& id_, util::ptr sprite_, const SourceInfo& source_) : TileData(id_, source_), + depth(id_.z >= source_.max_zoom ? mapMaxZoom - id_.z : 1), glyphAtlas(glyphAtlas_), glyphStore(glyphStore_), spriteAtlas(spriteAtlas_), sprite(sprite_), style(style_), - depth(id.z >= source.max_zoom ? mapMaxZoom - id.z : 1) { + collision(util::make_unique(id_.z, 4096, source_.tile_size, depth)) { } VectorTileData::~VectorTileData() { @@ -35,7 +37,7 @@ VectorTileData::~VectorTileData() { } void VectorTileData::parse() { - if (state != State::loaded && state != State::partial) { + if (getState() != State::loaded && getState() != State::partial) { return; } @@ -48,24 +50,24 @@ void VectorTileData::parse() { TileParser parser(*vt, *this, style, glyphAtlas, glyphStore, spriteAtlas, sprite); parser.parse(); - if (state == State::obsolete) { + if (getState() == State::obsolete) { return; } if (parser.isPartialParse()) { - state = State::partial; + setState(State::partial); } else { - state = State::parsed; + setState(State::parsed); } } catch (const std::exception& ex) { Log::Error(Event::ParseTile, "Parsing [%d/%d/%d] failed: %s", id.z, id.x, id.y, ex.what()); - state = State::obsolete; + setState(State::obsolete); return; } } Bucket* VectorTileData::getBucket(StyleLayer const& layer) { - if (!ready() || !layer.bucket) { + if (!isReady() || !layer.bucket) { return nullptr; } @@ -91,3 +93,11 @@ void VectorTileData::setBucket(StyleLayer const& layer, std::unique_ptr buckets[layer.bucket->name] = std::move(bucket); } + +void VectorTileData::setState(const State& state_) { + TileData::setState(state_); + + if (isImmutable()) { + collision.reset(); + } +} diff --git a/src/mbgl/map/vector_tile_data.hpp b/src/mbgl/map/vector_tile_data.hpp index 146bc93229..5fe351ca0c 100644 --- a/src/mbgl/map/vector_tile_data.hpp +++ b/src/mbgl/map/vector_tile_data.hpp @@ -16,6 +16,7 @@ namespace mbgl { class Bucket; +class Collision; class Painter; class SourceInfo; class StyleLayer; @@ -45,6 +46,14 @@ public: void setBucket(StyleLayer const &layer_desc, std::unique_ptr bucket); + void setState(const State& state) override; + + inline Collision* getCollision() const { + return collision.get(); + } + + const float depth; + protected: // Holds the actual geometries in this tile. FillVertexBuffer fillVertexBuffer; @@ -69,8 +78,7 @@ private: std::unordered_map> buckets; mutable std::mutex bucketsMutex; -public: - const float depth; + std::unique_ptr collision; }; } diff --git a/src/mbgl/renderer/painter.cpp b/src/mbgl/renderer/painter.cpp index e2a3df6253..56d814c03a 100644 --- a/src/mbgl/renderer/painter.cpp +++ b/src/mbgl/renderer/painter.cpp @@ -381,7 +381,7 @@ std::vector Painter::determineRenderOrder(const Style& style) { const auto& tiles = layer.bucket->source->getTiles(); for (auto tile : tiles) { assert(tile); - if (!tile->data && !tile->data->ready()) { + if (!tile->data && !tile->data->isReady()) { continue; } -- cgit v1.2.1