tei/MapLibreParticleLab
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[core] Fix flickering caused by regression in #7586

Browse Source 
It should be safe to invoke GeometryTileWorker::setData multiple times without invoking GeometryTileWorker::setLayers. Therefore GeometryTileWorker::redoLayout() must not consume the layers.
This commit is contained in:
John Firebaugh
2017-01-09 13:05:13 -08:00
parent ea4c0b77c3
commit e7778509c5
13 changed files with 123 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
cmake/test-files.cmake
View File

@@ -90,6 +90,7 @@ set(MBGL_TEST_FILES
test/text/quads.test.cpp
# tile
test/tile/geojson_tile.test.cpp
test/tile/geometry_tile_data.test.cpp
test/tile/raster_tile.test.cpp
test/tile/tile_coordinate.test.cpp

4
src/mbgl/geometry/feature_index.cpp
View File

@@ -151,8 +151,8 @@ optional<GeometryCoordinates> FeatureIndex::translateQueryGeometry(
return translated;
}
void FeatureIndex::addBucketLayerName(const std::string& bucketName, const std::string& layerID) {
bucketLayerIDs[bucketName].push_back(layerID);
void FeatureIndex::setBucketLayerIDs(const std::string& bucketName, const std::vector<std::string>& layerIDs) {
bucketLayerIDs[bucketName] = layerIDs;
}
} // namespace mbgl

2
src/mbgl/geometry/feature_index.hpp
View File

@@ -52,7 +52,7 @@ public:
const float bearing,
const float pixelsToTileUnits);
void addBucketLayerName(const std::string& bucketName, const std::string& layerName);
void setBucketLayerIDs(const std::string& bucketName, const std::vector<std::string>& layerIDs);
private:
void addFeature(

6
src/mbgl/layout/symbol_layout.cpp
View File

@@ -27,7 +27,7 @@ namespace mbgl {
using namespace style;
SymbolLayout::SymbolLayout(std::vector<std::unique_ptr<Layer>> layers_,
SymbolLayout::SymbolLayout(std::vector<std::string> layerIDs_,
std::string sourceLayerName_,
uint32_t overscaling_,
float zoom_,
@@ -37,7 +37,7 @@ SymbolLayout::SymbolLayout(std::vector<std::unique_ptr<Layer>> layers_,
style::SymbolLayoutProperties::Evaluated layout_,
float textMaxSize_,
SpriteAtlas& spriteAtlas_)
: layers(std::move(layers_)),
: layerIDs(std::move(layerIDs_)),
sourceLayerName(std::move(sourceLayerName_)),
overscaling(overscaling_),
zoom(zoom_),
@@ -254,7 +254,7 @@ void SymbolLayout::addFeature(const SymbolFeature& feature,
? SymbolPlacementType::Point
: layout.get<SymbolPlacement>();
const float textRepeatDistance = symbolSpacing / 2;
IndexedSubfeature indexedFeature = {feature.index, sourceLayerName, layers.at(0)->getID(), symbolInstances.size()};
IndexedSubfeature indexedFeature = {feature.index, sourceLayerName, layerIDs.at(0), symbolInstances.size()};
auto addSymbolInstance = [&] (const GeometryCoordinates& line, Anchor& anchor) {
// https://github.com/mapbox/vector-tile-spec/tree/master/2.1#41-layers

4
src/mbgl/layout/symbol_layout.hpp
View File

@@ -28,7 +28,7 @@ struct Anchor;
class SymbolLayout {
public:
SymbolLayout(std::vector<std::unique_ptr<style::Layer>>,
SymbolLayout(std::vector<std::string> layerIDs_,
std::string sourceLayerName_,
uint32_t overscaling,
float zoom,
@@ -56,7 +56,7 @@ public:
State state = Pending;
const std::vector<std::unique_ptr<style::Layer>> layers;
const std::vector<std::string> layerIDs;
const std::string sourceLayerName;
private:

11
src/mbgl/style/group_by_layout.cpp
View File

@@ -32,16 +32,15 @@ std::string layoutKey(const Layer& layer) {
return s.GetString();
}
std::vector<std::vector<std::unique_ptr<Layer>>> groupByLayout(std::vector<std::unique_ptr<Layer>> layers) {
std::unordered_map<std::string, std::vector<std::unique_ptr<Layer>>> map;
std::vector<std::vector<const Layer*>> groupByLayout(const std::vector<std::unique_ptr<Layer>>& layers) {
std::unordered_map<std::string, std::vector<const Layer*>> map;
for (auto& layer : layers) {
auto& vector = map[layoutKey(*layer)];
vector.push_back(std::move(layer));
map[layoutKey(*layer)].push_back(layer.get());
}
std::vector<std::vector<std::unique_ptr<Layer>>> result;
std::vector<std::vector<const Layer*>> result;
for (auto& pair : map) {
result.push_back(std::move(pair.second));
result.push_back(pair.second);
}
return result;

2
src/mbgl/style/group_by_layout.hpp
View File

@@ -8,7 +8,7 @@ namespace style {
class Layer;
std::vector<std::vector<std::unique_ptr<Layer>>> groupByLayout(std::vector<std::unique_ptr<Layer>>);
std::vector<std::vector<const Layer*>> groupByLayout(const std::vector<std::unique_ptr<Layer>>&);
} // namespace style
} // namespace mbgl

2
src/mbgl/style/layers/symbol_layer_impl.cpp
View File

@@ -31,7 +31,7 @@ std::unique_ptr<Bucket> SymbolLayer::Impl::createBucket(BucketParameters&, const
std::unique_ptr<SymbolLayout> SymbolLayer::Impl::createLayout(BucketParameters& parameters,
const GeometryTileLayer& layer,
std::vector<std::unique_ptr<Layer>> group) const {
std::vector<std::string> group) const {
PropertyEvaluationParameters p(parameters.tileID.overscaledZ);
SymbolLayoutProperties::Evaluated evaluated = layout.evaluate(p);

2
src/mbgl/style/layers/symbol_layer_impl.hpp
View File

@@ -52,7 +52,7 @@ public:
std::unique_ptr<Bucket> createBucket(BucketParameters&, const GeometryTileLayer&) const override;
std::unique_ptr<SymbolLayout> createLayout(BucketParameters&, const GeometryTileLayer&,
std::vector<std::unique_ptr<Layer>>) const;
std::vector<std::string>) const;
SymbolPropertyValues iconPropertyValues(const SymbolLayoutProperties::Evaluated&) const;
SymbolPropertyValues textPropertyValues(const SymbolLayoutProperties::Evaluated&) const;

13
src/mbgl/tile/geometry_tile_worker.cpp
View File

@@ -214,7 +214,7 @@ void GeometryTileWorker::redoLayout() {
auto featureIndex = std::make_unique<FeatureIndex>();
BucketParameters parameters { id, obsolete, *featureIndex, mode };
std::vector<std::vector<std::unique_ptr<Layer>>> groups = groupByLayout(std::move(*layers));
std::vector<std::vector<const Layer*>> groups = groupByLayout(*layers);
for (auto& group : groups) {
if (obsolete) {
return;
@@ -231,13 +231,16 @@ void GeometryTileWorker::redoLayout() {
continue;
}
std::vector<std::string> layerIDs;
for (const auto& layer : group) {
featureIndex->addBucketLayerName(leader.getID(), layer->getID());
layerIDs.push_back(layer->getID());
}
featureIndex->setBucketLayerIDs(leader.getID(), layerIDs);
if (leader.is<SymbolLayer>()) {
symbolLayoutMap.emplace(leader.getID(),
leader.as<SymbolLayer>()->impl->createLayout(parameters, *geometryLayer, std::move(group)));
leader.as<SymbolLayer>()->impl->createLayout(parameters, *geometryLayer, layerIDs));
} else {
std::shared_ptr<Bucket> bucket = leader.baseImpl->createBucket(parameters, *geometryLayer);
if (!bucket->hasData()) {
@@ -321,8 +324,8 @@ void GeometryTileWorker::attemptPlacement() {
}
std::shared_ptr<Bucket> bucket = symbolLayout->place(*collisionTile);
for (const auto& layer : symbolLayout->layers) {
buckets.emplace(layer->getID(), bucket);
for (const auto& layerID : symbolLayout->layerIDs) {
buckets.emplace(layerID, bucket);
}
}

22
test/src/mbgl/test/stub_tile_observer.hpp Normal file
View File

@@ -0,0 +1,22 @@
#pragma once
#include <mbgl/tile/tile_observer.hpp>
using namespace mbgl;
/**
* An implementation of TileObserver that forwards all methods to dynamically-settable lambas.
*/
class StubTileObserver : public TileObserver {
public:
void onTileChanged(Tile& tile) override {
if (tileChanged) tileChanged(tile);
}
void onTileError(Tile& tile, std::exception_ptr error) override {
if (tileError) tileError(tile, error);
}
std::function<void (Tile&)> tileChanged;
std::function<void (Tile&, std::exception_ptr)> tileError;
};

8
test/style/group_by_layout.test.cpp
View File

@@ -12,7 +12,7 @@ TEST(GroupByLayout, Related) {
std::vector<std::unique_ptr<Layer>> layers;
layers.push_back(std::make_unique<LineLayer>("a", "source"));
layers.push_back(std::make_unique<LineLayer>("b", "source"));
auto result = groupByLayout(std::move(layers));
auto result = groupByLayout(layers);
ASSERT_EQ(1u, result.size());
ASSERT_EQ(2u, result[0].size());
}
@@ -21,7 +21,7 @@ TEST(GroupByLayout, UnrelatedType) {
std::vector<std::unique_ptr<Layer>> layers;
layers.push_back(std::make_unique<BackgroundLayer>("background"));
layers.push_back(std::make_unique<CircleLayer>("circle", "source"));
auto result = groupByLayout(std::move(layers));
auto result = groupByLayout(layers);
ASSERT_EQ(2u, result.size());
}
@@ -30,7 +30,7 @@ TEST(GroupByLayout, UnrelatedFilter) {
layers.push_back(std::make_unique<LineLayer>("a", "source"));
layers.push_back(std::make_unique<LineLayer>("b", "source"));
layers[0]->as<LineLayer>()->setFilter(EqualsFilter());
auto result = groupByLayout(std::move(layers));
auto result = groupByLayout(layers);
ASSERT_EQ(2u, result.size());
}
@@ -39,6 +39,6 @@ TEST(GroupByLayout, UnrelatedLayout) {
layers.push_back(std::make_unique<LineLayer>("a", "source"));
layers.push_back(std::make_unique<LineLayer>("b", "source"));
layers[0]->as<LineLayer>()->setLineCap(LineCapType::Square);
auto result = groupByLayout(std::move(layers));
auto result = groupByLayout(layers);
ASSERT_EQ(2u, result.size());
}

72
test/tile/geojson_tile.test.cpp Normal file
View File

@@ -0,0 +1,72 @@
#include <mbgl/test/util.hpp>
#include <mbgl/test/fake_file_source.hpp>
#include <mbgl/test/stub_tile_observer.hpp>
#include <mbgl/tile/geojson_tile.hpp>
#include <mbgl/tile/tile_loader_impl.hpp>
#include <mbgl/util/default_thread_pool.hpp>
#include <mbgl/util/run_loop.hpp>
#include <mbgl/map/transform.hpp>
#include <mbgl/style/style.hpp>
#include <mbgl/style/update_parameters.hpp>
#include <mbgl/style/layers/circle_layer.hpp>
#include <mbgl/annotation/annotation_manager.hpp>
#include <memory>
using namespace mbgl;
using namespace mbgl::style;
class GeoJSONTileTest {
public:
FakeFileSource fileSource;
TransformState transformState;
util::RunLoop loop;
ThreadPool threadPool { 1 };
AnnotationManager annotationManager { 1.0 };
style::Style style { fileSource, 1.0 };
Tileset tileset { { "https://example.com" }, { 0, 22 }, "none" };
style::UpdateParameters updateParameters {
1.0,
MapDebugOptions(),
transformState,
threadPool,
fileSource,
MapMode::Continuous,
annotationManager,
style
};
};
TEST(GeoJSONTile, Issue7648) {
GeoJSONTileTest test;
GeoJSONTile tile(OverscaledTileID(0, 0, 0), "source", test.updateParameters);
test.style.addLayer(std::make_unique<CircleLayer>("circle", "source"));
StubTileObserver observer;
observer.tileChanged = [&] (const Tile&) {
// Once present, the bucket should never "disappear", which would cause
// flickering.
ASSERT_NE(nullptr, tile.getBucket(*test.style.getLayer("circle")));
};
tile.setObserver(&observer);
tile.setPlacementConfig({});
mapbox::geometry::feature_collection<int16_t> features;
features.push_back(mapbox::geometry::feature<int16_t> {
mapbox::geometry::point<int16_t>(0, 0)
});
tile.updateData(features);
while (!tile.isComplete()) {
test.loop.runOnce();
}
tile.updateData(features);
while (!tile.isComplete()) {
test.loop.runOnce();
}
}