[core] Change ImagePosition from storing non-padded rect to padded rect to mirror the JS implementation

6 files changed, 22 insertions, 46 deletions

diff --git a/src/mbgl/renderer/image_atlas.cpp b/src/mbgl/renderer/image_atlas.cpp
index e38bd923ee..71a0010972 100644
--- a/src/mbgl/renderer/image_atlas.cpp
+++ b/src/mbgl/renderer/image_atlas.cpp
@@ -9,7 +9,7 @@ static constexpr uint32_t padding = 1;
 
 ImagePosition::ImagePosition(const mapbox::Bin& bin, const style::Image::Impl& image, uint32_t version_)
     : pixelRatio(image.pixelRatio),
-      textureRect(bin.x + padding, bin.y + padding, bin.w - padding * 2, bin.h - padding * 2),
+      paddedRect(bin.x, bin.y, bin.w, bin.h),
       version(version_),
       stretchX(image.stretchX),
       stretchY(image.stretchY),
@@ -65,7 +65,7 @@ void populateImagePatches(
             auto updatedImage = imageManager.getSharedImage(name);
             if (updatedImage == nullptr) continue;
 
-            patches.emplace_back(*updatedImage, position.textureRect);
+            patches.emplace_back(*updatedImage, position.paddedRect);
             position.version = version;
         }
     }
diff --git a/src/mbgl/renderer/image_atlas.hpp b/src/mbgl/renderer/image_atlas.hpp
index e5305b0e42..8be975e98c 100644
--- a/src/mbgl/renderer/image_atlas.hpp
+++ b/src/mbgl/renderer/image_atlas.hpp
@@ -22,24 +22,19 @@ public:
 
     static constexpr const uint16_t padding = 1u;
     float pixelRatio;
-    Rect<uint16_t> textureRect;
+    Rect<uint16_t> paddedRect;
     uint32_t version;
     style::ImageStretches stretchX;
     style::ImageStretches stretchY;
     style::ImageContent content;
 
     std::array<uint16_t, 2> tl() const {
-        return {{
-            textureRect.x,
-            textureRect.y
-        }};
+        return {{static_cast<uint16_t>(paddedRect.x + padding), static_cast<uint16_t>(paddedRect.y + padding)}};
     }
 
     std::array<uint16_t, 2> br() const {
-        return {{
-            static_cast<uint16_t>(textureRect.x + textureRect.w),
-            static_cast<uint16_t>(textureRect.y + textureRect.h)
-        }};
+        return {{static_cast<uint16_t>(paddedRect.x + paddedRect.w - padding),
+                 static_cast<uint16_t>(paddedRect.y + paddedRect.h - padding)}};
     }
 
     std::array<uint16_t, 4> tlbr() const {
@@ -50,29 +45,21 @@ public:
 
     std::array<float, 2> displaySize() const {
         return {{
-            textureRect.w / pixelRatio,
-            textureRect.h / pixelRatio,
+            static_cast<float>(paddedRect.w - padding * 2) / pixelRatio,
+            static_cast<float>(paddedRect.h - padding * 2) / pixelRatio,
         }};
     }
-
-    Rect<uint16_t> paddedTextureRect() const {
-        return {static_cast<uint16_t>(textureRect.x - padding),
-                static_cast<uint16_t>(textureRect.y - padding),
-                static_cast<uint16_t>(textureRect.w + padding * 2),
-                static_cast<uint16_t>(textureRect.h + padding * 2)};
-    }
 };
 
 using ImagePositions = std::map<std::string, ImagePosition>;
 
 class ImagePatch {
 public:
-    ImagePatch(Immutable<style::Image::Impl> image_,
-               const Rect<uint16_t>& textureRect_)
-        : image(std::move(image_))
-        , textureRect(textureRect_) {}
+    ImagePatch(Immutable<style::Image::Impl> image_, const Rect<uint16_t>& paddedRect_)
+        : image(std::move(image_)), paddedRect(paddedRect_) {}
+
     Immutable<style::Image::Impl> image;
-    Rect<uint16_t> textureRect;
+    Rect<uint16_t> paddedRect;
 };
 
 class ImageAtlas {
diff --git a/src/mbgl/text/quads.cpp b/src/mbgl/text/quads.cpp
index 1f07ad4427..9d4e83c0d6 100644
--- a/src/mbgl/text/quads.cpp
+++ b/src/mbgl/text/quads.cpp
@@ -20,7 +20,7 @@ SymbolQuads getIconQuads(const PositionedIcon& shapedIcon, WritingModeType writi
     // If you have a 10px icon that isn't perfectly aligned to the pixel grid it will cover 11 actual
     // pixels. The quad needs to be padded to account for this, otherwise they'll look slightly clipped
     // on one edge in some cases.
-    const float border = ImagePosition::padding;
+    constexpr auto border = ImagePosition::padding;
 
     // Expand the box to respect the 1 pixel border in the atlas image. We're using `image.paddedRect - border`
     // instead of image.displaySize because we only pad with one pixel for retina images as well, and the
@@ -28,18 +28,12 @@ SymbolQuads getIconQuads(const PositionedIcon& shapedIcon, WritingModeType writi
     // Unlike the JavaScript version, we're _not_ including the padding in the texture rect, so the
     // logic "dimension * padded / non-padded - dimension" is swapped.
     const float iconWidth = shapedIcon.right() - shapedIcon.left();
-    const float expandX = (iconWidth * (static_cast<float>(image.textureRect.w) + 2.0f * border) /
-                               static_cast<float>(image.textureRect.w) -
-                           iconWidth) /
-                          2.0f;
+    const float expandX = (iconWidth * image.paddedRect.w / (image.paddedRect.w - 2 * border) - iconWidth) / 2.0f;
     const float left = shapedIcon.left() - expandX;
     const float right = shapedIcon.right() + expandX;
 
     const float iconHeight = shapedIcon.bottom() - shapedIcon.top();
-    const float expandY = (iconHeight * (static_cast<float>(image.textureRect.h) + 2.0f * border) /
-                               static_cast<float>(image.textureRect.h) -
-                           iconHeight) /
-                          2.0f;
+    const float expandY = (iconHeight * image.paddedRect.h / (image.paddedRect.h - 2 * border) - iconHeight) / 2.0f;
     const float top = shapedIcon.top() - expandY;
     const float bottom = shapedIcon.bottom() + expandY;
 
@@ -62,14 +56,6 @@ SymbolQuads getIconQuads(const PositionedIcon& shapedIcon, WritingModeType writi
         br = util::matrixMultiply(matrix, br);
     }
 
-    // Icon quad is padded, so texture coordinates also need to be padded.
-    Rect<uint16_t> textureRect {
-        static_cast<uint16_t>(image.textureRect.x - border),
-        static_cast<uint16_t>(image.textureRect.y - border),
-        static_cast<uint16_t>(image.textureRect.w + border * 2),
-        static_cast<uint16_t>(image.textureRect.h + border * 2)
-    };
-
     Point<float> pixelOffsetTL;
     Point<float> pixelOffsetBR;
     Point<float> minFontScale;
@@ -78,7 +64,7 @@ SymbolQuads getIconQuads(const PositionedIcon& shapedIcon, WritingModeType writi
                        tr,
                        bl,
                        br,
-                       textureRect,
+                       image.paddedRect,
                        writingMode,
                        {0.0f, 0.0f},
                        iconType == SymbolContent::IconSDF,
diff --git a/src/mbgl/text/shaping.cpp b/src/mbgl/text/shaping.cpp
index fb1aa04346..2b23deadea 100644
--- a/src/mbgl/text/shaping.cpp
+++ b/src/mbgl/text/shaping.cpp
@@ -436,7 +436,7 @@ void shapeLines(Shaping& shaping,
                 metrics.left = ImagePosition::padding;
                 metrics.top = -Glyph::borderSize;
                 metrics.advance = vertical ? displaySize[1] : displaySize[0];
-                rect = image->second.paddedTextureRect();
+                rect = image->second.paddedRect;
 
                 // If needed, allow to set scale factor for an image using
                 // alias "image-scale" that could be alias for "font-scale"
diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp
index 67e4459104..04ee5f9df7 100644
--- a/src/mbgl/tile/geometry_tile.cpp
+++ b/src/mbgl/tile/geometry_tile.cpp
@@ -99,7 +99,10 @@ void GeometryTileRenderData::upload(gfx::UploadPass& uploadPass) {
 
     if (atlasTextures->icon && !imagePatches.empty()) {
         for (const auto& imagePatch : imagePatches) { // patch updated images.
-            uploadPass.updateTextureSub(*atlasTextures->icon, imagePatch.image->image, imagePatch.textureRect.x, imagePatch.textureRect.y);
+            uploadPass.updateTextureSub(*atlasTextures->icon,
+                                        imagePatch.image->image,
+                                        imagePatch.paddedRect.x + ImagePosition::padding,
+                                        imagePatch.paddedRect.y + ImagePosition::padding);
         }
         imagePatches.clear();
     }
diff --git a/test/renderer/pattern_atlas.test.cpp b/test/renderer/pattern_atlas.test.cpp
index 2a19e463bd..350a64adb4 100644
--- a/test/renderer/pattern_atlas.test.cpp
+++ b/test/renderer/pattern_atlas.test.cpp
@@ -51,7 +51,7 @@ TEST(PatternAtlas, Updates) {
     ASSERT_TRUE(added);
     auto found = patternAtlas.getPattern("one");
     ASSERT_TRUE(found);
-    EXPECT_EQ(added->textureRect, found->textureRect);
+    EXPECT_EQ(added->paddedRect, found->paddedRect);
 
     auto a = *found;
     EXPECT_EQ(1, a.tl()[0]);