[core] remove bilinear, nearest scaling, fix #3164

8 files changed, 41 insertions, 254 deletions

diff --git a/package.json b/package.json
index 273e1a4dd0..0e1ca758b6 100644
--- a/package.json
+++ b/package.json
@@ -26,7 +26,7 @@
   ],
   "devDependencies": {
     "aws-sdk": "^2.2.21",
-    "mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#c401ec0acb064f819cddd182c63ee9e019bfb18f",
+    "mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#c8982f16ef5d39164b2e4f33bc3f993f9ac17502",
     "node-gyp": "^3.2.1",
     "request": "^2.67.0",
     "tape": "^4.2.2"
diff --git a/src/mbgl/sprite/sprite_atlas.cpp b/src/mbgl/sprite/sprite_atlas.cpp
index a20f6f0fe3..497721a61c 100644
--- a/src/mbgl/sprite/sprite_atlas.cpp
+++ b/src/mbgl/sprite/sprite_atlas.cpp
@@ -7,7 +7,6 @@
 #include <mbgl/util/math.hpp>
 #include <mbgl/util/std.hpp>
 #include <mbgl/util/constants.hpp>
-#include <mbgl/util/scaling.hpp>
 #include <mbgl/util/thread_context.hpp>
 
 #include <cassert>
@@ -45,9 +44,6 @@ Rect<SpriteAtlas::dimension> SpriteAtlas::allocateImage(float src_width, float s
         return rect;
     }
 
-    rect.originalW = pixel_width;
-    rect.originalH = pixel_height;
-
     return rect;
 }
 
@@ -101,6 +97,34 @@ mapbox::util::optional<SpriteAtlasPosition> SpriteAtlas::getPosition(const std::
     };
 }
 
+void copyBitmap(const uint32_t *src, const uint32_t srcStride, const uint32_t srcX, const uint32_t srcY,
+        uint32_t *const dst, const uint32_t dstStride, const uint32_t dstX, const uint32_t dstY, int dstSize,
+        const int width, const int height, const bool wrap) {
+
+    int srcI = srcY * srcStride + srcX;
+    int dstI = dstY * dstStride + dstX;
+    int x, y;
+
+    if (wrap) {
+        // add 1 pixel wrapped padding on each side of the image
+        dstI -= dstStride;
+        for (y = -1; y <= height; y++, srcI = ((y + height) % height + srcY) * srcStride + srcX, dstI += dstStride) {
+            for (x = -1; x <= width; x++) {
+                const int dstIndex = (dstI + x + dstSize) % dstSize;
+                dst[dstIndex] = src[srcI + ((x + width) % width)];
+            }
+        }
+
+    } else {
+        for (y = 0; y < height; y++, srcI += srcStride, dstI += dstStride) {
+            for (x = 0; x < width; x++) {
+                const int dstIndex = (dstI + x + dstSize) % dstSize;
+                dst[dstIndex] = src[srcI + x];
+            }
+        }
+    }
+}
+
 void SpriteAtlas::copy(const Holder& holder, const bool wrap) {
     if (!data) {
         data = std::make_unique<uint32_t[]>(pixelWidth * pixelHeight);
@@ -109,51 +133,13 @@ void SpriteAtlas::copy(const Holder& holder, const bool wrap) {
 
     const uint32_t *srcData = reinterpret_cast<const uint32_t *>(holder.texture->data.data());
     if (!srcData) return;
-    const vec2<uint32_t> srcSize { holder.texture->pixelWidth, holder.texture->pixelHeight };
-    const Rect<uint32_t> srcPos { 0, 0, srcSize.x, srcSize.y };
-    const auto& dst = holder.pos;
-
-    const int offset = 1;
-
     uint32_t *const dstData = data.get();
-    const vec2<uint32_t> dstSize{ pixelWidth, pixelHeight };
-    const Rect<uint32_t> dstPos{ static_cast<uint32_t>((offset + dst.x) * pixelRatio),
-                                 static_cast<uint32_t>((offset + dst.y) * pixelRatio),
-                                 static_cast<uint32_t>(dst.originalW * pixelRatio),
-                                 static_cast<uint32_t>(dst.originalH * pixelRatio) };
 
-    util::bilinearScale(srcData, srcSize, srcPos, dstData, dstSize, dstPos, wrap);
+    const int padding = 1;
 
-    // Add borders around the copied image if required.
-    if (wrap) {
-        // We're copying from the same image so we don't have to scale again.
-        const uint32_t border = 1;
-        const uint32_t borderX = dstPos.x != 0 ? border : 0;
-        const uint32_t borderY = dstPos.y != 0 ? border : 0;
-
-        // Left border
-        util::nearestNeighborScale(
-            dstData, dstSize, { dstPos.x + dstPos.w - borderX, dstPos.y, borderX, dstPos.h },
-            dstData, dstSize, { dstPos.x - borderX, dstPos.y, borderX, dstPos.h });
-
-        // Right border
-        util::nearestNeighborScale(dstData, dstSize, { dstPos.x, dstPos.y, border, dstPos.h },
-                                   dstData, dstSize,
-                                   { dstPos.x + dstPos.w, dstPos.y, border, dstPos.h });
-
-        // Top border
-        util::nearestNeighborScale(
-            dstData, dstSize, { dstPos.x - borderX, dstPos.y + dstPos.h - borderY,
-                                dstPos.w + border + borderX, borderY },
-            dstData, dstSize,
-            { dstPos.x - borderX, dstPos.y - borderY, dstPos.w + 2 * borderX, borderY });
-
-        // Bottom border
-        util::nearestNeighborScale(
-            dstData, dstSize, { dstPos.x - borderX, dstPos.y, dstPos.w + 2 * borderX, border },
-            dstData, dstSize,
-            { dstPos.x - borderX, dstPos.y + dstPos.h, dstPos.w + border + borderX, border });
-    }
+    copyBitmap(srcData, holder.texture->pixelWidth, 0, 0,
+            dstData, pixelWidth, (holder.pos.x + padding) * pixelRatio, (holder.pos.y + padding) * pixelRatio, pixelWidth * pixelHeight,
+            holder.texture->pixelWidth, holder.texture->pixelHeight, wrap);
 
     dirty = true;
 }
diff --git a/src/mbgl/util/rect.hpp b/src/mbgl/util/rect.hpp
index 7bb3214b9b..1dc8cb8af9 100644
--- a/src/mbgl/util/rect.hpp
+++ b/src/mbgl/util/rect.hpp
@@ -9,7 +9,6 @@ struct Rect {
     inline Rect(T x_, T y_, T w_, T h_) : x(x_), y(y_), w(w_), h(h_) {}
     T x = 0, y = 0;
     T w = 0, h = 0;
-    T originalW = 0, originalH = 0;
 
     template <typename Number>
     inline Rect operator *(Number value) const {
diff --git a/src/mbgl/util/scaling.cpp b/src/mbgl/util/scaling.cpp
deleted file mode 100644
index 9b32650511..0000000000
--- a/src/mbgl/util/scaling.cpp
+++ /dev/null
@@ -1,112 +0,0 @@
-#include "scaling.hpp"
-
-namespace {
-
-using namespace mbgl;
-
-inline uint8_t bilinearInterpolate(uint8_t tl, uint8_t tr, uint8_t bl, uint8_t br, double dx, double dy) {
-    const double t = dx * (tr - tl) + tl;
-    const double b = dx * (br - bl) + bl;
-    return t + dy * (b - t);
-}
-
-template <size_t i>
-inline const uint8_t& b(const uint32_t& w) {
-    return reinterpret_cast<const uint8_t*>(&w)[i];
-}
-
-template <size_t i>
-inline uint8_t& b(uint32_t& w) {
-    return reinterpret_cast<uint8_t*>(&w)[i];
-}
-
-vec2<double> getFactor(const Rect<uint32_t>& srcPos, const Rect<uint32_t>& dstPos) {
-    return {
-        double(srcPos.w) / dstPos.w,
-        double(srcPos.h) / dstPos.h
-    };
-}
-
-vec2<uint32_t> getBounds(const vec2<uint32_t>& srcSize, const Rect<uint32_t>& srcPos,
-                         const vec2<uint32_t>& dstSize, const Rect<uint32_t>& dstPos,
-                         const vec2<double>& factor) {
-    if (srcPos.x > srcSize.x || srcPos.y > srcSize.y ||
-        dstPos.x > dstSize.x || dstPos.y > dstSize.y) {
-        // Source or destination position is out of range.
-        return { 0, 0 };
-    }
-
-    // Make sure we don't read/write values out of range.
-    return { std::min(uint32_t(double(srcSize.x - srcPos.x) / factor.x),
-                      std::min(dstSize.x - dstPos.x, dstPos.w)),
-             std::min(uint32_t(double(srcSize.y - srcPos.y) / factor.y),
-                      std::min(dstSize.y - dstPos.y, dstPos.h)) };
-}
-} // namespace
-
-namespace mbgl {
-namespace util {
-
-void bilinearScale(const uint32_t* srcData, const vec2<uint32_t>& srcSize,
-                   const Rect<uint32_t>& srcPos, uint32_t* dstData, const vec2<uint32_t>& dstSize,
-                   const Rect<uint32_t>& dstPos, bool wrap) {
-    const auto factor = getFactor(srcPos, dstPos);
-    const auto bounds = getBounds(srcSize, srcPos, dstSize, dstPos, factor);
-
-    uint32_t x, y;
-    size_t i = dstSize.x * dstPos.y + dstPos.x;
-    for (y = 0; y < bounds.y; y++) {
-        const double fractY = y * factor.y;
-        const uint32_t Y0 = fractY;
-        const uint32_t Y1 = wrap ? (Y0 + 1) % srcPos.h : (Y0 + 1);
-        const uint32_t srcY0 = srcPos.y + Y0;
-        const uint32_t srcY1 = std::min(srcPos.y + Y1, srcSize.y - 1);
-        for (x = 0; x < bounds.x; x++) {
-            const double fractX = x * factor.x;
-            const uint32_t X0 = fractX;
-            const uint32_t X1 = wrap ? (X0 + 1) % srcPos.w : (X0 + 1);
-            const uint32_t srcX0 = srcPos.x + X0;
-            const uint32_t srcX1 = std::min(srcPos.x + X1, srcSize.x - 1);
-
-            const uint32_t tl = srcData[srcSize.x * srcY0 + srcX0];
-            const uint32_t tr = srcData[srcSize.x * srcY0 + srcX1];
-            const uint32_t bl = srcData[srcSize.x * srcY1 + srcX0];
-            const uint32_t br = srcData[srcSize.x * srcY1 + srcX1];
-
-            const double dx = fractX - X0;
-            const double dy = fractY - Y0;
-            uint32_t& dst = dstData[i + x];
-            b<0>(dst) = bilinearInterpolate(b<0>(tl), b<0>(tr), b<0>(bl), b<0>(br), dx, dy);
-            b<1>(dst) = bilinearInterpolate(b<1>(tl), b<1>(tr), b<1>(bl), b<1>(br), dx, dy);
-            b<2>(dst) = bilinearInterpolate(b<2>(tl), b<2>(tr), b<2>(bl), b<2>(br), dx, dy);
-            b<3>(dst) = bilinearInterpolate(b<3>(tl), b<3>(tr), b<3>(bl), b<3>(br), dx, dy);
-        }
-        i += dstSize.x;
-    }
-}
-
-void nearestNeighborScale(const uint32_t* srcData, const vec2<uint32_t>& srcSize,
-                          const Rect<uint32_t>& srcPos, uint32_t* dstData,
-                          const vec2<uint32_t>& dstSize, const Rect<uint32_t>& dstPos) {
-    const auto factor = getFactor(srcPos, dstPos);
-    const auto bounds = getBounds(srcSize, srcPos, dstSize, dstPos, factor);
-
-    double fractSrcY = srcPos.y;
-    double fractSrcX;
-    size_t i = dstSize.x * dstPos.y + dstPos.x;
-    uint32_t srcY;
-    uint32_t x, y;
-    for (y = 0; y < bounds.y; y++) {
-        fractSrcX = srcPos.x;
-        srcY = srcSize.x * uint32_t(fractSrcY);
-        for (x = 0; x < bounds.x; x++) {
-            dstData[i + x] = srcData[srcY + uint32_t(fractSrcX)];
-            fractSrcX += factor.x;
-        }
-        i += dstSize.x;
-        fractSrcY += factor.y;
-    }
-}
-
-} // namespace util
-} // namespace mbgl
diff --git a/src/mbgl/util/scaling.hpp b/src/mbgl/util/scaling.hpp
deleted file mode 100644
index a2d582de1c..0000000000
--- a/src/mbgl/util/scaling.hpp
+++ /dev/null
@@ -1,23 +0,0 @@
-#ifndef MBGL_UTIL_SCALING
-#define MBGL_UTIL_SCALING
-
-
-#include <mbgl/util/vec.hpp>
-#include <mbgl/util/rect.hpp>
-
-#include <cstdint>
-
-namespace mbgl {
-namespace util {
-
-void bilinearScale(const uint32_t* srcData, const vec2<uint32_t>& srcSize,
-                   const Rect<uint32_t>& srcPos, uint32_t* dstData, const vec2<uint32_t>& dstSize,
-                   const Rect<uint32_t>& dstPos, bool wrap);
-
-void nearestNeighborScale(const uint32_t* srcData, const vec2<uint32_t>& srcSize,
-                          const Rect<uint32_t>& srcPos, uint32_t* dstData,
-                          const vec2<uint32_t>& dstSize, const Rect<uint32_t>& dstPos);
-} // namespace util
-} // namespace mbgl
-
-#endif
diff --git a/test/miscellaneous/bilinear.cpp b/test/miscellaneous/bilinear.cpp
deleted file mode 100644
index 3184e5739d..0000000000
--- a/test/miscellaneous/bilinear.cpp
+++ /dev/null
@@ -1,54 +0,0 @@
-#include "../fixtures/util.hpp"
-#include <mbgl/util/compression.hpp>
-#include <mbgl/util/scaling.hpp>
-#include <mbgl/util/premultiply.hpp>
-#include <mbgl/util/image.hpp>
-#include <mbgl/util/io.hpp>
-
-#include <algorithm>
-#include <cstring>
-
-using namespace mbgl;
-
-TEST(Bilinear, Scaling) {
-    // We're reading from a custom "image format" that has a uint32_t width/height prefix and then
-    // raw RGBA data. We can't use the built-in PNG reading routines because they are reading
-    // premultiplied images by default.
-    const std::string sprite = util::decompress(util::read_file("test/fixtures/sprites/bright.bin"));
-    const uint8_t *src = reinterpret_cast<const uint8_t *>(sprite.data());
-    ASSERT_GT(sprite.length(), 8u);
-    const uint32_t width = src[0] << 24 | src[1] << 16 | src[2] << 8 | src[3];
-    const uint32_t height = src[4] << 24 | src[5] << 16 | src[6] << 8 | src[7];
-    ASSERT_EQ(sprite.length(), 2 * sizeof(uint32_t) + width * height * sizeof(uint32_t));
-
-    const uint32_t *srcData = reinterpret_cast<const uint32_t *>(src + 8);
-    const vec2<uint32_t> srcSize { width, height };
-    const vec2<uint32_t> dstSize { 128, 128 };
-
-    UnassociatedImage dst { dstSize.x, dstSize.y };
-    uint32_t *dstData = reinterpret_cast<uint32_t*>(dst.data.get());
-    std::fill(dstData, dstData + dstSize.x * dstSize.y, 0xFFFF00FF);
-
-    util::bilinearScale(srcData, srcSize, { 0, 0, 24, 24 }, dstData, dstSize, { 8, 8, 24, 24 }, false);
-    util::bilinearScale(srcData, srcSize, { 26, 0, 24, 24 }, dstData, dstSize, { 0, 40, 48, 48 }, false);
-    util::bilinearScale(srcData, srcSize, { 26, 26, 24, 24 }, dstData, dstSize, { 52, 40, 36, 36 }, false);
-    util::bilinearScale(srcData, srcSize, { 26, 26, 24, 24 }, dstData, dstSize, { 52, 40, 36, 36 }, false);
-    util::bilinearScale(srcData, srcSize, { 104, 0, 24, 24 }, dstData, dstSize, { 96, 0, 48, 48 }, false);
-    util::bilinearScale(srcData, srcSize, { 52, 260, 24, 24 }, dstData, dstSize, { 108, 108, 38, 38 }, false);
-    util::bilinearScale(srcData, srcSize, { 380, 0, 24, 24 }, dstData, dstSize, { 36, 0, 24, 24 }, false);
-    util::bilinearScale(srcData, srcSize, { 396, 396, 24, 24 }, dstData, dstSize, { 0, 0, 50, 50 }, false);
-    util::bilinearScale(srcData, srcSize, { 380, 182, 12, 12 }, dstData, dstSize, { 52, 80, 24, 24 }, false);
-
-    // From the bottom
-    util::bilinearScale(srcData, srcSize, { 252, 380, 12, 12 }, dstData, dstSize, { 0, 90, 12, 12 }, false);
-    util::bilinearScale(srcData, srcSize, { 252, 380, 12, 12 }, dstData, dstSize, { 18, 90, 24, 24 }, false);
-
-    const std::string data { reinterpret_cast<char *>(dstData), dstSize.x * dstSize.y * sizeof(uint32_t) };
-    util::write_file("test/fixtures/sprites/atlas_actual.png", encodePNG(util::premultiply(std::move(dst))));
-    util::write_file("test/fixtures/sprites/atlas_actual.bin", util::compress(data));
-
-    const std::string reference = util::decompress(util::read_file("test/fixtures/sprites/atlas_reference.bin"));
-
-    EXPECT_EQ(reference.size(), data.size());
-    EXPECT_TRUE(0 == std::memcmp(data.data(), reference.data(), data.size()));
-}
diff --git a/test/sprite/sprite_atlas.cpp b/test/sprite/sprite_atlas.cpp
index fd78e971fc..8e3c4317c0 100644
--- a/test/sprite/sprite_atlas.cpp
+++ b/test/sprite/sprite_atlas.cpp
@@ -34,8 +34,6 @@ TEST(Sprite, SpriteAtlas) {
     EXPECT_EQ(0, metro.pos.y);
     EXPECT_EQ(20, metro.pos.w);
     EXPECT_EQ(20, metro.pos.h);
-    EXPECT_EQ(18, metro.pos.originalW);
-    EXPECT_EQ(18, metro.pos.originalH);
     EXPECT_EQ(18, metro.texture->width);
     EXPECT_EQ(18, metro.texture->height);
     EXPECT_EQ(18, metro.texture->pixelWidth);
@@ -45,12 +43,12 @@ TEST(Sprite, SpriteAtlas) {
     EXPECT_TRUE(atlas.getData());
 
     auto pos = *atlas.getPosition("metro", false);
-    EXPECT_DOUBLE_EQ(20, pos.size[0]);
-    EXPECT_DOUBLE_EQ(20, pos.size[1]);
+    EXPECT_DOUBLE_EQ(18, pos.size[0]);
+    EXPECT_DOUBLE_EQ(18, pos.size[1]);
     EXPECT_DOUBLE_EQ(1.0f / 63, pos.tl[0]);
     EXPECT_DOUBLE_EQ(1.0f / 112, pos.tl[1]);
-    EXPECT_DOUBLE_EQ(21.0f / 63, pos.br[0]);
-    EXPECT_DOUBLE_EQ(21.0f / 112, pos.br[1]);
+    EXPECT_DOUBLE_EQ(19.0f / 63, pos.br[0]);
+    EXPECT_DOUBLE_EQ(19.0f / 112, pos.br[1]);
 
     auto missing = atlas.getImage("doesnotexist", false);
     EXPECT_FALSE(missing);
@@ -68,12 +66,10 @@ TEST(Sprite, SpriteAtlas) {
     EXPECT_EQ(0, metro2.pos.y);
     EXPECT_EQ(20, metro2.pos.w);
     EXPECT_EQ(20, metro2.pos.h);
-    EXPECT_EQ(18, metro2.pos.originalW);
-    EXPECT_EQ(18, metro2.pos.originalH);
 
     const size_t bytes = atlas.getTextureWidth() * atlas.getTextureHeight() * 4;
     const auto hash = test::crc64(reinterpret_cast<const char*>(atlas.getData()), bytes);
-    EXPECT_EQ(0x9875FC0595489A9Fu, hash) << std::hex << hash;
+    EXPECT_EQ(11868256915183397177u, hash) << std::hex << hash;
 
     // util::write_file(
     //     "test/fixtures/annotations/atlas1.png",
@@ -98,10 +94,8 @@ TEST(Sprite, SpriteAtlasSize) {
     auto metro = *atlas.getImage("metro", false);
     EXPECT_EQ(0, metro.pos.x);
     EXPECT_EQ(0, metro.pos.y);
-    EXPECT_EQ(20, metro.pos.w);
-    EXPECT_EQ(20, metro.pos.h);
-    EXPECT_EQ(18, metro.pos.originalW);
-    EXPECT_EQ(18, metro.pos.originalH);
+    EXPECT_EQ(16, metro.pos.w);
+    EXPECT_EQ(16, metro.pos.h);
     EXPECT_EQ(18, metro.texture->width);
     EXPECT_EQ(18, metro.texture->height);
     EXPECT_EQ(18, metro.texture->pixelWidth);
@@ -110,7 +104,7 @@ TEST(Sprite, SpriteAtlasSize) {
 
     const size_t bytes = atlas.getTextureWidth() * atlas.getTextureHeight() * 4;
     const auto hash = test::crc64(reinterpret_cast<const char*>(atlas.getData()), bytes);
-    EXPECT_EQ(0x2CDDA7DBB04D116Du, hash) << std::hex << hash;
+    EXPECT_EQ(18324190582232646342u, hash) << std::hex << hash;
 
     // util::write_file(
     //     "test/fixtures/annotations/atlas2.png",
@@ -134,8 +128,6 @@ TEST(Sprite, SpriteAtlasUpdates) {
     EXPECT_EQ(0, one.pos.y);
     EXPECT_EQ(20, one.pos.w);
     EXPECT_EQ(16, one.pos.h);
-    EXPECT_EQ(16, one.pos.originalW);
-    EXPECT_EQ(12, one.pos.originalH);
     EXPECT_EQ(16, one.texture->width);
     EXPECT_EQ(12, one.texture->height);
     EXPECT_EQ(16, one.texture->pixelWidth);
diff --git a/test/test.gypi b/test/test.gypi
index f8818e2084..21b2038876 100644
--- a/test/test.gypi
+++ b/test/test.gypi
@@ -47,7 +47,6 @@
         'miscellaneous/async_task.cpp',
         'miscellaneous/clip_ids.cpp',
         'miscellaneous/binpack.cpp',
-        'miscellaneous/bilinear.cpp',
         'miscellaneous/comparisons.cpp',
         'miscellaneous/functions.cpp',
         'miscellaneous/geo.cpp',