From 7a010df9560e8807b9be938bb5ff7360e8e730a6 Mon Sep 17 00:00:00 2001
From: Asheem Mamoowala <asheem.mamoowala@mapbox.com>
Date: Thu, 20 Apr 2017 12:39:51 -0700
Subject: [core] Add ImageSource support to style parsers

---
 cmake/core-files.cmake                             |  5 ++
 include/mbgl/style/conversion/coordinate.hpp       | 37 ++++++++++
 include/mbgl/style/conversion/source.hpp           | 48 ++++++++++++
 include/mbgl/style/sources/image_source.hpp        | 41 +++++++++++
 include/mbgl/style/types.hpp                       |  3 +-
 platform/default/mbgl/storage/offline_download.cpp |  2 +
 src/mbgl/style/parser.cpp                          | 11 ++-
 src/mbgl/style/sources/image_source.cpp            | 85 ++++++++++++++++++++++
 src/mbgl/style/sources/image_source_impl.cpp       | 42 +++++++++++
 src/mbgl/style/sources/image_source_impl.hpp       | 31 ++++++++
 src/mbgl/style/types.cpp                           |  1 +
 .../style_parser/center-not-latlong.info.json      |  7 ++
 .../style_parser/center-not-latlong.style.json     |  4 +
 .../style_parser/image-coordinates.info.json       |  7 ++
 .../style_parser/image-coordinates.style.json      | 14 ++++
 test/fixtures/style_parser/image-url.info.json     |  7 ++
 test/fixtures/style_parser/image-url.style.json    |  9 +++
 test/style/source.test.cpp                         | 40 ++++++++++
 18 files changed, 389 insertions(+), 5 deletions(-)
 create mode 100644 include/mbgl/style/conversion/coordinate.hpp
 create mode 100644 include/mbgl/style/sources/image_source.hpp
 create mode 100644 src/mbgl/style/sources/image_source.cpp
 create mode 100644 src/mbgl/style/sources/image_source_impl.cpp
 create mode 100644 src/mbgl/style/sources/image_source_impl.hpp
 create mode 100644 test/fixtures/style_parser/center-not-latlong.info.json
 create mode 100644 test/fixtures/style_parser/center-not-latlong.style.json
 create mode 100644 test/fixtures/style_parser/image-coordinates.info.json
 create mode 100644 test/fixtures/style_parser/image-coordinates.style.json
 create mode 100644 test/fixtures/style_parser/image-url.info.json
 create mode 100644 test/fixtures/style_parser/image-url.style.json

diff --git a/cmake/core-files.cmake b/cmake/core-files.cmake
index e56f2fdc37..f314f9e0ce 100644
--- a/cmake/core-files.cmake
+++ b/cmake/core-files.cmake
@@ -355,6 +355,7 @@ set(MBGL_CORE_FILES
 
     # style/conversion
     include/mbgl/style/conversion/constant.hpp
+    include/mbgl/style/conversion/coordinate.hpp
     include/mbgl/style/conversion/data_driven_property_value.hpp
     include/mbgl/style/conversion/filter.hpp
     include/mbgl/style/conversion/function.hpp
@@ -437,11 +438,15 @@ set(MBGL_CORE_FILES
 
     # style/sources
     include/mbgl/style/sources/geojson_source.hpp
+    include/mbgl/style/sources/image_source.hpp
     include/mbgl/style/sources/raster_source.hpp
     include/mbgl/style/sources/vector_source.hpp
     src/mbgl/style/sources/geojson_source.cpp
     src/mbgl/style/sources/geojson_source_impl.cpp
     src/mbgl/style/sources/geojson_source_impl.hpp
+    src/mbgl/style/sources/image_source.cpp
+    src/mbgl/style/sources/image_source_impl.cpp
+    src/mbgl/style/sources/image_source_impl.hpp
     src/mbgl/style/sources/raster_source.cpp
     src/mbgl/style/sources/raster_source_impl.cpp
     src/mbgl/style/sources/raster_source_impl.hpp
diff --git a/include/mbgl/style/conversion/coordinate.hpp b/include/mbgl/style/conversion/coordinate.hpp
new file mode 100644
index 0000000000..0adbb9a7ee
--- /dev/null
+++ b/include/mbgl/style/conversion/coordinate.hpp
@@ -0,0 +1,37 @@
+#pragma once
+
+#include <mbgl/style/conversion.hpp>
+#include <mbgl/util/geo.hpp>
+
+namespace mbgl {
+namespace style {
+namespace conversion {
+            
+template<>
+struct Converter<std::unique_ptr<LatLng>> {
+public:
+    template <class V>
+    optional<std::unique_ptr<LatLng>> operator() (const V& value, Error& error) const {
+        if (!isArray(value) || arrayLength(value) < 2 ) {
+            error = { "coordinate array must contain numeric longtitude and latitude values" };
+            return {};
+        }
+        //Style spec uses GeoJSON convention for specifying coordinates
+        optional<float> latitude = toNumber(arrayMember(value, 1));
+        optional<float> longitude = toNumber(arrayMember(value, 0));
+    
+        if (!latitude || !longitude) {
+            error = { "coordinate array must contain numeric longtitude and latitude values" };
+            return {};
+        }
+        if (*latitude < -90 || *latitude > 90 ){
+            error = { "coordinate latitude must be between -90 and 90" };
+            return {};
+        }
+        return { std::make_unique<LatLng>(*latitude, *longitude) };
+    }
+};
+
+} // namespace conversion
+} // namespace style
+} // namespace mbgl
diff --git a/include/mbgl/style/conversion/source.hpp b/include/mbgl/style/conversion/source.hpp
index dc7cdc0d42..e121e36955 100644
--- a/include/mbgl/style/conversion/source.hpp
+++ b/include/mbgl/style/conversion/source.hpp
@@ -1,6 +1,7 @@
 #pragma once
 
 #include <mbgl/style/conversion.hpp>
+#include <mbgl/style/conversion/coordinate.hpp>
 #include <mbgl/style/conversion/geojson.hpp>
 #include <mbgl/style/conversion/geojson_options.hpp>
 #include <mbgl/style/conversion/tileset.hpp>
@@ -8,6 +9,8 @@
 #include <mbgl/style/sources/geojson_source.hpp>
 #include <mbgl/style/sources/raster_source.hpp>
 #include <mbgl/style/sources/vector_source.hpp>
+#include <mbgl/style/sources/image_source.hpp>
+#include <mbgl/util/geo.hpp>
 
 namespace mbgl {
 namespace style {
@@ -16,6 +19,7 @@ namespace conversion {
 template <>
 struct Converter<std::unique_ptr<Source>> {
 public:
+    
     template <class V>
     optional<std::unique_ptr<Source>> operator()(const V& value, Error& error, const std::string& id) const {
         if (!isObject(value)) {
@@ -41,6 +45,8 @@ public:
             return convertVectorSource(id, value, error);
         } else if (*type == "geojson") {
             return convertGeoJSONSource(id, value, error);
+        } else if (*type == "image") {
+            return convertImageSource(id, value, error);
         } else {
             error = { "invalid source type" };
             return {};
@@ -136,6 +142,48 @@ private:
 
         return { std::move(result) };
     }
+    
+    template <class V>
+    optional<std::unique_ptr<Source>> convertImageSource(const std::string& id,
+                                                        const V& value,
+                                                        Error& error) const {
+        auto urlValue = objectMember(value, "url");
+        if (!urlValue) {
+            error = { "Image source must have a url value" };
+            return {};
+        }
+        
+        auto urlString = toString(*urlValue);
+        if (!urlString) {
+            error = { "Image url must be a URL string" };
+            return {};
+        }
+        
+        auto coordinatesValue = objectMember(value, "coordinates");
+        if (!coordinatesValue) {
+            error = { "Image source must have a coordinates values" };
+            return {};
+        }
+        
+        if (!isArray(*coordinatesValue) || arrayLength(*coordinatesValue) != 4) {
+            error = { "Image coordinates must be an array of four longitude latitude pairs" };
+            return {};
+        }
+        
+        std::vector<LatLng> coordinates;
+        coordinates.reserve(4);
+        for( std::size_t i=0; i < arrayLength(*coordinatesValue); i++) {
+            auto latLng = conversion::convert<LatLng>(arrayMember(*coordinatesValue,i), error);
+            if (!latLng) {
+                return {};
+            }
+            coordinates.push_back(*latLng);
+        }
+        auto result = std::make_unique<ImageSource>(id, coordinates);
+        result->setURL(*urlString);
+
+        return { std::move(result) };
+    }
 };
 
 } // namespace conversion
diff --git a/include/mbgl/style/sources/image_source.hpp b/include/mbgl/style/sources/image_source.hpp
new file mode 100644
index 0000000000..e7e598a972
--- /dev/null
+++ b/include/mbgl/style/sources/image_source.hpp
@@ -0,0 +1,41 @@
+#pragma once
+
+#include <mbgl/style/source.hpp>
+#include <mbgl/util/image.hpp>
+#include <mbgl/util/optional.hpp>
+
+namespace mbgl {
+class LatLng;
+class AsyncRequest;
+
+namespace style {
+
+class ImageSource : public Source {
+public:
+    ImageSource(std::string id, const std::vector<LatLng>);
+    ~ImageSource() override;
+
+    const std::string& getURL() const;
+    void setURL(const std::string& url) ;
+
+    void setImage(mbgl::UnassociatedImage);
+
+    void setCoordinates(const std::vector<LatLng>&);
+    std::vector<LatLng> getCoordinates() const;
+
+    class Impl;
+    const Impl& impl() const;
+
+    void loadDescription(FileSource&) final;
+private:
+    optional<std::string> url;
+    std::unique_ptr<AsyncRequest> req;
+};
+
+template <>
+inline bool Source::is<ImageSource>() const {
+    return getType() == SourceType::Image;
+}
+
+} // namespace style
+} // namespace mbgl
diff --git a/include/mbgl/style/types.hpp b/include/mbgl/style/types.hpp
index e0436efb67..44b16f16e7 100644
--- a/include/mbgl/style/types.hpp
+++ b/include/mbgl/style/types.hpp
@@ -10,7 +10,8 @@ enum class SourceType : uint8_t {
     Raster,
     GeoJSON,
     Video,
-    Annotations
+    Annotations,
+    Image
 };
 
 namespace style {
diff --git a/platform/default/mbgl/storage/offline_download.cpp b/platform/default/mbgl/storage/offline_download.cpp
index 235baac12c..99b4447711 100644
--- a/platform/default/mbgl/storage/offline_download.cpp
+++ b/platform/default/mbgl/storage/offline_download.cpp
@@ -120,6 +120,7 @@ OfflineRegionStatus OfflineDownload::getStatus() const {
 
         case SourceType::Video:
         case SourceType::Annotations:
+        case SourceType::Image:
             break;
         }
     }
@@ -197,6 +198,7 @@ void OfflineDownload::activateDownload() {
 
             case SourceType::Video:
             case SourceType::Annotations:
+            case SourceType::Image:
                 break;
             }
         }
diff --git a/src/mbgl/style/parser.cpp b/src/mbgl/style/parser.cpp
index ea96bda502..725feb3fc2 100644
--- a/src/mbgl/style/parser.cpp
+++ b/src/mbgl/style/parser.cpp
@@ -2,6 +2,7 @@
 #include <mbgl/style/layer_impl.hpp>
 #include <mbgl/style/rapidjson_conversion.hpp>
 #include <mbgl/style/conversion.hpp>
+#include <mbgl/style/conversion/coordinate.hpp>
 #include <mbgl/style/conversion/source.hpp>
 #include <mbgl/style/conversion/layer.hpp>
 #include <mbgl/style/conversion/light.hpp>
@@ -55,10 +56,12 @@ StyleParseResult Parser::parse(const std::string& json) {
 
     if (document.HasMember("center")) {
         const JSValue& value = document["center"];
-        if (value.IsArray() && value.Size() >= 2) {
-            // Style spec uses lon/lat order
-            latLng = LatLng(value[1].IsNumber() ? value[1].GetDouble() : 0,
-                            value[0].IsNumber() ? value[0].GetDouble() : 0);
+        conversion::Error error;
+        auto convertedLatLng = conversion::convert<LatLng>(value, error);
+        if (convertedLatLng) {
+            latLng = *convertedLatLng;
+        } else {
+            Log::Warning(Event::ParseStyle, "center coordinate must be a longitude, latitude pair");
         }
     }
 
diff --git a/src/mbgl/style/sources/image_source.cpp b/src/mbgl/style/sources/image_source.cpp
new file mode 100644
index 0000000000..4f86befd0b
--- /dev/null
+++ b/src/mbgl/style/sources/image_source.cpp
@@ -0,0 +1,85 @@
+#include <mbgl/style/sources/image_source.hpp>
+#include <mbgl/style/sources/image_source_impl.hpp>
+#include <mbgl/util/geo.hpp>
+#include <mbgl/style/source_observer.hpp>
+#include <mbgl/util/premultiply.hpp>
+#include <mbgl/storage/file_source.hpp>
+
+namespace mbgl {
+namespace style {
+
+ImageSource::ImageSource(std::string id, const std::vector<LatLng> coords_)
+    : Source(makeMutable<Impl>(std::move(id), coords_)) {
+}
+
+ImageSource::~ImageSource() = default;
+
+const ImageSource::Impl& ImageSource::impl() const {
+    return static_cast<const Impl&>(*baseImpl);
+}
+
+void ImageSource::setCoordinates(const std::vector<LatLng>& coords_) {
+    baseImpl = makeMutable<Impl>(impl(), coords_);
+    observer->onSourceChanged(*this);
+}
+
+std::vector<LatLng> ImageSource::getCoordinates() const {
+    return impl().getCoordinates();
+}
+
+void ImageSource::setURL(const std::string& url_) {
+    url = std::move(url_);
+    // Signal that the source description needs a reload
+    if (loaded || req) {
+        loaded = false;
+        req.reset();
+        observer->onSourceDescriptionChanged(*this);
+    }
+}
+
+void ImageSource::setImage(mbgl::UnassociatedImage image_) {
+    url = {};
+    if (req) {
+        req.reset();
+    }
+    loaded = true;
+    baseImpl = makeMutable<Impl>(impl(), std::move(image_));
+    observer->onSourceChanged(*this);
+}
+
+const std::string& ImageSource::getURL() const {
+    return *url;
+}
+
+void ImageSource::loadDescription(FileSource& fileSource) {
+    if (!url) {
+        loaded = true;
+    }
+
+    if (req ||  loaded) {
+        return;
+    }
+    const Resource imageResource { Resource::Unknown, *url, {}, Resource::Necessity::Required };
+
+    req = fileSource.request(imageResource, [this](Response res) {
+        if (res.error) {
+            observer->onSourceError(*this, std::make_exception_ptr(std::runtime_error(res.error->message)));
+        } else if (res.notModified) {
+            return;
+        } else if (res.noContent) {
+            observer->onSourceError(*this, std::make_exception_ptr(std::runtime_error("unexpectedly empty image url")));
+        } else {
+            try {
+                UnassociatedImage image = util::unpremultiply(decodeImage(*res.data));
+                baseImpl = makeMutable<Impl>(impl(), std::move(image));
+            } catch (...) {
+                observer->onSourceError(*this, std::current_exception());
+            }
+            loaded = true;
+            observer->onSourceLoaded(*this);
+        }
+    });
+}
+
+} // namespace style
+} // namespace mbgl
diff --git a/src/mbgl/style/sources/image_source_impl.cpp b/src/mbgl/style/sources/image_source_impl.cpp
new file mode 100644
index 0000000000..e16b33b4ca
--- /dev/null
+++ b/src/mbgl/style/sources/image_source_impl.cpp
@@ -0,0 +1,42 @@
+#include <mbgl/style/sources/image_source_impl.hpp>
+#include <mbgl/util/geo.hpp>
+
+namespace mbgl {
+namespace style {
+
+ImageSource::Impl::Impl(std::string id_, std::vector<LatLng> coords_)
+    : Source::Impl(SourceType::Image, std::move(id_)),
+    coords(std::move(coords_)) {
+}
+
+ImageSource::Impl::Impl(const Impl& other, std::vector<LatLng> coords_)
+    : Source::Impl(other),
+    coords(std::move(coords_)),
+    image(other.image.clone()) {
+}
+
+ImageSource::Impl::Impl(const Impl& rhs, UnassociatedImage image_)
+    : Source::Impl(rhs),
+    coords(rhs.coords),
+    image(std::move(image_)) {
+}
+ImageSource::Impl::~Impl() = default;
+
+void ImageSource::Impl::setImage(UnassociatedImage image_) {
+    image = std::move(image_);
+}
+
+const UnassociatedImage& ImageSource::Impl::getImage() const {
+    return image;
+}
+
+std::vector<LatLng> ImageSource::Impl::getCoordinates() const {
+    return coords;
+}
+
+optional<std::string> ImageSource::Impl::getAttribution() const {
+    return {};
+}
+
+} // namespace style
+} // namespace mbgl
diff --git a/src/mbgl/style/sources/image_source_impl.hpp b/src/mbgl/style/sources/image_source_impl.hpp
new file mode 100644
index 0000000000..439db4fa64
--- /dev/null
+++ b/src/mbgl/style/sources/image_source_impl.hpp
@@ -0,0 +1,31 @@
+#pragma once
+
+#include <mbgl/style/source_impl.hpp>
+#include <mbgl/style/sources/image_source.hpp>
+#include <mbgl/util/image.hpp>
+
+namespace mbgl {
+class LatLng;
+
+namespace style {
+
+class ImageSource::Impl : public Source::Impl {
+public:
+    Impl(std::string id, std::vector<LatLng> coords);
+    Impl(const Impl& rhs, std::vector<LatLng> coords);
+    Impl(const Impl& rhs, UnassociatedImage image);
+
+    ~Impl() final;
+
+    void setImage(UnassociatedImage );
+    const mbgl::UnassociatedImage& getImage() const;
+    std::vector<LatLng> getCoordinates() const;
+
+    optional<std::string> getAttribution() const final;
+private:
+    std::vector<LatLng> coords;
+    mbgl::UnassociatedImage image;
+};
+
+} // namespace style
+} // namespace mbgl
diff --git a/src/mbgl/style/types.cpp b/src/mbgl/style/types.cpp
index b37e73ffb1..4fbf767e11 100644
--- a/src/mbgl/style/types.cpp
+++ b/src/mbgl/style/types.cpp
@@ -11,6 +11,7 @@ MBGL_DEFINE_ENUM(SourceType, {
     { SourceType::GeoJSON, "geojson" },
     { SourceType::Video, "video" },
     { SourceType::Annotations, "annotations" },
+    { SourceType::Image, "image" },
 });
 
 MBGL_DEFINE_ENUM(VisibilityType, {
diff --git a/test/fixtures/style_parser/center-not-latlong.info.json b/test/fixtures/style_parser/center-not-latlong.info.json
new file mode 100644
index 0000000000..c79de0a525
--- /dev/null
+++ b/test/fixtures/style_parser/center-not-latlong.info.json
@@ -0,0 +1,7 @@
+{
+    "default": {
+        "log": [
+	    [1, "WARNING", "ParseStyle", "center coordinate must be a longitude, latitude pair"]
+        ]
+    }
+}
\ No newline at end of file
diff --git a/test/fixtures/style_parser/center-not-latlong.style.json b/test/fixtures/style_parser/center-not-latlong.style.json
new file mode 100644
index 0000000000..eb847868f5
--- /dev/null
+++ b/test/fixtures/style_parser/center-not-latlong.style.json
@@ -0,0 +1,4 @@
+{
+  "version": 8,
+  "center" : [ 75.123, 181.123]
+}
\ No newline at end of file
diff --git a/test/fixtures/style_parser/image-coordinates.info.json b/test/fixtures/style_parser/image-coordinates.info.json
new file mode 100644
index 0000000000..5ef44badba
--- /dev/null
+++ b/test/fixtures/style_parser/image-coordinates.info.json
@@ -0,0 +1,7 @@
+{
+  "default": {
+    "log": [
+      [1, "WARNING", "ParseStyle", "Image coordinates must be an array of four longitude latitude pairs"]
+    ]
+  }
+}
\ No newline at end of file
diff --git a/test/fixtures/style_parser/image-coordinates.style.json b/test/fixtures/style_parser/image-coordinates.style.json
new file mode 100644
index 0000000000..51b0e93ee7
--- /dev/null
+++ b/test/fixtures/style_parser/image-coordinates.style.json
@@ -0,0 +1,14 @@
+{
+  "version": 8,
+  "center" : [ 75.123, 81.123],
+  "sources": {
+    "image": {
+      "type": "image",
+      "url": "local://0.png",
+      "coordinates": [
+        [ 12.2344, 87.23434],
+	[-12.234, 12.41242]
+      ]
+    }
+  }
+}
\ No newline at end of file
diff --git a/test/fixtures/style_parser/image-url.info.json b/test/fixtures/style_parser/image-url.info.json
new file mode 100644
index 0000000000..988e89c011
--- /dev/null
+++ b/test/fixtures/style_parser/image-url.info.json
@@ -0,0 +1,7 @@
+{
+  "default": {
+    "log": [
+      [1, "WARNING", "ParseStyle", "Image source must have a url value"]
+    ]
+  }
+}
\ No newline at end of file
diff --git a/test/fixtures/style_parser/image-url.style.json b/test/fixtures/style_parser/image-url.style.json
new file mode 100644
index 0000000000..94614cb7ab
--- /dev/null
+++ b/test/fixtures/style_parser/image-url.style.json
@@ -0,0 +1,9 @@
+{
+  "version": 8,
+  "center" : [ 75.123, 81.123],
+  "sources": {
+    "image": {
+      "type": "image"
+    }
+  }
+}
\ No newline at end of file
diff --git a/test/style/source.test.cpp b/test/style/source.test.cpp
index 880bcd986c..e674d69ed0 100644
--- a/test/style/source.test.cpp
+++ b/test/style/source.test.cpp
@@ -7,6 +7,7 @@
 #include <mbgl/style/sources/raster_source.hpp>
 #include <mbgl/style/sources/vector_source.hpp>
 #include <mbgl/style/sources/geojson_source.hpp>
+#include <mbgl/style/sources/image_source.hpp>
 
 #include <mbgl/renderer/sources/render_raster_source.hpp>
 #include <mbgl/renderer/sources/render_vector_source.hpp>
@@ -16,6 +17,9 @@
 #include <mbgl/util/run_loop.hpp>
 #include <mbgl/util/string.hpp>
 #include <mbgl/util/io.hpp>
+#include <mbgl/util/premultiply.hpp>
+#include <mbgl/util/image.hpp>
+
 #include <mbgl/util/tileset.hpp>
 #include <mbgl/util/default_thread_pool.hpp>
 #include <mbgl/util/logging.hpp>
@@ -436,3 +440,39 @@ TEST(Source, GeoJSonSourceUrlUpdate) {
 
     test.run();
 }
+
+TEST(Source, ImageSourceImageUpdate) {
+    SourceTest test;
+
+    test.fileSource.response = [&] (const Resource& resource) {
+        EXPECT_EQ("http://url", resource.url);
+        Response response;
+        response.data = std::make_unique<std::string>(util::read_file("test/fixtures/image/no_profile.png"));
+        return response;
+    };
+    test.styleObserver.sourceChanged = [&] (Source&) {
+        // Should be called (test will hang if it doesn't)
+        test.end();
+    };
+    std::vector<LatLng> coords;
+
+    ImageSource source("source", coords);
+    source.setURL("http://url");
+    source.setObserver(&test.styleObserver);
+
+    // Load initial, so the source state will be loaded=true
+    source.loadDescription(test.fileSource);
+    UnassociatedImage rgba({ 1, 1 });
+    rgba.data[0] = 255;
+    rgba.data[1] = 254;
+    rgba.data[2] = 253;
+    rgba.data[3] = 128;
+
+    // Schedule an update
+    test.loop.invoke([&] () {
+        // Update the url
+        source.setImage(std::move(rgba));
+    });
+
+    test.run();
+}
-- 
cgit v1.2.1