Improve tile lifecycle determinism (#2819)
This commit is contained in:
@@ -9,6 +9,12 @@
|
||||
-keep class com.google.gson.JsonObject { *; }
|
||||
-keep class com.google.gson.JsonPrimitive { *; }
|
||||
-dontnote com.google.gson.**
|
||||
-keep enum org.maplibre.android.tile.TileOperation
|
||||
-keepclassmembers class * extends java.lang.Enum {
|
||||
<fields>;
|
||||
public static **[] values();
|
||||
public static ** valueOf(java.lang.String);
|
||||
}
|
||||
|
||||
# dontnote for keeps the entry point x but not the descriptor class y
|
||||
-dontnote org.maplibre.android.maps.MapLibreMap$OnFpsChangedListener
|
||||
|
||||
@@ -229,7 +229,7 @@ void GeometryTile::setData(std::unique_ptr<const GeometryTileData> data_) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (data_) {
|
||||
if (!pending) {
|
||||
observer->onTileAction(id, sourceID, TileOperation::StartParse);
|
||||
}
|
||||
|
||||
@@ -245,12 +245,18 @@ void GeometryTile::setData(std::unique_ptr<const GeometryTileData> data_) {
|
||||
void GeometryTile::reset() {
|
||||
MLN_TRACE_FUNC();
|
||||
|
||||
// Mark the tile as pending again if it was complete before to prevent
|
||||
// signaling a complete state despite pending parse operations.
|
||||
pending = true;
|
||||
// If there is pending work, indicate that work has been cancelled.
|
||||
// Clear the pending status.
|
||||
if (pending) {
|
||||
observer->onTileAction(id, sourceID, TileOperation::Cancelled);
|
||||
pending = false;
|
||||
}
|
||||
|
||||
observer->onTileAction(id, sourceID, TileOperation::Cancelled);
|
||||
// Reset the tile to an unloaded state to avoid signaling completion
|
||||
// after clearing the tile's pending status.
|
||||
loaded = false;
|
||||
|
||||
// Reset the worker to the `NeedsParse` state.
|
||||
++correlationID;
|
||||
worker.self().invoke(&GeometryTileWorker::reset, correlationID);
|
||||
}
|
||||
@@ -266,7 +272,10 @@ void GeometryTile::setLayers(const std::vector<Immutable<LayerProperties>>& laye
|
||||
|
||||
// Mark the tile as pending again if it was complete before to prevent
|
||||
// signaling a complete state despite pending parse operations.
|
||||
pending = true;
|
||||
if (!pending) {
|
||||
pending = true;
|
||||
observer->onTileAction(id, sourceID, TileOperation::StartParse);
|
||||
}
|
||||
|
||||
std::vector<Immutable<LayerProperties>> impls;
|
||||
impls.reserve(layers.size());
|
||||
@@ -308,6 +317,7 @@ void GeometryTile::onLayout(std::shared_ptr<LayoutResult> result, const uint64_t
|
||||
renderable = true;
|
||||
if (resultCorrelationID == correlationID) {
|
||||
pending = false;
|
||||
observer->onTileAction(id, sourceID, TileOperation::EndParse);
|
||||
}
|
||||
|
||||
layoutResult = std::move(result);
|
||||
@@ -332,16 +342,13 @@ void GeometryTile::onLayout(std::shared_ptr<LayoutResult> result, const uint64_t
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (!pending) {
|
||||
observer->onTileAction(id, sourceID, TileOperation::EndParse);
|
||||
}
|
||||
}
|
||||
|
||||
void GeometryTile::onError(std::exception_ptr err, const uint64_t resultCorrelationID) {
|
||||
loaded = true;
|
||||
if (resultCorrelationID == correlationID) {
|
||||
pending = false;
|
||||
observer->onTileAction(id, sourceID, TileOperation::Error);
|
||||
}
|
||||
observer->onTileError(*this, std::move(err));
|
||||
}
|
||||
|
||||
@@ -66,13 +66,13 @@ void RasterDEMTile::setMetadata(std::optional<Timestamp> modified_, std::optiona
|
||||
|
||||
void RasterDEMTile::setData(const std::shared_ptr<const std::string>& data) {
|
||||
if (!obsolete) {
|
||||
pending = true;
|
||||
++correlationID;
|
||||
|
||||
if (data) {
|
||||
if (!pending) {
|
||||
observer->onTileAction(id, sourceID, TileOperation::StartParse);
|
||||
}
|
||||
|
||||
pending = true;
|
||||
worker.self().invoke(&RasterDEMTileWorker::parse, data, correlationID, encoding);
|
||||
}
|
||||
}
|
||||
@@ -83,10 +83,10 @@ void RasterDEMTile::onParsed(std::unique_ptr<HillshadeBucket> result, const uint
|
||||
loaded = true;
|
||||
if (resultCorrelationID == correlationID) {
|
||||
pending = false;
|
||||
observer->onTileAction(id, sourceID, TileOperation::EndParse);
|
||||
}
|
||||
renderable = static_cast<bool>(bucket);
|
||||
observer->onTileChanged(*this);
|
||||
observer->onTileAction(id, sourceID, TileOperation::EndParse);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -94,6 +94,7 @@ void RasterDEMTile::onError(std::exception_ptr err, const uint64_t resultCorrela
|
||||
loaded = true;
|
||||
if (resultCorrelationID == correlationID) {
|
||||
pending = false;
|
||||
observer->onTileAction(id, sourceID, TileOperation::Error);
|
||||
}
|
||||
observer->onTileError(*this, std::move(err));
|
||||
}
|
||||
@@ -158,6 +159,9 @@ void RasterDEMTile::cancel() {
|
||||
|
||||
void RasterDEMTile::markObsolete() {
|
||||
obsolete = true;
|
||||
if (pending) {
|
||||
observer->onTileAction(id, sourceID, TileOperation::Cancelled);
|
||||
}
|
||||
pending = false;
|
||||
mailbox->abandon();
|
||||
}
|
||||
|
||||
@@ -55,13 +55,13 @@ void RasterTile::setMetadata(std::optional<Timestamp> modified_, std::optional<T
|
||||
|
||||
void RasterTile::setData(const std::shared_ptr<const std::string>& data) {
|
||||
if (!obsolete) {
|
||||
pending = true;
|
||||
++correlationID;
|
||||
|
||||
if (data) {
|
||||
if (!pending) {
|
||||
observer->onTileAction(id, sourceID, TileOperation::StartParse);
|
||||
}
|
||||
|
||||
pending = true;
|
||||
worker.self().invoke(&RasterTileWorker::parse, data, correlationID);
|
||||
}
|
||||
}
|
||||
@@ -72,10 +72,10 @@ void RasterTile::onParsed(std::unique_ptr<RasterBucket> result, const uint64_t r
|
||||
loaded = true;
|
||||
if (resultCorrelationID == correlationID) {
|
||||
pending = false;
|
||||
observer->onTileAction(id, sourceID, TileOperation::EndParse);
|
||||
}
|
||||
renderable = static_cast<bool>(bucket);
|
||||
observer->onTileChanged(*this);
|
||||
observer->onTileAction(id, sourceID, TileOperation::EndParse);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -83,6 +83,7 @@ void RasterTile::onError(std::exception_ptr err, const uint64_t resultCorrelatio
|
||||
loaded = true;
|
||||
if (resultCorrelationID == correlationID) {
|
||||
pending = false;
|
||||
observer->onTileAction(id, sourceID, TileOperation::Error);
|
||||
}
|
||||
observer->onTileError(*this, std::move(err));
|
||||
}
|
||||
@@ -111,6 +112,9 @@ void RasterTile::cancel() {
|
||||
|
||||
void RasterTile::markObsolete() {
|
||||
obsolete = true;
|
||||
if (pending) {
|
||||
observer->onTileAction(id, sourceID, TileOperation::Cancelled);
|
||||
}
|
||||
pending = false;
|
||||
mailbox->abandon();
|
||||
}
|
||||
|
||||
@@ -1698,6 +1698,12 @@ TEST(Map, ObserveTileLifecycle) {
|
||||
};
|
||||
std::mutex tileMutex;
|
||||
std::vector<TileEntry> tileOps;
|
||||
// We expect to see a valid lifecycle for every tile in this list.
|
||||
const std::vector<OverscaledTileID> expectedTiles = {
|
||||
{10, 0, 10, 163, 395}, {10, 0, 10, 163, 396}, {10, 0, 10, 164, 395}, {10, 0, 10, 164, 396},
|
||||
// Lower zooms can also be seen, but not always, so we
|
||||
// ignore them.
|
||||
};
|
||||
|
||||
struct ShaderEntry {
|
||||
shaders::BuiltIn id;
|
||||
@@ -1762,17 +1768,14 @@ TEST(Map, ObserveTileLifecycle) {
|
||||
}
|
||||
}
|
||||
|
||||
// We expect to see a valid lifecycle for every tile in this list.
|
||||
const std::vector<OverscaledTileID> expectedTiles = {
|
||||
{10, 0, 10, 163, 395}, {10, 0, 10, 163, 396}, {10, 0, 10, 164, 395}, {10, 0, 10, 164, 396},
|
||||
// Lower zooms can also be seen, but not always, so we
|
||||
// ignore them.
|
||||
};
|
||||
|
||||
for (const auto& tile : expectedTiles) {
|
||||
TileOperation stage = TileOperation::NullOp;
|
||||
bool parsing = false;
|
||||
|
||||
for (const auto& op : tileOps) {
|
||||
// Suppressing warning-as-error to use range-based for. Index is desired for debugging context.
|
||||
// NOLINTNEXTLINE
|
||||
for (size_t i = 0; i < tileOps.size(); i++) {
|
||||
const auto& op = tileOps[i];
|
||||
if (op.id != tile) continue;
|
||||
switch (op.op) {
|
||||
case TileOperation::RequestedFromCache: {
|
||||
@@ -1781,26 +1784,34 @@ TEST(Map, ObserveTileLifecycle) {
|
||||
break;
|
||||
}
|
||||
case TileOperation::RequestedFromNetwork: {
|
||||
EXPECT_THAT(
|
||||
stage,
|
||||
testing::AnyOf(
|
||||
TileOperation::StartParse, TileOperation::EndParse, TileOperation::RequestedFromCache));
|
||||
// Parsing happens concurrently with the file source request and can start and stop between requests
|
||||
EXPECT_THAT(stage,
|
||||
testing::AnyOf(
|
||||
TileOperation::StartParse, TileOperation::EndParse, TileOperation::LoadFromCache));
|
||||
stage = TileOperation::RequestedFromNetwork;
|
||||
break;
|
||||
}
|
||||
case TileOperation::LoadFromNetwork: {
|
||||
EXPECT_THAT(stage, TileOperation::RequestedFromNetwork);
|
||||
// Parsing happens concurrently with the file source request and can start and stop between requests
|
||||
EXPECT_THAT(
|
||||
stage,
|
||||
testing::AnyOf(
|
||||
TileOperation::StartParse, TileOperation::EndParse, TileOperation::RequestedFromNetwork));
|
||||
stage = TileOperation::LoadFromNetwork;
|
||||
break;
|
||||
}
|
||||
case TileOperation::LoadFromCache: {
|
||||
EXPECT_EQ(stage, TileOperation::RequestedFromCache);
|
||||
EXPECT_THAT(stage, testing::AnyOf(TileOperation::RequestedFromCache, TileOperation::StartParse));
|
||||
stage = TileOperation::LoadFromCache;
|
||||
break;
|
||||
}
|
||||
case TileOperation::StartParse: {
|
||||
EXPECT_THAT(stage, testing::AnyOf(TileOperation::LoadFromNetwork, TileOperation::LoadFromCache));
|
||||
// Parsing is expected to be started early during the request process by a call to `setLayers`
|
||||
EXPECT_THAT(stage,
|
||||
testing::AnyOf(TileOperation::RequestedFromCache, TileOperation::RequestedFromNetwork));
|
||||
EXPECT_FALSE(parsing); // We must not already be parsing when seeing this marker.
|
||||
stage = TileOperation::StartParse;
|
||||
parsing = true;
|
||||
break;
|
||||
}
|
||||
case TileOperation::Cancelled: {
|
||||
@@ -1811,6 +1822,7 @@ TEST(Map, ObserveTileLifecycle) {
|
||||
TileOperation::LoadFromCache,
|
||||
TileOperation::StartParse));
|
||||
stage = TileOperation::Cancelled;
|
||||
parsing = false;
|
||||
break;
|
||||
}
|
||||
case TileOperation::EndParse: {
|
||||
@@ -1831,6 +1843,8 @@ TEST(Map, ObserveTileLifecycle) {
|
||||
testing::AnyOf(TileOperation::StartParse,
|
||||
TileOperation::LoadFromNetwork,
|
||||
TileOperation::RequestedFromNetwork));
|
||||
EXPECT_TRUE(parsing); // We must have been parsing to see the EndParse marker.
|
||||
parsing = false;
|
||||
stage = TileOperation::EndParse;
|
||||
break;
|
||||
}
|
||||
@@ -1844,5 +1858,6 @@ TEST(Map, ObserveTileLifecycle) {
|
||||
}
|
||||
|
||||
EXPECT_THAT(stage, testing::AnyOf(TileOperation::EndParse, TileOperation::Cancelled));
|
||||
EXPECT_FALSE(parsing);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user