summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--CHANGELOG.md3
-rw-r--r--platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/views/MapView.java4
-rw-r--r--platform/ios/src/MGLMapView.mm4
-rw-r--r--platform/osx/src/MGLMapView.mm4
-rw-r--r--src/mbgl/sprite/sprite_image.cpp12
-rw-r--r--src/mbgl/sprite/sprite_parser.cpp23
-rw-r--r--test/sprite/sprite_image.cpp8
-rw-r--r--test/sprite/sprite_parser.cpp6
-rw-r--r--test/sprite/sprite_store.cpp18
9 files changed, 39 insertions, 43 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index f0b07198c9..d684e7e8b2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,8 @@
## Android master
+* Fixed crash caused by annotation image with non-integer width or height ([#3031](https://github.com/mapbox/mapbox-gl-native/issues/3031))
+
## 3.0.0
* Added Camera API ([#3244](https://github.com/mapbox/mapbox-gl-native/issues/3244))
@@ -53,6 +55,7 @@ Known issues:
- New API to provide a custom callout view to the map for annotations. ([#3456](https://github.com/mapbox/mapbox-gl-native/pull/3456))
- Made telemetry on/off setting available in-app. ([#3445](https://github.com/mapbox/mapbox-gl-native/pull/3445))
- Fixed an issue with users not being counted by Mapbox if they had disabled telemetry. ([#3495](https://github.com/mapbox/mapbox-gl-native/pull/3495))
+- Fixed crash caused by MGLAnnotationImage with non-integer width or height ([#2198](https://github.com/mapbox/mapbox-gl-native/issues/2198))
## iOS 3.0.1
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/views/MapView.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/views/MapView.java
index 451dbfdcf5..9f6a675715 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/views/MapView.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/views/MapView.java
@@ -2314,8 +2314,8 @@ public final class MapView extends FrameLayout {
mNativeMapView.addAnnotationIcon(
id,
- (int) (bitmap.getWidth() / scale),
- (int) (bitmap.getHeight() / scale),
+ bitmap.getWidth(),
+ bitmap.getHeight(),
scale, buffer.array());
}
diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm
index 5b1ccd1628..3e63cd4b76 100644
--- a/platform/ios/src/MGLMapView.mm
+++ b/platform/ios/src/MGLMapView.mm
@@ -2346,8 +2346,8 @@ std::chrono::steady_clock::duration MGLDurationInSeconds(float duration)
// add sprite
auto cSpriteImage = std::make_shared<mbgl::SpriteImage>(
- uint16_t(annotationImage.image.size.width),
- uint16_t(annotationImage.image.size.height),
+ uint16_t(width),
+ uint16_t(height),
float(annotationImage.image.scale),
std::move(pixels));
diff --git a/platform/osx/src/MGLMapView.mm b/platform/osx/src/MGLMapView.mm
index 302a9e8c44..596b909fa2 100644
--- a/platform/osx/src/MGLMapView.mm
+++ b/platform/osx/src/MGLMapView.mm
@@ -1590,8 +1590,8 @@ public:
// Get the image’s raw pixel data as an RGBA buffer.
std::string pixelString((const char *)rep.bitmapData, rep.pixelsWide * rep.pixelsHigh * 4 /* RGBA */);
- auto cSpriteImage = std::make_shared<mbgl::SpriteImage>((uint16_t)rep.size.width,
- (uint16_t)rep.size.height,
+ auto cSpriteImage = std::make_shared<mbgl::SpriteImage>((uint16_t)rep.pixelsWide,
+ (uint16_t)rep.pixelsHigh,
(float)(rep.pixelsWide / size.width),
std::move(pixelString));
NSString *symbolName = [MGLAnnotationSpritePrefix stringByAppendingString:annotationImage.reuseIdentifier];
diff --git a/src/mbgl/sprite/sprite_image.cpp b/src/mbgl/sprite/sprite_image.cpp
index d5e4a7828e..e55d676b34 100644
--- a/src/mbgl/sprite/sprite_image.cpp
+++ b/src/mbgl/sprite/sprite_image.cpp
@@ -6,16 +6,16 @@
namespace mbgl {
-SpriteImage::SpriteImage(const uint16_t width_,
- const uint16_t height_,
+SpriteImage::SpriteImage(const uint16_t pixelWidth_,
+ const uint16_t pixelHeight_,
const float pixelRatio_,
std::string&& data_,
bool sdf_)
- : width(width_),
- height(height_),
+ : width(std::ceil(pixelWidth_ / pixelRatio_)),
+ height(std::ceil(pixelHeight_ / pixelRatio_)),
pixelRatio(pixelRatio_),
- pixelWidth(std::ceil(width * pixelRatio)),
- pixelHeight(std::ceil(height * pixelRatio)),
+ pixelWidth(pixelWidth_),
+ pixelHeight(pixelHeight_),
data(std::move(data_)),
sdf(sdf_) {
const size_t size = pixelWidth * pixelHeight * 4;
diff --git a/src/mbgl/sprite/sprite_parser.cpp b/src/mbgl/sprite/sprite_parser.cpp
index 1aa95310c5..6942d009f3 100644
--- a/src/mbgl/sprite/sprite_parser.cpp
+++ b/src/mbgl/sprite/sprite_parser.cpp
@@ -15,37 +15,30 @@ namespace mbgl {
SpriteImagePtr createSpriteImage(const PremultipliedImage& image,
const uint16_t srcX,
const uint16_t srcY,
- const uint16_t srcWidth,
- const uint16_t srcHeight,
+ const uint16_t width,
+ const uint16_t height,
const double ratio,
const bool sdf) {
// Disallow invalid parameter configurations.
- if (srcWidth == 0 || srcHeight == 0 || ratio <= 0 || ratio > 10 || srcWidth > 1024 ||
- srcHeight > 1024) {
+ if (width == 0 || height == 0 || ratio <= 0 || ratio > 10 || width > 1024 ||
+ height > 1024) {
Log::Warning(Event::Sprite, "Can't create sprite with invalid metrics");
return nullptr;
}
- const uint16_t width = std::ceil(double(srcWidth) / ratio);
- const uint16_t dstWidth = std::ceil(width * ratio);
- assert(dstWidth >= srcWidth);
- const uint16_t height = std::ceil(double(srcHeight) / ratio);
- const uint16_t dstHeight = std::ceil(height * ratio);
- assert(dstHeight >= srcHeight);
-
- std::string data(dstWidth * dstHeight * 4, '\0');
+ std::string data(width * height * 4, '\0');
auto srcData = reinterpret_cast<const uint32_t*>(image.data.get());
auto dstData = reinterpret_cast<uint32_t*>(const_cast<char*>(data.data()));
- const int32_t maxX = std::min(uint32_t(image.width), uint32_t(srcWidth + srcX)) - srcX;
+ const int32_t maxX = std::min(uint32_t(image.width), uint32_t(width + srcX)) - srcX;
assert(maxX <= int32_t(image.width));
- const int32_t maxY = std::min(uint32_t(image.height), uint32_t(srcHeight + srcY)) - srcY;
+ const int32_t maxY = std::min(uint32_t(image.height), uint32_t(height + srcY)) - srcY;
assert(maxY <= int32_t(image.height));
// Copy from the source image into our individual sprite image
for (uint16_t y = 0; y < maxY; ++y) {
- const auto dstRow = y * dstWidth;
+ const auto dstRow = y * width;
const auto srcRow = (y + srcY) * image.width + srcX;
for (uint16_t x = 0; x < maxX; ++x) {
dstData[dstRow + x] = srcData[srcRow + x];
diff --git a/test/sprite/sprite_image.cpp b/test/sprite/sprite_image.cpp
index 8bc88fcc84..1ed4e56ff6 100644
--- a/test/sprite/sprite_image.cpp
+++ b/test/sprite/sprite_image.cpp
@@ -28,7 +28,7 @@ TEST(Sprite, SpriteImageZeroRatio) {
SpriteImage(16, 16, 0, "");
FAIL() << "Expected exception";
} catch (util::SpriteImageException& ex) {
- EXPECT_STREQ("Sprite image dimensions may not be zero", ex.what());
+ EXPECT_STREQ("Sprite image pixel count mismatch", ex.what());
}
}
@@ -43,7 +43,7 @@ TEST(Sprite, SpriteImageMismatchedData) {
TEST(Sprite, SpriteImage) {
std::string pixels(32 * 24 * 4, '\0');
- SpriteImage sprite(16, 12, 2, std::move(pixels));
+ SpriteImage sprite(32, 24, 2, std::move(pixels));
EXPECT_EQ(16, sprite.width);
EXPECT_EQ(32, sprite.pixelWidth);
EXPECT_EQ(12, sprite.height);
@@ -54,8 +54,8 @@ TEST(Sprite, SpriteImage) {
TEST(Sprite, SpriteImageFractionalRatio) {
std::string pixels(20 * 12 * 4, '\0');
- SpriteImage sprite(13, 8, 1.5, std::move(pixels));
- EXPECT_EQ(13, sprite.width);
+ SpriteImage sprite(20, 12, 1.5, std::move(pixels));
+ EXPECT_EQ(14, sprite.width);
EXPECT_EQ(20, sprite.pixelWidth);
EXPECT_EQ(8, sprite.height);
EXPECT_EQ(12, sprite.pixelHeight);
diff --git a/test/sprite/sprite_parser.cpp b/test/sprite/sprite_parser.cpp
index 0eb298b23e..9eb19a8095 100644
--- a/test/sprite/sprite_parser.cpp
+++ b/test/sprite/sprite_parser.cpp
@@ -106,10 +106,10 @@ TEST(Sprite, SpriteImageCreation1_5x) {
ASSERT_TRUE(sprite2.get());
EXPECT_EQ(24, sprite2->width);
EXPECT_EQ(24, sprite2->height);
- EXPECT_EQ(36, sprite2->pixelWidth);
- EXPECT_EQ(36, sprite2->pixelHeight);
+ EXPECT_EQ(35, sprite2->pixelWidth);
+ EXPECT_EQ(35, sprite2->pixelHeight);
EXPECT_EQ(1.5, sprite2->pixelRatio);
- EXPECT_EQ(0x134A530C742DD141u, test::crc64(sprite2->data));
+ EXPECT_EQ(14312995667113444493u, test::crc64(sprite2->data));
}
TEST(Sprite, SpriteParsing) {
diff --git a/test/sprite/sprite_store.cpp b/test/sprite/sprite_store.cpp
index 8f9aebbe67..5056b89cb7 100644
--- a/test/sprite/sprite_store.cpp
+++ b/test/sprite/sprite_store.cpp
@@ -13,9 +13,9 @@ using namespace mbgl;
TEST(SpriteStore, SpriteStore) {
FixtureLog log;
- const auto sprite1 = std::make_shared<SpriteImage>(8, 8, 2, std::string(16 * 16 * 4, '\0'));
- const auto sprite2 = std::make_shared<SpriteImage>(8, 8, 2, std::string(16 * 16 * 4, '\0'));
- const auto sprite3 = std::make_shared<SpriteImage>(8, 8, 2, std::string(16 * 16 * 4, '\0'));
+ const auto sprite1 = std::make_shared<SpriteImage>(16, 16, 2, std::string(16 * 16 * 4, '\0'));
+ const auto sprite2 = std::make_shared<SpriteImage>(16, 16, 2, std::string(16 * 16 * 4, '\0'));
+ const auto sprite3 = std::make_shared<SpriteImage>(16, 16, 2, std::string(16 * 16 * 4, '\0'));
using Sprites = std::map<std::string, std::shared_ptr<const SpriteImage>>;
SpriteStore store(1);
@@ -90,8 +90,8 @@ TEST(SpriteStore, OtherPixelRatio) {
}
TEST(SpriteStore, Multiple) {
- const auto sprite1 = std::make_shared<SpriteImage>(8, 8, 2, std::string(16 * 16 * 4, '\0'));
- const auto sprite2 = std::make_shared<SpriteImage>(8, 8, 2, std::string(16 * 16 * 4, '\0'));
+ const auto sprite1 = std::make_shared<SpriteImage>(16, 16, 2, std::string(16 * 16 * 4, '\0'));
+ const auto sprite2 = std::make_shared<SpriteImage>(16, 16, 2, std::string(16 * 16 * 4, '\0'));
using Sprites = std::map<std::string, std::shared_ptr<const SpriteImage>>;
SpriteStore store(1);
@@ -109,8 +109,8 @@ TEST(SpriteStore, Multiple) {
TEST(SpriteStore, Replace) {
FixtureLog log;
- const auto sprite1 = std::make_shared<SpriteImage>(8, 8, 2, std::string(16 * 16 * 4, '\0'));
- const auto sprite2 = std::make_shared<SpriteImage>(8, 8, 2, std::string(16 * 16 * 4, '\0'));
+ const auto sprite1 = std::make_shared<SpriteImage>(16, 16, 2, std::string(16 * 16 * 4, '\0'));
+ const auto sprite2 = std::make_shared<SpriteImage>(16, 16, 2, std::string(16 * 16 * 4, '\0'));
using Sprites = std::map<std::string, std::shared_ptr<const SpriteImage>>;
SpriteStore store(1);
@@ -126,8 +126,8 @@ TEST(SpriteStore, Replace) {
TEST(SpriteStore, ReplaceWithDifferentDimensions) {
FixtureLog log;
- const auto sprite1 = std::make_shared<SpriteImage>(8, 8, 2, std::string(16 * 16 * 4, '\0'));
- const auto sprite2 = std::make_shared<SpriteImage>(9, 9, 2, std::string(18 * 18 * 4, '\0'));
+ const auto sprite1 = std::make_shared<SpriteImage>(16, 16, 2, std::string(16 * 16 * 4, '\0'));
+ const auto sprite2 = std::make_shared<SpriteImage>(18, 18, 2, std::string(18 * 18 * 4, '\0'));
using Sprites = std::map<std::string, std::shared_ptr<const SpriteImage>>;
SpriteStore store(1);