diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp index 4c31203f401..56c0f692282 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,11 +245,13 @@ void GeometryTile::setData(std::unique_ptr data_) { void GeometryTile::reset() { MLN_TRACE_FUNC(); + if (pending) { + observer->onTileAction(id, sourceID, TileOperation::Cancelled); + pending = false; + } + // Mark the tile as pending again if it was complete before to prevent // signaling a complete state despite pending parse operations. - pending = true; - - observer->onTileAction(id, sourceID, TileOperation::Cancelled); ++correlationID; worker.self().invoke(&GeometryTileWorker::reset, correlationID); @@ -266,7 +268,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 +313,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 +338,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 61fa833e80c..3f409e2dde7 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,18 +1768,12 @@ 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) { + 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: { @@ -1783,10 +1783,10 @@ TEST(Map, ObserveTileLifecycle) { } case 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::RequestedFromCache)); + EXPECT_THAT(stage, + testing::AnyOf(TileOperation::StartParse, + TileOperation::EndParse, + TileOperation::LoadFromCache)); stage = TileOperation::RequestedFromNetwork; break; } @@ -1800,12 +1800,15 @@ TEST(Map, ObserveTileLifecycle) { 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;