[core] add more detail to invalid metrics error in createSpriteImage

2 files changed, 120 insertions, 11 deletions

diff --git a/src/mbgl/sprite/sprite_parser.cpp b/src/mbgl/sprite/sprite_parser.cpp
index 8d9b35cb77..c528a678d2 100644
--- a/src/mbgl/sprite/sprite_parser.cpp
+++ b/src/mbgl/sprite/sprite_parser.cpp
@@ -20,11 +20,35 @@ SpriteImagePtr createSpriteImage(const PremultipliedImage& image,
                                  const double ratio,
                                  const bool sdf) {
     // Disallow invalid parameter configurations.
-    if (width <= 0 || height <= 0 || width > 1024 || height > 1024 ||
-        ratio <= 0 || ratio > 10 ||
-        srcX >= image.size.width || srcY >= image.size.height ||
-        srcX + width > image.size.width || srcY + height > image.size.height) {
-        Log::Error(Event::Sprite, "Can't create sprite with invalid metrics");
+    bool invalidParam = false;
+    std::stringstream invalidParamMessage;
+    invalidParamMessage << "Can't create sprite with invalid metrics: ";
+
+    if (width <= 0 || width > 1024) {
+        invalidParam = true;
+        invalidParamMessage << "invalid width: " << width;
+    } else if (height <= 0 || height > 1024) {
+        invalidParam = true;
+        invalidParamMessage << "invalid height: " << height;
+    } else if (ratio <= 0 || ratio > 10) {
+        invalidParam = true;
+        invalidParamMessage << "invalid ratio: " << ratio;
+    } else if (srcX >= image.size.width || srcX + width > image.size.width) {
+        invalidParam = true;
+        invalidParamMessage << "mismatched width: " <<
+            "image.size.height: " << image.size.width <<
+            ", srcX: " << srcX <<
+            ", width: " << width;
+    } else if (srcY >= image.size.height || srcY + height > image.size.height) {
+        invalidParam = true;
+        invalidParamMessage << "mismatched height: " <<
+            "image.size.height: " << image.size.height <<
+            ", srcY: " << srcY <<
+            ", height: " << height;
+    }
+
+    if (invalidParam) {
+        Log::Error(Event::Sprite, invalidParamMessage.str());
         return nullptr;
     }
 
diff --git a/test/sprite/sprite_parser.test.cpp b/test/sprite/sprite_parser.test.cpp
index 7bedf7bc28..d241c832b8 100644
--- a/test/sprite/sprite_parser.test.cpp
+++ b/test/sprite/sprite_parser.test.cpp
@@ -41,12 +41,97 @@ TEST(Sprite, SpriteImageCreationInvalid) {
     ASSERT_EQ(nullptr, createSpriteImage(image_1x, 0, 0, image_1x.size.width + 1, 16, 1, false));   // right edge out of bounds
     ASSERT_EQ(nullptr, createSpriteImage(image_1x, 0, 0, 16, image_1x.size.height + 1, 1, false));  // bottom edge out of bounds
 
-    EXPECT_EQ(13u, log.count({
+    EXPECT_EQ(1u, log.count({
+                      EventSeverity::Error,
+                      Event::Sprite,
+                      int64_t(-1),
+                      "Can't create sprite with invalid metrics: invalid width: 0",
+                  }));
+
+    EXPECT_EQ(1u, log.count({
+                      EventSeverity::Error,
+                      Event::Sprite,
+                      int64_t(-1),
+                      "Can't create sprite with invalid metrics: invalid height: 0",
+                  }));
+
+    EXPECT_EQ(1u, log.count({
+                      EventSeverity::Error,
+                      Event::Sprite,
+                      int64_t(-1),
+                      "Can't create sprite with invalid metrics: invalid width: 4294967295",
+                  }));
+
+    EXPECT_EQ(1u, log.count({
+                      EventSeverity::Error,
+                      Event::Sprite,
+                      int64_t(-1),
+                      "Can't create sprite with invalid metrics: invalid height: 4294967295",
+                  }));
+
+    EXPECT_EQ(1u, log.count({
+                      EventSeverity::Error,
+                      Event::Sprite,
+                      int64_t(-1),
+                      "Can't create sprite with invalid metrics: invalid ratio: 0",
+                  }));
+
+    EXPECT_EQ(1u, log.count({
+                      EventSeverity::Error,
+                      Event::Sprite,
+                      int64_t(-1),
+                      "Can't create sprite with invalid metrics: invalid ratio: -1",
+                  }));
+
+    EXPECT_EQ(1u, log.count({
                       EventSeverity::Error,
                       Event::Sprite,
                       int64_t(-1),
-                      "Can't create sprite with invalid metrics",
+                      "Can't create sprite with invalid metrics: invalid ratio: 23",
                   }));
+
+    EXPECT_EQ(1u, log.count({
+                      EventSeverity::Error,
+                      Event::Sprite,
+                      int64_t(-1),
+                      "Can't create sprite with invalid metrics: invalid width: 2048",
+                  }));
+
+    EXPECT_EQ(1u, log.count({
+                      EventSeverity::Error,
+                      Event::Sprite,
+                      int64_t(-1),
+                      "Can't create sprite with invalid metrics: invalid height: 1025",
+                  }));
+
+    EXPECT_EQ(1u, log.count({
+                      EventSeverity::Error,
+                      Event::Sprite,
+                      int64_t(-1),
+                      "Can't create sprite with invalid metrics: mismatched width: image.size.height: 200, srcX: 4294967295, width: 16",
+                  }));
+
+    EXPECT_EQ(1u, log.count({
+                      EventSeverity::Error,
+                      Event::Sprite,
+                      int64_t(-1),
+                      "Can't create sprite with invalid metrics: mismatched height: image.size.height: 299, srcY: 4294967295, height: 16",
+                  }));
+
+    EXPECT_EQ(1u, log.count({
+                      EventSeverity::Error,
+                      Event::Sprite,
+                      int64_t(-1),
+                      "Can't create sprite with invalid metrics: mismatched width: image.size.height: 200, srcX: 0, width: 201",
+                  }));
+
+    EXPECT_EQ(1u, log.count({
+                      EventSeverity::Error,
+                      Event::Sprite,
+                      int64_t(-1),
+                      "Can't create sprite with invalid metrics: mismatched height: image.size.height: 299, srcY: 0, height: 300",
+                  }));
+
 }
 
 TEST(Sprite, SpriteImageCreation1x) {
@@ -228,7 +313,7 @@ TEST(Sprite, SpriteParsingEmptyImage) {
                       EventSeverity::Error,
                       Event::Sprite,
                       int64_t(-1),
-                      "Can't create sprite with invalid metrics",
+                      "Can't create sprite with invalid metrics: invalid width: 0",
                   }));
 }
 
@@ -261,7 +346,7 @@ TEST(Sprite, SpriteParsingWidthTooBig) {
                       EventSeverity::Error,
                       Event::Sprite,
                       int64_t(-1),
-                      "Can't create sprite with invalid metrics",
+                      "Can't create sprite with invalid metrics: invalid width: 0",
                   }));
 }
 
@@ -284,7 +369,7 @@ TEST(Sprite, SpriteParsingNegativeWidth) {
                       EventSeverity::Error,
                       Event::Sprite,
                       int64_t(-1),
-                      "Can't create sprite with invalid metrics",
+                      "Can't create sprite with invalid metrics: invalid width: 0",
                   }));
 }
 
@@ -301,6 +386,6 @@ TEST(Sprite, SpriteParsingNullRatio) {
                       EventSeverity::Error,
                       Event::Sprite,
                       int64_t(-1),
-                      "Can't create sprite with invalid metrics",
+                      "Can't create sprite with invalid metrics: invalid ratio: 0",
                   }));
 }