Do not hold a reference to the Style at the [Live|Vector]TileData

Layers are added and removed dynamically on the Map thread when we use shape
annotations and we are iterating style.layers on the Worker thread without
any lock.

Now when we create a [Live|Vector]TileData at the Map thread we give it
a layers list (instead of the Style) and [Live|Vector]TileData will ultimately
make a copy of this list (on the Map thread, so no concurrency with
adding shape annotations).

The copy should be relatively cheap because we are using a shared pointer
for storing the layers on the list.
This commit is contained in:
Thiago Marcos P. Santos
2015-06-17 16:41:36 +03:00
committed by John Firebaugh
parent 4f3fc3c2c9
commit 0581df8787
7 changed files with 27 additions and 24 deletions

View File

@@ -10,7 +10,8 @@ using namespace mbgl;
LiveTileData::LiveTileData(const TileID& id_,
AnnotationManager& annotationManager_,
Style& style_,
const std::vector<util::ptr<StyleLayer>>& layers_,
Worker& workers_,
GlyphAtlas& glyphAtlas_,
GlyphStore& glyphStore_,
SpriteAtlas& spriteAtlas_,
@@ -18,7 +19,7 @@ LiveTileData::LiveTileData(const TileID& id_,
const SourceInfo& source_,
float angle_,
bool collisionDebug_)
: VectorTileData::VectorTileData(id_, style_, glyphAtlas_, glyphStore_,
: VectorTileData::VectorTileData(id_, layers_, workers_, glyphAtlas_, glyphStore_,
spriteAtlas_, sprite_, source_, angle_, collisionDebug_),
annotationManager(annotationManager_) {
// live features are always ready
@@ -43,7 +44,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, layers, 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());

View File

@@ -11,7 +11,8 @@ class LiveTileData : public VectorTileData {
public:
LiveTileData(const TileID&,
AnnotationManager&,
Style&,
const std::vector<util::ptr<StyleLayer>>&,
Worker&,
GlyphAtlas&,
GlyphStore&,
SpriteAtlas&,

View File

@@ -294,7 +294,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<VectorTileData>(normalized_id, style, glyphAtlas,
std::make_shared<VectorTileData>(normalized_id, style.layers, style.workers, glyphAtlas,
glyphStore, spriteAtlas, sprite, info,
transformState.getAngle(), data.getCollisionDebug());
new_tile.data->request(style.workers, transformState.getPixelRatio(), callback);
@@ -304,7 +304,7 @@ TileData::State Source::addTile(MapData& data,
style.workers, transformState.getPixelRatio(), callback);
} else if (info.type == SourceType::Annotations) {
new_tile.data = std::make_shared<LiveTileData>(normalized_id, data.annotationManager,
style, glyphAtlas,
style.layers, style.workers, glyphAtlas,
glyphStore, spriteAtlas, sprite, info,
transformState.getAngle(), data.getCollisionDebug());
new_tile.data->reparse(style.workers, callback);

View File

@@ -8,7 +8,6 @@
#include <mbgl/renderer/line_bucket.hpp>
#include <mbgl/renderer/symbol_bucket.hpp>
#include <mbgl/util/constants.hpp>
#include <mbgl/style/style.hpp>
#include <locale>
@@ -21,14 +20,14 @@ TileParser::~TileParser() = default;
TileParser::TileParser(const GeometryTile& geometryTile_,
VectorTileData& tile_,
const Style& style_,
const std::vector<util::ptr<StyleLayer>>& layers_,
GlyphAtlas& glyphAtlas_,
GlyphStore& glyphStore_,
SpriteAtlas& spriteAtlas_,
const util::ptr<Sprite>& sprite_)
: geometryTile(geometryTile_),
tile(tile_),
style(style_),
layers(layers_),
glyphAtlas(glyphAtlas_),
glyphStore(glyphStore_),
spriteAtlas(spriteAtlas_),
@@ -42,7 +41,7 @@ bool TileParser::obsolete() const {
}
void TileParser::parse() {
for (const auto& layer_desc : style.layers) {
for (const auto& layer_desc : layers) {
// Cancel early when parsing.
if (obsolete()) {
return;

View File

@@ -6,6 +6,7 @@
#include <mbgl/style/filter_expression.hpp>
#include <mbgl/style/class_properties.hpp>
#include <mbgl/style/style_bucket.hpp>
#include <mbgl/style/style_layer.hpp>
#include <mbgl/text/glyph.hpp>
#include <mbgl/util/ptr.hpp>
#include <mbgl/util/noncopyable.hpp>
@@ -22,7 +23,6 @@ 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 Style& style,
const std::vector<util::ptr<StyleLayer>>& layers,
GlyphAtlas& glyphAtlas,
GlyphStore& glyphStore,
SpriteAtlas& spriteAtlas,
@@ -61,8 +61,9 @@ private:
const GeometryTile& geometryTile;
VectorTileData& tile;
std::vector<util::ptr<StyleLayer>> layers;
// Cross-thread shared data.
const Style& style;
GlyphAtlas& glyphAtlas;
GlyphStore& glyphStore;
SpriteAtlas& spriteAtlas;

View File

@@ -9,12 +9,12 @@
#include <mbgl/util/pbf.hpp>
#include <mbgl/util/worker.hpp>
#include <mbgl/util/work_request.hpp>
#include <mbgl/style/style.hpp>
using namespace mbgl;
VectorTileData::VectorTileData(const TileID& id_,
Style& style_,
const std::vector<util::ptr<StyleLayer>>& layers_,
Worker& workers_,
GlyphAtlas& glyphAtlas_,
GlyphStore& glyphStore_,
SpriteAtlas& spriteAtlas_,
@@ -23,11 +23,12 @@ 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<CollisionTile>(id_.z, 4096, source_.tile_size * id.overscaling, angle, collisionDebug)),
lastAngle(angle),
currentAngle(angle) {
@@ -51,7 +52,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, layers, glyphAtlas, glyphStore, spriteAtlas, sprite);
parser.parse();
if (getState() == State::obsolete) {
@@ -128,15 +129,14 @@ 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);
workRequest = 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) {
for (const auto& layer_desc : 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 : style.layers) {
for (const auto& layer_desc : layers) {
auto bucket = getBucket(*layer_desc);
if (bucket) {
bucket->swapRenderData();

View File

@@ -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&,
Style&,
const std::vector<util::ptr<StyleLayer>>&,
Worker&,
GlyphAtlas&,
GlyphStore&,
SpriteAtlas&,
@@ -66,11 +66,12 @@ protected:
TriangleElementsBuffer triangleElementsBuffer;
LineElementsBuffer lineElementsBuffer;
std::vector<util::ptr<StyleLayer>> layers;
Worker& workers;
GlyphAtlas& glyphAtlas;
GlyphStore& glyphStore;
SpriteAtlas& spriteAtlas;
util::ptr<Sprite> sprite;
Style& style;
private:
// Contains all the Bucket objects for the tile. Buckets are render