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

Commit

Permalink
[core] Source should receive a ref to MapData just once
Browse files Browse the repository at this point in the history
  • Loading branch information
brunoabinader committed Nov 27, 2015
1 parent 1f0820c commit d55aa79
Show file tree
Hide file tree
Showing 9 changed files with 54 additions and 38 deletions.
6 changes: 4 additions & 2 deletions src/mbgl/annotation/annotation_manager.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include <mbgl/annotation/annotation_manager.hpp>
#include <mbgl/annotation/annotation_tile.hpp>
#include <mbgl/map/map_data.hpp>
#include <mbgl/style/style.hpp>
#include <mbgl/layer/symbol_layer.hpp>

Expand All @@ -10,7 +11,8 @@ namespace mbgl {
const std::string AnnotationManager::SourceID = "com.mapbox.annotations";
const std::string AnnotationManager::PointLayerID = "com.mapbox.annotations.points";

AnnotationManager::AnnotationManager() = default;
AnnotationManager::AnnotationManager(MapData& data_) : data(data_) {}

AnnotationManager::~AnnotationManager() = default;

AnnotationIDs
Expand Down Expand Up @@ -108,7 +110,7 @@ std::unique_ptr<AnnotationTile> AnnotationManager::getTile(const TileID& tileID)
void AnnotationManager::updateStyle(Style& style) {
// Create annotation source, point layer, and point bucket
if (!style.getSource(SourceID)) {
std::unique_ptr<Source> source = std::make_unique<Source>();
std::unique_ptr<Source> source = std::make_unique<Source>(data);
source->info.type = SourceType::Annotations;
source->info.source_id = SourceID;
source->enabled = true;
Expand Down
4 changes: 3 additions & 1 deletion src/mbgl/annotation/annotation_manager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

namespace mbgl {

class MapData;
class PointAnnotation;
class ShapeAnnotation;
class AnnotationTile;
Expand All @@ -21,7 +22,7 @@ class Style;

class AnnotationManager : private util::noncopyable {
public:
AnnotationManager();
AnnotationManager(MapData&);
~AnnotationManager();

AnnotationIDs addPointAnnotations(const std::vector<PointAnnotation>&, const uint8_t maxZoom);
Expand All @@ -42,6 +43,7 @@ class AnnotationManager : private util::noncopyable {
private:
std::unique_ptr<AnnotationTile> getTile(const TileID&);

MapData& data;
AnnotationID nextID = 0;
PointAnnotationImpl::Tree pointTree;
PointAnnotationImpl::Map pointAnnotations;
Expand Down
1 change: 1 addition & 0 deletions src/mbgl/map/map_data.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class MapData {
: mode(mode_)
, contextMode(contextMode_)
, pixelRatio(pixelRatio_)
, annotationManager(*this)
, animationTime(Duration::zero())
, defaultFadeDuration(mode_ == MapMode::Continuous ? std::chrono::milliseconds(300) : Duration::zero())
, defaultTransitionDuration(Duration::zero())
Expand Down
46 changes: 22 additions & 24 deletions src/mbgl/map/source.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ std::string SourceInfo::tileURL(const TileID& id, float pixelRatio) const {
return result;
}

Source::Source() {}
Source::Source(MapData& data_) : data(data_) {}

Source::~Source() = default;

Expand Down Expand Up @@ -231,23 +231,22 @@ TileData::State Source::hasTile(const TileID& id) {
bool Source::handlePartialTile(const TileID& id, Worker&) {
const TileID normalized_id = id.normalized();

auto it = tile_data.find(normalized_id);
if (it == tile_data.end()) {
auto it = tileDataMap.find(normalized_id);
if (it == tileDataMap.end()) {
return true;
}

auto data = it->second.lock();
if (!data) {
auto tileData = it->second.lock();
if (!tileData) {
return true;
}

return data->parsePending([this]() {
return tileData->parsePending([this]() {
emitTileLoaded(false);
});
}

TileData::State Source::addTile(MapData& data,
const TransformState& transformState,
TileData::State Source::addTile(const TransformState& transformState,
Style& style,
TexturePool& texturePool,
const TileID& id) {
Expand All @@ -265,8 +264,8 @@ TileData::State Source::addTile(MapData& data,
// Try to find the associated TileData object.
const TileID normalized_id = id.normalized();

auto it = tile_data.find(normalized_id);
if (it != tile_data.end()) {
auto it = tileDataMap.find(normalized_id);
if (it != tileDataMap.end()) {
// Create a shared_ptr handle. Note that this might be empty!
new_tile.data = it->second.lock();
}
Expand All @@ -281,7 +280,7 @@ TileData::State Source::addTile(MapData& data,
}

if (!new_tile.data) {
auto callback = std::bind(&Source::tileLoadingCompleteCallback, this, normalized_id, transformState, data.getCollisionDebug());
auto callback = std::bind(&Source::tileLoadingCompleteCallback, this, normalized_id, transformState);

// If we don't find working tile data, we're just going to load it.
if (info.type == SourceType::Raster) {
Expand Down Expand Up @@ -310,7 +309,7 @@ TileData::State Source::addTile(MapData& data,
callback);
}

tile_data.emplace(new_tile.data->id, new_tile.data);
tileDataMap.emplace(new_tile.data->id, new_tile.data);
}

return new_tile.data->getState();
Expand Down Expand Up @@ -408,8 +407,7 @@ void Source::findLoadedParent(const TileID& id, int32_t minCoveringZoom, std::fo
}
}

bool Source::update(MapData& data,
const TransformState& transformState,
bool Source::update(const TransformState& transformState,
Style& style,
TexturePool& texturePool,
bool shouldReparsePartialTiles) {
Expand Down Expand Up @@ -449,7 +447,7 @@ bool Source::update(MapData& data,
}
break;
case TileData::State::invalid:
state = addTile(data, transformState, style, texturePool, id);
state = addTile(transformState, style, texturePool, id);
break;
default:
break;
Expand Down Expand Up @@ -500,7 +498,7 @@ bool Source::update(MapData& data,
});

// Remove all the expired pointers from the set.
util::erase_if(tile_data, [&retain_data, &tileCache](std::pair<const TileID, std::weak_ptr<TileData>> &pair) {
util::erase_if(tileDataMap, [&retain_data, &tileCache](std::pair<const TileID, std::weak_ptr<TileData>> &pair) {
const util::ptr<TileData> tile = pair.second.lock();
if (!tile) {
return true;
Expand Down Expand Up @@ -548,23 +546,23 @@ void Source::setObserver(Observer* observer) {
observer_ = observer;
}

void Source::tileLoadingCompleteCallback(const TileID& normalized_id, const TransformState& transformState, bool collisionDebug) {
auto it = tile_data.find(normalized_id);
if (it == tile_data.end()) {
void Source::tileLoadingCompleteCallback(const TileID& normalized_id, const TransformState& transformState) {
auto it = tileDataMap.find(normalized_id);
if (it == tileDataMap.end()) {
return;
}

util::ptr<TileData> data = it->second.lock();
if (!data) {
util::ptr<TileData> tileData = it->second.lock();
if (!tileData) {
return;
}

if (data->getState() == TileData::State::obsolete && !data->getError().empty()) {
emitTileLoadingFailed(data->getError());
if (tileData->getState() == TileData::State::obsolete && !tileData->getError().empty()) {
emitTileLoadingFailed(tileData->getError());
return;
}

data->redoPlacement({ transformState.getAngle(), transformState.getPitch(), collisionDebug });
tileData->redoPlacement({ transformState.getAngle(), transformState.getPitch(), data.getCollisionDebug() });
emitTileLoaded(true);
}

Expand Down
14 changes: 7 additions & 7 deletions src/mbgl/map/source.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class Source : private util::noncopyable {
virtual void onTileLoadingFailed(std::exception_ptr error) = 0;
};

Source();
Source(MapData&);
~Source();

void load();
Expand All @@ -72,8 +72,7 @@ class Source : private util::noncopyable {
// will return true if all the tiles were scheduled for updating of false if
// they were not. shouldReparsePartialTiles must be set to "true" if there is
// new data available that a tile in the "partial" state might be interested at.
bool update(MapData&,
const TransformState&,
bool update(const TransformState&,
Style&,
TexturePool&,
bool shouldReparsePartialTiles);
Expand All @@ -95,7 +94,7 @@ class Source : private util::noncopyable {
bool enabled;

private:
void tileLoadingCompleteCallback(const TileID& normalized_id, const TransformState& transformState, bool collisionDebug);
void tileLoadingCompleteCallback(const TileID&, const TransformState&);

void emitSourceLoaded();
void emitSourceLoadingFailed(const std::string& message);
Expand All @@ -108,8 +107,7 @@ class Source : private util::noncopyable {
int32_t coveringZoomLevel(const TransformState&) const;
std::forward_list<TileID> coveringTiles(const TransformState&) const;

TileData::State addTile(MapData&,
const TransformState&,
TileData::State addTile(const TransformState&,
Style&,
TexturePool&,
const TileID&);
Expand All @@ -119,14 +117,16 @@ class Source : private util::noncopyable {

double getZoom(const TransformState &state) const;

MapData& data;

bool loaded = false;

// Stores the time when this source was most recently updated.
TimePoint updated = TimePoint::min();

std::map<TileID, std::unique_ptr<Tile>> tiles;
std::vector<Tile*> tilePtrs;
std::map<TileID, std::weak_ptr<TileData>> tile_data;
std::map<TileID, std::weak_ptr<TileData>> tileDataMap;
TileCache cache;

std::unique_ptr<FileRequest> req;
Expand Down
4 changes: 2 additions & 2 deletions src/mbgl/style/style.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ void Style::setJSON(const std::string& json, const std::string&) {
return;
}

StyleParser parser;
StyleParser parser(data);
parser.parse(doc);

for (auto& source : parser.getSources()) {
Expand Down Expand Up @@ -106,7 +106,7 @@ void Style::update(const TransformState& transform,
TexturePool& texturePool) {
bool allTilesUpdated = true;
for (const auto& source : sources) {
if (!source->update(data, transform, *this, texturePool, shouldReparsePartialTiles)) {
if (!source->update(transform, *this, texturePool, shouldReparsePartialTiles)) {
allTilesUpdated = false;
}
}
Expand Down
7 changes: 6 additions & 1 deletion src/mbgl/style/style_parser.cpp
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
#include <mbgl/style/style_parser.hpp>
#include <mbgl/style/style_layer.hpp>

#include <mbgl/map/map_data.hpp>
#include <mbgl/platform/log.hpp>

#include <algorithm>

namespace mbgl {

StyleParser::StyleParser(MapData& data_)
: data(data_) {
}

void StyleParser::parse(const JSVal& document) {
if (document.HasMember("version")) {
version = document["version"].GetInt();
Expand Down Expand Up @@ -49,7 +54,7 @@ void StyleParser::parseSources(const JSVal& value) {
const JSVal& nameVal = itr->name;
const JSVal& sourceVal = itr->value;

std::unique_ptr<Source> source = std::make_unique<Source>();
std::unique_ptr<Source> source = std::make_unique<Source>(data);

source->info.source_id = { nameVal.GetString(), nameVal.GetStringLength() };

Expand Down
5 changes: 5 additions & 0 deletions src/mbgl/style/style_parser.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,16 @@

namespace mbgl {

class MapData;
class StyleLayer;
class Source;

using JSVal = rapidjson::Value;

class StyleParser {
public:
StyleParser(MapData&);

void parse(const JSVal&);

std::vector<std::unique_ptr<Source>>&& getSources() {
Expand All @@ -45,6 +48,8 @@ class StyleParser {
void parseLayer(const std::string& id, const JSVal&, util::ptr<StyleLayer>&);
void parseVisibility(StyleLayer&, const JSVal& value);

MapData& data;

std::uint8_t version;

std::vector<std::unique_ptr<Source>> sources;
Expand Down
5 changes: 4 additions & 1 deletion test/miscellaneous/style_parser.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "../fixtures/util.hpp"

#include <mbgl/map/map_data.hpp>
#include <mbgl/style/style_parser.hpp>
#include <mbgl/util/io.hpp>

Expand Down Expand Up @@ -35,7 +36,9 @@ TEST_P(StyleParserTest, ParseStyle) {
FixtureLogObserver* observer = new FixtureLogObserver();
Log::setObserver(std::unique_ptr<Log::Observer>(observer));

StyleParser parser;
double fakePixelRatio = 1.0;
MapData data(MapMode::Continuous, GLContextMode::Unique, fakePixelRatio);
StyleParser parser(data);
parser.parse(styleDoc);

for (auto it = infoDoc.MemberBegin(), end = infoDoc.MemberEnd(); it != end; it++) {
Expand Down

1 comment on commit d55aa79

@jfirebaugh
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't agree with this change -- it adds MapData dependencies to AnnotationManager and StyleParser when we are trying to reduce dependencies on MapData.

Please sign in to comment.