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

Commit

Permalink
Rework GeometryTile* interface
Browse files Browse the repository at this point in the history
  • Loading branch information
@jfirebaugh
jfirebaugh committed Mar 10, 2015
1 parent 2d8bf82 commit 50b5c15
Show file tree
Hide file tree
Showing 9 changed files with 112 additions and 297 deletions.
24 changes: 8 additions & 16 deletions src/mbgl/map/geometry_tile.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,24 +1,16 @@
#include <mbgl/map/geometry_tile.hpp>
#include <mbgl/style/filter_expression_private.hpp>

#include <iostream>
namespace mbgl {

using namespace mbgl;

std::ostream& mbgl::operator<<(std::ostream& os, const GeometryFeatureType& type) {
switch (type) {
case GeometryFeatureType::Unknown: return os << "Unknown";
case GeometryFeatureType::Point: return os << "Point";
case GeometryFeatureType::LineString: return os << "LineString";
case GeometryFeatureType::Polygon: return os << "Polygon";
default: return os << "Invalid";
mapbox::util::optional<Value> GeometryTileFeatureExtractor::getValue(const std::string& key) const {
if (key == "$type") {
return Value(uint64_t(feature.getType()));
}

return feature.getValue(key);
}

std::ostream& mbgl::operator<<(std::ostream& os, const GeometryTileFeature& feature) {
os << "Feature(" << feature.getID() << "): " << feature.getType() << std::endl;
for (const auto& prop : feature.getProperties()) {
os << " - " << prop.first << ": " << &prop.second << std::endl;
}
return os;
template bool evaluate(const FilterExpression&, const GeometryTileFeatureExtractor&);

}
68 changes: 16 additions & 52 deletions src/mbgl/map/geometry_tile.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
#include <mbgl/util/ptr.hpp>
#include <mbgl/util/variant.hpp>
#include <mbgl/util/vec.hpp>
#include <mbgl/util/noncopyable.hpp>

#include <cstdint>
#include <iosfwd>
#include <map>
#include <string>
#include <type_traits>
Expand All @@ -34,69 +34,33 @@ using Geometry = mapbox::util::variant<std::false_type, GeometryPoint, GeometryL

typedef std::vector<Geometry> GeometryCollection;

std::ostream& operator<<(std::ostream&, const GeometryFeatureType& type);

class GeometryTileLayer;

class GeometryTileFeature {
class GeometryTileFeature : public mbgl::util::noncopyable {
public:
virtual uint64_t getID() const = 0;
virtual GeometryFeatureType getType() const = 0;
virtual std::map<std::string, Value> getProperties() const = 0;
virtual GeometryCollection nextGeometry() = 0;
virtual mapbox::util::optional<Value> getValue(const std::string& key) const = 0;
virtual GeometryCollection getGeometries() const = 0;
};

std::ostream& operator<<(std::ostream&, const GeometryTileFeature& feature);

template <typename Tags>
class GeometryTileTagExtractor {
public:
GeometryTileTagExtractor(const GeometryTileLayer&);

inline void setTags(const Tags& tags_) { tags = tags_; }
virtual mapbox::util::optional<Value> getValue(const std::string &key) const;

inline void setType(GeometryFeatureType type_) { type = type_; }
inline GeometryFeatureType getType() const { return type; }

class GeometryTileLayer : public mbgl::util::noncopyable {
public:
GeometryFeatureType type = GeometryFeatureType::Unknown;
Tags tags;

protected:
const GeometryTileLayer& layer;
virtual std::size_t featureCount() const = 0;
virtual const util::ptr<const GeometryTileFeature> feature(std::size_t i) const = 0;
};

class GeometryFilteredTileLayer {
class GeometryTile : public mbgl::util::noncopyable {
public:
class iterator {
public:
void operator++();
bool operator!=(const iterator&) const;
GeometryTileFeature& operator*() const;
};

public:
virtual iterator begin() const = 0;
virtual iterator end() const = 0;
virtual const util::ptr<const GeometryTileLayer> getLayer(const std::string&) const = 0;
};

std::ostream& operator<<(std::ostream&, const PositionedGlyph&);

class GeometryTileLayer {
class GeometryTileFeatureExtractor {
public:
virtual const std::string& getName() const = 0;
virtual uint32_t getExtent() const = 0;
virtual const std::vector<std::string>& getKeys() const = 0;
virtual const std::unordered_map<std::string, uint32_t>& getKeyIndex() const = 0;
virtual const std::vector<Value>& getValues() const = 0;
virtual const std::map<std::string, std::map<Value, Shaping>>& getShaping() const = 0;
virtual util::ptr<GeometryFilteredTileLayer> createFilteredTileLayer(const FilterExpression&) const = 0;
};
GeometryTileFeatureExtractor(const GeometryTileFeature& feature_)
: feature(feature_) {}

class GeometryTile {
public:
virtual const util::ptr<const GeometryTileLayer> getLayer(const std::string&) const = 0;
mapbox::util::optional<Value> getValue(const std::string& key) const;

private:
const GeometryTileFeature& feature;
};

}
Expand Down
40 changes: 19 additions & 21 deletions src/mbgl/map/tile_parser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,14 +187,14 @@ std::unique_ptr<Bucket> TileParser::createBucket(const StyleBucket &bucketDesc)
if (tile.id.z >= std::ceil(bucketDesc.max_zoom)) return nullptr;
if (bucketDesc.visibility == mbgl::VisibilityType::None) return nullptr;

const util::ptr<const GeometryTileLayer> layer = vectorTile.getLayer(bucketDesc.source_layer);
if (layer.get() != nullptr) {
auto layer = vectorTile.getLayer(bucketDesc.source_layer);
if (layer) {
if (bucketDesc.type == StyleLayerType::Fill) {
return createFillBucket(layer, bucketDesc);
return createFillBucket(*layer, bucketDesc);
} else if (bucketDesc.type == StyleLayerType::Line) {
return createLineBucket(layer, bucketDesc);
return createLineBucket(*layer, bucketDesc);
} else if (bucketDesc.type == StyleLayerType::Symbol) {
return createSymbolBucket(layer, bucketDesc);
return createSymbolBucket(*layer, bucketDesc);
} else if (bucketDesc.type == StyleLayerType::Raster) {
return nullptr;
} else {
Expand All @@ -212,25 +212,23 @@ std::unique_ptr<Bucket> TileParser::createBucket(const StyleBucket &bucketDesc)
}

template <class Bucket>
void TileParser::addBucketGeometries(Bucket& bucket, const util::ptr<const GeometryTileLayer> layer, const FilterExpression &filter) {

util::ptr<GeometryFilteredTileLayer> filtered_layer = layer->createFilteredTileLayer(filter);

for (auto feature_it = filtered_layer->begin(); feature_it != filtered_layer->end(); ++feature_it) {
void TileParser::addBucketGeometries(Bucket& bucket, const GeometryTileLayer& layer, const FilterExpression &filter) {
for (std::size_t i = 0; i < layer.featureCount(); i++) {
auto feature = layer.feature(i);

if (obsolete())
return;

GeometryCollection geometry = (*feature_it).nextGeometry();
while (geometry.size()) {
bucket->addGeometry(geometry);
geometry = (*feature_it).nextGeometry();
}
GeometryTileFeatureExtractor extractor(*feature);
if (!evaluate(filter, extractor))
continue;

bucket->addGeometry(feature->getGeometries());
}
}

std::unique_ptr<Bucket> TileParser::createFillBucket(const util::ptr<const GeometryTileLayer> layer,
const StyleBucket &bucket_desc) {
std::unique_ptr<Bucket> TileParser::createFillBucket(const GeometryTileLayer& layer,
const StyleBucket& bucket_desc) {
auto fill = parseStyleLayoutFill(bucket_desc, tile.id.z);
auto bucket = util::make_unique<FillBucket>(std::move(fill),
tile.fillVertexBuffer,
Expand All @@ -240,8 +238,8 @@ std::unique_ptr<Bucket> TileParser::createFillBucket(const util::ptr<const Geome
return obsolete() ? nullptr : std::move(bucket);
}

std::unique_ptr<Bucket> TileParser::createLineBucket(const util::ptr<const GeometryTileLayer> layer,
const StyleBucket &bucket_desc) {
std::unique_ptr<Bucket> TileParser::createLineBucket(const GeometryTileLayer& layer,
const StyleBucket& bucket_desc) {
auto line = parseStyleLayoutLine(bucket_desc, tile.id.z);
auto bucket = util::make_unique<LineBucket>(std::move(line),
tile.lineVertexBuffer,
Expand All @@ -251,8 +249,8 @@ std::unique_ptr<Bucket> TileParser::createLineBucket(const util::ptr<const Geome
return obsolete() ? nullptr : std::move(bucket);
}

std::unique_ptr<Bucket> TileParser::createSymbolBucket(const util::ptr<const GeometryTileLayer> layer,
const StyleBucket &bucket_desc) {
std::unique_ptr<Bucket> TileParser::createSymbolBucket(const GeometryTileLayer& layer,
const StyleBucket& bucket_desc) {
auto symbol = parseStyleLayoutSymbol(bucket_desc, tile.id.z);
auto bucket = util::make_unique<SymbolBucket>(std::move(symbol), *collision);
bucket->addFeatures(
Expand Down
11 changes: 6 additions & 5 deletions src/mbgl/map/tile_parser.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,13 @@ class TileParser : private util::noncopyable
bool obsolete() const;
void parseStyleLayers(util::ptr<const StyleLayerGroup> group);

std::unique_ptr<Bucket> createBucket(const StyleBucket& bucketDesc);
std::unique_ptr<Bucket> createFillBucket(const util::ptr<const GeometryTileLayer> layer, const StyleBucket& bucketDesc);
std::unique_ptr<Bucket> createLineBucket(const util::ptr<const GeometryTileLayer> layer, const StyleBucket& bucketDesc);
std::unique_ptr<Bucket> createSymbolBucket(const util::ptr<const GeometryTileLayer> layer, const StyleBucket& bucketDesc);
std::unique_ptr<Bucket> createBucket(const StyleBucket&);
std::unique_ptr<Bucket> createFillBucket(const GeometryTileLayer&, const StyleBucket&);
std::unique_ptr<Bucket> createLineBucket(const GeometryTileLayer&, const StyleBucket&);
std::unique_ptr<Bucket> createSymbolBucket(const GeometryTileLayer&, const StyleBucket&);

template <class Bucket> void addBucketGeometries(Bucket& bucket, const util::ptr<const GeometryTileLayer> layer, const FilterExpression& filter);
template <class Bucket>
void addBucketGeometries(Bucket&, const GeometryTileLayer&, const FilterExpression&);

private:
const VectorTile vectorTile;
Expand Down
Loading

5 comments on commit 50b5c15

@kkaefer
Copy link
Member

Choose a reason for hiding this comment

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

This commit made tile parsing significantly slower:

instruments1 2015-03-31 14-19-28

vs.

instruments 2015-03-31 14-19-48

Note that both views are parsing the same set of tiles, but the new one takes about twice as long. The culprit seems to be iterating through features. The code removed in this commit did this in an efficient way by only parsing the required parts and quickly advancing to the next as soon as a match fails. The new code requires parsing the entire feature including all tags, before it can do a comparison. @jfirebaugh, can we revert this commit?

@jfirebaugh
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This commit is necessary for annotations support, but I believe the interface can still support a lazier parsing strategy. I'll take a look at this today.

@mb12
Copy link

@mb12 mb12 commented on 50b5c15 Mar 31, 2015

Choose a reason for hiding this comment

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

Is there some way to monitor changes in performance so that these get flagged automatically?

@kkaefer
Copy link
Member

Choose a reason for hiding this comment

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

Performance regressions are really hard to catch because we would need to run them in a highly controlled environment (= not the cloud) to havE any sort of meaning. In addition, performance characteristics are different based on the architecture of the CPU. We're deploying this code on a vast number of architectures (x86, arm, mips, all with a 64 bit equivalent) as well.

@incanus
Copy link
Contributor

@incanus incanus commented on 50b5c15 Apr 1, 2015

Choose a reason for hiding this comment

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

Yeah, what @kkaefer said, though once I finish #830 we may be able to automate some sorts of client-actions-triggering-parsers/renders-under-the-hood that we can measure with completion callbacks. It's a little shaky but maybe better than nothing.

Please sign in to comment.