diff --git a/src/mbgl/annotation/sprite_parser.cpp b/src/mbgl/annotation/sprite_parser.cpp index 0f9ba97c9..f867b97f4 100644 --- a/src/mbgl/annotation/sprite_parser.cpp +++ b/src/mbgl/annotation/sprite_parser.cpp @@ -9,6 +9,7 @@ #include #include +#include namespace mbgl { @@ -99,7 +100,7 @@ inline bool getBoolean(const rapidjson::Value& value, const char* name, const bo } // namespace -Sprites parseSprite(const std::string& image, const std::string& json) { +SpriteParseResult parseSprite(const std::string& image, const std::string& json) { using namespace rapidjson; Sprites sprites; @@ -107,20 +108,18 @@ Sprites parseSprite(const std::string& image, const std::string& json) { // Parse the sprite image. const util::Image raster(image); if (!raster) { - Log::Warning(Event::Sprite, "Could not parse sprite image"); - return sprites; + return std::string("Could not parse sprite image"); } Document doc; doc.Parse<0>(json.c_str()); if (doc.HasParseError()) { - Log::Warning(Event::Sprite, std::string{ "Failed to parse JSON: " } + doc.GetParseError() + - " at offset " + std::to_string(doc.GetErrorOffset())); - return sprites; + std::stringstream message; + message << "Failed to parse JSON: " << doc.GetParseError() << " at offset " << doc.GetErrorOffset(); + return message.str(); } else if (!doc.IsObject()) { - Log::Warning(Event::Sprite, "Sprite JSON root must be an object"); - return sprites; + return std::string("Sprite JSON root must be an object"); } else { for (Value::ConstMemberIterator itr = doc.MemberBegin(); itr != doc.MemberEnd(); ++itr) { const std::string name = { itr->name.GetString(), itr->name.GetStringLength() }; diff --git a/src/mbgl/annotation/sprite_parser.hpp b/src/mbgl/annotation/sprite_parser.hpp index 71228aacf..d7e995335 100644 --- a/src/mbgl/annotation/sprite_parser.hpp +++ b/src/mbgl/annotation/sprite_parser.hpp @@ -3,6 +3,7 @@ #include #include +#include #include #include @@ -31,8 +32,13 @@ SpriteImagePtr createSpriteImage(const util::Image& image, using Sprites = std::map; + +using SpriteParseResult = mapbox::util::variant< + Sprites, // success + std::string>; // error + // Parses an image and an associated JSON file and returns the sprite objects. -Sprites parseSprite(const std::string& image, const std::string& json); +SpriteParseResult parseSprite(const std::string& image, const std::string& json); } // namespace mbgl diff --git a/src/mbgl/map/sprite.cpp b/src/mbgl/map/sprite.cpp index 900b2efa4..5f6190f58 100644 --- a/src/mbgl/map/sprite.cpp +++ b/src/mbgl/map/sprite.cpp @@ -85,11 +85,19 @@ Sprite::~Sprite() { void Sprite::emitSpriteLoadedIfComplete() { assert(loader); - if (loader->loadedImage && loader->loadedJSON && observer) { - std::unique_ptr data(std::move(loader->data)); - loader.reset(); - observer->onSpriteLoaded(std::move(data)); + if (!loader->loadedImage || !loader->loadedJSON || !observer) { + return; + } + + std::unique_ptr data(std::move(loader->data)); + loader.reset(); + + auto result = parseSprite(data->image, data->json); + if (result.is()) { + observer->onSpriteLoaded(result.get()); + } else { + emitSpriteLoadingFailed(result.get()); } } diff --git a/src/mbgl/map/sprite.hpp b/src/mbgl/map/sprite.hpp index d8f91fd9c..b2020e7d6 100644 --- a/src/mbgl/map/sprite.hpp +++ b/src/mbgl/map/sprite.hpp @@ -1,6 +1,7 @@ #ifndef MBGL_STYLE_SPRITE #define MBGL_STYLE_SPRITE +#include #include #include #include @@ -27,7 +28,7 @@ public: public: virtual ~Observer() = default; - virtual void onSpriteLoaded(std::unique_ptr) = 0; + virtual void onSpriteLoaded(const Sprites& sprites) = 0; virtual void onSpriteLoadingFailed(std::exception_ptr error) = 0; }; diff --git a/src/mbgl/style/style.cpp b/src/mbgl/style/style.cpp index 48106d8f3..e091c92f5 100644 --- a/src/mbgl/style/style.cpp +++ b/src/mbgl/style/style.cpp @@ -4,7 +4,6 @@ #include #include #include -#include #include #include #include @@ -191,9 +190,9 @@ void Style::onTileLoadingFailed(std::exception_ptr error) { emitResourceLoadingFailed(error); } -void Style::onSpriteLoaded(std::unique_ptr spriteData) { +void Style::onSpriteLoaded(const Sprites& sprites) { // Add all sprite images to the SpriteStore object - spriteStore->setSprites(parseSprite(spriteData->image, spriteData->json)); + spriteStore->setSprites(sprites); shouldReparsePartialTiles = true; emitTileDataChanged(); diff --git a/src/mbgl/style/style.hpp b/src/mbgl/style/style.hpp index 359ab4d83..d6b762a83 100644 --- a/src/mbgl/style/style.hpp +++ b/src/mbgl/style/style.hpp @@ -89,7 +89,7 @@ private: void onTileLoadingFailed(std::exception_ptr error) override; // Sprite::Observer implementation. - void onSpriteLoaded(std::unique_ptr) override; + void onSpriteLoaded(const Sprites& sprites) override; void onSpriteLoadingFailed(std::exception_ptr error) override; void emitTileDataChanged(); diff --git a/test/annotations/sprite_atlas.cpp b/test/annotations/sprite_atlas.cpp index 16140b051..0eece6c47 100644 --- a/test/annotations/sprite_atlas.cpp +++ b/test/annotations/sprite_atlas.cpp @@ -12,9 +12,11 @@ using namespace mbgl; TEST(Annotations, SpriteAtlas) { FixtureLog log; + auto spriteParseResult = parseSprite(util::read_file("test/fixtures/annotations/emerald.png"), + util::read_file("test/fixtures/annotations/emerald.json")); + SpriteStore store; - store.setSprites(parseSprite(util::read_file("test/fixtures/annotations/emerald.png"), - util::read_file("test/fixtures/annotations/emerald.json"))); + store.setSprites(spriteParseResult.get()); SpriteAtlas atlas(63, 112, 1, store); @@ -85,9 +87,12 @@ TEST(Annotations, SpriteAtlas) { } TEST(Annotations, SpriteAtlasSize) { + auto spriteParseResult = parseSprite(util::read_file("test/fixtures/annotations/emerald.png"), + util::read_file("test/fixtures/annotations/emerald.json")); + SpriteStore store; - store.setSprites(parseSprite(util::read_file("test/fixtures/annotations/emerald.png"), - util::read_file("test/fixtures/annotations/emerald.json"))); + store.setSprites(spriteParseResult.get()); + SpriteAtlas atlas(63, 112, 1.4, store); EXPECT_DOUBLE_EQ(1.4f, atlas.getPixelRatio()); diff --git a/test/annotations/sprite_parser.cpp b/test/annotations/sprite_parser.cpp index 280a19c50..703315258 100644 --- a/test/annotations/sprite_parser.cpp +++ b/test/annotations/sprite_parser.cpp @@ -117,7 +117,7 @@ TEST(Annotations, SpriteParsing) { const auto image_1x = util::read_file("test/fixtures/annotations/emerald.png"); const auto json_1x = util::read_file("test/fixtures/annotations/emerald.json"); - const auto images = parseSprite(image_1x, json_1x); + const auto images = parseSprite(image_1x, json_1x).get(); std::set names; std::transform(images.begin(), images.end(), std::inserter(names, names.begin()), @@ -210,22 +210,13 @@ TEST(Annotations, SpriteParsing) { } TEST(Annotations, SpriteParsingInvalidJSON) { - FixtureLog log; - const auto image_1x = util::read_file("test/fixtures/annotations/emerald.png"); const auto json_1x = R"JSON({ "image": " })JSON"; - const auto images = parseSprite(image_1x, json_1x); - EXPECT_EQ(0u, images.size()); + const auto error = parseSprite(image_1x, json_1x).get(); - EXPECT_EQ( - 1u, - log.count({ - EventSeverity::Warning, - Event::Sprite, - int64_t(-1), - "Failed to parse JSON: lacks ending quotation before the end of string at offset 10", - })); + EXPECT_EQ(error, + std::string("Failed to parse JSON: lacks ending quotation before the end of string at offset 10")); } TEST(Annotations, SpriteParsingEmptyImage) { @@ -234,7 +225,7 @@ TEST(Annotations, SpriteParsingEmptyImage) { const auto image_1x = util::read_file("test/fixtures/annotations/emerald.png"); const auto json_1x = R"JSON({ "image": {} })JSON"; - const auto images = parseSprite(image_1x, json_1x); + const auto images = parseSprite(image_1x, json_1x).get(); EXPECT_EQ(0u, images.size()); EXPECT_EQ(1u, log.count({ @@ -251,7 +242,7 @@ TEST(Annotations, SpriteParsingSimpleWidthHeight) { const auto image_1x = util::read_file("test/fixtures/annotations/emerald.png"); const auto json_1x = R"JSON({ "image": { "width": 32, "height": 32 } })JSON"; - const auto images = parseSprite(image_1x, json_1x); + const auto images = parseSprite(image_1x, json_1x).get(); EXPECT_EQ(1u, images.size()); } @@ -261,7 +252,7 @@ TEST(Annotations, SpriteParsingWidthTooBig) { const auto image_1x = util::read_file("test/fixtures/annotations/emerald.png"); const auto json_1x = R"JSON({ "image": { "width": 65536, "height": 32 } })JSON"; - const auto images = parseSprite(image_1x, json_1x); + const auto images = parseSprite(image_1x, json_1x).get(); EXPECT_EQ(0u, images.size()); EXPECT_EQ(1u, log.count({ @@ -284,7 +275,7 @@ TEST(Annotations, SpriteParsingNegativeWidth) { const auto image_1x = util::read_file("test/fixtures/annotations/emerald.png"); const auto json_1x = R"JSON({ "image": { "width": -1, "height": 32 } })JSON"; - const auto images = parseSprite(image_1x, json_1x); + const auto images = parseSprite(image_1x, json_1x).get(); EXPECT_EQ(0u, images.size()); EXPECT_EQ(1u, log.count({ @@ -307,7 +298,7 @@ TEST(Annotations, SpriteParsingNullRatio) { const auto image_1x = util::read_file("test/fixtures/annotations/emerald.png"); const auto json_1x = R"JSON({ "image": { "width": 32, "height": 32, "pixelRatio": 0 } })JSON"; - const auto images = parseSprite(image_1x, json_1x); + const auto images = parseSprite(image_1x, json_1x).get(); EXPECT_EQ(0u, images.size()); EXPECT_EQ(1u, log.count({