[core] Make Image safer

Provide Image::copy, which handles copying rectangles from a source to a destination, with thorough bounds checking.

Also fixes an indexing error in SpriteAtlas, where the top row of pixels in a wrapped image was copied from the wrong source row.

4 files changed, 39 insertions, 13 deletions

diff --git a/test/fixtures/sprite_atlas/basic/expected.png b/test/fixtures/sprite_atlas/basic/expected.png
index e886e060fe..cd13d16df6 100644
--- a/test/fixtures/sprite_atlas/basic/expected.png
+++ b/test/fixtures/sprite_atlas/basic/expected.png
diff --git a/test/text/glyph_atlas.test.cpp b/test/text/glyph_atlas.test.cpp
index e229cd117b..1ad2af9d82 100644
--- a/test/text/glyph_atlas.test.cpp
+++ b/test/text/glyph_atlas.test.cpp
@@ -145,16 +145,12 @@ TEST(GlyphAtlas, LoadingCancel) {
 
 TEST(GlyphAtlas, InvalidSDFGlyph) {
     GlyphSet glyphSet;
-    glyphSet.insert(65, SDFGlyph{ 65 /* ASCII 'A' */,
-                                  "x" /* bitmap is too short */,
-                                  { 1 /* width */, 1 /* height */, 0 /* left */, 0 /* top */,
-                                    0 /* advance */ } });
     glyphSet.insert(66, SDFGlyph{ 66 /* ASCII 'B' */,
-                                  std::string(7 * 7, 'x'), /* correct */
+                                  AlphaImage({7, 7}), /* correct */
                                   { 1 /* width */, 1 /* height */, 0 /* left */, 0 /* top */,
                                     0 /* advance */ } });
     glyphSet.insert(67, SDFGlyph{ 67 /* ASCII 'C' */,
-                                  std::string(518 * 8, 'x'), /* correct */
+                                  AlphaImage({518, 8}), /* correct */
                                   { 512 /* width */, 2 /* height */, 0 /* left */, 0 /* top */,
                                     0 /* advance */ } });
 
@@ -165,11 +161,10 @@ TEST(GlyphAtlas, InvalidSDFGlyph) {
     GlyphPositions positions;
     test.glyphAtlas.addGlyphs(1, std::u16string{u"ABC"}, fontStack, glyphSet, positions);
 
-    ASSERT_EQ(3u, positions.size());
+    ASSERT_EQ(2u, positions.size());
 
-    // 'A' was not placed because the bitmap size is invalid.
-    ASSERT_NE(positions.end(), positions.find(65));
-    ASSERT_EQ((Rect<uint16_t>{ 0, 0, 0, 0 }), positions[65].rect);
+    // 'A' was not placed because not in the glyph set.
+    ASSERT_EQ(positions.end(), positions.find(65));
 
     // 'B' was placed at the top left.
     ASSERT_NE(positions.end(), positions.find(66));
diff --git a/test/text/glyph_pbf.test.cpp b/test/text/glyph_pbf.test.cpp
index 1e28dfbc31..be3ca3359b 100644
--- a/test/text/glyph_pbf.test.cpp
+++ b/test/text/glyph_pbf.test.cpp
@@ -44,15 +44,17 @@ TEST(GlyphPBF, Parsing) {
     glyphAtlasObserver.glyphsLoaded = [&](const FontStack&, const GlyphRange&) {
         loop.stop();
 
-        auto sdfs = glyphAtlas.getGlyphSet(fontStack)->getSDFs();
+        const auto& sdfs = glyphAtlas.getGlyphSet(fontStack)->getSDFs();
 
         // The fake glyphs don't contain a glyph that has the ID 0; it only contains glyphs with
         // undefined IDs, but the parser should remove them.
 
         EXPECT_TRUE(sdfs.size() == 1);
         EXPECT_TRUE(sdfs.find(69) != sdfs.end());
-        auto& sdf = sdfs[69];
-        EXPECT_EQ("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"s, sdf.bitmap);
+        auto& sdf = sdfs.at(69);
+        AlphaImage expected({7, 7});
+        expected.fill('x');
+        EXPECT_EQ(expected, sdf.bitmap);
         EXPECT_EQ(1u, sdf.metrics.width);
         EXPECT_EQ(1u, sdf.metrics.height);
         EXPECT_EQ(20, sdf.metrics.left);
diff --git a/test/util/image.test.cpp b/test/util/image.test.cpp
index b15ddc1b3f..0cd4a7d8af 100644
--- a/test/util/image.test.cpp
+++ b/test/util/image.test.cpp
@@ -86,6 +86,35 @@ TEST(Image, WebPTile) {
 }
 #endif // !defined(__ANDROID__) && !defined(__APPLE__) && !defined(QT_IMAGE_DECODERS)
 
+TEST(Image, Copy) {
+    PremultipliedImage src5({5, 5});
+    PremultipliedImage dst5({5, 5});
+    PremultipliedImage src10({10, 10});
+    PremultipliedImage dst10({10, 10});
+
+    EXPECT_THROW(PremultipliedImage::copy(src5, dst10, {0, 0}, {0, 0}, {6, 0}), std::out_of_range);
+    EXPECT_THROW(PremultipliedImage::copy(src5, dst10, {0, 0}, {0, 0}, {0, 6}), std::out_of_range);
+    EXPECT_THROW(PremultipliedImage::copy(src5, dst10, {1, 1}, {0, 0}, {5, 0}), std::out_of_range);
+    EXPECT_THROW(PremultipliedImage::copy(src5, dst10, {1, 1}, {0, 0}, {0, 5}), std::out_of_range);
+
+    EXPECT_THROW(PremultipliedImage::copy(src10, dst5, {0, 0}, {0, 0}, {6, 0}), std::out_of_range);
+    EXPECT_THROW(PremultipliedImage::copy(src10, dst5, {0, 0}, {0, 0}, {0, 6}), std::out_of_range);
+    EXPECT_THROW(PremultipliedImage::copy(src10, dst5, {0, 0}, {1, 1}, {5, 0}), std::out_of_range);
+    EXPECT_THROW(PremultipliedImage::copy(src10, dst5, {0, 0}, {1, 1}, {0, 5}), std::out_of_range);
+
+    const uint32_t max = std::numeric_limits<uint32_t>::max();
+
+    EXPECT_THROW(PremultipliedImage::copy(src10, dst10, {max, 0}, {0, 0}, {1, 0}), std::out_of_range);
+    EXPECT_THROW(PremultipliedImage::copy(src10, dst10, {0, max}, {0, 0}, {0, 1}), std::out_of_range);
+    EXPECT_THROW(PremultipliedImage::copy(src10, dst10, {0, 0}, {max, 0}, {1, 0}), std::out_of_range);
+    EXPECT_THROW(PremultipliedImage::copy(src10, dst10, {0, 0}, {0, max}, {0, 1}), std::out_of_range);
+
+    EXPECT_THROW(PremultipliedImage::copy(src10, dst10, {1, 0}, {0, 0}, {max, 0}), std::out_of_range);
+    EXPECT_THROW(PremultipliedImage::copy(src10, dst10, {0, 1}, {0, 0}, {0, max}), std::out_of_range);
+    EXPECT_THROW(PremultipliedImage::copy(src10, dst10, {0, 0}, {1, 0}, {max, 0}), std::out_of_range);
+    EXPECT_THROW(PremultipliedImage::copy(src10, dst10, {0, 0}, {0, 1}, {0, max}), std::out_of_range);
+}
+
 TEST(Image, Premultiply) {
     UnassociatedImage rgba({ 1, 1 });
     rgba.data[0] = 255;