From bc8418b0682ce9cc7ac508350ea88201fda90a4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Thu, 20 Sep 2018 14:33:08 +0200 Subject: [PATCH] [core] Always request 1x and @2x sprite images for portability 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. --- platform/default/mbgl/storage/offline_download.cpp | 10 ++++++---- test/storage/offline_download.test.cpp | 12 +++++++----- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/platform/default/mbgl/storage/offline_download.cpp b/platform/default/mbgl/storage/offline_download.cpp index 118f3aad880..17af0abf3c0 100644 --- a/platform/default/mbgl/storage/offline_download.cpp +++ b/platform/default/mbgl/storage/offline_download.cpp @@ -209,7 +209,7 @@ OfflineRegionStatus OfflineDownload::getStatus() const { } if (!parser.spriteURL.empty()) { - result->requiredResourceCount += 2; + result->requiredResourceCount += 4; } return *result; @@ -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(); diff --git a/test/storage/offline_download.test.cpp b/test/storage/offline_download.test.cpp index 492e68e8691..5fc0e752dfb 100644 --- a/test/storage/offline_download.test.cpp +++ b/test/storage/offline_download.test.cpp @@ -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"); }; @@ -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"); }; @@ -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); @@ -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()); } @@ -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()); }