[core] Add image sections to format expression

8 files changed, 151 insertions, 92 deletions

diff --git a/include/mbgl/style/expression/format_expression.hpp b/include/mbgl/style/expression/format_expression.hpp
index 180df0139d..248a3cf6d4 100644
--- a/include/mbgl/style/expression/format_expression.hpp
+++ b/include/mbgl/style/expression/format_expression.hpp
@@ -8,12 +8,16 @@ namespace style {
 namespace expression {
 
 struct FormatExpressionSection {
-    FormatExpressionSection(std::unique_ptr<Expression> text_,
-                            optional<std::unique_ptr<Expression>> fontScale_,
-                            optional<std::unique_ptr<Expression>> textFont_,
-                            optional<std::unique_ptr<Expression>> textColor_);
-    
-    std::shared_ptr<Expression> text;
+    explicit FormatExpressionSection(std::unique_ptr<Expression> content_);
+
+    void setTextSectionOptions(optional<std::unique_ptr<Expression>> fontScale_,
+                               optional<std::unique_ptr<Expression>> textFont_,
+                               optional<std::unique_ptr<Expression>> textColor_);
+
+    // Content can be expression that evaluates to String or Image.
+    std::shared_ptr<Expression> content;
+
+    // Text related section options.
     optional<std::shared_ptr<Expression>> fontScale;
     optional<std::shared_ptr<Expression>> textFont;
     optional<std::shared_ptr<Expression>> textColor;
diff --git a/include/mbgl/style/expression/formatted.hpp b/include/mbgl/style/expression/formatted.hpp
index 09edad240f..d089e12d9d 100644
--- a/include/mbgl/style/expression/formatted.hpp
+++ b/include/mbgl/style/expression/formatted.hpp
@@ -1,6 +1,7 @@
 #pragma once
 
 #include <mbgl/style/conversion.hpp>
+#include <mbgl/style/expression/image.hpp>
 #include <mbgl/util/color.hpp>
 #include <mbgl/util/font_stack.hpp>
 #include <mbgl/util/optional.hpp>
@@ -17,17 +18,19 @@ extern const char* const kFormattedSectionTextFont;
 extern const char* const kFormattedSectionTextColor;
 
 struct FormattedSection {
-    FormattedSection(std::string text_,
-                     optional<double> fontScale_,
-                     optional<FontStack> fontStack_,
-                     optional<Color> textColor_)
-        : text(std::move(text_))
-        , fontScale(std::move(fontScale_))
-        , fontStack(std::move(fontStack_))
-        , textColor(std::move(textColor_))
-    {}
+    explicit FormattedSection(std::string text_,
+                              optional<double> fontScale_,
+                              optional<FontStack> fontStack_,
+                              optional<Color> textColor_)
+        : text(std::move(text_)),
+          fontScale(std::move(fontScale_)),
+          fontStack(std::move(fontStack_)),
+          textColor(std::move(textColor_)) {}
+
+    explicit FormattedSection(Image image_) : image(std::move(image_)) {}
 
     std::string text;
+    optional<Image> image;
     optional<double> fontScale;
     optional<FontStack> fontStack;
     optional<Color> textColor;
@@ -50,10 +53,8 @@ public:
     std::string toString() const;
     mbgl::Value toObject() const;
 
-    bool empty() const {
-        return sections.empty() || sections.at(0).text.empty();
-    }
-     
+    bool empty() const;
+
     std::vector<FormattedSection> sections;
 };
             
diff --git a/src/mbgl/style/conversion/function.cpp b/src/mbgl/style/conversion/function.cpp
index 4b3b8f6c6a..e3beb44d22 100644
--- a/src/mbgl/style/conversion/function.cpp
+++ b/src/mbgl/style/conversion/function.cpp
@@ -42,9 +42,8 @@ bool hasTokens(const std::string& source) {
     
 std::unique_ptr<Expression> convertTokenStringToFormatExpression(const std::string& source) {
     auto textExpression = convertTokenStringToExpression(source);
-    std::vector<FormatExpressionSection> sections;
-    sections.emplace_back(std::move(textExpression), nullopt, nullopt, nullopt);
-    return std::make_unique<FormatExpression>(sections);
+    std::vector<FormatExpressionSection> sections{FormatExpressionSection(std::move(textExpression))};
+    return std::make_unique<FormatExpression>(std::move(sections));
 }
 
 std::unique_ptr<Expression> convertTokenStringToImageExpression(const std::string& source) {
diff --git a/src/mbgl/style/expression/dsl.cpp b/src/mbgl/style/expression/dsl.cpp
index 6405149fe9..c8d98e61a4 100644
--- a/src/mbgl/style/expression/dsl.cpp
+++ b/src/mbgl/style/expression/dsl.cpp
@@ -237,9 +237,8 @@ std::unique_ptr<Expression> format(const char* value) {
 }
 
 std::unique_ptr<Expression> format(std::unique_ptr<Expression> input) {
-    std::vector<FormatExpressionSection> sections;
-    sections.emplace_back(std::move(input), nullopt, nullopt, nullopt);
-    return std::make_unique<FormatExpression>(sections);
+    std::vector<FormatExpressionSection> sections{FormatExpressionSection(std::move(input))};
+    return std::make_unique<FormatExpression>(std::move(sections));
 }
 
 std::unique_ptr<Expression> image(const char* value) {
diff --git a/src/mbgl/style/expression/format_expression.cpp b/src/mbgl/style/expression/format_expression.cpp
index 743942c769..201e189f41 100644
--- a/src/mbgl/style/expression/format_expression.cpp
+++ b/src/mbgl/style/expression/format_expression.cpp
@@ -6,21 +6,23 @@ namespace mbgl {
 namespace style {
 namespace expression {
 
-FormatExpressionSection::FormatExpressionSection(std::unique_ptr<Expression> text_,
-                                                 optional<std::unique_ptr<Expression>> fontScale_,
-                                                 optional<std::unique_ptr<Expression>> textFont_,
-                                                 optional<std::unique_ptr<Expression>> textColor_)
-    : text(std::move(text_))
-{
+FormatExpressionSection::FormatExpressionSection(std::unique_ptr<Expression> content_) : content(std::move(content_)) {}
+
+void FormatExpressionSection::setTextSectionOptions(optional<std::unique_ptr<Expression>> fontScale_,
+                                                    optional<std::unique_ptr<Expression>> textFont_,
+                                                    optional<std::unique_ptr<Expression>> textColor_) {
     if (fontScale_) {
+        assert(*fontScale_);
         fontScale = std::shared_ptr<Expression>(std::move(*fontScale_));
     }
 
     if (textFont_) {
+        assert(*textFont_);
         textFont = std::shared_ptr<Expression>(std::move(*textFont_));
     }
 
     if (textColor_) {
+        assert(*textColor_);
         textColor = std::shared_ptr<Expression>(std::move(*textColor_));
     }
 }
@@ -34,68 +36,69 @@ using namespace mbgl::style::conversion;
 
 ParseResult FormatExpression::parse(const Convertible& value, ParsingContext& ctx) {
     std::size_t argsLength = arrayLength(value);
-    if (argsLength < 3) {
-        ctx.error("Expected at least two arguments.");
+    if (argsLength < 2) {
+        ctx.error("Expected at least one argument.");
         return ParseResult();
     }
-    
-    if ((argsLength - 1) % 2 != 0) {
-        ctx.error("Expected an even number of arguments.");
+
+    if (isObject(arrayMember(value, 1))) {
+        ctx.error("First argument must be an image or text section.");
         return ParseResult();
     }
-    
+
     std::vector<FormatExpressionSection> sections;
-    for (std::size_t i = 1; i < argsLength - 1; i += 2) {
-        auto textArg = arrayMember(value, i);
-        ParseResult text = ctx.parse(textArg, 1, {type::Value});
-        if (!text) {
-            return ParseResult();
-        }
-        auto options = arrayMember(value, i + 1);
-        if (!isObject(options)) {
-            ctx.error("Format options argument must be an object.");
-            return ParseResult();
-        }
-        
-        const optional<Convertible> fontScaleOption = objectMember(options, kFormattedSectionFontScale);
-        ParseResult fontScale;
-        if (fontScaleOption) {
-            fontScale = ctx.parse(*fontScaleOption, 1, {type::Number});
-            if (!fontScale) {
-                return ParseResult();
+    bool nextTokenMayBeObject = false;
+    for (std::size_t i = 1; i <= argsLength - 1; ++i) {
+        auto arg = arrayMember(value, i);
+
+        if (nextTokenMayBeObject && isObject(arg)) {
+            nextTokenMayBeObject = false;
+
+            const optional<Convertible> fontScaleOption = objectMember(arg, kFormattedSectionFontScale);
+            ParseResult fontScale;
+            if (fontScaleOption) {
+                fontScale = ctx.parse(*fontScaleOption, 1, {type::Number});
+                if (!fontScale) {
+                    return ParseResult();
+                }
             }
-        }
-        
-        const optional<Convertible> textFontOption = objectMember(options, kFormattedSectionTextFont);
-        ParseResult textFont;
-        if (textFontOption) {
-            textFont = ctx.parse(*textFontOption, 1, {type::Array(type::String)});
-            if (!textFont) {
-                return ParseResult();
+
+            const optional<Convertible> textFontOption = objectMember(arg, kFormattedSectionTextFont);
+            ParseResult textFont;
+            if (textFontOption) {
+                textFont = ctx.parse(*textFontOption, 1, {type::Array(type::String)});
+                if (!textFont) {
+                    return ParseResult();
+                }
+            }
+            const optional<Convertible> textColorOption = objectMember(arg, kFormattedSectionTextColor);
+            ParseResult textColor;
+            if (textColorOption) {
+                textColor = ctx.parse(*textColorOption, 1, {type::Color});
+                if (!textColor) {
+                    return ParseResult();
+                }
             }
-        }
 
-        const optional<Convertible> textColorOption = objectMember(options, kFormattedSectionTextColor);
-        ParseResult textColor;
-        if (textColorOption) {
-            textColor = ctx.parse(*textColorOption, 1, {type::Color});
-            if (!textColor) {
+            sections.back().setTextSectionOptions(std::move(fontScale), std::move(textFont), std::move(textColor));
+        } else {
+            ParseResult parsedArg = ctx.parse(arg, 1, {type::Value});
+            if (!parsedArg) {
+                ctx.error("Cannot parse formatted section.");
                 return ParseResult();
             }
-        }
 
-        sections.emplace_back(std::move(*text),
-                              std::move(fontScale),
-                              std::move(textFont),
-                              std::move(textColor));
+            nextTokenMayBeObject = true;
+            sections.emplace_back(std::move(*parsedArg));
+        }
     }
-    
+
     return ParseResult(std::make_unique<FormatExpression>(std::move(sections)));
 }
 
 void FormatExpression::eachChild(const std::function<void(const Expression&)>& fn) const {
     for (auto& section : sections) {
-        fn(*section.text);
+        fn(*section.content);
         if (section.fontScale) {
             fn(**section.fontScale);
         }
@@ -117,7 +120,7 @@ bool FormatExpression::operator==(const Expression& e) const {
         for (std::size_t i = 0; i < sections.size(); i++) {
             const auto& lhsSection = sections.at(i);
             const auto& rhsSection = rhs->sections.at(i);
-            if (*lhsSection.text != *rhsSection.text) {
+            if (*lhsSection.content != *rhsSection.content) {
                 return false;
             }
             if ((lhsSection.fontScale && (!rhsSection.fontScale || **lhsSection.fontScale != **rhsSection.fontScale)) ||
@@ -141,7 +144,7 @@ bool FormatExpression::operator==(const Expression& e) const {
 mbgl::Value FormatExpression::serialize() const {
     std::vector<mbgl::Value> serialized{{ getOperator() }};
     for (const auto& section : sections) {
-        serialized.push_back(section.text->serialize());
+        serialized.push_back(section.content->serialize());
         std::unordered_map<std::string, mbgl::Value> options;
         if (section.fontScale) {
             options.emplace(kFormattedSectionFontScale, (*section.fontScale)->serialize());
@@ -160,14 +163,25 @@ mbgl::Value FormatExpression::serialize() const {
 EvaluationResult FormatExpression::evaluate(const EvaluationContext& params) const {
     std::vector<FormattedSection> evaluatedSections;
     for (const auto& section : sections) {
-        auto textResult = section.text->evaluate(params);
-        if (!textResult) {
-            return textResult.error();
+        auto contentResult = section.content->evaluate(params);
+        if (!contentResult) {
+            return contentResult.error();
         }
-        
-        optional<std::string> evaluatedText = toString(*textResult);
-        if (!evaluatedText) {
-            return EvaluationError({ "Could not coerce format expression text input to string." });
+
+        optional<std::string> evaluatedText;
+        if (typeOf(*contentResult) == type::Image) {
+            const auto& image = contentResult->get<Image>();
+            // Omit sections with empty image ids.
+            if (!image.id().empty()) {
+                evaluatedSections.emplace_back(image);
+            }
+            // Continue evaluation of a next section, as the image section does not have section options.
+            continue;
+        } else {
+            evaluatedText = toString(*contentResult);
+            if (!evaluatedText) {
+                return EvaluationError({"Could not coerce format expression text input to string."});
+            }
         }
 
         optional<double> evaluatedFontScale;
diff --git a/src/mbgl/style/expression/formatted.cpp b/src/mbgl/style/expression/formatted.cpp
index 4591a50ed1..4069dd8a7c 100644
--- a/src/mbgl/style/expression/formatted.cpp
+++ b/src/mbgl/style/expression/formatted.cpp
@@ -35,6 +35,16 @@ std::string Formatted::toString() const {
     return result;
 }
 
+bool Formatted::empty() const {
+    if (sections.empty()) {
+        return true;
+    }
+
+    return !std::any_of(sections.begin(), sections.end(), [](const FormattedSection& section) {
+        return !section.text.empty() || (section.image && !section.image->empty());
+    });
+}
+
 mbgl::Value Formatted::toObject() const {
     mapbox::base::ValueObject result;
     mapbox::base::ValueArray sectionValues;
@@ -58,6 +68,7 @@ mbgl::Value Formatted::toObject() const {
         } else {
             serializedSection.emplace("textColor", NullValue());
         }
+        serializedSection.emplace("image", section.image ? section.image->toValue() : NullValue());
         sectionValues.emplace_back(serializedSection);
     }
     result.emplace("sections", std::move(sectionValues));
@@ -76,14 +87,37 @@ optional<Formatted> Converter<Formatted>::operator()(const Convertible& value, E
     if (isArray(value)) {
         std::vector<FormattedSection> sections;
         for (std::size_t i = 0; i < arrayLength(value); ++i) {
-            Convertible section = arrayMember(value, i);
+            const Convertible& section = arrayMember(value, i);
             std::size_t sectionLength = arrayLength(section);
             if (sectionLength < 1) {
-                error.message = "Section has to contain a text and optional parameters.";
+                error.message = "Section has to contain a text and optional parameters or an image.";
                 return nullopt;
             }
 
-            optional<std::string> sectionText = toString(arrayMember(section, 0));
+            const Convertible& firstElement = arrayMember(section, 0);
+            if (isArray(firstElement)) {
+                if (arrayLength(firstElement) < 2) {
+                    error.message = "Image section has to contain image name.";
+                    return nullopt;
+                }
+
+                optional<std::string> imageOp = toString(arrayMember(firstElement, 0));
+                if (!imageOp || *imageOp != "image") {
+                    error.message = "Serialized image section has to contain 'image' operator.";
+                    return nullopt;
+                }
+
+                optional<std::string> imageArg = toString(arrayMember(firstElement, 1));
+                if (!imageArg) {
+                    error.message = "Serialized image section agument has to be of a String type.";
+                    return nullopt;
+                }
+
+                sections.emplace_back(Image(*imageArg));
+                continue;
+            }
+
+            optional<std::string> sectionText = toString(firstElement);
             if (!sectionText) {
                 error.message = "Section has to contain a text.";
                 return nullopt;
@@ -93,7 +127,7 @@ optional<Formatted> Converter<Formatted>::operator()(const Convertible& value, E
             optional<FontStack> textFont;
             optional<Color> textColor;
             if (sectionLength > 1) {
-                Convertible sectionParams = arrayMember(section, 1);
+                const Convertible& sectionParams = arrayMember(section, 1);
                 if (!isObject(sectionParams)) {
                     error.message = "Parameters have to be enclosed in an object.";
                     return nullopt;
diff --git a/src/mbgl/style/expression/value.cpp b/src/mbgl/style/expression/value.cpp
index 7609230b52..aac1c61655 100644
--- a/src/mbgl/style/expression/value.cpp
+++ b/src/mbgl/style/expression/value.cpp
@@ -149,13 +149,18 @@ mbgl::Value ValueConverter<mbgl::Value>::fromExpressionValue(const Value& value)
             static std::string formatOperator("format");
             serialized.emplace_back(formatOperator);
             for (const auto& section : formatted.sections) {
+                if (section.image) {
+                    serialized.emplace_back(std::vector<mbgl::Value>{std::string("image"), section.image->id()});
+                    continue;
+                }
+
                 serialized.emplace_back(section.text);
                 std::unordered_map<std::string, mbgl::Value> options;
                 
                 if (section.fontScale) {
                     options.emplace("font-scale", *section.fontScale);
                 }
-                
+
                 if (section.fontStack) {
                     std::vector<mbgl::Value> fontStack;
                     for (const auto& font : *section.fontStack) {
diff --git a/test/style/style_layer.test.cpp b/test/style/style_layer.test.cpp
index 2d6ef5881a..1d60197c2d 100644
--- a/test/style/style_layer.test.cpp
+++ b/test/style/style_layer.test.cpp
@@ -343,7 +343,8 @@ void testHasOverrides(LayoutType& layout) {
     EXPECT_FALSE(MockOverrides::hasOverrides(layout.template get<TextField>()));
 
     // Expression, overridden text-color.
-    FormatExpressionSection section(literal(""), nullopt, nullopt, toColor(literal("red")));
+    FormatExpressionSection section(literal(""));
+    section.setTextSectionOptions(nullopt, nullopt, toColor(literal("red")));
     auto formatExprOverride = std::make_unique<FormatExpression>(std::vector<FormatExpressionSection>{section});
     PropertyExpression<Formatted> propExprOverride(std::move(formatExprOverride));
     layout.template get<TextField>() = PropertyValueType<Formatted>(std::move(propExprOverride));
@@ -351,7 +352,9 @@ void testHasOverrides(LayoutType& layout) {
 
     // Nested expressions, overridden text-color.
     auto formattedExpr1 = format("first paragraph");
-    std::vector<FormatExpressionSection> sections{ { literal("second paragraph"), nullopt, nullopt, toColor(literal("blue")) } };
+    FormatExpressionSection secondParagraph(literal("second paragraph"));
+    secondParagraph.setTextSectionOptions(nullopt, nullopt, toColor(literal("blue")));
+    std::vector<FormatExpressionSection> sections{{std::move(secondParagraph)}};
     auto formattedExpr2 = std::make_unique<FormatExpression>(std::move(sections));
     std::unordered_map<std::string, std::shared_ptr<Expression>> branches{ { "1st", std::move(formattedExpr1) },
                                                                            { "2nd", std::move(formattedExpr2) } };