From 2455275bf47e1671a92eb2c1296c6b48f11f6b2e Mon Sep 17 00:00:00 2001 From: Ansis Brammanis Date: Tue, 2 Apr 2019 12:57:45 -0400 Subject: add onStyleImageMissing to allow dynamically loaded or generated images (#14253) Also make `Style#updateImage(...)` much faster when the image doesn't change size. This can be useful for asynchronously generating images. --- include/mbgl/map/map_observer.hpp | 2 + include/mbgl/renderer/renderer_observer.hpp | 4 + platform/darwin/src/MGLRendererFrontend.h | 7 +- platform/default/src/mbgl/gl/headless_frontend.cpp | 8 +- platform/glfw/glfw_renderer_frontend.cpp | 7 +- src/core-files.json | 1 + src/mbgl/gfx/context.hpp | 14 +++ src/mbgl/gl/context.cpp | 18 ++++ src/mbgl/gl/context.hpp | 1 + src/mbgl/map/map_impl.cpp | 10 +++ src/mbgl/map/map_impl.hpp | 1 + src/mbgl/renderer/image_atlas.cpp | 40 +++++++-- src/mbgl/renderer/image_atlas.hpp | 16 +++- src/mbgl/renderer/image_manager.cpp | 100 ++++++++++++++++++--- src/mbgl/renderer/image_manager.hpp | 20 ++++- src/mbgl/renderer/image_manager_observer.hpp | 14 +++ src/mbgl/renderer/renderer_impl.cpp | 13 ++- src/mbgl/renderer/renderer_impl.hpp | 5 ++ src/mbgl/style/image_impl.hpp | 1 + src/mbgl/style/style_impl.cpp | 1 + src/mbgl/tile/geometry_tile.cpp | 8 +- src/mbgl/tile/geometry_tile.hpp | 2 +- src/mbgl/tile/geometry_tile_worker.cpp | 5 +- src/mbgl/tile/geometry_tile_worker.hpp | 3 +- test/renderer/image_manager.test.cpp | 16 ++-- 25 files changed, 280 insertions(+), 37 deletions(-) create mode 100644 src/mbgl/renderer/image_manager_observer.hpp diff --git a/include/mbgl/map/map_observer.hpp b/include/mbgl/map/map_observer.hpp index 8ad9e93d0b..75e389df9f 100644 --- a/include/mbgl/map/map_observer.hpp +++ b/include/mbgl/map/map_observer.hpp @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include @@ -46,6 +47,7 @@ public: virtual void onDidFinishLoadingStyle() {} virtual void onSourceChanged(style::Source&) {} virtual void onDidBecomeIdle() {} + virtual void onStyleImageMissing(const std::string&) {} }; } // namespace mbgl diff --git a/include/mbgl/renderer/renderer_observer.hpp b/include/mbgl/renderer/renderer_observer.hpp index 0a76d01ca7..1a9d3ff9bc 100644 --- a/include/mbgl/renderer/renderer_observer.hpp +++ b/include/mbgl/renderer/renderer_observer.hpp @@ -31,6 +31,10 @@ public: // Final frame virtual void onDidFinishRenderingMap() {} + + // Style is missing an image + using StyleImageMissingCallback = std::function; + virtual void onStyleImageMissing(const std::string&, StyleImageMissingCallback done) { done(); } }; } // namespace mbgl diff --git a/platform/darwin/src/MGLRendererFrontend.h b/platform/darwin/src/MGLRendererFrontend.h index 2df67ca4e4..c0e03351c6 100644 --- a/platform/darwin/src/MGLRendererFrontend.h +++ b/platform/darwin/src/MGLRendererFrontend.h @@ -49,7 +49,12 @@ public: mbgl::BackendScope guard { mbglBackend, mbgl::BackendScope::ScopeType::Implicit }; - renderer->render(*updateParameters); + // onStyleImageMissing might be called during a render. The user implemented method + // could trigger a call to MGLRenderFrontend#update which overwrites `updateParameters`. + // Copy the shared pointer here so that the parameters aren't destroyed while `render(...)` is + // still using them. + auto updateParameters_ = updateParameters; + renderer->render(*updateParameters_); } mbgl::Renderer* getRenderer() { diff --git a/platform/default/src/mbgl/gl/headless_frontend.cpp b/platform/default/src/mbgl/gl/headless_frontend.cpp index f3dae2dbc9..c311e2df41 100644 --- a/platform/default/src/mbgl/gl/headless_frontend.cpp +++ b/platform/default/src/mbgl/gl/headless_frontend.cpp @@ -20,7 +20,13 @@ HeadlessFrontend::HeadlessFrontend(Size size_, float pixelRatio_, Scheduler& sch asyncInvalidate([this] { if (renderer && updateParameters) { mbgl::BackendScope guard { backend }; - renderer->render(*updateParameters); + + // onStyleImageMissing might be called during a render. The user implemented method + // could trigger a call to MGLRenderFrontend#update which overwrites `updateParameters`. + // Copy the shared pointer here so that the parameters aren't destroyed while `render(...)` is + // still using them. + auto updateParameters_ = updateParameters; + renderer->render(*updateParameters_); } }), renderer(std::make_unique(backend, pixelRatio, scheduler, mode, programCacheDir, localFontFamily)) { diff --git a/platform/glfw/glfw_renderer_frontend.cpp b/platform/glfw/glfw_renderer_frontend.cpp index 73205f1c56..9c5320cc78 100644 --- a/platform/glfw/glfw_renderer_frontend.cpp +++ b/platform/glfw/glfw_renderer_frontend.cpp @@ -32,7 +32,12 @@ void GLFWRendererFrontend::render() { mbgl::BackendScope guard { glfwView, mbgl::BackendScope::ScopeType::Implicit }; - renderer->render(*updateParameters); + // onStyleImageMissing might be called during a render. The user implemented method + // could trigger a call to MGLRenderFrontend#update which overwrites `updateParameters`. + // Copy the shared pointer here so that the parameters aren't destroyed while `render(...)` is + // still using them. + auto updateParameters_ = updateParameters; + renderer->render(*updateParameters_); } mbgl::Renderer* GLFWRendererFrontend::getRenderer() { diff --git a/src/core-files.json b/src/core-files.json index 139b93e368..b257666e3e 100644 --- a/src/core-files.json +++ b/src/core-files.json @@ -615,6 +615,7 @@ "mbgl/renderer/group_by_layout.hpp": "src/mbgl/renderer/group_by_layout.hpp", "mbgl/renderer/image_atlas.hpp": "src/mbgl/renderer/image_atlas.hpp", "mbgl/renderer/image_manager.hpp": "src/mbgl/renderer/image_manager.hpp", + "mbgl/renderer/image_manager_observer.hpp": "src/mbgl/renderer/image_manager_observer.hpp", "mbgl/renderer/layers/render_background_layer.hpp": "src/mbgl/renderer/layers/render_background_layer.hpp", "mbgl/renderer/layers/render_circle_layer.hpp": "src/mbgl/renderer/layers/render_circle_layer.hpp", "mbgl/renderer/layers/render_custom_layer.hpp": "src/mbgl/renderer/layers/render_custom_layer.hpp", diff --git a/src/mbgl/gfx/context.hpp b/src/mbgl/gfx/context.hpp index ff84cf34e8..e898006ff5 100644 --- a/src/mbgl/gfx/context.hpp +++ b/src/mbgl/gfx/context.hpp @@ -100,11 +100,25 @@ public: texture.size = image.size; } + template + void updateTextureSub(Texture& texture, + const Image& image, + const uint16_t offsetX, + const uint16_t offsetY, + TextureChannelDataType type = TextureChannelDataType::UnsignedByte) { + assert(image.size.width + offsetX <= texture.size.width); + assert(image.size.height + offsetY <= texture.size.height); + auto format = image.channels == 4 ? TexturePixelType::RGBA : TexturePixelType::Alpha; + updateTextureResourceSub(*texture.resource, offsetX, offsetY, image.size, image.data.get(), format, type); + } + protected: virtual std::unique_ptr createTextureResource( Size, const void* data, TexturePixelType, TextureChannelDataType) = 0; virtual void updateTextureResource(const TextureResource&, Size, const void* data, TexturePixelType, TextureChannelDataType) = 0; + virtual void updateTextureResourceSub(const TextureResource&, uint16_t xOffset, uint16_t yOffset, Size, const void* data, + TexturePixelType, TextureChannelDataType) = 0; public: DrawScope createDrawScope() { diff --git a/src/mbgl/gl/context.cpp b/src/mbgl/gl/context.cpp index 5ce035f691..6cee364379 100644 --- a/src/mbgl/gl/context.cpp +++ b/src/mbgl/gl/context.cpp @@ -546,6 +546,24 @@ void Context::updateTextureResource(const gfx::TextureResource& resource, Enum::to(type), data)); } +void Context::updateTextureResourceSub(const gfx::TextureResource& resource, + const uint16_t xOffset, + const uint16_t yOffset, + const Size size, + const void* data, + gfx::TexturePixelType format, + gfx::TextureChannelDataType type) { + // Always use texture unit 0 for manipulating it. + activeTextureUnit = 0; + texture[0] = static_cast(resource).texture; + MBGL_CHECK_ERROR(glTexSubImage2D(GL_TEXTURE_2D, 0, + xOffset, yOffset, + size.width, size.height, + Enum::to(format), + Enum::to(type), data)); +} + + std::unique_ptr Context::createDrawScopeResource() { return std::make_unique(createVertexArray()); } diff --git a/src/mbgl/gl/context.hpp b/src/mbgl/gl/context.hpp index a4e6184ddb..d46727cb7a 100644 --- a/src/mbgl/gl/context.hpp +++ b/src/mbgl/gl/context.hpp @@ -211,6 +211,7 @@ private: std::unique_ptr createTextureResource(Size, const void* data, gfx::TexturePixelType, gfx::TextureChannelDataType) override; void updateTextureResource(const gfx::TextureResource&, Size, const void* data, gfx::TexturePixelType, gfx::TextureChannelDataType) override; + void updateTextureResourceSub(const gfx::TextureResource&, const uint16_t xOffset, const uint16_t yOffset, Size, const void* data, gfx::TexturePixelType, gfx::TextureChannelDataType) override; std::unique_ptr createDrawScopeResource() override; diff --git a/src/mbgl/map/map_impl.cpp b/src/mbgl/map/map_impl.cpp index 42561cf8bf..1ec7255822 100644 --- a/src/mbgl/map/map_impl.cpp +++ b/src/mbgl/map/map_impl.cpp @@ -171,4 +171,14 @@ void Map::Impl::jumpTo(const CameraOptions& camera) { onUpdate(); } +void Map::Impl::onStyleImageMissing(const std::string& id, std::function done) { + + if (style->getImage(id) == nullptr) { + observer.onStyleImageMissing(id); + } + + done(); + onUpdate(); +} + } // namespace mbgl diff --git a/src/mbgl/map/map_impl.hpp b/src/mbgl/map/map_impl.hpp index 7086148276..f233f78275 100644 --- a/src/mbgl/map/map_impl.hpp +++ b/src/mbgl/map/map_impl.hpp @@ -47,6 +47,7 @@ public: void onDidFinishRenderingFrame(RenderMode, bool) final; void onWillStartRenderingMap() final; void onDidFinishRenderingMap() final; + void onStyleImageMissing(const std::string&, std::function) final; // Map void jumpTo(const CameraOptions&); diff --git a/src/mbgl/renderer/image_atlas.cpp b/src/mbgl/renderer/image_atlas.cpp index b39c788ced..282f135ac9 100644 --- a/src/mbgl/renderer/image_atlas.cpp +++ b/src/mbgl/renderer/image_atlas.cpp @@ -1,4 +1,6 @@ #include +#include +#include #include @@ -6,14 +8,15 @@ namespace mbgl { static constexpr uint32_t padding = 1; -ImagePosition::ImagePosition(const mapbox::Bin& bin, const style::Image::Impl& image) +ImagePosition::ImagePosition(const mapbox::Bin& bin, const style::Image::Impl& image, uint32_t version_) : pixelRatio(image.pixelRatio), textureRect( bin.x + padding, bin.y + padding, bin.w - padding * 2, bin.h - padding * 2 - ) { + ), + version(version_) { } const mapbox::Bin& _packImage(mapbox::ShelfPack& pack, const style::Image::Impl& image, ImageAtlas& resultImage, ImageType imageType) { @@ -49,7 +52,30 @@ const mapbox::Bin& _packImage(mapbox::ShelfPack& pack, const style::Image::Impl& return bin; } -ImageAtlas makeImageAtlas(const ImageMap& icons, const ImageMap& patterns) { +void ImageAtlas::patchUpdatedImages(gfx::Context& context, gfx::Texture& atlasTexture, const ImageManager& imageManager) { + for (auto& updatedImageVersion : imageManager.updatedImageVersions) { + auto iconPosition = iconPositions.find(updatedImageVersion.first); + if (iconPosition != iconPositions.end()) { + patchUpdatedImage(context, atlasTexture, iconPosition->second, imageManager, updatedImageVersion.first, updatedImageVersion.second); + } + auto patternPosition = patternPositions.find(updatedImageVersion.first); + if (patternPosition != patternPositions.end()) { + patchUpdatedImage(context, atlasTexture, patternPosition->second, imageManager, updatedImageVersion.first, updatedImageVersion.second); + } + } +} + +void ImageAtlas::patchUpdatedImage(gfx::Context& context, gfx::Texture& atlasTexture, ImagePosition& position, const ImageManager& imageManager, const std::string& name, uint16_t version) { + if (position.version == version) return; + + auto updatedImage = imageManager.getImage(name); + if (updatedImage == nullptr) return; + + context.updateTextureSub(atlasTexture, updatedImage->image, position.textureRect.x, position.textureRect.y); + position.version = version; +} + +ImageAtlas makeImageAtlas(const ImageMap& icons, const ImageMap& patterns, const std::unordered_map& versionMap) { ImageAtlas result; mapbox::ShelfPack::ShelfPackOptions options; @@ -59,13 +85,17 @@ ImageAtlas makeImageAtlas(const ImageMap& icons, const ImageMap& patterns) { for (const auto& entry : icons) { const style::Image::Impl& image = *entry.second; const mapbox::Bin& bin = _packImage(pack, image, result, ImageType::Icon); - result.iconPositions.emplace(image.id, ImagePosition { bin, image }); + auto it = versionMap.find(entry.first); + auto version = it != versionMap.end() ? it->second : 0; + result.iconPositions.emplace(image.id, ImagePosition { bin, image, version }); } for (const auto& entry : patterns) { const style::Image::Impl& image = *entry.second; const mapbox::Bin& bin = _packImage(pack, image, result, ImageType::Pattern); - result.patternPositions.emplace(image.id, ImagePosition { bin, image }); + auto it = versionMap.find(entry.first); + auto version = it != versionMap.end() ? it->second : 0; + result.patternPositions.emplace(image.id, ImagePosition { bin, image, version }); } pack.shrink(); diff --git a/src/mbgl/renderer/image_atlas.hpp b/src/mbgl/renderer/image_atlas.hpp index 3af31a75f8..080a490ab2 100644 --- a/src/mbgl/renderer/image_atlas.hpp +++ b/src/mbgl/renderer/image_atlas.hpp @@ -9,12 +9,20 @@ namespace mbgl { +namespace gfx { + class Texture; + class Context; +} // namespace gfx + +class ImageManager; + class ImagePosition { public: - ImagePosition(const mapbox::Bin&, const style::Image::Impl&); + ImagePosition(const mapbox::Bin&, const style::Image::Impl&, uint32_t version = 0); float pixelRatio; Rect textureRect; + uint32_t version; std::array tl() const { return {{ @@ -51,8 +59,12 @@ public: PremultipliedImage image; ImagePositions iconPositions; ImagePositions patternPositions; + + void patchUpdatedImages(gfx::Context&, gfx::Texture&, const ImageManager&); +private: + void patchUpdatedImage(gfx::Context&, gfx::Texture&, ImagePosition& position, const ImageManager& imageManager, const std::string& name, uint16_t version); }; -ImageAtlas makeImageAtlas(const ImageMap&, const ImageMap&); +ImageAtlas makeImageAtlas(const ImageMap&, const ImageMap&, const std::unordered_map& versionMap); } // namespace mbgl diff --git a/src/mbgl/renderer/image_manager.cpp b/src/mbgl/renderer/image_manager.cpp index 9c9f6c6e08..d2994d6f2d 100644 --- a/src/mbgl/renderer/image_manager.cpp +++ b/src/mbgl/renderer/image_manager.cpp @@ -1,9 +1,16 @@ #include #include #include +#include namespace mbgl { +static ImageManagerObserver nullObserver; + +void ImageManager::setObserver(ImageManagerObserver* observer_) { + observer = observer_ ? observer_ : &nullObserver; +} + void ImageManager::setLoaded(bool loaded_) { if (loaded == loaded_) { return; @@ -13,7 +20,7 @@ void ImageManager::setLoaded(bool loaded_) { if (loaded) { for (const auto& entry : requestors) { - notify(*entry.first, entry.second); + checkMissingAndNotify(*entry.first, entry.second); } requestors.clear(); } @@ -28,9 +35,23 @@ void ImageManager::addImage(Immutable image_) { images.emplace(image_->id, std::move(image_)); } -void ImageManager::updateImage(Immutable image_) { +bool ImageManager::updateImage(Immutable image_) { + auto oldImage = images.find(image_->id); + assert(oldImage != images.end()); + if (oldImage == images.end()) return false; + + auto sizeChanged = oldImage->second->image.size != image_->image.size; + + if (sizeChanged) { + updatedImageVersions.erase(image_->id); + } else { + updatedImageVersions[image_->id]++; + } + removeImage(image_->id); addImage(std::move(image_)); + + return sizeChanged; } void ImageManager::removeImage(const std::string& id) { @@ -60,10 +81,15 @@ const style::Image::Impl* ImageManager::getImage(const std::string& id) const { } void ImageManager::getImages(ImageRequestor& requestor, ImageRequestPair&& pair) { - // If the sprite has been loaded, or if all the icon dependencies are already present - // (i.e. if they've been addeded via runtime styling), then notify the requestor immediately. - // Otherwise, delay notification until the sprite is loaded. At that point, if any of the - // dependencies are still unavailable, we'll just assume they are permanently missing. + // remove previous requests from this tile + removeRequestor(requestor); + + // If all the icon dependencies are already present ((i.e. if they've been addeded via + // runtime styling), then notify the requestor immediately. Otherwise, if the + // sprite has not loaded, then wait for it. When the sprite has loaded check + // if all icons are available. If any are missing, call `onStyleImageMissing` + // to give the user a chance to provide the icon. If they are not provided + // by the next frame we'll assume they are permanently missing. bool hasAllDependencies = true; if (!isLoaded()) { for (const auto& dependency : pair.first) { @@ -72,29 +98,83 @@ void ImageManager::getImages(ImageRequestor& requestor, ImageRequestPair&& pair) } } } - if (isLoaded() || hasAllDependencies) { - notify(requestor, std::move(pair)); - } else { + + if (hasAllDependencies) { + notify(requestor, pair); + } else if (!isLoaded()) { requestors.emplace(&requestor, std::move(pair)); + } else { + checkMissingAndNotify(requestor, std::move(pair)); } } void ImageManager::removeRequestor(ImageRequestor& requestor) { requestors.erase(&requestor); + missingImageRequestors.erase(&requestor); +} + +void ImageManager::notifyIfMissingImageAdded() { + for (auto it = missingImageRequestors.begin(); it != missingImageRequestors.end();) { + if (it->second.callbacksRemaining == 0) { + notify(*it->first, it->second.pair); + missingImageRequestors.erase(it++); + } else { + it++; + } + } +} + +void ImageManager::checkMissingAndNotify(ImageRequestor& requestor, const ImageRequestPair& pair) { + unsigned int missing = 0; + for (const auto& dependency : pair.first) { + auto it = images.find(dependency.first); + if (it == images.end()) { + missing++; + } + } + + if (missing > 0) { + ImageRequestor* requestorPtr = &requestor; + + missingImageRequestors.emplace(requestorPtr, MissingImageRequestPair { std::move(pair), missing }); + + for (const auto& dependency : pair.first) { + auto it = images.find(dependency.first); + if (it == images.end()) { + assert(observer != nullptr); + observer->onStyleImageMissing(dependency.first, [this, requestorPtr]() { + auto requestorIt = missingImageRequestors.find(requestorPtr); + if (requestorIt != missingImageRequestors.end()) { + assert(requestorIt->second.callbacksRemaining > 0); + requestorIt->second.callbacksRemaining--; + } + }); + } + } + + } else { + notify(requestor, pair); + } } void ImageManager::notify(ImageRequestor& requestor, const ImageRequestPair& pair) const { ImageMap iconMap; ImageMap patternMap; + ImageVersionMap versionMap; for (const auto& dependency : pair.first) { auto it = images.find(dependency.first); if (it != images.end()) { dependency.second == ImageType::Pattern ? patternMap.emplace(*it) : iconMap.emplace(*it); + + auto versionIt = updatedImageVersions.find(dependency.first); + if (versionIt != updatedImageVersions.end()) { + versionMap.emplace(versionIt->first, versionIt->second); + } } } - requestor.onImagesAvailable(iconMap, patternMap, pair.second); + requestor.onImagesAvailable(iconMap, patternMap, std::move(versionMap), pair.second); } void ImageManager::dumpDebugLogs() const { diff --git a/src/mbgl/renderer/image_manager.hpp b/src/mbgl/renderer/image_manager.hpp index e56f30ac3f..99887ae384 100644 --- a/src/mbgl/renderer/image_manager.hpp +++ b/src/mbgl/renderer/image_manager.hpp @@ -6,6 +6,7 @@ #include #include #include +#include #include @@ -21,7 +22,7 @@ class Context; class ImageRequestor { public: virtual ~ImageRequestor() = default; - virtual void onImagesAvailable(ImageMap icons, ImageMap patterns, uint64_t imageCorrelationID) = 0; + virtual void onImagesAvailable(ImageMap icons, ImageMap patterns, ImageVersionMap versionMap, uint64_t imageCorrelationID) = 0; }; /* @@ -39,6 +40,8 @@ public: ImageManager(); ~ImageManager(); + void setObserver(ImageManagerObserver*); + void setLoaded(bool); bool isLoaded() const; @@ -47,20 +50,31 @@ public: const style::Image::Impl* getImage(const std::string&) const; void addImage(Immutable); - void updateImage(Immutable); + bool updateImage(Immutable); void removeImage(const std::string&); void getImages(ImageRequestor&, ImageRequestPair&&); void removeRequestor(ImageRequestor&); + void notifyIfMissingImageAdded(); + + ImageVersionMap updatedImageVersions; private: + void checkMissingAndNotify(ImageRequestor&, const ImageRequestPair&); void notify(ImageRequestor&, const ImageRequestPair&) const; bool loaded = false; - std::unordered_map requestors; + std::map requestors; + struct MissingImageRequestPair { + ImageRequestPair pair; + unsigned int callbacksRemaining; + }; + std::map missingImageRequestors; ImageMap images; + ImageManagerObserver* observer = nullptr; + // Pattern stuff public: optional getPattern(const std::string& name); diff --git a/src/mbgl/renderer/image_manager_observer.hpp b/src/mbgl/renderer/image_manager_observer.hpp new file mode 100644 index 0000000000..3dc53c9b66 --- /dev/null +++ b/src/mbgl/renderer/image_manager_observer.hpp @@ -0,0 +1,14 @@ +#pragma once + +#include + +namespace mbgl { + +class ImageManagerObserver { +public: + virtual ~ImageManagerObserver() = default; + + virtual void onStyleImageMissing(const std::string&, std::function done) { done(); } +}; + +} // namespace mbgl diff --git a/src/mbgl/renderer/renderer_impl.cpp b/src/mbgl/renderer/renderer_impl.cpp index 059bf6bf8d..7144a2dcee 100644 --- a/src/mbgl/renderer/renderer_impl.cpp +++ b/src/mbgl/renderer/renderer_impl.cpp @@ -83,6 +83,7 @@ void Renderer::Impl::render(const UpdateParameters& updateParameters) { if (!imageManager) { imageManager = std::make_unique(); + imageManager->setObserver(this); } if (updateParameters.mode != MapMode::Continuous) { @@ -143,6 +144,9 @@ void Renderer::Impl::render(const UpdateParameters& updateParameters) { const ImageDifference imageDiff = diffImages(imageImpls, updateParameters.images); imageImpls = updateParameters.images; + // Only trigger tile reparse for changed images. Changed images only need a relayout when they have a different size. + bool hasImageDiff = !imageDiff.removed.empty(); + // Remove removed images from sprite atlas. for (const auto& entry : imageDiff.removed) { imageManager->removeImage(entry.first); @@ -155,9 +159,10 @@ void Renderer::Impl::render(const UpdateParameters& updateParameters) { // Update changed images. for (const auto& entry : imageDiff.changed) { - imageManager->updateImage(entry.second.after); + hasImageDiff = imageManager->updateImage(entry.second.after) || hasImageDiff; } + imageManager->notifyIfMissingImageAdded(); imageManager->setLoaded(updateParameters.spriteLoaded); @@ -215,8 +220,6 @@ void Renderer::Impl::render(const UpdateParameters& updateParameters) { renderSources.emplace(entry.first, std::move(renderSource)); } - const bool hasImageDiff = !(imageDiff.added.empty() && imageDiff.removed.empty() && imageDiff.changed.empty()); - // Update all sources. for (const auto& source : *sourceImpls) { std::vector> filteredLayers; @@ -837,4 +840,8 @@ void Renderer::Impl::onTileChanged(RenderSource&, const OverscaledTileID&) { observer->onInvalidate(); } +void Renderer::Impl::onStyleImageMissing(const std::string& id, std::function done) { + observer->onStyleImageMissing(id, std::move(done)); +} + } // namespace mbgl diff --git a/src/mbgl/renderer/renderer_impl.hpp b/src/mbgl/renderer/renderer_impl.hpp index a036bfc7ff..624b4c273c 100644 --- a/src/mbgl/renderer/renderer_impl.hpp +++ b/src/mbgl/renderer/renderer_impl.hpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -34,6 +35,7 @@ class LineAtlas; class CrossTileSymbolIndex; class Renderer::Impl : public GlyphManagerObserver, + public ImageManagerObserver, public RenderSourceObserver{ public: Impl(RendererBackend&, float pixelRatio_, Scheduler&, GLContextMode, @@ -86,6 +88,9 @@ private: void onTileChanged(RenderSource&, const OverscaledTileID&) override; void onTileError(RenderSource&, const OverscaledTileID&, std::exception_ptr) override; + // ImageManagerObserver implementation + void onStyleImageMissing(const std::string&, std::function) override; + void updateFadingTiles(); friend class Renderer; diff --git a/src/mbgl/style/image_impl.hpp b/src/mbgl/style/image_impl.hpp index 54b5e6487b..b2decbf781 100644 --- a/src/mbgl/style/image_impl.hpp +++ b/src/mbgl/style/image_impl.hpp @@ -34,5 +34,6 @@ enum class ImageType : bool { using ImageMap = std::unordered_map>; using ImageDependencies = std::unordered_map; using ImageRequestPair = std::pair; +using ImageVersionMap = std::unordered_map; } // namespace mbgl diff --git a/src/mbgl/style/style_impl.cpp b/src/mbgl/style/style_impl.cpp index d65e9c9de2..fde5aa632d 100644 --- a/src/mbgl/style/style_impl.cpp +++ b/src/mbgl/style/style_impl.cpp @@ -254,6 +254,7 @@ bool Style::Impl::isLoaded() const { void Style::Impl::addImage(std::unique_ptr image) { images.remove(image->getID()); // We permit using addImage to update. images.add(std::move(image)); + observer->onUpdate(); } void Style::Impl::removeImage(const std::string& id) { diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp index d0b55a17d4..9165c03267 100644 --- a/src/mbgl/tile/geometry_tile.cpp +++ b/src/mbgl/tile/geometry_tile.cpp @@ -156,8 +156,8 @@ void GeometryTile::getGlyphs(GlyphDependencies glyphDependencies) { glyphManager.getGlyphs(*this, std::move(glyphDependencies)); } -void GeometryTile::onImagesAvailable(ImageMap images, ImageMap patterns, uint64_t imageCorrelationID) { - worker.self().invoke(&GeometryTileWorker::onImagesAvailable, std::move(images), std::move(patterns), imageCorrelationID); +void GeometryTile::onImagesAvailable(ImageMap images, ImageMap patterns, ImageVersionMap versionMap, uint64_t imageCorrelationID) { + worker.self().invoke(&GeometryTileWorker::onImagesAvailable, std::move(images), std::move(patterns), std::move(versionMap), imageCorrelationID); } void GeometryTile::getImages(ImageRequestPair pair) { @@ -192,6 +192,10 @@ void GeometryTile::upload(gfx::Context& context) { iconAtlasTexture = context.createTexture(iconAtlas.image); iconAtlas.image = {}; } + + if (iconAtlasTexture) { + iconAtlas.patchUpdatedImages(context, *iconAtlasTexture, imageManager); + } } Bucket* GeometryTile::getBucket(const Layer::Impl& layer) const { diff --git a/src/mbgl/tile/geometry_tile.hpp b/src/mbgl/tile/geometry_tile.hpp index b652e7ba5d..53a8910c27 100644 --- a/src/mbgl/tile/geometry_tile.hpp +++ b/src/mbgl/tile/geometry_tile.hpp @@ -37,7 +37,7 @@ public: void setShowCollisionBoxes(const bool showCollisionBoxes) override; void onGlyphsAvailable(GlyphMap) override; - void onImagesAvailable(ImageMap, ImageMap, uint64_t imageCorrelationID) override; + void onImagesAvailable(ImageMap, ImageMap, ImageVersionMap versionMap, uint64_t imageCorrelationID) override; void getGlyphs(GlyphDependencies); void getImages(ImageRequestPair); diff --git a/src/mbgl/tile/geometry_tile_worker.cpp b/src/mbgl/tile/geometry_tile_worker.cpp index 943e782af1..0b703cd68b 100644 --- a/src/mbgl/tile/geometry_tile_worker.cpp +++ b/src/mbgl/tile/geometry_tile_worker.cpp @@ -278,12 +278,13 @@ void GeometryTileWorker::onGlyphsAvailable(GlyphMap newGlyphMap) { symbolDependenciesChanged(); } -void GeometryTileWorker::onImagesAvailable(ImageMap newIconMap, ImageMap newPatternMap, uint64_t imageCorrelationID_) { +void GeometryTileWorker::onImagesAvailable(ImageMap newIconMap, ImageMap newPatternMap, ImageVersionMap newVersionMap, uint64_t imageCorrelationID_) { if (imageCorrelationID != imageCorrelationID_) { return; // Ignore outdated image request replies. } imageMap = std::move(newIconMap); patternMap = std::move(newPatternMap); + versionMap = std::move(newVersionMap); pendingImageDependencies.clear(); symbolDependenciesChanged(); } @@ -441,7 +442,7 @@ void GeometryTileWorker::finalizeLayout() { MBGL_TIMING_START(watch) optional glyphAtlasImage; - ImageAtlas iconAtlas = makeImageAtlas(imageMap, patternMap); + ImageAtlas iconAtlas = makeImageAtlas(imageMap, patternMap, versionMap); if (!layouts.empty()) { GlyphAtlas glyphAtlas = makeGlyphAtlas(glyphMap); glyphAtlasImage = std::move(glyphAtlas.image); diff --git a/src/mbgl/tile/geometry_tile_worker.hpp b/src/mbgl/tile/geometry_tile_worker.hpp index 485d1315a6..706bac0154 100644 --- a/src/mbgl/tile/geometry_tile_worker.hpp +++ b/src/mbgl/tile/geometry_tile_worker.hpp @@ -44,7 +44,7 @@ public: void setShowCollisionBoxes(bool showCollisionBoxes_, uint64_t correlationID_); void onGlyphsAvailable(GlyphMap glyphs); - void onImagesAvailable(ImageMap icons, ImageMap patterns, uint64_t imageCorrelationID); + void onImagesAvailable(ImageMap icons, ImageMap patterns, ImageVersionMap versionMap, uint64_t imageCorrelationID); private: void coalesced(); @@ -96,6 +96,7 @@ private: GlyphMap glyphMap; ImageMap imageMap; ImageMap patternMap; + ImageVersionMap versionMap; bool showCollisionBoxes; bool firstLoad = true; diff --git a/test/renderer/image_manager.test.cpp b/test/renderer/image_manager.test.cpp index 20451827ce..b73d9b5c7a 100644 --- a/test/renderer/image_manager.test.cpp +++ b/test/renderer/image_manager.test.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -107,11 +108,11 @@ TEST(ImageManager, RemoveReleasesBinPackRect) { class StubImageRequestor : public ImageRequestor { public: - void onImagesAvailable(ImageMap icons, ImageMap patterns, uint64_t imageCorrelationID_) final { - if (imagesAvailable && imageCorrelationID == imageCorrelationID_) imagesAvailable(icons, patterns); + void onImagesAvailable(ImageMap icons, ImageMap patterns, std::unordered_map versionMap, uint64_t imageCorrelationID_) final { + if (imagesAvailable && imageCorrelationID == imageCorrelationID_) imagesAvailable(icons, patterns, versionMap); } - std::function imagesAvailable; + std::function)> imagesAvailable; uint64_t imageCorrelationID = 0; }; @@ -120,7 +121,10 @@ TEST(ImageManager, NotifiesRequestorWhenSpriteIsLoaded) { StubImageRequestor requestor; bool notified = false; - requestor.imagesAvailable = [&] (ImageMap, ImageMap) { + ImageManagerObserver observer; + imageManager.setObserver(&observer); + + requestor.imagesAvailable = [&] (ImageMap, ImageMap, std::unordered_map) { notified = true; }; @@ -131,6 +135,8 @@ TEST(ImageManager, NotifiesRequestorWhenSpriteIsLoaded) { ASSERT_FALSE(notified); imageManager.setLoaded(true); + ASSERT_FALSE(notified); + imageManager.notifyIfMissingImageAdded(); ASSERT_TRUE(notified); } @@ -139,7 +145,7 @@ TEST(ImageManager, NotifiesRequestorImmediatelyIfDependenciesAreSatisfied) { StubImageRequestor requestor; bool notified = false; - requestor.imagesAvailable = [&] (ImageMap, ImageMap) { + requestor.imagesAvailable = [&] (ImageMap, ImageMap, std::unordered_map) { notified = true; }; -- cgit v1.2.1