[core] Use optional where SpriteAtlas return values might not exist

Fixes #3162

6 files changed, 55 insertions, 47 deletions

diff --git a/src/mbgl/renderer/painter.cpp b/src/mbgl/renderer/painter.cpp
index 70d2c75c0d..49b4526f58 100644
--- a/src/mbgl/renderer/painter.cpp
+++ b/src/mbgl/renderer/painter.cpp
@@ -276,19 +276,20 @@ void Painter::renderBackground(const BackgroundLayer& layer) {
     const BackgroundPaintProperties& properties = layer.paint;
 
     if (!properties.pattern.value.to.empty()) {
-        if ((properties.opacity >= 1.0f) != (pass == RenderPass::Opaque))
+        mapbox::util::optional<SpriteAtlasPosition> imagePosA = spriteAtlas->getPosition(properties.pattern.value.from, true);
+        mapbox::util::optional<SpriteAtlasPosition> imagePosB = spriteAtlas->getPosition(properties.pattern.value.to, true);
+
+        if ((properties.opacity >= 1.0f) != (pass == RenderPass::Opaque) || !imagePosA || !imagePosB)
             return;
 
-        SpriteAtlasPosition imagePosA = spriteAtlas->getPosition(properties.pattern.value.from, true);
-        SpriteAtlasPosition imagePosB = spriteAtlas->getPosition(properties.pattern.value.to, true);
         float zoomFraction = state.getZoomFraction();
 
         config.program = patternShader->program;
         patternShader->u_matrix = identityMatrix;
-        patternShader->u_pattern_tl_a = imagePosA.tl;
-        patternShader->u_pattern_br_a = imagePosA.br;
-        patternShader->u_pattern_tl_b = imagePosB.tl;
-        patternShader->u_pattern_br_b = imagePosB.br;
+        patternShader->u_pattern_tl_a = (*imagePosA).tl;
+        patternShader->u_pattern_br_a = (*imagePosA).br;
+        patternShader->u_pattern_tl_b = (*imagePosB).tl;
+        patternShader->u_pattern_br_b = (*imagePosB).br;
         patternShader->u_mix = properties.pattern.value.t;
         patternShader->u_opacity = properties.opacity;
 
@@ -296,7 +297,7 @@ void Painter::renderBackground(const BackgroundLayer& layer) {
         PrecisionPoint center = state.latLngToPoint(latLng);
         float scale = 1 / std::pow(2, zoomFraction);
 
-        std::array<float, 2> sizeA = imagePosA.size;
+        std::array<float, 2> sizeA = (*imagePosA).size;
         mat3 matrixA;
         matrix::identity(matrixA);
         matrix::scale(matrixA, matrixA,
@@ -310,7 +311,7 @@ void Painter::renderBackground(const BackgroundLayer& layer) {
                        scale * state.getWidth()  / 2,
                       -scale * state.getHeight() / 2);
 
-        std::array<float, 2> sizeB = imagePosB.size;
+        std::array<float, 2> sizeB = (*imagePosB).size;
         mat3 matrixB;
         matrix::identity(matrixB);
         matrix::scale(matrixB, matrixB,
diff --git a/src/mbgl/renderer/painter_fill.cpp b/src/mbgl/renderer/painter_fill.cpp
index 4bee5b6d1e..beebdfcf83 100644
--- a/src/mbgl/renderer/painter_fill.cpp
+++ b/src/mbgl/renderer/painter_fill.cpp
@@ -59,30 +59,30 @@ void Painter::renderFill(FillBucket& bucket, const FillLayer& layer, const TileI
     }
 
     if (pattern) {
-        // Image fill.
-        if (pass == RenderPass::Translucent) {
+        mapbox::util::optional<SpriteAtlasPosition> posA = spriteAtlas->getPosition(properties.pattern.value.from, true);
+        mapbox::util::optional<SpriteAtlasPosition> posB = spriteAtlas->getPosition(properties.pattern.value.to, true);
 
-            const SpriteAtlasPosition posA = spriteAtlas->getPosition(properties.pattern.value.from, true);
-            const SpriteAtlasPosition posB = spriteAtlas->getPosition(properties.pattern.value.to, true);
+        // Image fill.
+        if (pass == RenderPass::Translucent && posA && posB) {
             float factor = 8.0 / std::pow(2, state.getIntegerZoom() - id.z) / id.overscaling;
 
             mat3 patternMatrixA;
             matrix::identity(patternMatrixA);
             matrix::scale(patternMatrixA, patternMatrixA,
-                    1.0f / (posA.size[0] * factor * properties.pattern.value.fromScale),
-                    1.0f / (posA.size[1] * factor * properties.pattern.value.fromScale));
+                    1.0f / ((*posA).size[0] * factor * properties.pattern.value.fromScale),
+                    1.0f / ((*posA).size[1] * factor * properties.pattern.value.fromScale));
             mat3 patternMatrixB;
             matrix::identity(patternMatrixB);
             matrix::scale(patternMatrixB, patternMatrixB,
-                    1.0f / (posB.size[0] * factor * properties.pattern.value.toScale),
-                    1.0f / (posB.size[1] * factor * properties.pattern.value.toScale));
+                    1.0f / ((*posB).size[0] * factor * properties.pattern.value.toScale),
+                    1.0f / ((*posB).size[1] * factor * properties.pattern.value.toScale));
 
             config.program = patternShader->program;
             patternShader->u_matrix = vtxMatrix;
-            patternShader->u_pattern_tl_a = posA.tl;
-            patternShader->u_pattern_br_a = posA.br;
-            patternShader->u_pattern_tl_b = posB.tl;
-            patternShader->u_pattern_br_b = posB.br;
+            patternShader->u_pattern_tl_a = (*posA).tl;
+            patternShader->u_pattern_br_a = (*posA).br;
+            patternShader->u_pattern_tl_b = (*posB).tl;
+            patternShader->u_pattern_br_b = (*posB).br;
             patternShader->u_opacity = properties.opacity;
             patternShader->u_image = 0;
             patternShader->u_mix = properties.pattern.value.t;
diff --git a/src/mbgl/renderer/painter_line.cpp b/src/mbgl/renderer/painter_line.cpp
index e33864622e..30f03ff030 100644
--- a/src/mbgl/renderer/painter_line.cpp
+++ b/src/mbgl/renderer/painter_line.cpp
@@ -102,8 +102,11 @@ void Painter::renderLine(LineBucket& bucket, const LineLayer& layer, const TileI
         bucket.drawLineSDF(*linesdfShader);
 
     } else if (!properties.pattern.value.from.empty()) {
-        SpriteAtlasPosition imagePosA = spriteAtlas->getPosition(properties.pattern.value.from, true);
-        SpriteAtlasPosition imagePosB = spriteAtlas->getPosition(properties.pattern.value.to, true);
+        mapbox::util::optional<SpriteAtlasPosition> imagePosA = spriteAtlas->getPosition(properties.pattern.value.from, true);
+        mapbox::util::optional<SpriteAtlasPosition> imagePosB = spriteAtlas->getPosition(properties.pattern.value.to, true);
+        
+        if (!imagePosA || !imagePosB)
+            return;
 
         float factor = 8.0 / std::pow(2, state.getIntegerZoom() - id.z) * id.overscaling;
 
@@ -115,12 +118,12 @@ void Painter::renderLine(LineBucket& bucket, const LineLayer& layer, const TileI
         linepatternShader->u_ratio = ratio;
         linepatternShader->u_blur = blur;
 
-        linepatternShader->u_pattern_size_a = {{imagePosA.size[0] * factor * properties.pattern.value.fromScale, imagePosA.size[1]}};
-        linepatternShader->u_pattern_tl_a = imagePosA.tl;
-        linepatternShader->u_pattern_br_a = imagePosA.br;
-        linepatternShader->u_pattern_size_b = {{imagePosB.size[0] * factor * properties.pattern.value.toScale, imagePosB.size[1]}};
-        linepatternShader->u_pattern_tl_b = imagePosB.tl;
-        linepatternShader->u_pattern_br_b = imagePosB.br;
+        linepatternShader->u_pattern_size_a = {{(*imagePosA).size[0] * factor * properties.pattern.value.fromScale, (*imagePosA).size[1]}};
+        linepatternShader->u_pattern_tl_a = (*imagePosA).tl;
+        linepatternShader->u_pattern_br_a = (*imagePosA).br;
+        linepatternShader->u_pattern_size_b = {{(*imagePosB).size[0] * factor * properties.pattern.value.toScale, (*imagePosB).size[1]}};
+        linepatternShader->u_pattern_tl_b = (*imagePosB).tl;
+        linepatternShader->u_pattern_br_b = (*imagePosB).br;
         linepatternShader->u_fade = properties.pattern.value.t;
         linepatternShader->u_opacity = properties.opacity;
         linepatternShader->u_extra = extra;
diff --git a/src/mbgl/renderer/symbol_bucket.cpp b/src/mbgl/renderer/symbol_bucket.cpp
index 1f2ad96bf2..f5737d7430 100644
--- a/src/mbgl/renderer/symbol_bucket.cpp
+++ b/src/mbgl/renderer/symbol_bucket.cpp
@@ -247,10 +247,10 @@ void SymbolBucket::addFeatures(uintptr_t tileUID,
         // if feature has icon, get sprite atlas position
         if (feature.sprite.length()) {
             auto image = spriteAtlas.getImage(feature.sprite, false);
-            if (image.pos.hasArea() && image.texture) {
-                shapedIcon = shapeIcon(image.pos, layout);
-                assert(image.texture);
-                if (image.texture->sdf) {
+            if (image) {
+                shapedIcon = shapeIcon((*image).pos, layout);
+                assert((*image).texture);
+                if ((*image).texture->sdf) {
                     sdfIcons = true;
                 }
             }
diff --git a/src/mbgl/sprite/sprite_atlas.cpp b/src/mbgl/sprite/sprite_atlas.cpp
index ae71f18f03..6ebebb9507 100644
--- a/src/mbgl/sprite/sprite_atlas.cpp
+++ b/src/mbgl/sprite/sprite_atlas.cpp
@@ -47,17 +47,17 @@ Rect<SpriteAtlas::dimension> SpriteAtlas::allocateImage(const size_t pixel_width
     return rect;
 }
 
-SpriteAtlasElement SpriteAtlas::getImage(const std::string& name, const bool wrap) {
+mapbox::util::optional<SpriteAtlasElement> SpriteAtlas::getImage(const std::string& name, const bool wrap) {
     std::lock_guard<std::recursive_mutex> lock(mtx);
 
     auto rect_it = images.find({ name, wrap });
     if (rect_it != images.end()) {
-        return { rect_it->second.pos, rect_it->second.texture };
+        return SpriteAtlasElement { rect_it->second.pos, rect_it->second.texture };
     }
 
     auto sprite = store.getSprite(name);
     if (!sprite) {
-        return { Rect<dimension> { 0, 0, 0, 0 }, nullptr };
+        return {};
     }
 
     Rect<dimension> rect = allocateImage(sprite->width, sprite->height);
@@ -65,19 +65,24 @@ SpriteAtlasElement SpriteAtlas::getImage(const std::string& name, const bool wra
         if (debug::spriteWarnings) {
             Log::Warning(Event::Sprite, "sprite atlas bitmap overflow");
         }
-        return { Rect<dimension> { 0, 0, 0, 0 }, nullptr };
+        return {};
     }
 
     const Holder& holder = images.emplace(Key{ name, wrap }, Holder{ sprite, rect }).first->second;
     copy(holder, wrap);
 
-    return { rect, sprite };
+    return SpriteAtlasElement { rect, sprite };
 }
 
-SpriteAtlasPosition SpriteAtlas::getPosition(const std::string& name, bool repeating) {
+mapbox::util::optional<SpriteAtlasPosition> SpriteAtlas::getPosition(const std::string& name, bool repeating) {
     std::lock_guard<std::recursive_mutex> lock(mtx);
 
-    auto rect = getImage(name, repeating).pos;
+    auto img = getImage(name, repeating);
+    if (!img) {
+        return {};
+    }
+
+    auto rect = (*img).pos;
     if (repeating) {
         // When the image is repeating, get the correct position of the image, rather than the
         // one rounded up to 4 pixels.
diff --git a/src/mbgl/sprite/sprite_atlas.hpp b/src/mbgl/sprite/sprite_atlas.hpp
index 0e3e8cf225..0d1e52cf81 100644
--- a/src/mbgl/sprite/sprite_atlas.hpp
+++ b/src/mbgl/sprite/sprite_atlas.hpp
@@ -6,6 +6,8 @@
 #include <mbgl/util/noncopyable.hpp>
 #include <mbgl/util/ptr.hpp>
 
+#include <mapbox/optional.hpp>
+
 #include <string>
 #include <map>
 #include <mutex>
@@ -41,15 +43,12 @@ public:
     SpriteAtlas(dimension width, dimension height, float pixelRatio, SpriteStore& store);
     ~SpriteAtlas();
 
-    // Returns the coordinates of an image that is sourced from the sprite image.
-    // This getter attempts to read the image from the sprite if it is already loaded.
-    // In that case, it copies it into the sprite atlas and returns the dimensions.
-    // Otherwise, it returns a 0/0/0/0 rect.
-    // This function is used during bucket creation.
-    SpriteAtlasElement getImage(const std::string& name, const bool wrap);
+    // If the sprite is loaded, copies the requsted image from it into the atlas and returns
+    // the resulting icon measurements. If not, returns an empty optional.
+    mapbox::util::optional<SpriteAtlasElement> getImage(const std::string& name, const bool wrap);
 
     // This function is used for getting the position during render time.
-    SpriteAtlasPosition getPosition(const std::string& name, bool repeating = false);
+    mapbox::util::optional<SpriteAtlasPosition> getPosition(const std::string& name, bool repeating = false);
 
     // Binds the atlas texture to the GPU, and uploads data if it is out of date.
     void bind(bool linear = false);