Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Commit

Permalink
Do not hold a reference to the Style at the [Live|Vector]TileData
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tmpsantos committed Jun 18, 2015
1 parent 1f12bce commit 0444544
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 24 deletions.
7 changes: 4 additions & 3 deletions src/mbgl/map/live_tile_data.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,16 @@ 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_,
util::ptr<Sprite> sprite_,
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
Expand All @@ -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());
Expand Down
3 changes: 2 additions & 1 deletion src/mbgl/map/live_tile_data.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ class LiveTileData : public VectorTileData {
public:
LiveTileData(const TileID&,
AnnotationManager&,
Style&,
const std::vector<util::ptr<StyleLayer>>&,
Worker&,
GlyphAtlas&,
GlyphStore&,
SpriteAtlas&,
Expand Down
4 changes: 2 additions & 2 deletions src/mbgl/map/source.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
7 changes: 3 additions & 4 deletions src/mbgl/map/tile_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand All @@ -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_),
Expand All @@ -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;
Expand Down
7 changes: 4 additions & 3 deletions src/mbgl/map/tile_parser.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -22,7 +23,6 @@ class GlyphAtlas;
class GlyphStore;
class SpriteAtlas;
class Sprite;
class Style;
class StyleBucket;
class StyleLayoutFill;
class StyleLayoutRaster;
Expand All @@ -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,
Expand All @@ -61,8 +61,9 @@ class TileParser : private util::noncopyable {
const GeometryTile& geometryTile;
VectorTileData& tile;

std::vector<util::ptr<StyleLayer>> layers;

// Cross-thread shared data.
const Style& style;
GlyphAtlas& glyphAtlas;
GlyphStore& glyphStore;
SpriteAtlas& spriteAtlas;
Expand Down
16 changes: 8 additions & 8 deletions src/mbgl/map/vector_tile_data.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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_,
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down
7 changes: 4 additions & 3 deletions src/mbgl/map/vector_tile_data.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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&,
Expand Down Expand Up @@ -66,11 +66,12 @@ class VectorTileData : public TileData {
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
Expand Down

0 comments on commit 0444544

Please sign in to comment.