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

Commit

Permalink
[core] GeometryCollection must not be implicitly copied
Browse files Browse the repository at this point in the history
  • Loading branch information
@pozdnyakov
pozdnyakov committed Jul 24, 2019
1 parent a242bd1 commit 2c2e858
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 38 deletions.
2 changes: 1 addition & 1 deletion src/mbgl/layout/pattern_layout.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ class PatternLayout : public Layout {
const auto i = patternFeature.i;
std::unique_ptr<GeometryTileFeature> feature = std::move(patternFeature.feature);
const PatternLayerMap& patterns = patternFeature.patterns;
GeometryCollection geometries = feature->getGeometries();
const GeometryCollection& geometries = feature->getGeometries();

bucket->addFeature(*feature, geometries, patternPositions, patterns);
featureIndex->insert(geometries, i, sourceLayerID, bucketLeaderID);
Expand Down
2 changes: 1 addition & 1 deletion src/mbgl/layout/symbol_feature.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class SymbolFeature : public GeometryTileFeature {
public:
SymbolFeature(std::unique_ptr<GeometryTileFeature> feature_) :
feature(std::move(feature_)),
geometry(feature->getGeometries()) // we need a mutable copy of the geometry for mergeLines()
geometry(feature->getGeometries().clone()) // we need a mutable copy of the geometry for mergeLines()
{}

FeatureType getType() const override { return feature->getType(); }
Expand Down
23 changes: 11 additions & 12 deletions src/mbgl/tile/geometry_tile_data.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,32 +57,31 @@ std::vector<GeometryCollection> classifyRings(const GeometryCollection& rings) {
std::size_t len = rings.size();

if (len <= 1) {
polygons.push_back(rings);
polygons.emplace_back(rings.clone());
return polygons;
}

GeometryCollection polygon;
int8_t ccw = 0;

for (std::size_t i = 0; i < len; i++) {
double area = signedArea(rings[i]);

if (area == 0)
continue;
for (const auto& ring : rings) {
double area = signedArea(ring);
if (area == 0) continue;

if (ccw == 0)
if (ccw == 0) {
ccw = (area < 0 ? -1 : 1);
}

if (ccw == (area < 0 ? -1 : 1) && !polygon.empty()) {
polygons.push_back(polygon);
polygon.clear();
polygons.emplace_back(std::move(polygon));
}

polygon.push_back(rings[i]);
polygon.emplace_back(ring);
}

if (!polygon.empty())
polygons.push_back(polygon);
if (!polygon.empty()) {
polygons.emplace_back(std::move(polygon));
}

return polygons;
}
Expand Down
7 changes: 7 additions & 0 deletions src/mbgl/tile/geometry_tile_data.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@ class GeometryCollection : public std::vector<GeometryCoordinates> {
GeometryCollection(Args&&... args) : std::vector<GeometryCoordinates>(std::forward<Args>(args)...) {}
GeometryCollection(std::initializer_list<GeometryCoordinates> args)
: std::vector<GeometryCoordinates>(std::move(args)) {}
GeometryCollection(GeometryCollection&&) = default;
GeometryCollection& operator=(GeometryCollection&&) = default;

GeometryCollection clone() const { return GeometryCollection(*this); }

private:
GeometryCollection(const GeometryCollection&) = default;
};

class GeometryTileFeature {
Expand Down
2 changes: 1 addition & 1 deletion src/mbgl/tile/geometry_tile_worker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ void GeometryTileWorker::parse() {
if (!filter(expression::EvaluationContext { static_cast<float>(this->id.overscaledZ), feature.get() }))
continue;

GeometryCollection geometries = feature->getGeometries();
const GeometryCollection& geometries = feature->getGeometries();
bucket->addFeature(*feature, geometries, {}, PatternLayerMap ());
featureIndex->insert(geometries, i, sourceLayerID, leaderImpl.id);
}
Expand Down
2 changes: 1 addition & 1 deletion test/gl/bucket.test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ TEST(Buckets, SymbolBucket) {

// SymbolBucket::addFeature() is a no-op.
GeometryCollection point { { { 0, 0 } } };
bucket.addFeature(StubGeometryTileFeature { {}, FeatureType::Point, point, properties }, point, {}, PatternLayerMap());
bucket.addFeature(StubGeometryTileFeature { {}, FeatureType::Point, std::move(point), properties }, point, {}, PatternLayerMap());
ASSERT_FALSE(bucket.hasData());
ASSERT_FALSE(bucket.needsUpload());

Expand Down
44 changes: 22 additions & 22 deletions test/util/merge_lines.test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,14 @@ TEST(MergeLines, SameText) {
input1.push_back(SymbolFeatureStub({}, FeatureType::LineString, {{{6, 0}, {7, 0}, {8, 0}}}, properties, aaa, {}, 0));
input1.push_back(SymbolFeatureStub({}, FeatureType::LineString, {{{5, 0}, {6, 0}}}, properties, aaa, {}, 0));

const std::vector<StubGeometryTileFeature> expected1 = {
{ {}, FeatureType::LineString, {{{0, 0}, {1, 0}, {2, 0}, {3, 0}, {4, 0}}}, properties },
{ {}, FeatureType::LineString, {{{4, 0}, {5, 0}, {6, 0}}}, properties },
{ {}, FeatureType::LineString, {{{5, 0}, {6, 0}, {7, 0}, {8, 0}, {9, 0}}}, properties },
{ {}, FeatureType::LineString, { emptyLine }, properties },
{ {}, FeatureType::LineString, { emptyLine }, properties },
{ {}, FeatureType::LineString, { emptyLine }, properties }
};
std::vector<StubGeometryTileFeature> expected1;
expected1.emplace_back(StubGeometryTileFeature({}, FeatureType::LineString, {{{0, 0}, {1, 0}, {2, 0}, {3, 0}, {4, 0}}}, properties));
expected1.emplace_back(StubGeometryTileFeature({}, FeatureType::LineString, {{{4, 0}, {5, 0}, {6, 0}}}, properties));
expected1.emplace_back(StubGeometryTileFeature({}, FeatureType::LineString, {{{5, 0}, {6, 0}, {7, 0}, {8, 0}, {9, 0}}}, properties));
expected1.emplace_back(StubGeometryTileFeature({}, FeatureType::LineString, { emptyLine }, properties));
expected1.emplace_back(StubGeometryTileFeature({}, FeatureType::LineString, { emptyLine }, properties));
expected1.emplace_back(StubGeometryTileFeature({}, FeatureType::LineString, { emptyLine }, properties));


mbgl::util::mergeLines(input1);

Expand All @@ -65,11 +65,11 @@ TEST(MergeLines, BothEnds) {
input2.push_back(SymbolFeatureStub { {}, FeatureType::LineString, {{{4, 0}, {5, 0}, {6, 0}}}, properties, aaa, {}, 0 });
input2.push_back(SymbolFeatureStub { {}, FeatureType::LineString, {{{2, 0}, {3, 0}, {4, 0}}}, properties, aaa, {}, 0 });

const std::vector<StubGeometryTileFeature> expected2 = {
{ {}, FeatureType::LineString, {{{0, 0}, {1, 0}, {2, 0}, {3, 0}, {4, 0}, {5, 0}, {6, 0}}}, properties },
{ {}, FeatureType::LineString, { emptyLine }, properties },
{ {}, FeatureType::LineString, { emptyLine }, properties }
};
std::vector<StubGeometryTileFeature> expected2;
expected2.emplace_back(StubGeometryTileFeature({}, FeatureType::LineString, {{{0, 0}, {1, 0}, {2, 0}, {3, 0}, {4, 0}, {5, 0}, {6, 0}}}, properties));
expected2.emplace_back(StubGeometryTileFeature({}, FeatureType::LineString, { emptyLine }, properties));
expected2.emplace_back(StubGeometryTileFeature({}, FeatureType::LineString, { emptyLine }, properties));


mbgl::util::mergeLines(input2);

Expand All @@ -85,11 +85,11 @@ TEST(MergeLines, CircularLines) {
input3.push_back(SymbolFeatureStub { {}, FeatureType::LineString, {{{2, 0}, {3, 0}, {4, 0}}}, properties, aaa, {}, 0 });
input3.push_back(SymbolFeatureStub { {}, FeatureType::LineString, {{{4, 0}, {0, 0}}}, properties, aaa, {}, 0 });

const std::vector<StubGeometryTileFeature> expected3 = {
{ {}, FeatureType::LineString, {{{0, 0}, {1, 0}, {2, 0}, {3, 0}, {4, 0}, {0, 0}}}, properties },
{ {}, FeatureType::LineString, { emptyLine }, properties },
{ {}, FeatureType::LineString, { emptyLine }, properties }
};
std::vector<StubGeometryTileFeature> expected3;
expected3.emplace_back(StubGeometryTileFeature({}, FeatureType::LineString, {{{0, 0}, {1, 0}, {2, 0}, {3, 0}, {4, 0}, {0, 0}}}, properties));
expected3.emplace_back(StubGeometryTileFeature({}, FeatureType::LineString, { emptyLine }, properties));
expected3.emplace_back(StubGeometryTileFeature({}, FeatureType::LineString, { emptyLine }, properties));


mbgl::util::mergeLines(input3);

Expand All @@ -102,20 +102,20 @@ TEST(MergeLines, EmptyOuterGeometry) {
std::vector<mbgl::SymbolFeature> input;
input.push_back(SymbolFeatureStub { {}, FeatureType::LineString, {}, properties, aaa, {}, 0 });

const std::vector<StubGeometryTileFeature> expected = { { {}, FeatureType::LineString, {}, properties } };
const StubGeometryTileFeature expected{ {}, FeatureType::LineString, {}, properties };

mbgl::util::mergeLines(input);

EXPECT_EQ(input[0].geometry, expected[0].getGeometries());
EXPECT_EQ(input[0].geometry, expected.getGeometries());
}

TEST(MergeLines, EmptyInnerGeometry) {
std::vector<mbgl::SymbolFeature> input;
input.push_back(SymbolFeatureStub { {}, FeatureType::LineString, {}, properties, aaa, {}, 0 });

const std::vector<StubGeometryTileFeature> expected = { { {}, FeatureType::LineString, {}, properties } };
const StubGeometryTileFeature expected{ {}, FeatureType::LineString, {}, properties };

mbgl::util::mergeLines(input);

EXPECT_EQ(input[0].geometry, expected[0].getGeometries());
EXPECT_EQ(input[0].geometry, expected.getGeometries());
}

0 comments on commit 2c2e858

Please sign in to comment.