Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Commit

Permalink
[core] Always request 1x and @2x sprite images for portability
Browse files Browse the repository at this point in the history
When creating a offline region, we've previously only requested the sprite image for the specified resolution. This lead to offline packs not being usable on devices that have a different pixel ratio. We're now requesting both 1x and 2x sprites. Some devices use even higher or fractional pixel ratios. However, we only ever use 1x and 2x sprite images in our requests.
  • Loading branch information
kkaefer committed Sep 20, 2018
1 parent 2f7a147 commit bc8418b
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 9 deletions.
10 changes: 6 additions & 4 deletions platform/default/mbgl/storage/offline_download.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ OfflineRegionStatus OfflineDownload::getStatus() const {
}

if (!parser.spriteURL.empty()) {
result->requiredResourceCount += 2;
result->requiredResourceCount += 4;
}

return *result;
Expand Down Expand Up @@ -306,9 +306,11 @@ void OfflineDownload::activateDownload() {
}

if (!parser.spriteURL.empty()) {
auto pixelRatio = definition.match([](auto& reg){ return reg.pixelRatio; });
queueResource(Resource::spriteImage(parser.spriteURL, pixelRatio));
queueResource(Resource::spriteJSON(parser.spriteURL, pixelRatio));
// Always request 1x and @2x sprite images for portability.
queueResource(Resource::spriteImage(parser.spriteURL, 1));
queueResource(Resource::spriteImage(parser.spriteURL, 2));
queueResource(Resource::spriteJSON(parser.spriteURL, 1));
queueResource(Resource::spriteJSON(parser.spriteURL, 2));
}

continueDownload();
Expand Down
12 changes: 7 additions & 5 deletions test/storage/offline_download.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,8 @@ TEST(OfflineDownload, Activate) {
};

test.fileSource.spriteImageResponse = [&] (const Resource& resource) {
EXPECT_EQ("http://127.0.0.1:3000/sprite.png", resource.url);
EXPECT_TRUE(resource.url == "http://127.0.0.1:3000/sprite.png" ||
resource.url == "http://127.0.0.1:3000/sprite@2x.png");
return test.response("sprite.png");
};

Expand All @@ -224,7 +225,8 @@ TEST(OfflineDownload, Activate) {
};

test.fileSource.spriteJSONResponse = [&] (const Resource& resource) {
EXPECT_EQ("http://127.0.0.1:3000/sprite.json", resource.url);
EXPECT_TRUE(resource.url == "http://127.0.0.1:3000/sprite.json" ||
resource.url == "http://127.0.0.1:3000/sprite@2x.json");
return test.response("sprite.json");
};

Expand All @@ -251,7 +253,7 @@ TEST(OfflineDownload, Activate) {

observer->statusChangedFn = [&] (OfflineRegionStatus status) {
if (status.complete()) {
EXPECT_EQ(262u, status.completedResourceCount); // 256 glyphs, 1 tile, 1 style, source, image, sprite image, and sprite json
EXPECT_EQ(264u, status.completedResourceCount); // 256 glyphs, 2 sprite images, 2 sprite jsons, 1 tile, 1 style, source, image
EXPECT_EQ(test.size, status.completedResourceSize);

download.setState(OfflineRegionDownloadState::Inactive);
Expand Down Expand Up @@ -334,7 +336,7 @@ TEST(OfflineDownload, GetStatusStyleComplete) {
EXPECT_EQ(OfflineRegionDownloadState::Inactive, status.downloadState);
EXPECT_EQ(1u, status.completedResourceCount);
EXPECT_EQ(test.size, status.completedResourceSize);
EXPECT_EQ(261u, status.requiredResourceCount);
EXPECT_EQ(263u, status.requiredResourceCount);
EXPECT_FALSE(status.requiredResourceCountIsPrecise);
EXPECT_FALSE(status.complete());
}
Expand All @@ -361,7 +363,7 @@ TEST(OfflineDownload, GetStatusStyleAndSourceComplete) {
EXPECT_EQ(OfflineRegionDownloadState::Inactive, status.downloadState);
EXPECT_EQ(2u, status.completedResourceCount);
EXPECT_EQ(test.size, status.completedResourceSize);
EXPECT_EQ(262u, status.requiredResourceCount);
EXPECT_EQ(264u, status.requiredResourceCount);
EXPECT_TRUE(status.requiredResourceCountIsPrecise);
EXPECT_FALSE(status.complete());
}
Expand Down

0 comments on commit bc8418b

Please sign in to comment.