Fix render transforms for all zoom levels

12 files changed, 183 insertions, 41 deletions

diff --git a/include/mbgl/style/conversion.hpp b/include/mbgl/style/conversion.hpp
index d6fb3a6dd0..27504a89b1 100644
--- a/include/mbgl/style/conversion.hpp
+++ b/include/mbgl/style/conversion.hpp
@@ -46,6 +46,7 @@ namespace conversion {
 
       * `toBool(v)` -- returns `optional<bool>`, absence indicating `v` is not a JSON boolean
       * `toNumber(v)` -- returns `optional<float>`, absence indicating `v` is not a JSON number
+      * `toDouble(v)` -- returns `optional<double>`, absence indicating `v` is not a JSON number
       * `toString(v)` -- returns `optional<std::string>`, absence indicating `v` is not a JSON string
       * `toValue(v)` -- returns `optional<mbgl::Value>`, a variant type, for generic conversion,
         absence indicating `v` is not a boolean, number, or string. Numbers should be converted to
diff --git a/include/mbgl/style/conversion/coordinate.hpp b/include/mbgl/style/conversion/coordinate.hpp
index 0adbb9a7ee..988e2b02f6 100644
--- a/include/mbgl/style/conversion/coordinate.hpp
+++ b/include/mbgl/style/conversion/coordinate.hpp
@@ -17,8 +17,8 @@ public:
             return {};
         }
         //Style spec uses GeoJSON convention for specifying coordinates
-        optional<float> latitude = toNumber(arrayMember(value, 1));
-        optional<float> longitude = toNumber(arrayMember(value, 0));
+        optional<double> latitude = toDouble(arrayMember(value, 1));
+        optional<double> longitude = toDouble(arrayMember(value, 0));
     
         if (!latitude || !longitude) {
             error = { "coordinate array must contain numeric longtitude and latitude values" };
diff --git a/platform/node/src/node_conversion.hpp b/platform/node/src/node_conversion.hpp
index 22daedef6a..d266745548 100644
--- a/platform/node/src/node_conversion.hpp
+++ b/platform/node/src/node_conversion.hpp
@@ -82,6 +82,14 @@ inline optional<float> toNumber(v8::Local<v8::Value> value) {
     return value->NumberValue();
 }
 
+inline optional<double> toDouble(v8::Local<v8::Value> value) {
+    Nan::HandleScope scope;
+    if (!value->IsNumber()) {
+        return {};
+    }
+    return value->NumberValue();
+}
+
 inline optional<std::string> toString(v8::Local<v8::Value> value) {
     Nan::HandleScope scope;
     if (!value->IsString()) {
diff --git a/src/mbgl/renderer/buckets/raster_bucket.cpp b/src/mbgl/renderer/buckets/raster_bucket.cpp
index 6c5c47aa72..6f1940fd36 100644
--- a/src/mbgl/renderer/buckets/raster_bucket.cpp
+++ b/src/mbgl/renderer/buckets/raster_bucket.cpp
@@ -11,8 +11,9 @@ using namespace style;
 RasterBucket::RasterBucket(UnassociatedImage&& image_) : image(std::move(image_)) {
 }
 
+
 void RasterBucket::upload(gl::Context& context) {
-    texture = context.createTexture(std::move(image));
+    texture = context.createTexture(image);
     if (vertices.vertexSize() > 0) {
         vertexBuffer = context.createVertexBuffer(std::move(vertices));
         indexBuffer = context.createIndexBuffer(std::move(indices));
diff --git a/src/mbgl/renderer/painter.hpp b/src/mbgl/renderer/painter.hpp
index 2b234aff76..d42160c0d7 100644
--- a/src/mbgl/renderer/painter.hpp
+++ b/src/mbgl/renderer/painter.hpp
@@ -86,6 +86,7 @@ public:
 
     void renderClippingMask(const UnwrappedTileID&, const ClipID&);
     void renderTileDebug(const RenderTile&);
+    void renderTileDebug(mat4& matrix);
     void renderFill(PaintParameters&, FillBucket&, const RenderFillLayer&, const RenderTile&);
     void renderFillExtrusion(PaintParameters&, FillExtrusionBucket&, const RenderFillExtrusionLayer&, const RenderTile&);
     void renderLine(PaintParameters&, LineBucket&, const RenderLineLayer&, const RenderTile&);
diff --git a/src/mbgl/renderer/painters/painter_debug.cpp b/src/mbgl/renderer/painters/painter_debug.cpp
index 4794d79d45..c7177f66d8 100644
--- a/src/mbgl/renderer/painters/painter_debug.cpp
+++ b/src/mbgl/renderer/painters/painter_debug.cpp
@@ -77,6 +77,34 @@ void Painter::renderTileDebug(const RenderTile& renderTile) {
     }
 }
 
+void Painter::renderTileDebug(mat4& matrix) {
+    if (frame.debugOptions == MapDebugOptions::NoDebug)
+        return;
+
+    static const style::PaintProperties<>::Evaluated properties {};
+    static const DebugProgram::PaintPropertyBinders paintAttibuteData(properties, 0);
+
+    if (frame.debugOptions & MapDebugOptions::TileBorders) {
+        programs->debug.draw(
+            context,
+            gl::LineStrip { 4.0f * frame.pixelRatio },
+            gl::DepthMode::disabled(),
+            gl::StencilMode::disabled(),
+            gl::ColorMode::unblended(),
+            DebugProgram::UniformValues {
+             uniforms::u_matrix::Value{ matrix },
+             uniforms::u_color::Value{ Color::red() }
+            },
+            tileVertexBuffer,
+            tileBorderIndexBuffer,
+            tileBorderSegments,
+            paintAttibuteData,
+            properties,
+            state.getZoom()
+        );
+    }
+}
+
 #ifndef NDEBUG
 void Painter::renderClipMasks(PaintParameters&) {
     context.setStencilMode(gl::StencilMode::disabled());
diff --git a/src/mbgl/renderer/sources/render_image_source.cpp b/src/mbgl/renderer/sources/render_image_source.cpp
index b66db1e124..21232ac22b 100644
--- a/src/mbgl/renderer/sources/render_image_source.cpp
+++ b/src/mbgl/renderer/sources/render_image_source.cpp
@@ -3,7 +3,10 @@
 #include <mbgl/renderer/buckets/raster_bucket.hpp>
 #include <mbgl/map/transform_state.hpp>
 #include <mbgl/util/tile_coordinate.hpp>
-#include <mbgl/gl/context.hpp>
+#include <mbgl/renderer/tile_parameters.hpp>
+#include <mbgl/util/tile_cover.hpp>
+#include <mbgl/math/log2.hpp>
+#include <mbgl/renderer/painter.hpp>
 
 namespace mbgl {
 
@@ -22,13 +25,21 @@ bool RenderImageSource::isLoaded() const {
 void RenderImageSource::startRender(algorithm::ClipIDGenerator& ,
                                     const mat4& projMatrix,
                                     const mat4& ,
-                                    const TransformState& transform) {
+                                    const TransformState& transformState) {
+
+    if (!loaded) {
+        return;
+    }
     matrix::identity(matrix);
-    transform.matrixFor(matrix, {0,0,0});
+    transformState.matrixFor(matrix, *tileId);
     matrix::multiply(matrix, projMatrix, matrix);
 }
 
-void RenderImageSource::finishRender(Painter& ) {
+void RenderImageSource::finishRender(Painter& painter) {
+    if (!loaded) {
+        return;
+    }
+    painter.renderTileDebug(matrix);
 }
 
 std::unordered_map<std::string, std::vector<Feature>>
@@ -48,28 +59,72 @@ void RenderImageSource::upload(gl::Context& context) {
     }
 }
 
-void RenderImageSource::updateTiles(const TileParameters& ) {
-    if(impl.loaded && !isLoaded()) {
-        //TODO: AHM: Is it possible to do this without making a clone ?
-        UnassociatedImage img = impl.getData().clone();
-        bucket = std::make_unique<RasterBucket>(std::move(img));
-        loaded = true;
-        auto coords = impl.getCoordinates();
-        GeometryCoordinates geomCoords;
-        for ( auto latLng : coords) {
-            geomCoords.push_back(TileCoordinate::toGeometryCoordinate(latLng));
-        }
-        assert(geomCoords.size() == 4);
-        bucket->vertices.emplace_back(RasterProgram::layoutVertex({ geomCoords[0].x, geomCoords[0].y }, { 0, 0 }));
-        bucket->vertices.emplace_back(RasterProgram::layoutVertex({ geomCoords[1].x, geomCoords[1].y }, { 32767, 0 }));
-        bucket->vertices.emplace_back(RasterProgram::layoutVertex({ geomCoords[3].x, geomCoords[3].y }, { 0, 32767 }));
-        bucket->vertices.emplace_back(RasterProgram::layoutVertex({ geomCoords[2].x, geomCoords[2].y }, { 32767, 32767 }));
-
-        bucket->indices.emplace_back(0, 1, 2);
-        bucket->indices.emplace_back(1, 2, 3);
-
-        bucket->segments.emplace_back(0, 0, 4, 6);
+void RenderImageSource::updateTiles(const TileParameters& parameters) {
+    if(!impl.loaded || isLoaded()) {
+        return;
+    }
+    auto transformState = parameters.transformState;
+    auto size = transformState.getSize();
+    double viewportHeight = size.height;
+
+    auto coords = impl.getCoordinates();
+
+    ScreenCoordinate nePixel = {-INFINITY, -INFINITY};
+    ScreenCoordinate swPixel = {INFINITY, INFINITY};
+
+    for (LatLng latLng : coords) {
+        ScreenCoordinate pixel = transformState.latLngToScreenCoordinate(latLng);
+        swPixel.x = std::min(swPixel.x, pixel.x);
+        nePixel.x = std::max(nePixel.x, pixel.x);
+        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;
+
+    // Calculate the zoom level.
+    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 zoom = transformState.getZoom() + util::log2(minScale);
+    zoom = util::clamp(zoom, transformState.getMinZoom(), transformState.getMaxZoom());
+
+    // Calculate Geometry Coordinates based on ideal Tile for these LatLng
+    auto imageBounds = LatLngBounds::hull(coords[0], coords[1]);
+    imageBounds.extend(coords[2]);
+    imageBounds.extend(coords[3]);
+    auto tileCover = util::tileCover(imageBounds, ::round(zoom));
+    tileId = std::make_unique<UnwrappedTileID>(tileCover[0].wrap, tileCover[0].canonical);
+    GeometryCoordinates geomCoords;
+    for ( auto latLng : coords) {
+        auto tc = TileCoordinate::fromLatLng(0, latLng);
+        auto gc = TileCoordinate::toGeometryCoordinate(tileCover[0], tc.p);
+        geomCoords.push_back(gc);
+    }
+
+    setupBucket(geomCoords);
+
+    loaded = true;
+}
+
+void RenderImageSource::setupBucket(GeometryCoordinates& geomCoords) {
+    UnassociatedImage img = impl.getData().clone();
+    bucket = std::make_unique<RasterBucket>(std::move(img));
+
+    //Set Bucket Vertices, Indices, and segments
+    bucket->vertices.emplace_back(RasterProgram::layoutVertex({ geomCoords[0].x, geomCoords[0].y }, { 0, 0 }));
+    bucket->vertices.emplace_back(RasterProgram::layoutVertex({ geomCoords[1].x, geomCoords[1].y }, { 32767, 0 }));
+    bucket->vertices.emplace_back(RasterProgram::layoutVertex({ geomCoords[3].x, geomCoords[3].y }, { 0, 32767 }));
+    bucket->vertices.emplace_back(RasterProgram::layoutVertex({ geomCoords[2].x, geomCoords[2].y }, { 32767, 32767 }));
+
+    bucket->indices.emplace_back(0, 1, 2);
+    bucket->indices.emplace_back(1, 2, 3);
+
+    bucket->segments.emplace_back(0, 0, 4, 6);
 }
 
 void RenderImageSource::render(Painter& painter, PaintParameters& parameters, const RenderLayer& layer)  {
diff --git a/src/mbgl/renderer/sources/render_image_source.hpp b/src/mbgl/renderer/sources/render_image_source.hpp
index f2485675db..ac69933c49 100644
--- a/src/mbgl/renderer/sources/render_image_source.hpp
+++ b/src/mbgl/renderer/sources/render_image_source.hpp
@@ -52,12 +52,15 @@ public:
     void dumpDebugLogs() const final;
 
 private:
+
+    void setupBucket(GeometryCoordinates& coordiantes);
     const style::ImageSource::Impl& impl;
     std::map<UnwrappedTileID, RenderTile> tiles;
     bool loaded;
-
+    std::unique_ptr<UnwrappedTileID> tileId;
     std::unique_ptr<RasterBucket> bucket;
     mat4 matrix;
+
 };
     
 } // namespace mbgl
diff --git a/src/mbgl/style/rapidjson_conversion.hpp b/src/mbgl/style/rapidjson_conversion.hpp
index 101fe67ec0..48a764ccb4 100644
--- a/src/mbgl/style/rapidjson_conversion.hpp
+++ b/src/mbgl/style/rapidjson_conversion.hpp
@@ -62,6 +62,13 @@ inline optional<float> toNumber(const JSValue& value) {
     return value.GetDouble();
 }
 
+inline optional<double> toDouble(const JSValue& value) {
+    if (!value.IsNumber()) {
+        return {};
+    }
+    return value.GetDouble();
+}
+
 inline optional<std::string> toString(const JSValue& value) {
     if (!value.IsString()) {
         return {};
diff --git a/src/mbgl/util/tile_coordinate.hpp b/src/mbgl/util/tile_coordinate.hpp
index e10899cc6d..bcd1c8444f 100644
--- a/src/mbgl/util/tile_coordinate.hpp
+++ b/src/mbgl/util/tile_coordinate.hpp
@@ -44,18 +44,6 @@ public:
                         std::numeric_limits<int16_t>::max()))
         };
     }
-
-    static GeometryCoordinate toGeometryCoordinate(const LatLng& latLng) {
-        auto projectedPoint = Projection::project(latLng, 1) / double (util::tileSize);
-        return {
-            int16_t(util::clamp<int64_t>(projectedPoint.x * util::EXTENT,
-                                         std::numeric_limits<int16_t>::min(),
-                                         std::numeric_limits<int16_t>::max())),
-            int16_t(util::clamp<int64_t>(projectedPoint.y * util::EXTENT,
-                                         std::numeric_limits<int16_t>::min(),
-                                         std::numeric_limits<int16_t>::max()))
-        };
-    }
 };
 
 } // namespace mbgl
diff --git a/test/src/mbgl/test/conversion_stubs.hpp b/test/src/mbgl/test/conversion_stubs.hpp
index e6581c5e53..d957cfb617 100644
--- a/test/src/mbgl/test/conversion_stubs.hpp
+++ b/test/src/mbgl/test/conversion_stubs.hpp
@@ -17,6 +17,7 @@ using ValueMap = std::unordered_map<std::string, Value>;
 using ValueVector = std::vector<Value>;
 class Value : public mbgl::variant<std::string,
                                    float,
+                                   double,
                                    bool,
                                    mapbox::util::recursive_wrapper<ValueMap>,
                                    mapbox::util::recursive_wrapper<ValueVector>> {
@@ -90,6 +91,16 @@ inline optional<float> toNumber(const Value& value) {
     return {};
 }
 
+
+inline optional<double> toDouble(const Value& value) {
+    if (value.is<double>()) {
+        return value.get<double>();
+    } else {
+        return {};
+    }
+    return {};
+}
+
 inline optional<std::string> toString(const Value& value) {
     if (value.is<std::string>()) {
         return value.get<std::string>();
diff --git a/test/style/source.test.cpp b/test/style/source.test.cpp
index 0473286c8b..be7ab0e890 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,38 @@ 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", "http://url", coords);
+    source.baseImpl->setObserver(&test.styleObserver);
+
+    // Load initial, so the source state will be loaded=true
+    source.baseImpl->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();
+}