From 2171ded437f5dff42f17b5e7eb38a01f6c44d2c0 Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Tue, 18 Dec 2018 17:26:31 -0800 Subject: [PATCH 1/5] [core] Support for excluding ideographic glyphs from offline downloads. --- include/mbgl/storage/offline.hpp | 6 +- platform/default/src/mbgl/storage/offline.cpp | 19 +-- .../src/mbgl/storage/offline_download.cpp | 13 +- src/mbgl/text/glyph_range.hpp | 2 + test/storage/offline.test.cpp | 6 +- test/storage/offline_database.test.cpp | 31 ++--- test/storage/offline_download.test.cpp | 115 +++++++++++++++--- 7 files changed, 146 insertions(+), 46 deletions(-) diff --git a/include/mbgl/storage/offline.hpp b/include/mbgl/storage/offline.hpp index b4e40cb5f3e..b3d258a7e33 100644 --- a/include/mbgl/storage/offline.hpp +++ b/include/mbgl/storage/offline.hpp @@ -29,7 +29,7 @@ class TileID; */ class OfflineTilePyramidRegionDefinition { public: - OfflineTilePyramidRegionDefinition(std::string, LatLngBounds, double, double, float); + OfflineTilePyramidRegionDefinition(std::string, LatLngBounds, double, double, float, bool); /* Private */ const std::string styleURL; @@ -37,6 +37,7 @@ class OfflineTilePyramidRegionDefinition { const double minZoom; const double maxZoom; const float pixelRatio; + const bool includeIdeographs; }; /* @@ -52,7 +53,7 @@ class OfflineTilePyramidRegionDefinition { */ class OfflineGeometryRegionDefinition { public: - OfflineGeometryRegionDefinition(std::string styleURL, Geometry, double minZoom, double maxZoom, float pixelRatio); + OfflineGeometryRegionDefinition(std::string styleURL, Geometry, double minZoom, double maxZoom, float pixelRatio, bool includeIdeographs); /* Private */ const std::string styleURL; @@ -60,6 +61,7 @@ class OfflineGeometryRegionDefinition { const double minZoom; const double maxZoom; const float pixelRatio; + const bool includeIdeographs; }; /* diff --git a/platform/default/src/mbgl/storage/offline.cpp b/platform/default/src/mbgl/storage/offline.cpp index e1ec0acb31d..fd945c724f3 100644 --- a/platform/default/src/mbgl/storage/offline.cpp +++ b/platform/default/src/mbgl/storage/offline.cpp @@ -16,12 +16,13 @@ namespace mbgl { // OfflineTilePyramidRegionDefinition OfflineTilePyramidRegionDefinition::OfflineTilePyramidRegionDefinition( - std::string styleURL_, LatLngBounds bounds_, double minZoom_, double maxZoom_, float pixelRatio_) + std::string styleURL_, LatLngBounds bounds_, double minZoom_, double maxZoom_, float pixelRatio_, bool includeIdeographs_) : styleURL(std::move(styleURL_)), bounds(std::move(bounds_)), minZoom(minZoom_), maxZoom(maxZoom_), - pixelRatio(pixelRatio_) { + pixelRatio(pixelRatio_), + includeIdeographs(includeIdeographs_) { if (minZoom < 0 || maxZoom < 0 || maxZoom < minZoom || pixelRatio < 0 || !std::isfinite(minZoom) || std::isnan(maxZoom) || !std::isfinite(pixelRatio)) { throw std::invalid_argument("Invalid offline region definition"); @@ -31,12 +32,13 @@ OfflineTilePyramidRegionDefinition::OfflineTilePyramidRegionDefinition( // OfflineGeometryRegionDefinition -OfflineGeometryRegionDefinition::OfflineGeometryRegionDefinition(std::string styleURL_, Geometry geometry_, double minZoom_, double maxZoom_, float pixelRatio_) +OfflineGeometryRegionDefinition::OfflineGeometryRegionDefinition(std::string styleURL_, Geometry geometry_, double minZoom_, double maxZoom_, float pixelRatio_, bool includeIdeographs_) : styleURL(styleURL_) , geometry(std::move(geometry_)) , minZoom(minZoom_) , maxZoom(maxZoom_) - , pixelRatio(pixelRatio_) { + , pixelRatio(pixelRatio_) + , includeIdeographs(includeIdeographs_){ if (minZoom < 0 || maxZoom < 0 || maxZoom < minZoom || pixelRatio < 0 || !std::isfinite(minZoom) || std::isnan(maxZoom) || !std::isfinite(pixelRatio)) { throw std::invalid_argument("Invalid offline region definition"); @@ -64,7 +66,8 @@ OfflineRegionDefinition decodeOfflineRegionDefinition(const std::string& region) || !(hasValidBounds() || hasValidGeometry()) || !doc.HasMember("min_zoom") || !doc["min_zoom"].IsDouble() || (doc.HasMember("max_zoom") && !doc["max_zoom"].IsDouble()) - || !doc.HasMember("pixel_ratio") || !doc["pixel_ratio"].IsDouble()) { + || !doc.HasMember("pixel_ratio") || !doc["pixel_ratio"].IsDouble() + || (doc.HasMember("include_ideographs") && !doc["include_ideographs"].IsBool())) { throw std::runtime_error("Malformed offline region definition"); } @@ -74,6 +77,7 @@ OfflineRegionDefinition decodeOfflineRegionDefinition(const std::string& region) double minZoom = doc["min_zoom"].GetDouble(); double maxZoom = doc.HasMember("max_zoom") ? doc["max_zoom"].GetDouble() : INFINITY; float pixelRatio = doc["pixel_ratio"].GetDouble(); + bool includeIdeographs = doc.HasMember("include_ideographs") ? doc["include_ideographs"].GetBool() : true; if (doc.HasMember("bounds")) { return OfflineTilePyramidRegionDefinition{ @@ -81,12 +85,12 @@ OfflineRegionDefinition decodeOfflineRegionDefinition(const std::string& region) LatLngBounds::hull( LatLng(doc["bounds"][0].GetDouble(), doc["bounds"][1].GetDouble()), LatLng(doc["bounds"][2].GetDouble(), doc["bounds"][3].GetDouble())), - minZoom, maxZoom, pixelRatio }; + minZoom, maxZoom, pixelRatio, includeIdeographs }; } else { return OfflineGeometryRegionDefinition{ styleURL, mapbox::geojson::convert>(doc["geometry"].GetObject()), - minZoom, maxZoom, pixelRatio }; + minZoom, maxZoom, pixelRatio, includeIdeographs }; }; } @@ -104,6 +108,7 @@ std::string encodeOfflineRegionDefinition(const OfflineRegionDefinition& region) } doc.AddMember("pixel_ratio", _region.pixelRatio, doc.GetAllocator()); + doc.AddMember("include_ideographs", _region.includeIdeographs, doc.GetAllocator()); }); // Encode specific properties diff --git a/platform/default/src/mbgl/storage/offline_download.cpp b/platform/default/src/mbgl/storage/offline_download.cpp index c97797a5a25..d60ae786dd4 100644 --- a/platform/default/src/mbgl/storage/offline_download.cpp +++ b/platform/default/src/mbgl/storage/offline_download.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -205,7 +206,10 @@ OfflineRegionStatus OfflineDownload::getStatus() const { } if (!parser.glyphURL.empty()) { - result->requiredResourceCount += parser.fontStacks().size() * GLYPH_RANGES_PER_FONT_STACK; + result->requiredResourceCount += parser.fontStacks().size() * + (definition.match([](auto& reg){ return reg.includeIdeographs; }) ? + GLYPH_RANGES_PER_FONT_STACK : + NON_IDEOGRAPH_GLYPH_RANGES_PER_FONT_STACK); } if (!parser.spriteURL.empty()) { @@ -298,9 +302,14 @@ void OfflineDownload::activateDownload() { } if (!parser.glyphURL.empty()) { + const bool includeIdeographs = definition.match([](auto& reg){ return reg.includeIdeographs; }); for (const auto& fontStack : parser.fontStacks()) { for (char16_t i = 0; i < GLYPH_RANGES_PER_FONT_STACK; i++) { - queueResource(Resource::glyphs(parser.glyphURL, fontStack, getGlyphRange(i * GLYPHS_PER_GLYPH_RANGE))); + // Assumes that if a glyph range starts with fixed width/ideographic characters, the entire + // range will be fixed width. + if (includeIdeographs || !util::i18n::allowsFixedWidthGlyphGeneration(i * GLYPHS_PER_GLYPH_RANGE)) { + queueResource(Resource::glyphs(parser.glyphURL, fontStack, getGlyphRange(i * GLYPHS_PER_GLYPH_RANGE))); + } } } } diff --git a/src/mbgl/text/glyph_range.hpp b/src/mbgl/text/glyph_range.hpp index 35a1f74b94f..17dc8c82725 100644 --- a/src/mbgl/text/glyph_range.hpp +++ b/src/mbgl/text/glyph_range.hpp @@ -11,6 +11,8 @@ using GlyphRange = std::pair; constexpr uint32_t GLYPHS_PER_GLYPH_RANGE = 256; constexpr uint32_t GLYPH_RANGES_PER_FONT_STACK = 256; +// 256 - 126 ranges skipped w/ i18n::allowsFixedWidthGlyphGeneration +constexpr uint32_t NON_IDEOGRAPH_GLYPH_RANGES_PER_FONT_STACK = 130; } // end namespace mbgl diff --git a/test/storage/offline.test.cpp b/test/storage/offline.test.cpp index 90f95703202..46e7d68c3f0 100644 --- a/test/storage/offline.test.cpp +++ b/test/storage/offline.test.cpp @@ -8,7 +8,7 @@ using SourceType = mbgl::style::SourceType; TEST(OfflineTilePyramidRegionDefinition, EncodeDecode) { - OfflineTilePyramidRegionDefinition region("mapbox://style", LatLngBounds::hull({ 37.6609, -122.5744 }, { 37.8271, -122.3204 }), 0, 20, 1.0); + OfflineTilePyramidRegionDefinition region("mapbox://style", LatLngBounds::hull({ 37.6609, -122.5744 }, { 37.8271, -122.3204 }), 0, 20, 1.0, true); auto encoded = encodeOfflineRegionDefinition(region); auto decoded = decodeOfflineRegionDefinition(encoded).get(); @@ -19,10 +19,11 @@ TEST(OfflineTilePyramidRegionDefinition, EncodeDecode) { EXPECT_EQ(decoded.pixelRatio, region.pixelRatio); EXPECT_EQ(decoded.bounds.southwest(), region.bounds.southwest()); EXPECT_EQ(decoded.bounds.northeast(), region.bounds.northeast()); + EXPECT_EQ(decoded.includeIdeographs, region.includeIdeographs); } TEST(OfflineGeometryRegionDefinition, EncodeDecode) { - OfflineGeometryRegionDefinition region("mapbox://style", Point(-122.5744, 37.6609), 0, 2, 1.0); + OfflineGeometryRegionDefinition region("mapbox://style", Point(-122.5744, 37.6609), 0, 2, 1.0, false); auto encoded = encodeOfflineRegionDefinition(region); auto decoded = decodeOfflineRegionDefinition(encoded).get(); @@ -32,4 +33,5 @@ TEST(OfflineGeometryRegionDefinition, EncodeDecode) { EXPECT_EQ(decoded.maxZoom, region.maxZoom); EXPECT_EQ(decoded.pixelRatio, region.pixelRatio); EXPECT_EQ(decoded.geometry, region.geometry); + EXPECT_EQ(decoded.includeIdeographs, region.includeIdeographs); } diff --git a/test/storage/offline_database.test.cpp b/test/storage/offline_database.test.cpp index 809771af07d..d6405fd4525 100644 --- a/test/storage/offline_database.test.cpp +++ b/test/storage/offline_database.test.cpp @@ -412,7 +412,7 @@ TEST(OfflineDatabase, PutTileNotFound) { TEST(OfflineDatabase, CreateRegion) { FixtureLog log; OfflineDatabase db(":memory:"); - OfflineTilePyramidRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0 }; + OfflineTilePyramidRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0, true }; OfflineRegionMetadata metadata {{ 1, 2, 3 }}; auto region = db.createRegion(definition, metadata); ASSERT_TRUE(region); @@ -426,6 +426,7 @@ TEST(OfflineDatabase, CreateRegion) { EXPECT_EQ(definition.minZoom, def.minZoom); EXPECT_EQ(definition.maxZoom, def.maxZoom); EXPECT_EQ(definition.pixelRatio, def.pixelRatio); + EXPECT_EQ(definition.includeIdeographs, def.includeIdeographs); }, [](auto&) { EXPECT_FALSE(false); } @@ -436,7 +437,7 @@ TEST(OfflineDatabase, CreateRegion) { TEST(OfflineDatabase, UpdateMetadata) { FixtureLog log; OfflineDatabase db(":memory:"); - OfflineTilePyramidRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0 }; + OfflineTilePyramidRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0, true }; OfflineRegionMetadata metadata {{ 1, 2, 3 }}; auto region = db.createRegion(definition, metadata); ASSERT_TRUE(region); @@ -452,7 +453,7 @@ TEST(OfflineDatabase, UpdateMetadata) { TEST(OfflineDatabase, ListRegions) { FixtureLog log; OfflineDatabase db(":memory:"); - OfflineTilePyramidRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0 }; + OfflineTilePyramidRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0, false }; OfflineRegionMetadata metadata {{ 1, 2, 3 }}; auto region = db.createRegion(definition, metadata); @@ -469,6 +470,7 @@ TEST(OfflineDatabase, ListRegions) { EXPECT_EQ(definition.minZoom, def.minZoom); EXPECT_EQ(definition.maxZoom, def.maxZoom); EXPECT_EQ(definition.pixelRatio, def.pixelRatio); + EXPECT_EQ(definition.includeIdeographs, def.includeIdeographs); }, [&](auto&) { EXPECT_FALSE(false); @@ -481,7 +483,7 @@ TEST(OfflineDatabase, ListRegions) { TEST(OfflineDatabase, GetRegionDefinition) { FixtureLog log; OfflineDatabase db(":memory:"); - OfflineTilePyramidRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0 }; + OfflineTilePyramidRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0, false }; OfflineRegionMetadata metadata {{ 1, 2, 3 }}; EXPECT_EQ(0u, log.uncheckedCount()); @@ -494,6 +496,7 @@ TEST(OfflineDatabase, GetRegionDefinition) { EXPECT_EQ(definition.minZoom, result.minZoom); EXPECT_EQ(definition.maxZoom, result.maxZoom); EXPECT_EQ(definition.pixelRatio, result.pixelRatio); + EXPECT_EQ(definition.includeIdeographs, result.includeIdeographs); }, [&](auto&) { EXPECT_FALSE(false); @@ -504,7 +507,7 @@ TEST(OfflineDatabase, GetRegionDefinition) { TEST(OfflineDatabase, DeleteRegion) { FixtureLog log; OfflineDatabase db(":memory:"); - OfflineTilePyramidRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0 }; + OfflineTilePyramidRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0, true }; OfflineRegionMetadata metadata {{ 1, 2, 3 }}; auto region = db.createRegion(definition, metadata); ASSERT_TRUE(region); @@ -526,7 +529,7 @@ TEST(OfflineDatabase, DeleteRegion) { TEST(OfflineDatabase, CreateRegionInfiniteMaxZoom) { FixtureLog log; OfflineDatabase db(":memory:"); - OfflineTilePyramidRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0 }; + OfflineTilePyramidRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0, false }; OfflineRegionMetadata metadata; auto region = db.createRegion(definition, metadata); ASSERT_TRUE(region); @@ -619,7 +622,7 @@ TEST(OfflineDatabase, PutEvictsLeastRecentlyUsedResources) { TEST(OfflineDatabase, PutRegionResourceDoesNotEvict) { FixtureLog log; OfflineDatabase db(":memory:", 1024 * 100); - OfflineTilePyramidRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0 }; + OfflineTilePyramidRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0, true }; auto region = db.createRegion(definition, OfflineRegionMetadata()); ASSERT_TRUE(region); @@ -656,7 +659,7 @@ TEST(OfflineDatabase, PutFailsWhenEvictionInsuffices) { TEST(OfflineDatabase, GetRegionCompletedStatus) { FixtureLog log; OfflineDatabase db(":memory:"); - OfflineTilePyramidRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0 }; + OfflineTilePyramidRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0, false }; OfflineRegionMetadata metadata; auto region = db.createRegion(definition, metadata); ASSERT_TRUE(region); @@ -695,7 +698,7 @@ TEST(OfflineDatabase, GetRegionCompletedStatus) { TEST(OfflineDatabase, HasRegionResource) { FixtureLog log; OfflineDatabase db(":memory:", 1024 * 100); - OfflineTilePyramidRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0 }; + OfflineTilePyramidRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0, true }; auto region = db.createRegion(definition, OfflineRegionMetadata()); ASSERT_TRUE(region); @@ -719,7 +722,7 @@ TEST(OfflineDatabase, HasRegionResource) { TEST(OfflineDatabase, HasRegionResourceTile) { FixtureLog log; OfflineDatabase db(":memory:", 1024 * 100); - OfflineTilePyramidRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0 }; + OfflineTilePyramidRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0, false }; auto region = db.createRegion(definition, OfflineRegionMetadata()); ASSERT_TRUE(region); @@ -753,7 +756,7 @@ TEST(OfflineDatabase, HasRegionResourceTile) { TEST(OfflineDatabase, OfflineMapboxTileCount) { FixtureLog log; OfflineDatabase db(":memory:"); - OfflineTilePyramidRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0 }; + OfflineTilePyramidRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0 , true}; OfflineRegionMetadata metadata; auto region1 = db.createRegion(definition, metadata); @@ -814,7 +817,7 @@ TEST(OfflineDatabase, OfflineMapboxTileCount) { TEST(OfflineDatabase, BatchInsertion) { FixtureLog log; OfflineDatabase db(":memory:", 1024 * 100); - OfflineTilePyramidRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0 }; + OfflineTilePyramidRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0, true }; auto region = db.createRegion(definition, OfflineRegionMetadata()); ASSERT_TRUE(region); @@ -840,7 +843,7 @@ TEST(OfflineDatabase, BatchInsertionMapboxTileCountExceeded) { FixtureLog log; OfflineDatabase db(":memory:", 1024 * 100); db.setOfflineMapboxTileCountLimit(1); - OfflineTilePyramidRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0 }; + OfflineTilePyramidRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0, false }; auto region = db.createRegion(definition, OfflineRegionMetadata()); ASSERT_TRUE(region); @@ -1051,7 +1054,7 @@ TEST(OfflineDatabase, TEST_REQUIRES_WRITE(DisallowedIO)) { // First, create a region object so that we can try deleting it later. OfflineTilePyramidRegionDefinition definition( - "mapbox://style", LatLngBounds::hull({ 37.66, -122.57 }, { 37.83, -122.32 }), 0, 8, 2); + "mapbox://style", LatLngBounds::hull({ 37.66, -122.57 }, { 37.83, -122.32 }), 0, 8, 2, false); auto region = db.createRegion(definition, {}); ASSERT_TRUE(region); diff --git a/test/storage/offline_download.test.cpp b/test/storage/offline_download.test.cpp index 2195585bf53..68f8ea24847 100644 --- a/test/storage/offline_download.test.cpp +++ b/test/storage/offline_download.test.cpp @@ -66,7 +66,7 @@ class OfflineTest { std::size_t size = 0; auto createRegion() { - OfflineTilePyramidRegionDefinition definition { "", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 1.0 }; + OfflineTilePyramidRegionDefinition definition { "", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 1.0, true }; OfflineRegionMetadata metadata; return db.createRegion(definition, metadata); } @@ -87,7 +87,7 @@ TEST(OfflineDownload, NoSubresources) { ASSERT_TRUE(region); OfflineDownload download( region->getID(), - OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), + OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0, false), test.db, test.fileSource); test.fileSource.styleResponse = [&] (const Resource& resource) { @@ -128,7 +128,7 @@ TEST(OfflineDownload, InlineSource) { ASSERT_TRUE(region); OfflineDownload download( region->getID(), - OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), + OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0, true), test.db, test.fileSource); test.fileSource.styleResponse = [&] (const Resource& resource) { @@ -169,7 +169,7 @@ TEST(OfflineDownload, GeoJSONSource) { ASSERT_TRUE(region); OfflineDownload download( region->getID(), - OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), + OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0, false), test.db, test.fileSource); test.fileSource.styleResponse = [&] (const Resource& resource) { @@ -205,7 +205,7 @@ TEST(OfflineDownload, Activate) { ASSERT_TRUE(region); OfflineDownload download( region->getID(), - OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), + OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0, true), test.db, test.fileSource); test.fileSource.styleResponse = [&] (const Resource& resource) { @@ -276,6 +276,83 @@ TEST(OfflineDownload, Activate) { test.loop.run(); } +TEST(OfflineDownload, ExcludeIdeographs) { + OfflineTest test; + auto region = test.createRegion(); + ASSERT_TRUE(region); + OfflineDownload download( + region->getID(), + OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0, false), + test.db, test.fileSource); + + test.fileSource.styleResponse = [&] (const Resource& resource) { + EXPECT_EQ("http://127.0.0.1:3000/style.json", resource.url); + return test.response("style.json"); + }; + + test.fileSource.spriteImageResponse = [&] (const Resource& resource) { + 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"); + }; + + test.fileSource.imageResponse = [&] (const Resource& resource) { + EXPECT_EQ("http://127.0.0.1:3000/radar.gif", resource.url); + return test.response("radar.gif"); + }; + + test.fileSource.spriteJSONResponse = [&] (const Resource& resource) { + 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"); + }; + + test.fileSource.glyphsResponse = [&] (const Resource&) { + return test.response("glyph.pbf"); + }; + + test.fileSource.sourceResponse = [&] (const Resource& resource) { + EXPECT_EQ("http://127.0.0.1:3000/streets.json", resource.url); + return test.response("streets.json"); + }; + + test.fileSource.tileResponse = [&] (const Resource& resource) { + const Resource::TileData& tile = *resource.tileData; + EXPECT_EQ("http://127.0.0.1:3000/{z}-{x}-{y}.vector.pbf", tile.urlTemplate); + EXPECT_EQ(1, tile.pixelRatio); + EXPECT_EQ(0, tile.x); + EXPECT_EQ(0, tile.y); + EXPECT_EQ(0, tile.z); + return test.response("0-0-0.vector.pbf"); + }; + + auto observer = std::make_unique(); + + observer->statusChangedFn = [&] (OfflineRegionStatus status) { + if (status.complete()) { + EXPECT_EQ(138u, status.completedResourceCount); // 130 glyphs, 2 sprite images, 2 sprite jsons, 1 tile, 1 style, source, image + EXPECT_EQ(test.size, status.completedResourceSize); + + download.setState(OfflineRegionDownloadState::Inactive); + OfflineRegionStatus computedStatus = download.getStatus(); + EXPECT_EQ(OfflineRegionDownloadState::Inactive, computedStatus.downloadState); + EXPECT_EQ(status.requiredResourceCount, computedStatus.requiredResourceCount); + EXPECT_EQ(status.completedResourceCount, computedStatus.completedResourceCount); + EXPECT_EQ(status.completedResourceSize, computedStatus.completedResourceSize); + EXPECT_EQ(status.completedTileCount, computedStatus.completedTileCount); + EXPECT_EQ(status.completedTileSize, computedStatus.completedTileSize); + EXPECT_TRUE(status.requiredResourceCountIsPrecise); + + test.loop.stop(); + } + }; + + download.setObserver(std::move(observer)); + download.setState(OfflineRegionDownloadState::Active); + + test.loop.run(); +} + TEST(OfflineDownload, DoesNotFloodTheFileSourceWithRequests) { OfflineTest test; FakeOnlineFileSource fileSource; @@ -283,7 +360,7 @@ TEST(OfflineDownload, DoesNotFloodTheFileSourceWithRequests) { ASSERT_TRUE(region); OfflineDownload download( region->getID(), - OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), + OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0, true), test.db, fileSource); auto observer = std::make_unique(); @@ -306,7 +383,7 @@ TEST(OfflineDownload, GetStatusNoResources) { ASSERT_TRUE(region); OfflineDownload download( region->getID(), - OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), + OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0, false), test.db, test.fileSource); OfflineRegionStatus status = download.getStatus(); @@ -324,7 +401,7 @@ TEST(OfflineDownload, GetStatusStyleComplete) { ASSERT_TRUE(region); OfflineDownload download( region->getID(), - OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), + OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0, true), test.db, test.fileSource); test.db.putRegionResource(1, @@ -347,7 +424,7 @@ TEST(OfflineDownload, GetStatusStyleAndSourceComplete) { ASSERT_TRUE(region); OfflineDownload download( region->getID(), - OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), + OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0, true), test.db, test.fileSource); test.db.putRegionResource(1, @@ -374,7 +451,7 @@ TEST(OfflineDownload, RequestError) { ASSERT_TRUE(region); OfflineDownload download( region->getID(), - OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), + OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0, false), test.db, test.fileSource); test.fileSource.styleResponse = [&] (const Resource&) { @@ -403,7 +480,7 @@ TEST(OfflineDownload, RequestErrorsAreRetried) { ASSERT_TRUE(region); OfflineDownload download( region->getID(), - OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), + OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0, true), test.db, test.fileSource); test.fileSource.styleResponse = [&] (const Resource&) { @@ -437,7 +514,7 @@ TEST(OfflineDownload, TileCountLimitExceededNoTileResponse) { ASSERT_TRUE(region); OfflineDownload download( region->getID(), - OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), + OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0, false), test.db, test.fileSource); uint64_t tileLimit = 0; @@ -480,7 +557,7 @@ TEST(OfflineDownload, TileCountLimitExceededWithTileResponse) { ASSERT_TRUE(region); OfflineDownload download( region->getID(), - OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), + OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0, true), test.db, test.fileSource); uint64_t tileLimit = 1; @@ -535,7 +612,7 @@ TEST(OfflineDownload, WithPreviouslyExistingTile) { ASSERT_TRUE(region); OfflineDownload download( region->getID(), - OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), + OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0, false), test.db, test.fileSource); test.fileSource.styleResponse = [&] (const Resource& resource) { @@ -570,7 +647,7 @@ TEST(OfflineDownload, ReactivatePreviouslyCompletedDownload) { ASSERT_TRUE(region); OfflineDownload download( region->getID(), - OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), + OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0, true), test.db, test.fileSource); test.fileSource.styleResponse = [&] (const Resource& resource) { @@ -596,7 +673,7 @@ TEST(OfflineDownload, ReactivatePreviouslyCompletedDownload) { OfflineDownload redownload( region->getID(), - OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), + OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0, false), test.db, test.fileSource); std::vector statusesAfterReactivate; @@ -638,7 +715,7 @@ TEST(OfflineDownload, Deactivate) { ASSERT_TRUE(region); OfflineDownload download( region->getID(), - OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), + OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0, false), test.db, test.fileSource); test.fileSource.styleResponse = [&] (const Resource& resource) { @@ -669,7 +746,7 @@ TEST(OfflineDownload, AllOfflineRequestsHaveLowPriority) { ASSERT_TRUE(region); OfflineDownload download( region->getID(), - OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), + OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0, true), test.db, test.fileSource); test.fileSource.styleResponse = [&] (const Resource& resource) { @@ -740,7 +817,7 @@ TEST(OfflineDownload, DiskFull) { OfflineDownload download( region->getID(), - OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), + OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0, false), test.db, test.fileSource); bool hasRequestedStyle = false; From 462455c553cbb9b3e414b8c4b3b3ebae118d8b45 Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Tue, 18 Dec 2018 17:26:31 -0800 Subject: [PATCH 2/5] [core] Expose "includeIdeographs" in offline download command line tool. --- bin/offline.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bin/offline.cpp b/bin/offline.cpp index 398f8427eea..e5ce1f75fe3 100644 --- a/bin/offline.cpp +++ b/bin/offline.cpp @@ -101,6 +101,7 @@ int main(int argc, char *argv[]) { args::ValueFlag minZoomValue(argumentParser, "number", "Min zoom level", {"minZoom"}); args::ValueFlag maxZoomValue(argumentParser, "number", "Max zoom level", {"maxZoom"}); args::ValueFlag pixelRatioValue(argumentParser, "number", "Pixel ratio", {"pixelRatio"}); + args::ValueFlag includeIdeographsValue(argumentParser, "boolean", "Include CJK glyphs", {"includeIdeographs"}); try { argumentParser.ParseCLI(argc, argv); @@ -127,6 +128,7 @@ int main(int argc, char *argv[]) { const double minZoom = minZoomValue ? args::get(minZoomValue) : 0.0; const double maxZoom = maxZoomValue ? args::get(maxZoomValue) : 15.0; const double pixelRatio = pixelRatioValue ? args::get(pixelRatioValue) : 1.0; + const bool includeIdeographs = includeIdeographsValue ? args::get(includeIdeographsValue) : false; const std::string output = outputValue ? args::get(outputValue) : "offline.db"; using namespace mbgl; @@ -136,7 +138,7 @@ int main(int argc, char *argv[]) { try { std::string json = readFile(geometryValue.Get()); auto geometry = parseGeometry(json); - return OfflineRegionDefinition{ OfflineGeometryRegionDefinition(style, geometry, minZoom, maxZoom, pixelRatio) }; + return OfflineRegionDefinition{ OfflineGeometryRegionDefinition(style, geometry, minZoom, maxZoom, pixelRatio, includeIdeographs) }; } catch(const std::runtime_error& e) { std::cerr << "Could not parse geojson file " << geometryValue.Get() << ": " << e.what() << std::endl; exit(1); @@ -148,7 +150,7 @@ int main(int argc, char *argv[]) { const double south = southValue ? args::get(southValue) : 38.1; const double east = eastValue ? args::get(eastValue) : -121.7; LatLngBounds boundingBox = LatLngBounds::hull(LatLng(north, west), LatLng(south, east)); - return OfflineRegionDefinition{ OfflineTilePyramidRegionDefinition(style, boundingBox, minZoom, maxZoom, pixelRatio) }; + return OfflineRegionDefinition{ OfflineTilePyramidRegionDefinition(style, boundingBox, minZoom, maxZoom, pixelRatio, includeIdeographs) }; } }(); From 1603e11104003717aa53815bb77a8ddfc09eb2df Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Tue, 18 Dec 2018 17:26:31 -0800 Subject: [PATCH 3/5] [ios, macos] Add "includesIdeographicGlyphs" option to MGLOfflineRegion. --- platform/darwin/src/MGLOfflineRegion.h | 13 +++ platform/darwin/src/MGLShapeOfflineRegion.mm | 23 +++-- .../darwin/src/MGLTilePyramidOfflineRegion.mm | 23 +++-- platform/darwin/test/MGLOfflineRegionTests.m | 11 ++- .../darwin/test/MGLOfflineStorageTests.mm | 2 +- .../app/MBXOfflinePacksTableViewController.m | 2 + platform/macos/app/Base.lproj/MapDocument.xib | 84 +++++++++++-------- platform/macos/app/MapDocument.m | 4 + 8 files changed, 109 insertions(+), 53 deletions(-) diff --git a/platform/darwin/src/MGLOfflineRegion.h b/platform/darwin/src/MGLOfflineRegion.h index 3e0f485e2cb..6a9bdbc3998 100644 --- a/platform/darwin/src/MGLOfflineRegion.h +++ b/platform/darwin/src/MGLOfflineRegion.h @@ -19,6 +19,19 @@ NS_ASSUME_NONNULL_BEGIN */ @property (nonatomic, readonly) NSURL *styleURL; +/** + Specifies whether to include ideographic glyphs in downloaded font data. + Ideographic glyphs make up the majority of downloaded font data, but + it is possible to configure the renderer to use locally installed fonts + instead of relying on fonts downloaded as part of the offline pack. + See `MGLIdeographicFontFamilyName` setting. Also, for regions outside of + China, Japan, and Korea, these glyphs will rarely appear for non-CJK users. + + By default, this property is set to `YES`, so that the offline pack will + include ideographic glyphs. + */ +@property (nonatomic) BOOL includesIdeographicGlyphs; + @end NS_ASSUME_NONNULL_END diff --git a/platform/darwin/src/MGLShapeOfflineRegion.mm b/platform/darwin/src/MGLShapeOfflineRegion.mm index 67b9941a498..25b6b8e1667 100644 --- a/platform/darwin/src/MGLShapeOfflineRegion.mm +++ b/platform/darwin/src/MGLShapeOfflineRegion.mm @@ -26,6 +26,7 @@ @implementation MGLShapeOfflineRegion { } @synthesize styleURL = _styleURL; +@synthesize includesIdeographicGlyphs = _includesIdeographicGlyphs; -(NSDictionary *)offlineStartEventAttributes { return @{ @@ -71,6 +72,7 @@ - (instancetype)initWithStyleURL:(NSURL *)styleURL shape:(MGLShape *)shape fromZ _shape = shape; _minimumZoomLevel = minimumZoomLevel; _maximumZoomLevel = maximumZoomLevel; + _includesIdeographicGlyphs = YES; } return self; } @@ -78,7 +80,9 @@ - (instancetype)initWithStyleURL:(NSURL *)styleURL shape:(MGLShape *)shape fromZ - (instancetype)initWithOfflineRegionDefinition:(const mbgl::OfflineGeometryRegionDefinition &)definition { NSURL *styleURL = [NSURL URLWithString:@(definition.styleURL.c_str())]; MGLShape *shape = MGLShapeFromGeoJSON(definition.geometry); - return [self initWithStyleURL:styleURL shape:shape fromZoomLevel:definition.minZoom toZoomLevel:definition.maxZoom]; + MGLShapeOfflineRegion* result = [self initWithStyleURL:styleURL shape:shape fromZoomLevel:definition.minZoom toZoomLevel:definition.maxZoom]; + result.includesIdeographicGlyphs = definition.includeIdeographs; + return result; } - (const mbgl::OfflineRegionDefinition)offlineRegionDefinition { @@ -90,7 +94,7 @@ - (instancetype)initWithOfflineRegionDefinition:(const mbgl::OfflineGeometryRegi return mbgl::OfflineGeometryRegionDefinition(_styleURL.absoluteString.UTF8String, _shape.geometryObject, _minimumZoomLevel, _maximumZoomLevel, - scaleFactor); + scaleFactor, _includesIdeographicGlyphs); } - (nullable instancetype)initWithCoder:(NSCoder *)coder { @@ -100,7 +104,9 @@ - (nullable instancetype)initWithCoder:(NSCoder *)coder { double minimumZoomLevel = [coder decodeDoubleForKey:@"minimumZoomLevel"]; double maximumZoomLevel = [coder decodeDoubleForKey:@"maximumZoomLevel"]; - return [self initWithStyleURL:styleURL shape:shape fromZoomLevel:minimumZoomLevel toZoomLevel:maximumZoomLevel]; + MGLShapeOfflineRegion* result = [self initWithStyleURL:styleURL shape:shape fromZoomLevel:minimumZoomLevel toZoomLevel:maximumZoomLevel]; + result.includesIdeographicGlyphs = [coder decodeBoolForKey:@"includesIdeographicGlyphs"]; + return result; } - (void)encodeWithCoder:(NSCoder *)coder @@ -109,10 +115,13 @@ - (void)encodeWithCoder:(NSCoder *)coder [coder encodeObject:_shape forKey:@"shape"]; [coder encodeDouble:_maximumZoomLevel forKey:@"maximumZoomLevel"]; [coder encodeDouble:_minimumZoomLevel forKey:@"minimumZoomLevel"]; + [coder encodeBool:_includesIdeographicGlyphs forKey:@"includesIdeographicGlyphs"]; } - (id)copyWithZone:(nullable NSZone *)zone { - return [[[self class] allocWithZone:zone] initWithStyleURL:_styleURL shape:_shape fromZoomLevel:_minimumZoomLevel toZoomLevel:_maximumZoomLevel]; + MGLShapeOfflineRegion* result = [[[self class] allocWithZone:zone] initWithStyleURL:_styleURL shape:_shape fromZoomLevel:_minimumZoomLevel toZoomLevel:_maximumZoomLevel]; + result.includesIdeographicGlyphs = _includesIdeographicGlyphs; + return result; } - (BOOL)isEqual:(id)other { @@ -127,13 +136,15 @@ - (BOOL)isEqual:(id)other { return (_minimumZoomLevel == otherRegion->_minimumZoomLevel && _maximumZoomLevel == otherRegion->_maximumZoomLevel && _shape.geometryObject == otherRegion->_shape.geometryObject - && [_styleURL isEqual:otherRegion->_styleURL]); + && [_styleURL isEqual:otherRegion->_styleURL] + && _includesIdeographicGlyphs == otherRegion->_includesIdeographicGlyphs); } - (NSUInteger)hash { return (_styleURL.hash + _shape.hash - + @(_minimumZoomLevel).hash + @(_maximumZoomLevel).hash); + + @(_minimumZoomLevel).hash + @(_maximumZoomLevel).hash + + @(_includesIdeographicGlyphs).hash); } @end diff --git a/platform/darwin/src/MGLTilePyramidOfflineRegion.mm b/platform/darwin/src/MGLTilePyramidOfflineRegion.mm index 4b19b76508a..a398d6baa4a 100644 --- a/platform/darwin/src/MGLTilePyramidOfflineRegion.mm +++ b/platform/darwin/src/MGLTilePyramidOfflineRegion.mm @@ -23,6 +23,7 @@ @implementation MGLTilePyramidOfflineRegion { } @synthesize styleURL = _styleURL; +@synthesize includesIdeographicGlyphs = _includesIdeographicGlyphs; -(NSDictionary *)offlineStartEventAttributes { return @{ @@ -67,6 +68,7 @@ - (instancetype)initWithStyleURL:(NSURL *)styleURL bounds:(MGLCoordinateBounds)b _bounds = bounds; _minimumZoomLevel = minimumZoomLevel; _maximumZoomLevel = maximumZoomLevel; + _includesIdeographicGlyphs = YES; } return self; } @@ -74,7 +76,9 @@ - (instancetype)initWithStyleURL:(NSURL *)styleURL bounds:(MGLCoordinateBounds)b - (instancetype)initWithOfflineRegionDefinition:(const mbgl::OfflineTilePyramidRegionDefinition &)definition { NSURL *styleURL = [NSURL URLWithString:@(definition.styleURL.c_str())]; MGLCoordinateBounds bounds = MGLCoordinateBoundsFromLatLngBounds(definition.bounds); - return [self initWithStyleURL:styleURL bounds:bounds fromZoomLevel:definition.minZoom toZoomLevel:definition.maxZoom]; + MGLTilePyramidOfflineRegion* result = [self initWithStyleURL:styleURL bounds:bounds fromZoomLevel:definition.minZoom toZoomLevel:definition.maxZoom]; + result.includesIdeographicGlyphs = definition.includeIdeographs; + return result; } - (const mbgl::OfflineRegionDefinition)offlineRegionDefinition { @@ -86,7 +90,7 @@ - (instancetype)initWithOfflineRegionDefinition:(const mbgl::OfflineTilePyramidR return mbgl::OfflineTilePyramidRegionDefinition(_styleURL.absoluteString.UTF8String, MGLLatLngBoundsFromCoordinateBounds(_bounds), _minimumZoomLevel, _maximumZoomLevel, - scaleFactor); + scaleFactor, _includesIdeographicGlyphs); } - (nullable instancetype)initWithCoder:(NSCoder *)coder { @@ -100,7 +104,9 @@ - (nullable instancetype)initWithCoder:(NSCoder *)coder { double minimumZoomLevel = [coder decodeDoubleForKey:@"minimumZoomLevel"]; double maximumZoomLevel = [coder decodeDoubleForKey:@"maximumZoomLevel"]; - return [self initWithStyleURL:styleURL bounds:bounds fromZoomLevel:minimumZoomLevel toZoomLevel:maximumZoomLevel]; + MGLTilePyramidOfflineRegion* result = [self initWithStyleURL:styleURL bounds:bounds fromZoomLevel:minimumZoomLevel toZoomLevel:maximumZoomLevel]; + result.includesIdeographicGlyphs = [coder decodeBoolForKey:@"includesIdeographicGlyphs"]; + return result; } - (void)encodeWithCoder:(NSCoder *)coder @@ -112,10 +118,13 @@ - (void)encodeWithCoder:(NSCoder *)coder [coder encodeDouble:_bounds.ne.longitude forKey:@"northEastLongitude"]; [coder encodeDouble:_maximumZoomLevel forKey:@"maximumZoomLevel"]; [coder encodeDouble:_minimumZoomLevel forKey:@"minimumZoomLevel"]; + [coder encodeBool:_includesIdeographicGlyphs forKey:@"includesIdeographicGlyphs"]; } - (id)copyWithZone:(nullable NSZone *)zone { - return [[[self class] allocWithZone:zone] initWithStyleURL:_styleURL bounds:_bounds fromZoomLevel:_minimumZoomLevel toZoomLevel:_maximumZoomLevel]; + MGLTilePyramidOfflineRegion* result = [[[self class] allocWithZone:zone] initWithStyleURL:_styleURL bounds:_bounds fromZoomLevel:_minimumZoomLevel toZoomLevel:_maximumZoomLevel]; + result.includesIdeographicGlyphs = _includesIdeographicGlyphs; + return result; } - (BOOL)isEqual:(id)other { @@ -130,14 +139,16 @@ - (BOOL)isEqual:(id)other { return (_minimumZoomLevel == otherRegion->_minimumZoomLevel && _maximumZoomLevel == otherRegion->_maximumZoomLevel && MGLCoordinateBoundsEqualToCoordinateBounds(_bounds, otherRegion->_bounds) - && [_styleURL isEqual:otherRegion->_styleURL]); + && [_styleURL isEqual:otherRegion->_styleURL] + && _includesIdeographicGlyphs == otherRegion->_includesIdeographicGlyphs); } - (NSUInteger)hash { return (_styleURL.hash + @(_bounds.sw.latitude).hash + @(_bounds.sw.longitude).hash + @(_bounds.ne.latitude).hash + @(_bounds.ne.longitude).hash - + @(_minimumZoomLevel).hash + @(_maximumZoomLevel).hash); + + @(_minimumZoomLevel).hash + @(_maximumZoomLevel).hash + + @(_includesIdeographicGlyphs).hash); } @end diff --git a/platform/darwin/test/MGLOfflineRegionTests.m b/platform/darwin/test/MGLOfflineRegionTests.m index eac6da9b54b..4d5767a8d26 100644 --- a/platform/darwin/test/MGLOfflineRegionTests.m +++ b/platform/darwin/test/MGLOfflineRegionTests.m @@ -25,8 +25,9 @@ - (void)testTilePyramidRegionEquality { XCTAssertEqualObjects(original.styleURL, copy.styleURL, @"Style URL has changed."); XCTAssert(MGLCoordinateBoundsEqualToCoordinateBounds(original.bounds, copy.bounds), @"Bounds have changed."); - XCTAssertEqual(original.minimumZoomLevel, original.minimumZoomLevel, @"Minimum zoom level has changed."); - XCTAssertEqual(original.maximumZoomLevel, original.maximumZoomLevel, @"Maximum zoom level has changed."); + XCTAssertEqual(original.minimumZoomLevel, copy.minimumZoomLevel, @"Minimum zoom level has changed."); + XCTAssertEqual(original.maximumZoomLevel, copy.maximumZoomLevel, @"Maximum zoom level has changed."); + XCTAssertEqual(original.includesIdeographicGlyphs, copy.includesIdeographicGlyphs, @"Include ideographs has changed."); } - (void)testGeometryRegionEquality { @@ -36,13 +37,15 @@ - (void)testGeometryRegionEquality { XCTAssertNil(error); MGLShapeOfflineRegion *original = [[MGLShapeOfflineRegion alloc] initWithStyleURL:[MGLStyle lightStyleURLWithVersion:MGLStyleDefaultVersion] shape:shape fromZoomLevel:5 toZoomLevel:10]; + original.includesIdeographicGlyphs = NO; MGLShapeOfflineRegion *copy = [original copy]; XCTAssertEqualObjects(original, copy, @"Shape region should be equal to its copy."); XCTAssertEqualObjects(original.styleURL, copy.styleURL, @"Style URL has changed."); XCTAssertEqualObjects(original.shape, copy.shape, @"Geometry has changed."); - XCTAssertEqual(original.minimumZoomLevel, original.minimumZoomLevel, @"Minimum zoom level has changed."); - XCTAssertEqual(original.maximumZoomLevel, original.maximumZoomLevel, @"Maximum zoom level has changed."); + XCTAssertEqual(original.minimumZoomLevel, copy.minimumZoomLevel, @"Minimum zoom level has changed."); + XCTAssertEqual(original.maximumZoomLevel, copy.maximumZoomLevel, @"Maximum zoom level has changed."); + XCTAssertEqual(original.includesIdeographicGlyphs, copy.includesIdeographicGlyphs, @"Include ideographs has changed."); } @end diff --git a/platform/darwin/test/MGLOfflineStorageTests.mm b/platform/darwin/test/MGLOfflineStorageTests.mm index f5d7ed28e12..6fb787b556c 100644 --- a/platform/darwin/test/MGLOfflineStorageTests.mm +++ b/platform/darwin/test/MGLOfflineStorageTests.mm @@ -140,7 +140,7 @@ - (void)testAddPackForGeometry { MGLShape *shape = [MGLShape shapeWithData: [geojson dataUsingEncoding:NSUTF8StringEncoding] encoding: NSUTF8StringEncoding error:&error]; XCTAssertNil(error); MGLShapeOfflineRegion *region = [[MGLShapeOfflineRegion alloc] initWithStyleURL:styleURL shape:shape fromZoomLevel:zoomLevel toZoomLevel:zoomLevel]; - + region.includesIdeographicGlyphs = NO; NSString *nameKey = @"Name"; NSString *name = @"Utrecht centrum"; diff --git a/platform/ios/app/MBXOfflinePacksTableViewController.m b/platform/ios/app/MBXOfflinePacksTableViewController.m index 497784f36b9..fe63f4a02ef 100644 --- a/platform/ios/app/MBXOfflinePacksTableViewController.m +++ b/platform/ios/app/MBXOfflinePacksTableViewController.m @@ -102,7 +102,9 @@ - (IBAction)addCurrentRegion:(id)sender { name = nameField.placeholder; } + NSString *fontFamilyName = [[NSBundle mainBundle] objectForInfoDictionaryKey:@"MGLIdeographicFontFamilyName"]; MGLTilePyramidOfflineRegion *region = [[MGLTilePyramidOfflineRegion alloc] initWithStyleURL:mapView.styleURL bounds:mapView.visibleCoordinateBounds fromZoomLevel:mapView.zoomLevel toZoomLevel:mapView.maximumZoomLevel]; + region.includesIdeographicGlyphs = fontFamilyName; NSData *context = [NSKeyedArchiver archivedDataWithRootObject:@{ MBXOfflinePackContextNameKey: name, }]; diff --git a/platform/macos/app/Base.lproj/MapDocument.xib b/platform/macos/app/Base.lproj/MapDocument.xib index 72fa024fcca..dd8237dbb55 100644 --- a/platform/macos/app/Base.lproj/MapDocument.xib +++ b/platform/macos/app/Base.lproj/MapDocument.xib @@ -1,14 +1,15 @@ - + - + + @@ -55,10 +56,10 @@ - + - + @@ -130,11 +131,11 @@ - - - + + - - - - - - - - - - + + @@ -384,16 +377,16 @@ - - + + - - + + @@ -406,7 +399,7 @@ - + + + + + + + + + + @@ -471,7 +483,7 @@ Gw - + diff --git a/platform/macos/app/MapDocument.m b/platform/macos/app/MapDocument.m index d2bc7e9bfc1..213aa33107e 100644 --- a/platform/macos/app/MapDocument.m +++ b/platform/macos/app/MapDocument.m @@ -82,6 +82,7 @@ @interface MapDocument () Date: Tue, 18 Dec 2018 17:26:31 -0800 Subject: [PATCH 4/5] [android] Add "localIdeographs" option to OfflineRegionDefinition. --- .../OfflineGeometryRegionDefinition.java | 29 ++++++++++++++++++- .../offline/OfflineRegionDefinition.java | 15 +++++++++- .../OfflineTilePyramidRegionDefinition.java | 29 ++++++++++++++++++- .../src/offline/offline_region_definition.cpp | 18 ++++++++---- 4 files changed, 82 insertions(+), 9 deletions(-) diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/offline/OfflineGeometryRegionDefinition.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/offline/OfflineGeometryRegionDefinition.java index 7cf9d4b2db2..01ceb66b2bc 100644 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/offline/OfflineGeometryRegionDefinition.java +++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/offline/OfflineGeometryRegionDefinition.java @@ -21,6 +21,8 @@ * tiles from minZoom up to the maximum zoom level provided by that source. *

* pixelRatio must be ≥ 0 and should typically be 1.0 or 2.0. + *

+ * if includeIdeographs is false, offline region will not include CJK glyphs */ public class OfflineGeometryRegionDefinition implements OfflineRegionDefinition, Parcelable { @@ -35,6 +37,23 @@ public class OfflineGeometryRegionDefinition implements OfflineRegionDefinition, private double maxZoom; @Keep private float pixelRatio; + @Keep + private boolean includeIdeographs; + + /** + * Constructor to create an OfflineGeometryRegionDefinition from parameters. + * + * @param styleURL the style + * @param geometry the geometry + * @param minZoom min zoom + * @param maxZoom max zoom + * @param pixelRatio pixel ratio of the device + */ + @Keep + public OfflineGeometryRegionDefinition( + String styleURL, Geometry geometry, double minZoom, double maxZoom, float pixelRatio) { + this(styleURL, geometry, minZoom, maxZoom, pixelRatio, true); + } /** * Constructor to create an OfflineGeometryRegionDefinition from parameters. @@ -44,16 +63,19 @@ public class OfflineGeometryRegionDefinition implements OfflineRegionDefinition, * @param minZoom min zoom * @param maxZoom max zoom * @param pixelRatio pixel ratio of the device + * @param includeIdeographs include glyphs for CJK languages */ @Keep public OfflineGeometryRegionDefinition( - String styleURL, Geometry geometry, double minZoom, double maxZoom, float pixelRatio) { + String styleURL, Geometry geometry, double minZoom, double maxZoom, float pixelRatio, + boolean includeIdeographs) { // Note: Also used in JNI this.styleURL = styleURL; this.geometry = geometry; this.minZoom = minZoom; this.maxZoom = maxZoom; this.pixelRatio = pixelRatio; + this.includeIdeographs = includeIdeographs; } /** @@ -110,6 +132,11 @@ public float getPixelRatio() { return pixelRatio; } + @Override + public boolean getIncludeIdeographs() { + return includeIdeographs; + } + @NonNull @Override public String getType() { diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/offline/OfflineRegionDefinition.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/offline/OfflineRegionDefinition.java index 473d905f1ac..782968fb004 100644 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/offline/OfflineRegionDefinition.java +++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/offline/OfflineRegionDefinition.java @@ -48,7 +48,20 @@ public interface OfflineRegionDefinition { float getPixelRatio(); /** - * Gest the type of the OfflineRegionDefinition for telemetry ("tileregion", "shaperegion"). + * Specifies whether to include ideographic glyphs in downloaded font data. + * Ideographic glyphs make up the majority of downloaded font data, but + * it is possible to configure the renderer to use locally installed fonts + * instead of relying on fonts downloaded as part of the offline pack. + * + * Defaults to `true` + * + * @return true if offline region will include ideographic glyphs + * @see MapboxMapOptions localIdeographFontFamily + */ + boolean getIncludeIdeographs(); + + /** + * Gets the type of the OfflineRegionDefinition for telemetry ("tileregion", "shaperegion"). * * @return The type of the OfflineRegionDefinition. */ diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/offline/OfflineTilePyramidRegionDefinition.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/offline/OfflineTilePyramidRegionDefinition.java index 8649c70acbd..a5339fcac05 100644 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/offline/OfflineTilePyramidRegionDefinition.java +++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/offline/OfflineTilePyramidRegionDefinition.java @@ -18,6 +18,8 @@ * tiles from minZoom up to the maximum zoom level provided by that source. *

* pixelRatio must be ≥ 0 and should typically be 1.0 or 2.0. + *

+ * if includeIdeographs is false, offline region will not include CJK glyphs */ public class OfflineTilePyramidRegionDefinition implements OfflineRegionDefinition, Parcelable { @@ -31,6 +33,23 @@ public class OfflineTilePyramidRegionDefinition implements OfflineRegionDefiniti private double maxZoom; @Keep private float pixelRatio; + @Keep + private boolean includeIdeographs; + + /** + * Constructor to create an OfflineTilePyramidDefinition from parameters. + * + * @param styleURL the style + * @param bounds the bounds + * @param minZoom min zoom + * @param maxZoom max zoom + * @param pixelRatio pixel ratio of the device + */ + @Keep + public OfflineTilePyramidRegionDefinition( + String styleURL, LatLngBounds bounds, double minZoom, double maxZoom, float pixelRatio) { + this(styleURL, bounds, minZoom, maxZoom, pixelRatio, true); + } /** * Constructor to create an OfflineTilePyramidDefinition from parameters. @@ -40,16 +59,19 @@ public class OfflineTilePyramidRegionDefinition implements OfflineRegionDefiniti * @param minZoom min zoom * @param maxZoom max zoom * @param pixelRatio pixel ratio of the device + * @param includeIdeographs include glyphs for CJK languages */ @Keep public OfflineTilePyramidRegionDefinition( - String styleURL, LatLngBounds bounds, double minZoom, double maxZoom, float pixelRatio) { + String styleURL, LatLngBounds bounds, double minZoom, double maxZoom, float pixelRatio, + boolean includeIdeographs) { // Note: Also used in JNI this.styleURL = styleURL; this.bounds = bounds; this.minZoom = minZoom; this.maxZoom = maxZoom; this.pixelRatio = pixelRatio; + this.includeIdeographs = includeIdeographs; } /** @@ -94,6 +116,11 @@ public float getPixelRatio() { return pixelRatio; } + @Override + public boolean getIncludeIdeographs() { + return includeIdeographs; + } + @NonNull @Override public String getType() { diff --git a/platform/android/src/offline/offline_region_definition.cpp b/platform/android/src/offline/offline_region_definition.cpp index 23e5b7466b9..bb9dfc8dd0a 100644 --- a/platform/android/src/offline/offline_region_definition.cpp +++ b/platform/android/src/offline/offline_region_definition.cpp @@ -29,14 +29,15 @@ mbgl::OfflineRegionDefinition OfflineRegionDefinition::getDefinition(JNIEnv& env jni::Local> OfflineTilePyramidRegionDefinition::New(jni::JNIEnv& env, const mbgl::OfflineTilePyramidRegionDefinition& definition) { static auto& javaClass = jni::Class::Singleton(env); - static auto constructor = javaClass.GetConstructor, jni::jdouble, jni::jdouble, jni::jfloat>(env); + static auto constructor = javaClass.GetConstructor, jni::jdouble, jni::jdouble, jni::jfloat, jni::jboolean>(env); return javaClass.New(env, constructor, jni::Make(env, definition.styleURL), LatLngBounds::New(env, definition.bounds), definition.minZoom, definition.maxZoom, - definition.pixelRatio); + definition.pixelRatio, + jni::jboolean(definition.includeIdeographs)); } mbgl::OfflineTilePyramidRegionDefinition OfflineTilePyramidRegionDefinition::getDefinition(jni::JNIEnv& env, const jni::Object& jDefinition) { @@ -47,13 +48,15 @@ mbgl::OfflineTilePyramidRegionDefinition OfflineTilePyramidRegionDefinition::get static auto minZoomF = javaClass.GetField(env, "minZoom"); static auto maxZoomF = javaClass.GetField(env, "maxZoom"); static auto pixelRatioF = javaClass.GetField(env, "pixelRatio"); + static auto includeIdeographsF = javaClass.GetField(env, "includeIdeographs"); return mbgl::OfflineTilePyramidRegionDefinition( jni::Make(env, jDefinition.Get(env, styleURLF)), LatLngBounds::getLatLngBounds(env, jDefinition.Get(env, boundsF)), jDefinition.Get(env, minZoomF), jDefinition.Get(env, maxZoomF), - jDefinition.Get(env, pixelRatioF) + jDefinition.Get(env, pixelRatioF), + jDefinition.Get(env, includeIdeographsF) ); } @@ -65,14 +68,15 @@ void OfflineTilePyramidRegionDefinition::registerNative(jni::JNIEnv& env) { jni::Local> OfflineGeometryRegionDefinition::New(jni::JNIEnv& env, const mbgl::OfflineGeometryRegionDefinition& definition) { static auto& javaClass = jni::Class::Singleton(env); - static auto constructor = javaClass.GetConstructor, jni::jdouble, jni::jdouble, jni::jfloat>(env); + static auto constructor = javaClass.GetConstructor, jni::jdouble, jni::jdouble, jni::jfloat, jni::jboolean>(env); return javaClass.New(env, constructor, jni::Make(env, definition.styleURL), geojson::Geometry::New(env, definition.geometry), definition.minZoom, definition.maxZoom, - definition.pixelRatio); + definition.pixelRatio, + jni::jboolean(definition.includeIdeographs)); } mbgl::OfflineGeometryRegionDefinition OfflineGeometryRegionDefinition::getDefinition(jni::JNIEnv& env, const jni::Object& jDefinition) { @@ -83,13 +87,15 @@ mbgl::OfflineGeometryRegionDefinition OfflineGeometryRegionDefinition::getDefini static auto minZoomF = javaClass.GetField(env, "minZoom"); static auto maxZoomF = javaClass.GetField(env, "maxZoom"); static auto pixelRatioF = javaClass.GetField(env, "pixelRatio"); + static auto includeIdeographsF = javaClass.GetField(env, "includeIdeographs"); return mbgl::OfflineGeometryRegionDefinition( jni::Make(env, jDefinition.Get(env, styleURLF)), geojson::Geometry::convert(env, jDefinition.Get(env, geometryF)), jDefinition.Get(env, minZoomF), jDefinition.Get(env, maxZoomF), - jDefinition.Get(env, pixelRatioF) + jDefinition.Get(env, pixelRatioF), + jDefinition.Get(env, includeIdeographsF) ); } From 822ea188d209c2dbc37f123f94e93bcacf9bf3ce Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Fri, 21 Dec 2018 10:44:45 -0800 Subject: [PATCH 5/5] [docs] Changelog entry for offline CJK glyph control. --- scripts/changelog_staging/no-offline-cjk.json | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 scripts/changelog_staging/no-offline-cjk.json diff --git a/scripts/changelog_staging/no-offline-cjk.json b/scripts/changelog_staging/no-offline-cjk.json new file mode 100644 index 00000000000..cfc63e89915 --- /dev/null +++ b/scripts/changelog_staging/no-offline-cjk.json @@ -0,0 +1,6 @@ +{ + "core": "Add `--includeIdeographs` option to mbgl-offline tool to include CJK glyphs in offline region, at the cost of increased size requirements.", + "darwin": "Added the `MGLOfflineRegion.includesIdeographicGlyphs` property, which you can set to NO to exclude CJK glyphs and save space.", + "android": "Added the `includeIdeographs` property to `OfflineRegionDefinition`, which you can set to false to exclude CJK glyphs and save space.", + "issue": 11561 +}