[core] Use shared pointer to manage Image source raster data and speed up change detection

6 files changed, 74 insertions, 41 deletions

diff --git a/src/mbgl/renderer/buckets/raster_bucket.cpp b/src/mbgl/renderer/buckets/raster_bucket.cpp
index 49ec0065c3..61548ee333 100644
--- a/src/mbgl/renderer/buckets/raster_bucket.cpp
+++ b/src/mbgl/renderer/buckets/raster_bucket.cpp
@@ -9,12 +9,19 @@ namespace mbgl {
 
 using namespace style;
 
-RasterBucket::RasterBucket(UnassociatedImage&& image_) : image(std::move(image_)) {
+RasterBucket::RasterBucket(UnassociatedImage&& image_) {
+    image = std::make_shared<UnassociatedImage>(std::move(image_));
 }
 
+RasterBucket::RasterBucket(std::shared_ptr<UnassociatedImage> image_): image(image_) {
+
+}
 void RasterBucket::upload(gl::Context& context) {
+    if (!hasData()) {
+        return;
+    }
     if (!texture) {
-        texture = context.createTexture(image);
+        texture = context.createTexture(*image);
     }
     if (!vertices.empty()) {
         vertexBuffer = context.createVertexBuffer(std::move(vertices));
@@ -32,6 +39,12 @@ void RasterBucket::clear() {
 
     uploaded = false;
 }
+
+void RasterBucket::setImage(std::shared_ptr<UnassociatedImage> image_) {
+    image = std::move(image_);
+    texture = {};
+    uploaded = false;
+}
 void RasterBucket::render(Painter& painter,
                           PaintParameters& parameters,
                           const RenderLayer& layer,
@@ -47,7 +60,7 @@ void RasterBucket::render(Painter& painter,
 }
 
 bool RasterBucket::hasData() const {
-    return true;
+    return !!image;
 }
 
 } // namespace mbgl
diff --git a/src/mbgl/renderer/buckets/raster_bucket.hpp b/src/mbgl/renderer/buckets/raster_bucket.hpp
index 44b9111b81..e92e4b51f7 100644
--- a/src/mbgl/renderer/buckets/raster_bucket.hpp
+++ b/src/mbgl/renderer/buckets/raster_bucket.hpp
@@ -14,7 +14,7 @@ namespace mbgl {
 class RasterBucket : public Bucket {
 public:
     RasterBucket(UnassociatedImage&&);
-
+    RasterBucket(std::shared_ptr<UnassociatedImage>);
     void upload(gl::Context&) override;
     void render(Painter&, PaintParameters&, const RenderLayer&, const RenderTile&) override;
     void render(Painter& painter,
@@ -24,7 +24,8 @@ public:
     bool hasData() const override;
 
     void clear();
-    UnassociatedImage image;
+    void setImage(std::shared_ptr<UnassociatedImage>);
+    std::shared_ptr<UnassociatedImage> image;
     optional<gl::Texture> texture;
 
     // Bucket specific vertices are used for Image Sources only
diff --git a/src/mbgl/renderer/sources/render_image_source.cpp b/src/mbgl/renderer/sources/render_image_source.cpp
index f1860b6b62..a17f97ee2f 100644
--- a/src/mbgl/renderer/sources/render_image_source.cpp
+++ b/src/mbgl/renderer/sources/render_image_source.cpp
@@ -14,7 +14,7 @@ namespace mbgl {
 using namespace style;
 
 RenderImageSource::RenderImageSource(Immutable<style::ImageSource::Impl> impl_)
-    : RenderSource(impl_), shouldRender(false) {
+    : RenderSource(impl_) {
 }
 
 RenderImageSource::~RenderImageSource() = default;
@@ -42,13 +42,13 @@ void RenderImageSource::startRender(Painter& painter) {
         matrices.push_back(matrix);
     }
 
-    if (bucket->needsUpload() && shouldRender) {
+    if (bucket->needsUpload()) {
         bucket->upload(painter.context);
     }
 }
 
 void RenderImageSource::finishRender(Painter& painter) {
-    if (!isLoaded() || !shouldRender) {
+    if (!isLoaded()) {
         return;
     }
     for (auto matrix : matrices) {
@@ -73,15 +73,24 @@ void RenderImageSource::update(Immutable<style::Source::Impl> baseImpl_,
                                const bool needsRendering,
                                const bool,
                                const TileParameters& parameters) {
-    std::swap(baseImpl, baseImpl_);
-
     enabled = needsRendering;
+    if (!needsRendering) {
+        return;
+    }
 
     auto transformState = parameters.transformState;
-    auto size = transformState.getSize();
-    double viewportHeight = size.height;
+    std::swap(baseImpl, baseImpl_);
 
     auto coords = impl().getCoordinates();
+    std::shared_ptr<UnassociatedImage> image = impl().getImage();
+
+    if (!image || !image->valid()) {
+        enabled = false;
+        return;
+    }
+
+    auto size = transformState.getSize();
+    const double viewportHeight = size.height;
 
     // Compute the screen coordinates at wrap=0 for the given LatLng
     ScreenCoordinate nePixel = { -INFINITY, -INFINITY };
@@ -94,38 +103,51 @@ void RenderImageSource::update(Immutable<style::Source::Impl> baseImpl_,
         swPixel.y = std::min(swPixel.y, viewportHeight - pixel.y);
         nePixel.y = std::max(nePixel.y, viewportHeight - pixel.y);
     }
-    double width = nePixel.x - swPixel.x;
-    double height = nePixel.y - swPixel.y;
+    const double width = nePixel.x - swPixel.x;
+    const double height = nePixel.y - swPixel.y;
 
     // Don't bother drawing the ImageSource unless it occupies >4 screen pixels
-    shouldRender = (width * height > 4);
-    if (!shouldRender) {
+    enabled = (width * height > 4);
+    if (!enabled) {
         return;
     }
 
     // Calculate the optimum zoom level to determine the tile ids to use for transforms
     double minScale = INFINITY;
-    if (width > 0 || height > 0) {
-        double scaleX = double(size.width) / width;
-        double scaleY = double(size.height) / height;
-        minScale = util::min(scaleX, scaleY);
-    }
+    double scaleX = double(size.width) / width;
+    double scaleY = double(size.height) / height;
+    minScale = util::min(scaleX, scaleY);
     double zoom = transformState.getZoom() + util::log2(minScale);
-    zoom = util::clamp(zoom, transformState.getMinZoom(), transformState.getMaxZoom());
-
+    zoom = std::floor(util::clamp(zoom, transformState.getMinZoom(), transformState.getMaxZoom()));
     auto imageBounds = LatLngBounds::hull(coords[0], coords[1]);
     imageBounds.extend(coords[2]);
     imageBounds.extend(coords[3]);
-    auto tileCover = util::tileCover(imageBounds, ::floor(zoom));
+    auto tileCover = util::tileCover(imageBounds, zoom);
     tileIds.clear();
     tileIds.push_back(tileCover[0]);
+    bool hasVisibleTile = false;
 
     // Add additional wrapped tile ids if neccessary
     auto idealTiles = util::tileCover(transformState, transformState.getZoom());
     for (auto tile : idealTiles) {
         if (tile.wrap != 0 && tileCover[0].canonical.isChildOf(tile.canonical)) {
             tileIds.push_back({ tile.wrap, tileCover[0].canonical });
+            hasVisibleTile = true;
         }
+        else if (!hasVisibleTile) {
+            for (auto coveringTile: tileCover) {
+                if(coveringTile.canonical == tile.canonical ||
+                    coveringTile.canonical.isChildOf(tile.canonical) ||
+                    tile.canonical.isChildOf(coveringTile.canonical)) {
+                    hasVisibleTile = true;
+                }
+            }
+        }
+    }
+
+    enabled = hasVisibleTile;
+    if (!enabled) {
+        return;
     }
 
     // Calculate Geometry Coordinates based on tile cover at ideal zoom
@@ -135,16 +157,13 @@ void RenderImageSource::update(Immutable<style::Source::Impl> baseImpl_,
         auto gc = TileCoordinate::toGeometryCoordinate(tileIds[0], tc.p);
         geomCoords.push_back(gc);
     }
-    
-    const UnassociatedImage& image = impl().getImage();
-    if (!image.valid()) {
-        return;
-    }
-    
-    if (!bucket || image != bucket->image) {
-        bucket = std::make_unique<RasterBucket>(image.clone());
+    if (!bucket) {
+        bucket = std::make_unique<RasterBucket>(image);
     } else {
         bucket->clear();
+        if (image != bucket->image) {
+            bucket->setImage(image);
+        }
     }
 
     // Set Bucket Vertices, Indices, and segments
@@ -166,7 +185,7 @@ void RenderImageSource::update(Immutable<style::Source::Impl> baseImpl_,
 void RenderImageSource::render(Painter& painter,
                                PaintParameters& parameters,
                                const RenderLayer& layer) {
-    if (isLoaded() && !bucket->needsUpload() && shouldRender) {
+    if (isEnabled() && isLoaded() && !bucket->needsUpload()) {
         for (auto matrix : matrices) {
             bucket->render(painter, parameters, layer, matrix);
         }
diff --git a/src/mbgl/renderer/sources/render_image_source.hpp b/src/mbgl/renderer/sources/render_image_source.hpp
index 41c4bd5483..6389251709 100644
--- a/src/mbgl/renderer/sources/render_image_source.hpp
+++ b/src/mbgl/renderer/sources/render_image_source.hpp
@@ -8,6 +8,7 @@ namespace mbgl {
 class RenderLayer;
 class PaintParameters;
 class RasterBucket;
+class LatLng;
 
 namespace gl {
 class Context;
@@ -52,7 +53,6 @@ private:
     std::vector<UnwrappedTileID> tileIds;
     std::unique_ptr<RasterBucket> bucket;
     std::vector<mat4> matrices;
-    bool shouldRender;
 };
 
 template <>
diff --git a/src/mbgl/style/sources/image_source_impl.cpp b/src/mbgl/style/sources/image_source_impl.cpp
index 98f3cc9db9..eb3e2635e5 100644
--- a/src/mbgl/style/sources/image_source_impl.cpp
+++ b/src/mbgl/style/sources/image_source_impl.cpp
@@ -12,17 +12,17 @@ ImageSource::Impl::Impl(std::string id_, std::array<LatLng, 4> coords_)
 ImageSource::Impl::Impl(const Impl& other, std::array<LatLng, 4> coords_)
     : Source::Impl(other),
     coords(std::move(coords_)),
-    image(other.image.clone()) {
+    image(other.image) {
 }
 
-ImageSource::Impl::Impl(const Impl& rhs, UnassociatedImage image_)
+ImageSource::Impl::Impl(const Impl& rhs, UnassociatedImage&& image_)
     : Source::Impl(rhs),
     coords(rhs.coords),
-    image(std::move(image_)) {
+    image(std::make_shared<UnassociatedImage>(std::move(image_))) {
 }
 ImageSource::Impl::~Impl() = default;
 
-const UnassociatedImage& ImageSource::Impl::getImage() const {
+std::shared_ptr<UnassociatedImage> ImageSource::Impl::getImage() const {
     return image;
 }
 
diff --git a/src/mbgl/style/sources/image_source_impl.hpp b/src/mbgl/style/sources/image_source_impl.hpp
index 5fd41ac6e6..e0999c34a5 100644
--- a/src/mbgl/style/sources/image_source_impl.hpp
+++ b/src/mbgl/style/sources/image_source_impl.hpp
@@ -13,17 +13,17 @@ class ImageSource::Impl : public Source::Impl {
 public:
     Impl(std::string id, std::array<LatLng, 4> coords);
     Impl(const Impl& rhs, std::array<LatLng, 4> coords);
-    Impl(const Impl& rhs, UnassociatedImage image);
+    Impl(const Impl& rhs, UnassociatedImage&& image);
 
     ~Impl() final;
 
-    const UnassociatedImage& getImage() const;
+    std::shared_ptr<UnassociatedImage> getImage() const;
     std::array<LatLng, 4> getCoordinates() const;
 
     optional<std::string> getAttribution() const final;
 private:
     std::array<LatLng, 4> coords;
-    UnassociatedImage image;
+    std::shared_ptr<UnassociatedImage> image;
 };
 
 } // namespace style