don't scale when adding to sprite atlas

package.json

@@ -26,7 +26,7 @@
   ],
   "devDependencies": {
     "aws-sdk": "^2.2.21",
-    "mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#46c907f48d2b3b8ae6531963cdc40cb07958fd01",
+    "mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#c8982f16ef5d39164b2e4f33bc3f993f9ac17502",
     "node-gyp": "^3.2.1",
     "request": "^2.67.0",
     "tape": "^4.2.2"

src/mbgl/renderer/painter_symbol.cpp

@@ -187,8 +187,9 @@ void Painter::renderSymbol(SymbolBucket& bucket, const SymbolLayer& layer, const
         const float fontScale = fontSize / 1.0f;
 
         SpriteAtlas* activeSpriteAtlas = layer.spriteAtlas;
-        activeSpriteAtlas->bind(state.isChanging() || layout.placement == PlacementType::Line
-                || angleOffset != 0 || fontScale != 1 || sdf || state.getPitch() != 0);
+        const bool iconScaled = fontScale != 1 || data.pixelRatio != activeSpriteAtlas->getPixelRatio() || bucket.iconsNeedLinear;
+        const bool iconTransformed = layout.placement == PlacementType::Line || angleOffset != 0 || state.getPitch() != 0;
+        activeSpriteAtlas->bind(sdf || state.isChanging() || iconScaled || iconTransformed);
 
         if (sdf) {
             renderSDF(bucket,

src/mbgl/renderer/symbol_bucket.cpp

@@ -248,11 +248,14 @@ void SymbolBucket::addFeatures(uintptr_t tileUID,
         if (feature.sprite.length()) {
             auto image = spriteAtlas.getImage(feature.sprite, false);
             if (image) {
-                shapedIcon = shapeIcon((*image).pos, layout);
+                shapedIcon = shapeIcon(*image, layout);
                 assert((*image).texture);
                 if ((*image).texture->sdf) {
                     sdfIcons = true;
                 }
+                if ((*image).relativePixelRatio != 1.0f) {
+                    iconsNeedLinear = true;
+                }
             }
         }
 

src/mbgl/renderer/symbol_bucket.hpp

@@ -112,6 +112,7 @@ class SymbolBucket : public Bucket {
 public:
     SymbolLayoutProperties layout;
     bool sdfIcons = false;
+    bool iconsNeedLinear = false;
 
 private:
 

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>
@@ -27,7 +26,11 @@ SpriteAtlas::SpriteAtlas(dimension width_, dimension height_, float pixelRatio_,
       dirty(true) {
 }
 
-Rect<SpriteAtlas::dimension> SpriteAtlas::allocateImage(const size_t pixel_width, const size_t pixel_height) {
+Rect<SpriteAtlas::dimension> SpriteAtlas::allocateImage(float src_width, float src_height) {
+
+    const uint16_t pixel_width = std::ceil(src_width / pixelRatio);
+    const uint16_t pixel_height = std::ceil(src_height / pixelRatio);
+
     // Increase to next number divisible by 4, but at least 1.
     // This is so we can scale down the texture coordinates and pack them
     // into 2 bytes rather than 4 bytes.
@@ -41,9 +44,6 @@ Rect<SpriteAtlas::dimension> SpriteAtlas::allocateImage(const size_t pixel_width
         return rect;
     }
 
-    rect.originalW = pixel_width;
-    rect.originalH = pixel_height;
-
     return rect;
 }
 
@@ -52,15 +52,15 @@ mapbox::util::optional<SpriteAtlasElement> SpriteAtlas::getImage(const std::stri
 
     auto rect_it = images.find({ name, wrap });
     if (rect_it != images.end()) {
-        return SpriteAtlasElement { rect_it->second.pos, rect_it->second.texture };
+        return SpriteAtlasElement { rect_it->second.pos, rect_it->second.texture, rect_it->second.texture->pixelRatio / pixelRatio };
     }
 
     auto sprite = store.getSprite(name);
     if (!sprite) {
         return {};
     }
 
-    Rect<dimension> rect = allocateImage(sprite->width, sprite->height);
+    Rect<dimension> rect = allocateImage(sprite->width * sprite->pixelRatio, sprite->height * sprite->pixelRatio);
     if (rect.w == 0) {
         if (debug::spriteWarnings) {
             Log::Warning(Event::Sprite, "sprite atlas bitmap overflow");
@@ -71,7 +71,7 @@ mapbox::util::optional<SpriteAtlasElement> SpriteAtlas::getImage(const std::stri
     const Holder& holder = images.emplace(Key{ name, wrap }, Holder{ sprite, rect }).first->second;
     copy(holder, wrap);
 
-    return SpriteAtlasElement { rect, sprite };
+    return SpriteAtlasElement { rect, sprite, sprite->pixelRatio / pixelRatio };
 }
 
 mapbox::util::optional<SpriteAtlasPosition> SpriteAtlas::getPosition(const std::string& name, bool repeating) {
@@ -83,28 +83,48 @@ mapbox::util::optional<SpriteAtlasPosition> SpriteAtlas::getPosition(const std::
     }
 
     auto rect = (*img).pos;
-    if (repeating) {
-        // When the image is repeating, get the correct position of the image, rather than the
-        // one rounded up to 4 pixels.
-        // TODO: Can't we just use originalW/originalH?
-        auto sprite = store.getSprite(name);
-        if (!sprite) {
-            return SpriteAtlasPosition {};
-        }
-
-        rect.w = sprite->width;
-        rect.h = sprite->height;
-    }
 
     const float padding = 1;
+    auto image = (*img).texture;
+
+    const float w = image->width * (*img).relativePixelRatio;
+    const float h = image->height * (*img).relativePixelRatio;
 
     return SpriteAtlasPosition {
-        {{ float(rect.w), float(rect.h) }},
-        {{ float(rect.x + padding)          / width, float(rect.y + padding)          / height }},
-        {{ float(rect.x + padding + rect.w) / width, float(rect.y + padding + rect.h) / height }}
+        {{ float(image->width), float(image->height) }},
+        {{ float(rect.x + padding)     / width, float(rect.y + padding)     / height }},
+        {{ float(rect.x + padding + w) / width, float(rect.y + padding + h) / height }}
     };
 }
 
+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);
@@ -113,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;
 }

src/mbgl/sprite/sprite_atlas.hpp

@@ -34,6 +34,7 @@ struct SpriteAtlasPosition {
 struct SpriteAtlasElement {
     const Rect<uint16_t> pos;
     const std::shared_ptr<const SpriteImage> texture;
+    const float relativePixelRatio;
 };
 
 class SpriteAtlas : public util::noncopyable {
@@ -83,7 +84,7 @@ class SpriteAtlas : public util::noncopyable {
 
     using Key = std::pair<std::string, bool>;
 
-    Rect<SpriteAtlas::dimension> allocateImage(size_t width, size_t height);
+    Rect<SpriteAtlas::dimension> allocateImage(float width, float height);
     void copy(const Holder& holder, const bool wrap);
 
     std::recursive_mutex mtx;

src/mbgl/text/quads.cpp

@@ -13,11 +13,13 @@ SymbolQuads getIconQuads(Anchor& anchor, const PositionedIcon& shapedIcon,
         const std::vector<Coordinate>& line, const SymbolLayoutProperties& layout,
         const bool alongLine) {
 
+    auto image = *(shapedIcon.image);
+
     const float border = 1.0;
     auto left = shapedIcon.left - border;
-    auto right = left + shapedIcon.image.w;
+    auto right = left + image.pos.w / image.relativePixelRatio;
     auto top = shapedIcon.top - border;
-    auto bottom = top + shapedIcon.image.h;
+    auto bottom = top + image.pos.h / image.relativePixelRatio;
     vec2<float> tl{left, top};
     vec2<float> tr{right, top};
     vec2<float> br{right, bottom};
@@ -51,7 +53,7 @@ SymbolQuads getIconQuads(Anchor& anchor, const PositionedIcon& shapedIcon,
     }
 
     SymbolQuads quads;
-    quads.emplace_back(tl, tr, bl, br, shapedIcon.image, 0, anchor, globalMinScale, std::numeric_limits<float>::infinity());
+    quads.emplace_back(tl, tr, bl, br, image.pos, 0, anchor, globalMinScale, std::numeric_limits<float>::infinity());
     return quads;
 }
 

src/mbgl/text/shaping.cpp

@@ -3,13 +3,13 @@
 
 namespace mbgl {
 
-PositionedIcon shapeIcon(const Rect<uint16_t>& image, const SymbolLayoutProperties& layout) {
+PositionedIcon shapeIcon(const SpriteAtlasElement& image, const SymbolLayoutProperties& layout) {
     float dx = layout.icon.offset.value[0];
     float dy = layout.icon.offset.value[1];
-    float x1 = dx - image.originalW / 2.0f;
-    float x2 = x1 + image.originalW;
-    float y1 = dy - image.originalH / 2.0f;
-    float y2 = y1 + image.originalH;
+    float x1 = dx - image.texture->width / 2.0f;
+    float x2 = x1 + image.texture->width;
+    float y1 = dy - image.texture->height / 2.0f;
+    float y2 = y1 + image.texture->height;
 
     return PositionedIcon(image, y1, y2, x1, x2);
 }

src/mbgl/text/shaping.hpp

@@ -2,29 +2,35 @@
 #define MBGL_TEXT_SHAPING
 
 #include <mbgl/text/glyph.hpp>
+#include <mbgl/sprite/sprite_atlas.hpp>
+#include <mbgl/sprite/sprite_image.hpp>
+#include <mapbox/optional.hpp>
 
 #include <mbgl/util/vec.hpp>
 
 namespace mbgl {
 
+    struct SpriteAtlasElement;
+
     class PositionedIcon {
         public:
             inline explicit PositionedIcon() {}
-            inline explicit PositionedIcon(Rect<uint16_t> _image,
+            inline explicit PositionedIcon(const SpriteAtlasElement& _image,
                     float _top, float _bottom, float _left, float _right) :
                 image(_image), top(_top), bottom(_bottom), left(_left), right(_right) {}
-            Rect<uint16_t> image;
+
+            mapbox::util::optional<SpriteAtlasElement> image;
             float top = 0;
             float bottom = 0;
             float left = 0;
             float right = 0;
 
-            operator bool() const { return image.hasArea(); }
+            operator bool() const { return image && (*image).pos.hasArea(); }
     };
 
     class SymbolLayoutProperties;
 
-    PositionedIcon shapeIcon(const Rect<uint16_t>& image, const SymbolLayoutProperties&);
+    PositionedIcon shapeIcon(const SpriteAtlasElement& image, const SymbolLayoutProperties&);
 
 } // namespace mbgl
 

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 {