diff --git a/platform/android/MapLibreAndroid/proguard-rules.pro b/platform/android/MapLibreAndroid/proguard-rules.pro index ade0552c244..55413ffe390 100644 --- a/platform/android/MapLibreAndroid/proguard-rules.pro +++ b/platform/android/MapLibreAndroid/proguard-rules.pro @@ -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 { + ; + 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 diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp index 4c31203f401..ee711c72b7e 100644 --- a/src/mbgl/tile/geometry_tile.cpp +++ b/src/mbgl/tile/geometry_tile.cpp @@ -229,7 +229,7 @@ void GeometryTile::setData(std::unique_ptr data_) { return; } - if (data_) { + if (!pending) { observer->onTileAction(id, sourceID, TileOperation::StartParse); } @@ -245,12 +245,18 @@ void GeometryTile::setData(std::unique_ptr 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>& 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> impls; impls.reserve(layers.size()); @@ -308,6 +317,7 @@ void GeometryTile::onLayout(std::shared_ptr 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 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)); } diff --git a/src/mbgl/tile/raster_dem_tile.cpp b/src/mbgl/tile/raster_dem_tile.cpp index ff5cf8b6088..3b65f94bfcf 100644 --- a/src/mbgl/tile/raster_dem_tile.cpp +++ b/src/mbgl/tile/raster_dem_tile.cpp @@ -66,13 +66,13 @@ void RasterDEMTile::setMetadata(std::optional modified_, std::optiona void RasterDEMTile::setData(const std::shared_ptr& 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 result, const uint loaded = true; if (resultCorrelationID == correlationID) { pending = false; + observer->onTileAction(id, sourceID, TileOperation::EndParse); } renderable = static_cast(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(); } diff --git a/src/mbgl/tile/raster_tile.cpp b/src/mbgl/tile/raster_tile.cpp index d89af17c355..9868e9395d5 100644 --- a/src/mbgl/tile/raster_tile.cpp +++ b/src/mbgl/tile/raster_tile.cpp @@ -55,13 +55,13 @@ void RasterTile::setMetadata(std::optional modified_, std::optional& 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 result, const uint64_t r loaded = true; if (resultCorrelationID == correlationID) { pending = false; + observer->onTileAction(id, sourceID, TileOperation::EndParse); } renderable = static_cast(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(); } diff --git a/test/map/map.test.cpp b/test/map/map.test.cpp index 5cf770c83fb..aaad37f299d 100644 --- a/test/map/map.test.cpp +++ b/test/map/map.test.cpp @@ -1698,6 +1698,12 @@ TEST(Map, ObserveTileLifecycle) { }; std::mutex tileMutex; std::vector tileOps; + // We expect to see a valid lifecycle for every tile in this list. + const std::vector 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 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); } }