Skip to content

Commit

Permalink
Adjust tile action events, emit only for correct correlationIDs
Browse files Browse the repository at this point in the history
  • Loading branch information
@mwilsnd
mwilsnd committed Sep 9, 2024
1 parent 6ba3644 commit 48a1936
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 29 deletions.
21 changes: 12 additions & 9 deletions src/mbgl/tile/geometry_tile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ void GeometryTile::setData(std::unique_ptr<const GeometryTileData> data_) {
return;
}

if (data_) {
if (!pending) {
observer->onTileAction(id, sourceID, TileOperation::StartParse);
}

Expand All @@ -245,11 +245,13 @@ void GeometryTile::setData(std::unique_ptr<const GeometryTileData> 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);
Expand All @@ -266,7 +268,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());
Expand Down Expand Up @@ -308,6 +313,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);
Expand All @@ -332,16 +338,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));
}
Expand Down
10 changes: 7 additions & 3 deletions src/mbgl/tile/raster_dem_tile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand All @@ -83,17 +83,18 @@ 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);
}
}

void RasterDEMTile::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));
}
Expand Down Expand Up @@ -158,6 +159,9 @@ void RasterDEMTile::cancel() {

void RasterDEMTile::markObsolete() {
obsolete = true;
if (pending) {
observer->onTileAction(id, sourceID, TileOperation::Cancelled);
}
pending = false;
mailbox->abandon();
}
Expand Down
10 changes: 7 additions & 3 deletions src/mbgl/tile/raster_tile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand All @@ -72,17 +72,18 @@ 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);
}
}

void RasterTile::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));
}
Expand Down Expand Up @@ -111,6 +112,9 @@ void RasterTile::cancel() {

void RasterTile::markObsolete() {
obsolete = true;
if (pending) {
observer->onTileAction(id, sourceID, TileOperation::Cancelled);
}
pending = false;
mailbox->abandon();
}
Expand Down
31 changes: 17 additions & 14 deletions test/map/map.test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1762,18 +1768,12 @@ 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) {
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: {
Expand All @@ -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;
}
Expand All @@ -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;
Expand Down

0 comments on commit 48a1936

Please sign in to comment.