[macos,ios] don't roundtrip through encodePNG when converting images

9 files changed, 129 insertions, 126 deletions

diff --git a/platform/darwin/mbgl/util/image+MGLAdditions.hpp b/platform/darwin/mbgl/util/image+MGLAdditions.hpp
new file mode 100644
index 0000000000..c738b4523d
--- /dev/null
+++ b/platform/darwin/mbgl/util/image+MGLAdditions.hpp
@@ -0,0 +1,12 @@
+#pragma once
+
+#include <mbgl/util/image.hpp>
+
+#include <CoreGraphics/CGImage.h>
+
+// Creates a CGImage from a PremultipliedImage, taking over the memory ownership.
+CGImageRef CGImageFromMGLPremultipliedImage(mbgl::PremultipliedImage&&);
+
+// Creates a PremultipliedImage by copying the pixels of the CGImage.
+// Does not alter the retain count of the supplied CGImage.
+mbgl::PremultipliedImage MGLPremultipliedImageFromCGImage(CGImageRef);
diff --git a/platform/darwin/src/image.mm b/platform/darwin/src/image.mm
index 3a707d4a36..57b680fbdb 100644
--- a/platform/darwin/src/image.mm
+++ b/platform/darwin/src/image.mm
@@ -1,126 +1,107 @@
-#include <mbgl/util/image.hpp>
+#include <mbgl/util/image+MGLAdditions.hpp>
 
 #import <ImageIO/ImageIO.h>
 
-#if TARGET_OS_IPHONE
-#import <MobileCoreServices/MobileCoreServices.h>
-#else
-#import <CoreServices/CoreServices.h>
-#endif
-
-namespace mbgl {
-
-std::string encodePNG(const PremultipliedImage& src) {
-    CGDataProviderRef provider = CGDataProviderCreateWithData(NULL, src.data.get(), src.bytes(), NULL);
+namespace {
+
+template <typename T, typename S, void (*Releaser)(S)>
+struct CFHandle {
+    CFHandle(T t_): t(t_) {}
+    ~CFHandle() { Releaser(t); }
+    T operator*() { return t; }
+    operator bool() { return t; }
+private:
+    T t;
+};
+
+} // namespace
+
+using CGImageHandle = CFHandle<CGImageRef, CGImageRef, CGImageRelease>;
+using CFDataHandle = CFHandle<CFDataRef, CFTypeRef, CFRelease>;
+using CGImageSourceHandle = CFHandle<CGImageSourceRef, CFTypeRef, CFRelease>;
+using CGDataProviderHandle = CFHandle<CGDataProviderRef, CGDataProviderRef, CGDataProviderRelease>;
+using CGColorSpaceHandle = CFHandle<CGColorSpaceRef, CGColorSpaceRef, CGColorSpaceRelease>;
+using CGContextHandle = CFHandle<CGContextRef, CGContextRef, CGContextRelease>;
+
+CGImageRef CGImageFromMGLPremultipliedImage(mbgl::PremultipliedImage&& src) {
+    // We're converting the PremultipliedImage's backing store to a CGDataProvider, and are taking
+    // over ownership of the memory.
+    CGDataProviderHandle provider(CGDataProviderCreateWithData(
+        NULL, src.data.get(), src.bytes(), [](void*, const void* data, size_t) {
+            delete[] reinterpret_cast<const decltype(src.data)::element_type*>(data);
+        }));
     if (!provider) {
-        return "";
+        return nil;
     }
 
-    CGColorSpaceRef color_space = CGColorSpaceCreateDeviceRGB();
-    if (!color_space) {
-        CGDataProviderRelease(provider);
-        return "";
-    }
+    // If we successfully created the provider, it will take over management of the memory segment.
+    src.data.release();
 
-    CGImageRef image =
-        CGImageCreate(src.size.width, src.size.height, 8, 32, 4 * src.size.width, color_space,
-                      kCGBitmapByteOrderDefault | kCGImageAlphaPremultipliedLast, provider, NULL,
-                      false, kCGRenderingIntentDefault);
-    if (!image) {
-        CGColorSpaceRelease(color_space);
-        CGDataProviderRelease(provider);
-        return "";
+    CGColorSpaceHandle colorSpace(CGColorSpaceCreateDeviceRGB());
+    if (!colorSpace) {
+        return nil;
     }
 
-    CFMutableDataRef data = CFDataCreateMutable(kCFAllocatorDefault, 0);
-    if (!data) {
-        CGImageRelease(image);
-        CGColorSpaceRelease(color_space);
-        CGDataProviderRelease(provider);
-        return "";
-    }
+    constexpr const size_t bitsPerComponent = 8;
+    constexpr const size_t bytesPerPixel = 4;
+    constexpr const size_t bitsPerPixel = bitsPerComponent * bytesPerPixel;
+    const size_t bytesPerRow = bytesPerPixel * src.size.width;
 
-    CGImageDestinationRef image_destination = CGImageDestinationCreateWithData(data, kUTTypePNG, 1, NULL);
-    if (!image_destination) {
-        CFRelease(data);
-        CGImageRelease(image);
-        CGColorSpaceRelease(color_space);
-        CGDataProviderRelease(provider);
-        return "";
+    return CGImageCreate(src.size.width, src.size.height, bitsPerComponent, bitsPerPixel,
+                         bytesPerRow, *colorSpace,
+                         kCGBitmapByteOrderDefault | kCGImageAlphaPremultipliedLast, *provider,
+                         NULL, false, kCGRenderingIntentDefault);
+}
+
+mbgl::PremultipliedImage MGLPremultipliedImageFromCGImage(CGImageRef src) {
+    const size_t width = CGImageGetWidth(src);
+    const size_t height = CGImageGetHeight(src);
+
+    mbgl::PremultipliedImage image({ static_cast<uint32_t>(width), static_cast<uint32_t>(height) });
+
+    CGColorSpaceHandle colorSpace(CGColorSpaceCreateDeviceRGB());
+    if (!colorSpace) {
+        throw std::runtime_error("CGColorSpaceCreateDeviceRGB failed");
     }
 
-    CGImageDestinationAddImage(image_destination, image, NULL);
-    CGImageDestinationFinalize(image_destination);
+    constexpr const size_t bitsPerComponent = 8;
+    constexpr const size_t bytesPerPixel = 4;
+    const size_t bytesPerRow = bytesPerPixel * width;
 
-    const std::string result {
-        reinterpret_cast<const char *>(CFDataGetBytePtr(data)),
-        static_cast<size_t>(CFDataGetLength(data))
-    };
+    CGContextHandle context(CGBitmapContextCreate(
+        image.data.get(), width, height, bitsPerComponent, bytesPerRow, *colorSpace,
+        kCGBitmapByteOrderDefault | kCGImageAlphaPremultipliedLast));
+    if (!context) {
+        throw std::runtime_error("CGBitmapContextCreate failed");
+    }
 
-    CFRelease(image_destination);
-    CFRelease(data);
-    CGImageRelease(image);
-    CGColorSpaceRelease(color_space);
-    CGDataProviderRelease(provider);
+    CGContextSetBlendMode(*context, kCGBlendModeCopy);
+    CGContextDrawImage(*context, CGRectMake(0, 0, width, height), src);
 
-    return result;
+    return image;
 }
 
-PremultipliedImage decodeImage(const std::string &source_data) {
-    CFDataRef data = CFDataCreateWithBytesNoCopy(kCFAllocatorDefault, reinterpret_cast<const unsigned char *>(source_data.data()), source_data.size(), kCFAllocatorNull);
+namespace mbgl {
+
+PremultipliedImage decodeImage(const std::string& source) {
+    CFDataHandle data(CFDataCreateWithBytesNoCopy(
+        kCFAllocatorDefault, reinterpret_cast<const unsigned char*>(source.data()), source.size(),
+        kCFAllocatorNull));
     if (!data) {
         throw std::runtime_error("CFDataCreateWithBytesNoCopy failed");
     }
 
-    CGImageSourceRef image_source = CGImageSourceCreateWithData(data, NULL);
-    if (!image_source) {
-        CFRelease(data);
+    CGImageSourceHandle imageSource(CGImageSourceCreateWithData(*data, NULL));
+    if (!imageSource) {
         throw std::runtime_error("CGImageSourceCreateWithData failed");
     }
 
-    CGImageRef image = CGImageSourceCreateImageAtIndex(image_source, 0, NULL);
+    CGImageHandle image(CGImageSourceCreateImageAtIndex(*imageSource, 0, NULL));
     if (!image) {
-        CFRelease(image_source);
-        CFRelease(data);
         throw std::runtime_error("CGImageSourceCreateImageAtIndex failed");
     }
 
-    CGColorSpaceRef color_space = CGColorSpaceCreateDeviceRGB();
-    if (!color_space) {
-        CGImageRelease(image);
-        CFRelease(image_source);
-        CFRelease(data);
-        throw std::runtime_error("CGColorSpaceCreateDeviceRGB failed");
-    }
-
-    PremultipliedImage result({ static_cast<uint32_t>(CGImageGetWidth(image)),
-                                static_cast<uint32_t>(CGImageGetHeight(image)) });
-
-    CGContextRef context =
-        CGBitmapContextCreate(result.data.get(), result.size.width, result.size.height, 8,
-                              result.stride(), color_space, kCGImageAlphaPremultipliedLast);
-    if (!context) {
-        CGColorSpaceRelease(color_space);
-        CGImageRelease(image);
-        CFRelease(image_source);
-        CFRelease(data);
-        throw std::runtime_error("CGBitmapContextCreate failed");
-    }
-
-    CGContextSetBlendMode(context, kCGBlendModeCopy);
-
-    CGRect rect = { { 0, 0 },
-                    { static_cast<CGFloat>(result.size.width),
-                      static_cast<CGFloat>(result.size.height) } };
-    CGContextDrawImage(context, rect, image);
-
-    CGContextRelease(context);
-    CGColorSpaceRelease(color_space);
-    CGImageRelease(image);
-    CFRelease(image_source);
-    CFRelease(data);
-
-    return result;
+    return MGLPremultipliedImageFromCGImage(*image);
 }
 
-}
+} // namespace mbgl
diff --git a/platform/default/png_writer.cpp b/platform/default/png_writer.cpp
index 13d60c34fe..9ef9052158 100644
--- a/platform/default/png_writer.cpp
+++ b/platform/default/png_writer.cpp
@@ -72,7 +72,7 @@ std::string encodePNG(const PremultipliedImage& pre) {
                 (12 + idat.size() /* IDAT */) + (12 /* IEND */));
     png.append(preamble, 8);
     addChunk(png, "IHDR", ihdr, 13);
-    addChunk(png, "IDAT", idat.data(), idat.size());
+    addChunk(png, "IDAT", idat.data(), static_cast<uint32_t>(idat.size()));
     addChunk(png, "IEND");
     return png;
 }
diff --git a/platform/ios/config.cmake b/platform/ios/config.cmake
index 1150171c54..b37aae9f32 100644
--- a/platform/ios/config.cmake
+++ b/platform/ios/config.cmake
@@ -44,7 +44,9 @@ macro(mbgl_platform_core)
         PRIVATE platform/default/utf.cpp
 
         # Image handling
+        PRIVATE platform/darwin/mbgl/util/image+MGLAdditions.hpp
         PRIVATE platform/darwin/src/image.mm
+        PRIVATE platform/default/png_writer.cpp
 
         # Headless view
         PRIVATE platform/default/mbgl/gl/headless_backend.cpp
diff --git a/platform/ios/src/UIImage+MGLAdditions.mm b/platform/ios/src/UIImage+MGLAdditions.mm
index 7b5737f5e4..d99a1f73ed 100644
--- a/platform/ios/src/UIImage+MGLAdditions.mm
+++ b/platform/ios/src/UIImage+MGLAdditions.mm
@@ -1,42 +1,31 @@
 #import "UIImage+MGLAdditions.h"
 
+#include <mbgl/util/image+MGLAdditions.hpp>
+
 @implementation UIImage (MGLAdditions)
 
 - (nullable instancetype)initWithMGLSpriteImage:(const mbgl::SpriteImage *)spriteImage
 {
-    std::string png = encodePNG(spriteImage->image);
-    NSData *data = [[NSData alloc] initWithBytes:png.data() length:png.size()];
-    if (self = [self initWithData:data scale:spriteImage->pixelRatio])
+    CGImageRef image = CGImageFromMGLPremultipliedImage(spriteImage->image.clone());
+    if (!image) {
+        return nil;
+    }
+
+    if (self = [self initWithCGImage:image scale:spriteImage->pixelRatio orientation:UIImageOrientationUp])
     {
         if (spriteImage->sdf)
         {
             self = [self imageWithRenderingMode:UIImageRenderingModeAlwaysTemplate];
         }
     }
+    CGImageRelease(image);
     return self;
 }
 
-- (std::unique_ptr<mbgl::SpriteImage>)mgl_spriteImage
-{
-    CGImageRef cgImage = self.CGImage;
-    size_t width = CGImageGetWidth(cgImage);
-    size_t height = CGImageGetHeight(cgImage);
-    CGColorSpaceRef colorSpace = CGColorSpaceCreateDeviceRGB();
-    mbgl::PremultipliedImage cPremultipliedImage({ static_cast<uint32_t>(width), static_cast<uint32_t>(height) });
-    size_t bytesPerPixel = 4;
-    size_t bytesPerRow = bytesPerPixel * width;
-    size_t bitsPerComponent = 8;
-
-    CGContextRef context = CGBitmapContextCreate(cPremultipliedImage.data.get(),
-      width, height, bitsPerComponent, bytesPerRow,
-      colorSpace, kCGImageAlphaPremultipliedLast);
-
-    CGContextDrawImage(context, CGRectMake(0, 0, width, height), cgImage);
-    CGContextRelease(context);
-    CGColorSpaceRelease(colorSpace);
-
+- (std::unique_ptr<mbgl::SpriteImage>)mgl_spriteImage {
     BOOL isTemplate = self.renderingMode == UIImageRenderingModeAlwaysTemplate;
-    return std::make_unique<mbgl::SpriteImage>(std::move(cPremultipliedImage), float(self.scale), isTemplate);
+    return std::make_unique<mbgl::SpriteImage>(MGLPremultipliedImageFromCGImage(self.CGImage),
+                                               float(self.scale), isTemplate);
 }
 
 @end
diff --git a/platform/macos/config.cmake b/platform/macos/config.cmake
index d7a9c894b8..c8927cec3b 100644
--- a/platform/macos/config.cmake
+++ b/platform/macos/config.cmake
@@ -40,7 +40,9 @@ macro(mbgl_platform_core)
         PRIVATE platform/default/utf.cpp
 
         # Image handling
+        PRIVATE platform/darwin/mbgl/util/image+MGLAdditions.hpp
         PRIVATE platform/darwin/src/image.mm
+        PRIVATE platform/default/png_writer.cpp
 
         # Headless view
         PRIVATE platform/default/mbgl/gl/headless_backend.cpp
diff --git a/platform/macos/src/MGLMapView.mm b/platform/macos/src/MGLMapView.mm
index e9d8fc5f45..e93e4a40c5 100644
--- a/platform/macos/src/MGLMapView.mm
+++ b/platform/macos/src/MGLMapView.mm
@@ -787,9 +787,7 @@ public:
 
         if (_isPrinting) {
             _isPrinting = NO;
-            std::string png = encodePNG(_mbglView->readStillImage());
-            NSData *data = [[NSData alloc] initWithBytes:png.data() length:png.size()];
-            NSImage *image = [[NSImage alloc] initWithData:data];
+            NSImage *image = [[NSImage alloc] initWithMGLPremultipliedImage:_mbglView->readStillImage()];
             [self performSelector:@selector(printWithImage:) withObject:image afterDelay:0];
         }
 
diff --git a/platform/macos/src/NSImage+MGLAdditions.h b/platform/macos/src/NSImage+MGLAdditions.h
index ee01a763a3..c6a80e372d 100644
--- a/platform/macos/src/NSImage+MGLAdditions.h
+++ b/platform/macos/src/NSImage+MGLAdditions.h
@@ -6,6 +6,8 @@ NS_ASSUME_NONNULL_BEGIN
 
 @interface NSImage (MGLAdditions)
 
+- (nullable instancetype)initWithMGLPremultipliedImage:(mbgl::PremultipliedImage&&)image;
+
 - (nullable instancetype)initWithMGLSpriteImage:(const mbgl::SpriteImage *)spriteImage;
 
 - (std::unique_ptr<mbgl::SpriteImage>)mgl_spriteImage;
diff --git a/platform/macos/src/NSImage+MGLAdditions.mm b/platform/macos/src/NSImage+MGLAdditions.mm
index 9c30d3c37b..397e291431 100644
--- a/platform/macos/src/NSImage+MGLAdditions.mm
+++ b/platform/macos/src/NSImage+MGLAdditions.mm
@@ -1,11 +1,28 @@
 #import "NSImage+MGLAdditions.h"
 
+#include <mbgl/util/image+MGLAdditions.hpp>
+
 @implementation NSImage (MGLAdditions)
 
+- (nullable instancetype)initWithMGLPremultipliedImage:(mbgl::PremultipliedImage&&)src {
+    CGImageRef image = CGImageFromMGLPremultipliedImage(std::move(src));
+    if (!image) {
+        return nil;
+    }
+
+    self = [self initWithCGImage:image size:NSZeroSize];
+    CGImageRelease(image);
+    return self;
+}
+
 - (nullable instancetype)initWithMGLSpriteImage:(const mbgl::SpriteImage *)spriteImage {
-    std::string png = encodePNG(spriteImage->image);
-    NSData *data = [[NSData alloc] initWithBytes:png.data() length:png.size()];
-    NSBitmapImageRep *rep = [NSBitmapImageRep imageRepWithData:data];
+    CGImageRef image = CGImageFromMGLPremultipliedImage(spriteImage->image.clone());
+    if (!image) {
+        return nil;
+    }
+
+    NSBitmapImageRep *rep = [[NSBitmapImageRep alloc] initWithCGImage:image];
+    CGImageRelease(image);
     if (self = [self initWithSize:NSMakeSize(spriteImage->getWidth(), spriteImage->getHeight())]) {
         [self addRepresentation:rep];
         [self setTemplate:spriteImage->sdf];