Merge pull request #1111 from mapbox/fix-sprite-padding

fix repeating pattern seams

6 files changed, 41 insertions, 40 deletions

diff --git a/src/mbgl/geometry/sprite_atlas.cpp b/src/mbgl/geometry/sprite_atlas.cpp
index 187253b147..c2686e2f34 100644
--- a/src/mbgl/geometry/sprite_atlas.cpp
+++ b/src/mbgl/geometry/sprite_atlas.cpp
@@ -165,37 +165,37 @@ void SpriteAtlas::copy(const Rect<dimension>& dst, const SpritePosition& src, co
                                   static_cast<uint32_t>(dst.originalW * pixelRatio),
                                   static_cast<uint32_t>(dst.originalH * pixelRatio) };
 
-    util::bilinearScale(srcData, srcSize, srcPos, dstData, dstSize, dstPos);
+    util::bilinearScale(srcData, srcSize, srcPos, dstData, dstSize, dstPos, wrap);
 
     // 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
-        if (dstPos.x >= border) {
-            util::nearestNeighborScale(
-                dstData, dstSize, { dstPos.x + dstPos.w - border - 1, dstPos.y, border, dstPos.h },
-                dstData, dstSize, { dstPos.x - border, dstPos.y, border, dstPos.h });
-        }
+        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
-        if (dstPos.y >= border) {
-            util::nearestNeighborScale(
-                dstData, dstSize, { dstPos.x - border, dstPos.y + dstPos.h - border - 1,
-                                    dstPos.w + 2 * border, border },
-                dstData, dstSize,
-                { dstPos.x - border, dstPos.y - border, dstPos.w + 2 * border, 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 - border, dstPos.y, dstPos.w + 2 * border, border },
+            dstData, dstSize, { dstPos.x - borderX, dstPos.y, dstPos.w + 2 * borderX, border },
             dstData, dstSize,
-            { dstPos.x - border, dstPos.y + dstPos.h, dstPos.w + 2 * border, border });
+            { dstPos.x - borderX, dstPos.y + dstPos.h, dstPos.w + border + borderX, border });
     }
 
     dirty = true;
diff --git a/src/mbgl/util/scaling.cpp b/src/mbgl/util/scaling.cpp
index a554b2e137..efc2709df3 100644
--- a/src/mbgl/util/scaling.cpp
+++ b/src/mbgl/util/scaling.cpp
@@ -49,38 +49,39 @@ 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) {
+                   const Rect<uint32_t>& dstPos, bool wrap) {
     const auto factor = getFactor(srcPos, dstPos);
     const auto bounds = getBounds(srcSize, srcPos, dstSize, dstPos, factor);
 
-    double fractSrcY = srcPos.y;
-    double fractSrcX;
     uint32_t x, y;
     size_t i = dstSize.x * dstPos.y + dstPos.x;
     for (y = 0; y < bounds.y; y++) {
-        fractSrcX = srcPos.x;
-        const uint32_t srcY0 = fractSrcY;
-        const uint32_t srcY1 = std::min(srcY0 + 1, srcSize.y - 1);
+        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 uint32_t srcX0 = fractSrcX;
-            const uint32_t srcX1 = std::min(srcX0 + 1, srcSize.x - 1);
+            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 = fractSrcX - srcX0;
-            const double dy = fractSrcY - srcY0;
+            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);
-            fractSrcX += factor.x;
         }
         i += dstSize.x;
-        fractSrcY += factor.y;
     }
 }
 
@@ -108,4 +109,4 @@ void nearestNeighborScale(const uint32_t* srcData, const vec2<uint32_t>& srcSize
 }
 
 }
-}
\ No newline at end of file
+}
diff --git a/src/mbgl/util/scaling.hpp b/src/mbgl/util/scaling.hpp
index d2625e9219..74a43eb621 100644
--- a/src/mbgl/util/scaling.hpp
+++ b/src/mbgl/util/scaling.hpp
@@ -12,7 +12,7 @@ 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);
+                   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,
diff --git a/test/fixtures/sprites/atlas_reference.bin b/test/fixtures/sprites/atlas_reference.bin
index 57eb28bd93..d75d961b60 100644
--- a/test/fixtures/sprites/atlas_reference.bin
+++ b/test/fixtures/sprites/atlas_reference.bin
diff --git a/test/fixtures/sprites/atlas_reference.png b/test/fixtures/sprites/atlas_reference.png
index 86fad30983..9107aefe09 100644
--- a/test/fixtures/sprites/atlas_reference.png
+++ b/test/fixtures/sprites/atlas_reference.png
diff --git a/test/miscellaneous/bilinear.cpp b/test/miscellaneous/bilinear.cpp
index b7730303a0..9808e20fef 100644
--- a/test/miscellaneous/bilinear.cpp
+++ b/test/miscellaneous/bilinear.cpp
@@ -28,19 +28,19 @@ TEST(Bilinear, Scaling) {
     uint32_t *dstData = dst.get();
     std::fill(dstData, dstData + dstSize.x * dstSize.y, 0xFFFF00FF);
 
-    util::bilinearScale(srcData, srcSize, { 0, 0, 24, 24 }, dstData, dstSize, { 8, 8, 24, 24 });
-    util::bilinearScale(srcData, srcSize, { 26, 0, 24, 24 }, dstData, dstSize, { 0, 40, 48, 48 });
-    util::bilinearScale(srcData, srcSize, { 26, 26, 24, 24 }, dstData, dstSize, { 52, 40, 36, 36 });
-    util::bilinearScale(srcData, srcSize, { 26, 26, 24, 24 }, dstData, dstSize, { 52, 40, 36, 36 });
-    util::bilinearScale(srcData, srcSize, { 104, 0, 24, 24 }, dstData, dstSize, { 96, 0, 48, 48 });
-    util::bilinearScale(srcData, srcSize, { 52, 260, 24, 24 }, dstData, dstSize, { 108, 108, 38, 38 });
-    util::bilinearScale(srcData, srcSize, { 380, 0, 24, 24 }, dstData, dstSize, { 36, 0, 24, 24 });
-    util::bilinearScale(srcData, srcSize, { 396, 396, 24, 24 }, dstData, dstSize, { 0, 0, 50, 50 });
-    util::bilinearScale(srcData, srcSize, { 380, 182, 12, 12 }, dstData, dstSize, { 52, 80, 24, 24 });
+    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 });
-    util::bilinearScale(srcData, srcSize, { 252, 380, 12, 12 }, dstData, dstSize, { 18, 90, 24, 24 });
+    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", util::compress_png(dstSize.x, dstSize.y, dstData));