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

[core] make sure tiles are not treated as complete until all worker operations completed

Browse Source 
Previously, when we started a worker operation that eventually throws an exception (e.g. due to the tile not being parseable), and then enqueue another worker operation while the first one is processing, we treated the worker as idle once the first operation's error callback fired, even though the second operation was still in progress. Due to our use of coalescing, I was unable to come up with a reliable test since we'd need to reproduce the behavior described above, which is timing dependent.
This commit is contained in:
Konstantin Käfer
2017-09-16 00:44:33 +02:00
parent 1a684c1f48
commit f659aa4eda
11 changed files with 56 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -140,9 +140,10 @@ void GeometryTile::setLayers(const std::vector<Immutable<Layer::Impl>>& layers)
worker.invoke(&GeometryTileWorker::setLayers, std::move(impls), correlationID);
}
void GeometryTile::onLayout(LayoutResult result) {
void GeometryTile::onLayout(LayoutResult result, const uint64_t resultCorrelationID) {
loaded = true;
renderable = true;
(void)resultCorrelationID;
nonSymbolBuckets = std::move(result.nonSymbolBuckets);
featureIndex = std::move(result.featureIndex);
data = std::move(result.tileData);
@@ -150,10 +151,10 @@ void GeometryTile::onLayout(LayoutResult result) {
observer->onTileChanged(*this);
}
void GeometryTile::onPlacement(PlacementResult result) {
void GeometryTile::onPlacement(PlacementResult result, const uint64_t resultCorrelationID) {
loaded = true;
renderable = true;
if (result.correlationID == correlationID) {
if (resultCorrelationID == correlationID) {
pending = false;
}
symbolBuckets = std::move(result.symbolBuckets);
@@ -170,9 +171,11 @@ void GeometryTile::onPlacement(PlacementResult result) {
observer->onTileChanged(*this);
}
void GeometryTile::onError(std::exception_ptr err) {
void GeometryTile::onError(std::exception_ptr err, const uint64_t resultCorrelationID) {
loaded = true;
pending = false;
if (resultCorrelationID == correlationID) {
pending = false;
}
observer->onTileError(*this, err);
}

20
src/mbgl/tile/geometry_tile.hpp
View File

@@ -69,18 +69,15 @@ public:
std::unordered_map<std::string, std::shared_ptr<Bucket>> nonSymbolBuckets;
std::unique_ptr<FeatureIndex> featureIndex;
std::unique_ptr<GeometryTileData> tileData;
uint64_t correlationID;
LayoutResult(std::unordered_map<std::string, std::shared_ptr<Bucket>> nonSymbolBuckets_,
std::unique_ptr<FeatureIndex> featureIndex_,
std::unique_ptr<GeometryTileData> tileData_,
uint64_t correlationID_)
std::unique_ptr<GeometryTileData> tileData_)
: nonSymbolBuckets(std::move(nonSymbolBuckets_)),
featureIndex(std::move(featureIndex_)),
tileData(std::move(tileData_)),
correlationID(correlationID_) {}
tileData(std::move(tileData_)) {}
};
void onLayout(LayoutResult);
void onLayout(LayoutResult, uint64_t correlationID);
class PlacementResult {
public:
@@ -88,22 +85,19 @@ public:
std::unique_ptr<CollisionTile> collisionTile;
optional<AlphaImage> glyphAtlasImage;
optional<PremultipliedImage> iconAtlasImage;
uint64_t correlationID;
PlacementResult(std::unordered_map<std::string, std::shared_ptr<Bucket>> symbolBuckets_,
std::unique_ptr<CollisionTile> collisionTile_,
optional<AlphaImage> glyphAtlasImage_,
optional<PremultipliedImage> iconAtlasImage_,
uint64_t correlationID_)
optional<PremultipliedImage> iconAtlasImage_)
: symbolBuckets(std::move(symbolBuckets_)),
collisionTile(std::move(collisionTile_)),
glyphAtlasImage(std::move(glyphAtlasImage_)),
iconAtlasImage(std::move(iconAtlasImage_)),
correlationID(correlationID_) {}
iconAtlasImage(std::move(iconAtlasImage_)) {}
};
void onPlacement(PlacementResult);
void onPlacement(PlacementResult, uint64_t correlationID);
void onError(std::exception_ptr);
void onError(std::exception_ptr, uint64_t correlationID);
float yStretch() const override;

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

@@ -88,7 +88,7 @@ void GeometryTileWorker::setData(std::unique_ptr<const GeometryTileData> data_,
break;
}
} catch (...) {
parent.invoke(&GeometryTile::onError, std::current_exception());
parent.invoke(&GeometryTile::onError, std::current_exception(), correlationID);
}
}
@@ -112,7 +112,7 @@ void GeometryTileWorker::setLayers(std::vector<Immutable<Layer::Impl>> layers_,
break;
}
} catch (...) {
parent.invoke(&GeometryTile::onError, std::current_exception());
parent.invoke(&GeometryTile::onError, std::current_exception(), correlationID);
}
}
@@ -136,7 +136,7 @@ void GeometryTileWorker::setPlacementConfig(PlacementConfig placementConfig_, ui
break;
}
} catch (...) {
parent.invoke(&GeometryTile::onError, std::current_exception());
parent.invoke(&GeometryTile::onError, std::current_exception(), correlationID);
}
}
@@ -161,7 +161,7 @@ void GeometryTileWorker::symbolDependenciesChanged() {
break;
}
} catch (...) {
parent.invoke(&GeometryTile::onError, std::current_exception());
parent.invoke(&GeometryTile::onError, std::current_exception(), correlationID);
}
}
@@ -187,7 +187,7 @@ void GeometryTileWorker::coalesced() {
break;
}
} catch (...) {
parent.invoke(&GeometryTile::onError, std::current_exception());
parent.invoke(&GeometryTile::onError, std::current_exception(), correlationID);
}
}
@@ -357,8 +357,7 @@ void GeometryTileWorker::redoLayout() {
std::move(buckets),
std::move(featureIndex),
*data ? (*data)->clone() : nullptr,
correlationID
});
}, correlationID);
attemptPlacement();
}
@@ -422,8 +421,7 @@ void GeometryTileWorker::attemptPlacement() {
std::move(collisionTile),
std::move(glyphAtlasImage),
std::move(iconAtlasImage),
correlationID
});
}, correlationID);
}
} // namespace mbgl

15
src/mbgl/tile/raster_tile.cpp
View File

@@ -37,18 +37,27 @@ void RasterTile::setData(std::shared_ptr<const std::string> data,
optional<Timestamp> expires_) {
modified = modified_;
expires = expires_;
worker.invoke(&RasterTileWorker::parse, data);
pending = true;
++correlationID;
worker.invoke(&RasterTileWorker::parse, data, correlationID);
}
void RasterTile::onParsed(std::unique_ptr<RasterBucket> result) {
void RasterTile::onParsed(std::unique_ptr<RasterBucket> result, const uint64_t resultCorrelationID) {
bucket = std::move(result);
loaded = true;
if (resultCorrelationID == correlationID) {
pending = false;
}
renderable = bucket ? true : false;
observer->onTileChanged(*this);
}
void RasterTile::onError(std::exception_ptr err) {
void RasterTile::onError(std::exception_ptr err, const uint64_t resultCorrelationID) {
loaded = true;
if (resultCorrelationID == correlationID) {
pending = false;
}
observer->onTileError(*this, err);
}

6
src/mbgl/tile/raster_tile.hpp
View File

@@ -36,8 +36,8 @@ public:
void setMask(TileMask&&) override;
void onParsed(std::unique_ptr<RasterBucket> result);
void onError(std::exception_ptr);
void onParsed(std::unique_ptr<RasterBucket> result, uint64_t correlationID);
void onError(std::exception_ptr, uint64_t correlationID);
private:
TileLoader<RasterTile> loader;
@@ -45,6 +45,8 @@ private:
std::shared_ptr<Mailbox> mailbox;
Actor<RasterTileWorker> worker;
uint64_t correlationID = 0;
// Contains the Bucket object for the tile. Buckets are render
// objects and they get added by tile parsing operations.
std::unique_ptr<RasterBucket> bucket;

8
src/mbgl/tile/raster_tile_worker.cpp
View File

@@ -10,17 +10,17 @@ RasterTileWorker::RasterTileWorker(ActorRef<RasterTileWorker>, ActorRef<RasterTi
: parent(std::move(parent_)) {
}
void RasterTileWorker::parse(std::shared_ptr<const std::string> data) {
void RasterTileWorker::parse(std::shared_ptr<const std::string> data, uint64_t correlationID) {
if (!data) {
parent.invoke(&RasterTile::onParsed, nullptr); // No data; empty tile.
parent.invoke(&RasterTile::onParsed, nullptr, correlationID); // No data; empty tile.
return;
}
try {
auto bucket = std::make_unique<RasterBucket>(decodeImage(*data));
parent.invoke(&RasterTile::onParsed, std::move(bucket));
parent.invoke(&RasterTile::onParsed, std::move(bucket), correlationID);
} catch (...) {
parent.invoke(&RasterTile::onError, std::current_exception());
parent.invoke(&RasterTile::onError, std::current_exception(), correlationID);
}
}

2
src/mbgl/tile/raster_tile_worker.hpp
View File

@@ -13,7 +13,7 @@ class RasterTileWorker {
public:
RasterTileWorker(ActorRef<RasterTileWorker>, ActorRef<RasterTile>);
void parse(std::shared_ptr<const std::string> data);
void parse(std::shared_ptr<const std::string> data, uint64_t correlationID);
private:
ActorRef<RasterTile> parent;

9
test/tile/annotation_tile.test.cpp
View File

@@ -60,8 +60,7 @@ TEST(AnnotationTile, Issue8289) {
std::unordered_map<std::string, std::shared_ptr<Bucket>>(),
std::make_unique<FeatureIndex>(),
std::move(data),
0
});
}, 0);
auto collisionTile = std::make_unique<CollisionTile>(PlacementConfig());
@@ -75,16 +74,14 @@ TEST(AnnotationTile, Issue8289) {
std::move(collisionTile),
{},
{},
0
});
}, 0);
// Simulate a second layout with empty data.
tile.onLayout(GeometryTile::LayoutResult {
std::unordered_map<std::string, std::shared_ptr<Bucket>>(),
std::make_unique<FeatureIndex>(),
std::make_unique<AnnotationTileData>(),
0
});
}, 0);
std::unordered_map<std::string, std::vector<Feature>> result;
GeometryCoordinates queryGeometry {{ Point<int16_t>(0, 0) }};

5
test/tile/geojson_tile.test.cpp
View File

@@ -109,8 +109,9 @@ TEST(GeoJSONTile, Issue9927) {
ASSERT_NE(nullptr, tile.getBucket(*layer.baseImpl));
// Then simulate a parsing failure and make sure that we keep it renderable in this situation
// as well.
tile.onError(std::make_exception_ptr(std::runtime_error("Parse error")));
// as well. We're using 3 as a correlationID since we've done two three calls that increment
// this counter (as part of the GeoJSONTile constructor, setLayers, and setPlacementConfig).
tile.onError(std::make_exception_ptr(std::runtime_error("Parse error")), 3);
ASSERT_TRUE(tile.isRenderable());
ASSERT_NE(nullptr, tile.getBucket(*layer.baseImpl));
}

6
test/tile/raster_tile.test.cpp
View File

@@ -53,7 +53,7 @@ TEST(RasterTile, setError) {
TEST(RasterTile, onError) {
RasterTileTest test;
RasterTile tile(OverscaledTileID(0, 0, 0), test.tileParameters, test.tileset);
tile.onError(std::make_exception_ptr(std::runtime_error("test")));
tile.onError(std::make_exception_ptr(std::runtime_error("test")), 0);
EXPECT_FALSE(tile.isRenderable());
EXPECT_TRUE(tile.isLoaded());
EXPECT_TRUE(tile.isComplete());
@@ -62,7 +62,7 @@ TEST(RasterTile, onError) {
TEST(RasterTile, onParsed) {
RasterTileTest test;
RasterTile tile(OverscaledTileID(0, 0, 0), test.tileParameters, test.tileset);
tile.onParsed(std::make_unique<RasterBucket>(PremultipliedImage{}));
tile.onParsed(std::make_unique<RasterBucket>(PremultipliedImage{}), 0);
EXPECT_TRUE(tile.isRenderable());
EXPECT_TRUE(tile.isLoaded());
EXPECT_TRUE(tile.isComplete());
@@ -85,7 +85,7 @@ TEST(RasterTile, onParsed) {
TEST(RasterTile, onParsedEmpty) {
RasterTileTest test;
RasterTile tile(OverscaledTileID(0, 0, 0), test.tileParameters, test.tileset);
tile.onParsed(nullptr);
tile.onParsed(nullptr, 0);
EXPECT_FALSE(tile.isRenderable());
EXPECT_TRUE(tile.isLoaded());
EXPECT_TRUE(tile.isComplete());

9
test/tile/vector_tile.test.cpp
View File

@@ -59,7 +59,8 @@ TEST(VectorTile, setError) {
TEST(VectorTile, onError) {
VectorTileTest test;
VectorTile tile(OverscaledTileID(0, 0, 0), "source", test.tileParameters, test.tileset);
tile.onError(std::make_exception_ptr(std::runtime_error("test")));
tile.onError(std::make_exception_ptr(std::runtime_error("test")), 0);
EXPECT_FALSE(tile.isRenderable());
EXPECT_TRUE(tile.isLoaded());
EXPECT_TRUE(tile.isComplete());
@@ -86,16 +87,14 @@ TEST(VectorTile, Issue7615) {
nullptr,
{},
{},
0
});
}, 0);
// Subsequent onLayout should not cause the existing symbol bucket to be discarded.
tile.onLayout(GeometryTile::LayoutResult {
std::unordered_map<std::string, std::shared_ptr<Bucket>>(),
nullptr,
nullptr,
0
});
}, 0);
EXPECT_EQ(symbolBucket.get(), tile.getBucket(*symbolLayer.baseImpl));
}