[core][ios][osx][android] make SpriteImage accept PremultipliedImage

the SpriteImage constructor signature changes from SpriteImage( uint16_t width, uint16_t height, float pixelRatio, std::string&& data, bool sdf = false); to SpriteImage(PremultipliedImage&&, float pixelRatio, bool sdf = false)
mapbox · Jan 20, 2016 · d34f8eb · d34f8eb
1 parent 26faa6a
commit d34f8eb
Show file tree

Hide file tree

Showing 20 changed files with 160 additions and 168 deletions.
diff --git a/include/mbgl/sprite/sprite_image.hpp b/include/mbgl/sprite/sprite_image.hpp
@@ -3,6 +3,7 @@
 
 #include <mbgl/util/noncopyable.hpp>
 #include <mbgl/util/geo.hpp>
+#include <mbgl/util/image.hpp>
 
 #include <string>
 #include <memory>
@@ -12,27 +13,18 @@ namespace mbgl {
 
 class SpriteImage : private util::noncopyable {
 public:
-    SpriteImage(
-        uint16_t width, uint16_t height, float pixelRatio, std::string&& data, bool sdf = false);
+    SpriteImage(PremultipliedImage&&, float pixelRatio, bool sdf = false);
 
-    // Logical dimensions of the sprite image.
-    const uint16_t width;
-    const uint16_t height;
+    PremultipliedImage image;
 
     // Pixel ratio of the sprite image.
     const float pixelRatio;
 
-    // Physical dimensions of the sprite image.
-    const uint16_t pixelWidth;
-    const uint16_t pixelHeight;
-
-    // A string of an RGBA8 representation of the sprite. It must have exactly
-    // (width * ratio) * (height * ratio) * 4 (RGBA) bytes. The scan lines may
-    // not have gaps between them (i.e. stride == 0).
-    const std::string data;
-
     // Whether this image should be interpreted as a signed distance field icon.
     const bool sdf;
+
+    float getWidth() const { return image.width / pixelRatio; }
+    float getHeight() const { return image.height / pixelRatio; }
 };
 
 } // namespace mbgl

diff --git a/platform/android/src/jni.cpp b/platform/android/src/jni.cpp
@@ -24,6 +24,7 @@
 #include <mbgl/platform/event.hpp>
 #include <mbgl/platform/log.hpp>
 #include <mbgl/storage/network_status.hpp>
+#include <mbgl/util/exception.hpp>
 
 #pragma clang diagnostic ignored "-Wunused-parameter"
 
@@ -1225,16 +1226,18 @@ void JNICALL nativeAddAnnotationIcon(JNIEnv *env, jobject obj, jlong nativeMapVi
 
     const std::string symbolName = std_string_from_jstring(env, symbol);
 
-    jbyte* pixelData = env->GetByteArrayElements(jpixels, nullptr);
     jsize size = env->GetArrayLength(jpixels);
-    std::string pixels(reinterpret_cast<char*>(pixelData), size);
-    env->ReleaseByteArrayElements(jpixels, pixelData, JNI_ABORT);
+    mbgl::PremultipliedImage premultipliedImage(width, height);
+
+    if (premultipliedImage.size() != uint32_t(size)) {
+        throw mbgl::util::SpriteImageException("Sprite image pixel count mismatch");
+    }
+
+    env->GetByteArrayRegion(jpixels, 0, size, reinterpret_cast<jbyte*>(premultipliedImage.data.get()));
 
     auto iconImage = std::make_shared<mbgl::SpriteImage>(
-        uint16_t(width),
-        uint16_t(height),
-        float(scale),
-        std::move(pixels));
+        std::move(premultipliedImage),
+        float(scale));
 
     nativeMapView->getMap().addAnnotationIcon(symbolName, iconImage);
 }

diff --git a/platform/default/glfw_view.cpp b/platform/default/glfw_view.cpp
@@ -200,8 +200,8 @@ GLFWView::makeSpriteImage(int width, int height, float pixelRatio) {
     const int w = std::ceil(pixelRatio * width);
     const int h = std::ceil(pixelRatio * height);
 
-    std::string pixels(w * h * 4, '\x00');
-    auto data = reinterpret_cast<uint32_t*>(const_cast<char*>(pixels.data()));
+    mbgl::PremultipliedImage image(w, h);
+    auto data = reinterpret_cast<uint32_t*>(image.data.get());
     const int dist = (w / 2) * (w / 2);
     for (int y = 0; y < h; y++) {
         for (int x = 0; x < w; x++) {
@@ -217,7 +217,7 @@ GLFWView::makeSpriteImage(int width, int height, float pixelRatio) {
         }
     }
 
-    return std::make_shared<mbgl::SpriteImage>(width, height, pixelRatio, std::move(pixels));
+    return std::make_shared<mbgl::SpriteImage>(std::move(image), pixelRatio);
 }
 
 void GLFWView::nextOrientation() {

diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm
@@ -2334,22 +2334,19 @@ - (void)installAnnotationImage:(MGLAnnotationImage *)annotationImage
     size_t width = CGImageGetWidth(image);
     size_t height = CGImageGetHeight(image);
     CGColorSpaceRef colorSpace = CGColorSpaceCreateDeviceRGB();
-    std::string pixels(width * height * 4, '\0');
+    mbgl::PremultipliedImage cPremultipliedImage(width, height);
     size_t bytesPerPixel = 4;
     size_t bytesPerRow = bytesPerPixel * width;
     size_t bitsPerComponent = 8;
-    char *pixelData = const_cast<char *>(pixels.data());
-    CGContextRef context = CGBitmapContextCreate(pixelData, width, height, bitsPerComponent, bytesPerRow, colorSpace, kCGImageAlphaPremultipliedLast);
+    CGContextRef context = CGBitmapContextCreate(cPremultipliedImage.data.get(), width, height, bitsPerComponent, bytesPerRow, colorSpace, kCGImageAlphaPremultipliedLast);
     CGContextDrawImage(context, CGRectMake(0, 0, width, height), image);
     CGContextRelease(context);
     CGColorSpaceRelease(colorSpace);
 
     // add sprite
     auto cSpriteImage = std::make_shared<mbgl::SpriteImage>(
-        uint16_t(width),
-        uint16_t(height),
-        float(annotationImage.image.scale),
-        std::move(pixels));
+        std::move(cPremultipliedImage), 
+        float(annotationImage.image.scale));
 
     // sprite upload
     NSString *symbolName = [MGLAnnotationSpritePrefix stringByAppendingString:annotationImage.reuseIdentifier];

diff --git a/platform/osx/src/MGLMapView.mm b/platform/osx/src/MGLMapView.mm
@@ -1590,10 +1590,11 @@ - (void)installAnnotationImage:(MGLAnnotationImage *)annotationImage {
 
     // 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.pixelsWide,
-                                                            (uint16_t)rep.pixelsHigh,
-                                                            (float)(rep.pixelsWide / size.width),
-                                                            std::move(pixelString));
+
+    mbgl::PremultipliedImage cPremultipliedImage(rep.pixelsWide, rep.pixelsHigh);
+    std::copy(rep.bitmapData, rep.bitmapData + cPremultipliedImage.size(), cPremultipliedImage.data.get());
+    auto cSpriteImage = std::make_shared<mbgl::SpriteImage>(std::move(cPremultipliedImage),
+                                                            (float)(rep.pixelsWide / size.width));
     NSString *symbolName = [MGLAnnotationSpritePrefix stringByAppendingString:annotationImage.reuseIdentifier];
     _mbglMap->addAnnotationIcon(symbolName.UTF8String, cSpriteImage);
 

diff --git a/src/mbgl/annotation/annotation_manager.cpp b/src/mbgl/annotation/annotation_manager.cpp
@@ -165,7 +165,7 @@ void AnnotationManager::removeIcon(const std::string& name) {
 
 double AnnotationManager::getTopOffsetPixelsForIcon(const std::string& name) {
     auto sprite = spriteStore.getSprite(name);
-    return sprite ? -sprite->height / 2 : 0;
+    return sprite ? -(sprite->image.height / sprite->pixelRatio) / 2 : 0;
 }
 
 } // namespace mbgl
diff --git a/src/mbgl/renderer/symbol_bucket.cpp b/src/mbgl/renderer/symbol_bucket.cpp
@@ -248,8 +248,8 @@ void SymbolBucket::addFeatures(uintptr_t tileUID,
             auto image = spriteAtlas.getImage(feature.sprite, false);
             if (image) {
                 shapedIcon = shapeIcon(*image, layout);
-                assert((*image).texture);
-                if ((*image).texture->sdf) {
+                assert((*image).spriteImage);
+                if ((*image).spriteImage->sdf) {
                     sdfIcons = true;
                 }
                 if ((*image).relativePixelRatio != 1.0f) {

diff --git a/src/mbgl/sprite/sprite_atlas.cpp b/src/mbgl/sprite/sprite_atlas.cpp
@@ -26,10 +26,10 @@ SpriteAtlas::SpriteAtlas(dimension width_, dimension height_, float pixelRatio_,
       dirty(true) {
 }
 
-Rect<SpriteAtlas::dimension> SpriteAtlas::allocateImage(float src_width, float src_height) {
+Rect<SpriteAtlas::dimension> SpriteAtlas::allocateImage(const SpriteImage& spriteImage) {
 
-    const uint16_t pixel_width = std::ceil(src_width / pixelRatio);
-    const uint16_t pixel_height = std::ceil(src_height / pixelRatio);
+    const uint16_t pixel_width = std::ceil(spriteImage.image.width / pixelRatio);
+    const uint16_t pixel_height = std::ceil(spriteImage.image.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
@@ -52,15 +52,15 @@ optional<SpriteAtlasElement> SpriteAtlas::getImage(const std::string& name, cons
 
     auto rect_it = images.find({ name, wrap });
     if (rect_it != images.end()) {
-        return SpriteAtlasElement { rect_it->second.pos, rect_it->second.texture, rect_it->second.texture->pixelRatio / pixelRatio };
+        return SpriteAtlasElement { rect_it->second.pos, rect_it->second.spriteImage, rect_it->second.spriteImage->pixelRatio / pixelRatio };
     }
 
     auto sprite = store.getSprite(name);
     if (!sprite) {
         return {};
     }
 
-    Rect<dimension> rect = allocateImage(sprite->width * sprite->pixelRatio, sprite->height * sprite->pixelRatio);
+    Rect<dimension> rect = allocateImage(*sprite);
     if (rect.w == 0) {
         if (debug::spriteWarnings) {
             Log::Warning(Event::Sprite, "sprite atlas bitmap overflow");
@@ -85,13 +85,13 @@ optional<SpriteAtlasPosition> SpriteAtlas::getPosition(const std::string& name,
     auto rect = (*img).pos;
 
     const float padding = 1;
-    auto image = (*img).texture;
+    auto spriteImage = (*img).spriteImage;
 
-    const float w = image->width * (*img).relativePixelRatio;
-    const float h = image->height * (*img).relativePixelRatio;
+    const float w = spriteImage->getWidth() * (*img).relativePixelRatio;
+    const float h = spriteImage->getHeight() * (*img).relativePixelRatio;
 
     return SpriteAtlasPosition {
-        {{ float(image->width), float(image->height) }},
+        {{ float(spriteImage->getWidth()), spriteImage->getHeight() }},
         {{ float(rect.x + padding)     / width, float(rect.y + padding)     / height }},
         {{ float(rect.x + padding + w) / width, float(rect.y + padding + h) / height }}
     };
@@ -131,15 +131,15 @@ void SpriteAtlas::copy(const Holder& holder, const bool wrap) {
         std::fill(data.get(), data.get() + pixelWidth * pixelHeight, 0);
     }
 
-    const uint32_t *srcData = reinterpret_cast<const uint32_t *>(holder.texture->data.data());
+    const uint32_t *srcData = reinterpret_cast<const uint32_t *>(holder.spriteImage->image.data.get());
     if (!srcData) return;
     uint32_t *const dstData = data.get();
 
     const int padding = 1;
 
-    copyBitmap(srcData, holder.texture->pixelWidth, 0, 0,
+    copyBitmap(srcData, uint32_t(holder.spriteImage->image.width), 0, 0,
             dstData, pixelWidth, (holder.pos.x + padding) * pixelRatio, (holder.pos.y + padding) * pixelRatio, pixelWidth * pixelHeight,
-            holder.texture->pixelWidth, holder.texture->pixelHeight, wrap);
+            uint32_t(holder.spriteImage->image.width), uint32_t(holder.spriteImage->image.height), wrap);
 
     dirty = true;
 }
@@ -168,8 +168,8 @@ void SpriteAtlas::updateDirty() {
         } else {
             // The two names match;
             Holder& holder = imageIterator->second;
-            holder.texture = spriteIterator->second;
-            if (holder.texture != nullptr) {
+            holder.spriteImage = spriteIterator->second;
+            if (holder.spriteImage != nullptr) {
                 copy(holder, imageIterator->first.second);
                 ++imageIterator;
             } else {
@@ -255,10 +255,10 @@ SpriteAtlas::~SpriteAtlas() {
     }
 }
 
-SpriteAtlas::Holder::Holder(const std::shared_ptr<const SpriteImage>& texture_,
+SpriteAtlas::Holder::Holder(const std::shared_ptr<const SpriteImage>& spriteImage_,
                             const Rect<dimension>& pos_)
-    : texture(texture_), pos(pos_) {
+    : spriteImage(spriteImage_), pos(pos_) {
 }
 
-SpriteAtlas::Holder::Holder(Holder&& h) : texture(std::move(h.texture)), pos(h.pos) {
+SpriteAtlas::Holder::Holder(Holder&& h) : spriteImage(std::move(h.spriteImage)), pos(h.pos) {
 }
diff --git a/src/mbgl/sprite/sprite_atlas.hpp b/src/mbgl/sprite/sprite_atlas.hpp
@@ -32,7 +32,7 @@ struct SpriteAtlasPosition {
 
 struct SpriteAtlasElement {
     Rect<uint16_t> pos;
-    std::shared_ptr<const SpriteImage> texture;
+    std::shared_ptr<const SpriteImage> spriteImage;
     float relativePixelRatio;
 };
 
@@ -77,13 +77,13 @@ class SpriteAtlas : public util::noncopyable {
     struct Holder : private util::noncopyable {
         inline Holder(const std::shared_ptr<const SpriteImage>&, const Rect<dimension>&);
         inline Holder(Holder&&);
-        std::shared_ptr<const SpriteImage> texture;
+        std::shared_ptr<const SpriteImage> spriteImage;
         const Rect<dimension> pos;
     };
 
     using Key = std::pair<std::string, bool>;
 
-    Rect<SpriteAtlas::dimension> allocateImage(float width, float height);
+    Rect<SpriteAtlas::dimension> allocateImage(const SpriteImage&);
     void copy(const Holder& holder, const bool wrap);
 
     std::recursive_mutex mtx;

diff --git a/src/mbgl/sprite/sprite_image.cpp b/src/mbgl/sprite/sprite_image.cpp
@@ -6,23 +6,17 @@
 
 namespace mbgl {
 
-SpriteImage::SpriteImage(const uint16_t pixelWidth_,
-                         const uint16_t pixelHeight_,
+SpriteImage::SpriteImage(PremultipliedImage&& image_,
                          const float pixelRatio_,
-                         std::string&& data_,
                          bool sdf_)
-    : width(std::ceil(pixelWidth_ / pixelRatio_)),
-      height(std::ceil(pixelHeight_ / pixelRatio_)),
+    : image(std::move(image_)),
       pixelRatio(pixelRatio_),
-      pixelWidth(pixelWidth_),
-      pixelHeight(pixelHeight_),
-      data(std::move(data_)),
       sdf(sdf_) {
-    const size_t size = pixelWidth * pixelHeight * 4;
-    if (size == 0) {
+
+    if (image.size() == 0) {
         throw util::SpriteImageException("Sprite image dimensions may not be zero");
-    } else if (size != data.size()) {
-        throw util::SpriteImageException("Sprite image pixel count mismatch");
+    } else if (pixelRatio <= 0) {
+        throw util::SpriteImageException("Sprite pixelRatio may not be <= 0");
     }
 }
 

diff --git a/src/mbgl/sprite/sprite_parser.cpp b/src/mbgl/sprite/sprite_parser.cpp
@@ -26,10 +26,10 @@ SpriteImagePtr createSpriteImage(const PremultipliedImage& image,
         return nullptr;
     }
 
-    std::string data(width * height * 4, '\0');
+    PremultipliedImage dstImage(width, height);
 
     auto srcData = reinterpret_cast<const uint32_t*>(image.data.get());
-    auto dstData = reinterpret_cast<uint32_t*>(const_cast<char*>(data.data()));
+    auto dstData = reinterpret_cast<uint32_t*>(dstImage.data.get());
 
     const int32_t maxX = std::min(uint32_t(image.width), uint32_t(width + srcX)) - srcX;
     assert(maxX <= int32_t(image.width));
@@ -45,7 +45,7 @@ SpriteImagePtr createSpriteImage(const PremultipliedImage& image,
         }
     }
 
-    return std::make_unique<const SpriteImage>(width, height, ratio, std::move(data), sdf);
+    return std::make_unique<const SpriteImage>(std::move(dstImage), ratio, sdf);
 }
 
 namespace {

diff --git a/src/mbgl/sprite/sprite_store.cpp b/src/mbgl/sprite/sprite_store.cpp
@@ -113,7 +113,7 @@ void SpriteStore::_setSprite(const std::string& name,
         auto it = sprites.find(name);
         if (it != sprites.end()) {
             // There is already a sprite with that name in our store.
-            if ((it->second->width != sprite->width || it->second->height != sprite->height)) {
+            if ((it->second->image.width != sprite->image.width || it->second->image.height != sprite->image.height)) {
                 Log::Warning(Event::Sprite, "Can't change sprite dimensions for '%s'", name.c_str());
                 return;
             }

diff --git a/src/mbgl/text/shaping.cpp b/src/mbgl/text/shaping.cpp
@@ -6,10 +6,10 @@ namespace mbgl {
 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.texture->width / 2.0f;
-    float x2 = x1 + image.texture->width;
-    float y1 = dy - image.texture->height / 2.0f;
-    float y2 = y1 + image.texture->height;
+    float x1 = dx - image.spriteImage->getWidth() / 2.0f;
+    float x2 = x1 + image.spriteImage->getWidth();
+    float y1 = dy - image.spriteImage->getHeight() / 2.0f;
+    float y2 = y1 + image.spriteImage->getHeight();
 
     return PositionedIcon(image, y1, y2, x1, x2);
 }

diff --git a/test/api/annotations.cpp b/test/api/annotations.cpp
@@ -16,7 +16,7 @@ using namespace mbgl;
 
 std::shared_ptr<SpriteImage> namedMarker(const std::string &name) {
     PremultipliedImage image = decodeImage(util::read_file("test/fixtures/sprites/" + name));
-    return std::make_shared<SpriteImage>(image.width, image.height, 1.0, std::string(reinterpret_cast<char*>(image.data.get()), image.size()));
+    return std::make_shared<SpriteImage>(std::move(image), 1.0);
 }
 
 namespace {

diff --git a/test/fixtures/util.cpp b/test/fixtures/util.cpp
@@ -89,6 +89,7 @@ Server::~Server() {
     }
 }
 
+
 // from https://gist.github.com/ArtemGr/997887
 uint64_t crc64(const char* data, size_t size) {
     boost::crc_optimal<64, 0x04C11DB7, 0, 0, false, false> crc;
@@ -100,6 +101,10 @@ uint64_t crc64(const std::string& str) {
     return crc64(str.data(), str.size());
 }
 
+uint64_t crc64(const PremultipliedImage &image) {
+    return crc64(reinterpret_cast<const char*>(image.data.get()), image.size());
+}
+
 PremultipliedImage render(Map& map, std::chrono::milliseconds timeout) {
     std::promise<PremultipliedImage> promise;
     map.renderStill([&](std::exception_ptr, PremultipliedImage&& image) {

diff --git a/test/fixtures/util.hpp b/test/fixtures/util.hpp
@@ -33,6 +33,7 @@ class Server {
 
 uint64_t crc64(const char*, size_t);
 uint64_t crc64(const std::string&);
+uint64_t crc64(const PremultipliedImage&);
 
 PremultipliedImage render(Map&,
                           std::chrono::milliseconds timeout = std::chrono::milliseconds(1000));