From 776612e59a5ade5a8174aec81ea46cd30e2adb01 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 21 Apr 2015 18:03:38 -0700 Subject: [PATCH] Clean up Sprite * Provide a callback so MapContext can re-render when it loads. * Cancel in-progress loading rather than relying on keeping it alive with a shared_ptr. * Remove / privatize as much as possible. --- src/mbgl/map/map_context.cpp | 5 ++- src/mbgl/map/sprite.cpp | 81 +++++++++++++++++------------------- src/mbgl/map/sprite.hpp | 25 ++++------- 3 files changed, 50 insertions(+), 61 deletions(-) diff --git a/src/mbgl/map/map_context.cpp b/src/mbgl/map/map_context.cpp index 0fa8884a157..05de1760601 100644 --- a/src/mbgl/map/map_context.cpp +++ b/src/mbgl/map/map_context.cpp @@ -99,7 +99,10 @@ util::ptr MapContext::getSprite() { const float pixelRatio = data.getTransformState().getPixelRatio(); const std::string &sprite_url = style->getSpriteURL(); if (!sprite || !sprite->hasPixelRatio(pixelRatio)) { - sprite = Sprite::Create(sprite_url, pixelRatio, env); + sprite = std::make_shared(sprite_url, pixelRatio, env, [this] { + assert(Environment::currentlyOn(ThreadType::Map)); + triggerUpdate(); + }); } return sprite; diff --git a/src/mbgl/map/sprite.cpp b/src/mbgl/map/sprite.cpp index 9b7411970c8..e5120809568 100644 --- a/src/mbgl/map/sprite.cpp +++ b/src/mbgl/map/sprite.cpp @@ -23,42 +23,15 @@ SpritePosition::SpritePosition(uint16_t x_, uint16_t y_, uint16_t width_, uint16 sdf(sdf_) { } -util::ptr Sprite::Create(const std::string &base_url, float pixelRatio, Environment &env) { - util::ptr sprite(std::make_shared(Key(), base_url, pixelRatio)); - sprite->load(env); - return sprite; -} - -Sprite::Sprite(const Key &, const std::string& base_url, float pixelRatio_) - : valid(base_url.length() > 0), - pixelRatio(pixelRatio_ > 1 ? 2 : 1), - spriteURL(base_url + (pixelRatio_ > 1 ? "@2x" : "") + ".png"), - jsonURL(base_url + (pixelRatio_ > 1 ? "@2x" : "") + ".json"), +Sprite::Sprite(const std::string& baseUrl, float pixelRatio_, Environment& env, std::function callback_) + : pixelRatio(pixelRatio_ > 1 ? 2 : 1), raster(), loadedImage(false), loadedJSON(false), - future(promise.get_future()) { -} - -bool Sprite::hasPixelRatio(float ratio) const { - return pixelRatio == (ratio > 1 ? 2 : 1); -} - - -void Sprite::waitUntilLoaded() const { - future.wait(); -} - -Sprite::operator bool() const { - return valid && isLoaded() && !pos.empty(); -} - + future(promise.get_future()), + callback(callback_) { -// Note: This is a separate function that must be called exactly once after creation -// The reason this isn't part of the constructor is that calling shared_from_this() in -// the constructor fails. -void Sprite::load(Environment &env) { - if (!valid) { + if (baseUrl.empty()) { // Treat a non-existent sprite as a successfully loaded empty sprite. loadedImage = true; loadedJSON = true; @@ -66,34 +39,48 @@ void Sprite::load(Environment &env) { return; } - util::ptr sprite = shared_from_this(); + std::string spriteURL(baseUrl + (pixelRatio_ > 1 ? "@2x" : "") + ".png"); + std::string jsonURL(baseUrl + (pixelRatio_ > 1 ? "@2x" : "") + ".json"); - env.request({ Resource::Kind::JSON, jsonURL }, [sprite](const Response &res) { + jsonRequest = env.request({ Resource::Kind::JSON, jsonURL }, [this](const Response &res) { + jsonRequest = nullptr; if (res.status == Response::Successful) { - sprite->body = res.data; - sprite->parseJSON(); + body = res.data; + parseJSON(); } else { Log::Warning(Event::Sprite, "Failed to load sprite info: %s", res.message.c_str()); } - sprite->loadedJSON = true; - sprite->complete(); + loadedJSON = true; + complete(); }); - env.request({ Resource::Kind::Image, spriteURL }, [sprite](const Response &res) { + spriteRequest = env.request({ Resource::Kind::Image, spriteURL }, [this](const Response &res) { + spriteRequest = nullptr; if (res.status == Response::Successful) { - sprite->image = res.data; - sprite->parseImage(); + image = res.data; + parseImage(); } else { Log::Warning(Event::Sprite, "Failed to load sprite image: %s", res.message.c_str()); } - sprite->loadedImage = true; - sprite->complete(); + loadedImage = true; + complete(); }); } +Sprite::~Sprite() { + if (jsonRequest) { + jsonRequest->cancel(); + } + + if (spriteRequest) { + spriteRequest->cancel(); + } +} + void Sprite::complete() { if (loadedImage && loadedJSON) { promise.set_value(); + callback(); } } @@ -101,6 +88,14 @@ bool Sprite::isLoaded() const { return loadedImage && loadedJSON; } +void Sprite::waitUntilLoaded() const { + future.wait(); +} + +bool Sprite::hasPixelRatio(float ratio) const { + return pixelRatio == (ratio > 1 ? 2 : 1); +} + void Sprite::parseImage() { raster = util::make_unique(image); if (!*raster) { diff --git a/src/mbgl/map/sprite.hpp b/src/mbgl/map/sprite.hpp index 6c1b3ba8e3a..125520a3d72 100644 --- a/src/mbgl/map/sprite.hpp +++ b/src/mbgl/map/sprite.hpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include @@ -15,6 +16,7 @@ namespace mbgl { class Environment; +class Request; class SpritePosition { public: @@ -31,15 +33,10 @@ class SpritePosition { bool sdf = false; }; -class Sprite : public std::enable_shared_from_this, private util::noncopyable { -private: - struct Key {}; - void load(Environment &env); - +class Sprite : private util::noncopyable { public: - Sprite(const Key &, const std::string& base_url, float pixelRatio); - static util::ptr - Create(const std::string &base_url, float pixelRatio, Environment &env); + Sprite(const std::string& baseUrl, float pixelRatio, Environment&, std::function callback); + ~Sprite(); const SpritePosition &getSpritePosition(const std::string& name) const; @@ -48,15 +45,7 @@ class Sprite : public std::enable_shared_from_this, private util::noncop void waitUntilLoaded() const; bool isLoaded() const; - operator bool() const; - -private: - const bool valid; - -public: const float pixelRatio; - const std::string spriteURL; - const std::string jsonURL; std::unique_ptr raster; private: @@ -64,7 +53,6 @@ class Sprite : public std::enable_shared_from_this, private util::noncop void parseImage(); void complete(); -private: std::string body; std::string image; std::atomic loadedImage; @@ -74,7 +62,10 @@ class Sprite : public std::enable_shared_from_this, private util::noncop std::promise promise; std::future future; + std::function callback; + Request* jsonRequest = nullptr; + Request* spriteRequest = nullptr; }; }