[core] Fix image type of Map::renderStill

It's a premultiplied image. This implies that we were misusing encodePNG
in most cases, as we were passing premultiplied pixels which were then
interpreted as unmultiplied. I changed encodePNG to accept premultipled
pixels, and unpremultiply in the implementations.

8 files changed, 19 insertions, 46 deletions

diff --git a/test/api/annotations.cpp b/test/api/annotations.cpp
index c4fea02079..ae9c804730 100644
--- a/test/api/annotations.cpp
+++ b/test/api/annotations.cpp
@@ -14,16 +14,16 @@
 
 using namespace mbgl;
 
-UnassociatedImage render(Map& map) {
-    std::promise<UnassociatedImage> promise;
-    map.renderStill([&](std::exception_ptr, UnassociatedImage&& image) {
+PremultipliedImage render(Map& map) {
+    std::promise<PremultipliedImage> promise;
+    map.renderStill([&](std::exception_ptr, PremultipliedImage&& image) {
         promise.set_value(std::move(image));
     });
     return std::move(promise.get_future().get());
 }
 
 void checkRendering(Map& map, const char * name) {
-    UnassociatedImage actual = render(map);
+    PremultipliedImage actual = render(map);
     test::checkImage(std::string("test/fixtures/annotations/") + name + "/",
                      actual, 0.0002, 0.1);
 }
diff --git a/test/api/api_misuse.cpp b/test/api/api_misuse.cpp
index 4d5fe11042..538f09a040 100644
--- a/test/api/api_misuse.cpp
+++ b/test/api/api_misuse.cpp
@@ -45,7 +45,7 @@ TEST(API, RenderWithoutStyle) {
     Map map(view, fileSource, MapMode::Still);
 
     std::promise<std::exception_ptr> promise;
-    map.renderStill([&promise](std::exception_ptr error, UnassociatedImage&&) {
+    map.renderStill([&promise](std::exception_ptr error, PremultipliedImage&&) {
         promise.set_value(error);
     });
 
diff --git a/test/api/repeated_render.cpp b/test/api/repeated_render.cpp
index 79349c67e7..77e4569dbf 100644
--- a/test/api/repeated_render.cpp
+++ b/test/api/repeated_render.cpp
@@ -25,8 +25,8 @@ TEST(API, RepeatedRender) {
 
     {
         map.setStyleJSON(style, "");
-        std::promise<UnassociatedImage> promise;
-        map.renderStill([&promise](std::exception_ptr, UnassociatedImage&& image) {
+        std::promise<PremultipliedImage> promise;
+        map.renderStill([&promise](std::exception_ptr, PremultipliedImage&& image) {
             promise.set_value(std::move(image));
         });
         auto result = std::move(promise.get_future().get());
@@ -37,8 +37,8 @@ TEST(API, RepeatedRender) {
 
     {
         map.setStyleJSON(style, "TEST_DATA/suite");
-        std::promise<UnassociatedImage> promise;
-        map.renderStill([&promise](std::exception_ptr, UnassociatedImage&& image) {
+        std::promise<PremultipliedImage> promise;
+        map.renderStill([&promise](std::exception_ptr, PremultipliedImage&& image) {
             promise.set_value(std::move(image));
         });
         auto result = std::move(promise.get_future().get());
diff --git a/test/fixtures/util.cpp b/test/fixtures/util.cpp
index aa6371a4c7..ebd530eaac 100644
--- a/test/fixtures/util.cpp
+++ b/test/fixtures/util.cpp
@@ -85,45 +85,17 @@ uint64_t crc64(const std::string& str) {
     return crc64(str.data(), str.size());
 }
 
-UnassociatedImage unpremultiply(const PremultipliedImage& src) {
-    UnassociatedImage dst { src.width, src.height };
-    std::copy(src.data.get(), src.data.get() + src.size(), dst.data.get());
-
-    uint8_t* data = dst.data.get();
-    for (size_t i = 0; i < dst.size(); i += 4) {
-        uint8_t& r = data[i + 0];
-        uint8_t& g = data[i + 1];
-        uint8_t& b = data[i + 2];
-        uint8_t& a = data[i + 3];
-        if (a) {
-            r = (255 * r + (a / 2)) / a;
-            g = (255 * g + (a / 2)) / a;
-            b = (255 * b + (a / 2)) / a;
-        }
-    }
-
-    return std::move(dst);
-}
-
 void checkImage(const std::string& base,
-                const UnassociatedImage& actual,
+                const PremultipliedImage& actual,
                 double imageThreshold,
                 double pixelThreshold) {
-    // TODO: the pixels produced by Map::renderStill are probably actually premultiplied,
-    // but Map::renderStill produces an UnassociatedImage. This probably should be fixed;
-    // here we just hack around it by copying the pixels to a PremultipliedImage (and
-    // un-premultiplying them when updating expected.png, since encodePNG wants
-    // un-premultiplied pixels).
-    PremultipliedImage actualActual { actual.width, actual.height };
-    std::copy(actual.data.get(), actual.data.get() + actual.size(), actualActual.data.get());
-
     if (getenv("UPDATE")) {
-        util::write_file(base + "/expected.png", encodePNG(unpremultiply(actualActual)));
+        util::write_file(base + "/expected.png", encodePNG(actual));
         return;
     }
 
     PremultipliedImage expected = decodeImage(util::read_file(base + "/expected.png"));
-    UnassociatedImage diff { expected.width, expected.height };
+    PremultipliedImage diff { expected.width, expected.height };
 
     ASSERT_EQ(expected.width, actual.width);
     ASSERT_EQ(expected.height, actual.height);
diff --git a/test/fixtures/util.hpp b/test/fixtures/util.hpp
index b369979e9c..16edfbace3 100644
--- a/test/fixtures/util.hpp
+++ b/test/fixtures/util.hpp
@@ -25,7 +25,7 @@ uint64_t crc64(const char*, size_t);
 uint64_t crc64(const std::string&);
 
 void checkImage(const std::string& base,
-                const UnassociatedImage& actual,
+                const PremultipliedImage& actual,
                 double imageThreshold = 0,
                 double pixelThreshold = 0);
 
diff --git a/test/miscellaneous/bilinear.cpp b/test/miscellaneous/bilinear.cpp
index 740025bbbc..3184e5739d 100644
--- a/test/miscellaneous/bilinear.cpp
+++ b/test/miscellaneous/bilinear.cpp
@@ -1,6 +1,7 @@
 #include "../fixtures/util.hpp"
 #include <mbgl/util/compression.hpp>
 #include <mbgl/util/scaling.hpp>
+#include <mbgl/util/premultiply.hpp>
 #include <mbgl/util/image.hpp>
 #include <mbgl/util/io.hpp>
 
@@ -43,7 +44,7 @@ TEST(Bilinear, Scaling) {
     util::bilinearScale(srcData, srcSize, { 252, 380, 12, 12 }, dstData, dstSize, { 18, 90, 24, 24 }, false);
 
     const std::string data { reinterpret_cast<char *>(dstData), dstSize.x * dstSize.y * sizeof(uint32_t) };
-    util::write_file("test/fixtures/sprites/atlas_actual.png", encodePNG(dst));
+    util::write_file("test/fixtures/sprites/atlas_actual.png", encodePNG(util::premultiply(std::move(dst))));
     util::write_file("test/fixtures/sprites/atlas_actual.bin", util::compress(data));
 
     const std::string reference = util::decompress(util::read_file("test/fixtures/sprites/atlas_reference.bin"));
diff --git a/test/miscellaneous/image.cpp b/test/miscellaneous/image.cpp
index 8a321583cb..2db62b96d3 100644
--- a/test/miscellaneous/image.cpp
+++ b/test/miscellaneous/image.cpp
@@ -7,7 +7,7 @@
 using namespace mbgl;
 
 TEST(Image, PNGRoundTrip) {
-    UnassociatedImage rgba { 1, 1 };
+    PremultipliedImage rgba { 1, 1 };
     rgba.data[0] = 128;
     rgba.data[1] = 0;
     rgba.data[2] = 0;
@@ -21,14 +21,14 @@ TEST(Image, PNGRoundTrip) {
 }
 
 TEST(Image, PNGRoundTripAlpha) {
-    UnassociatedImage rgba { 1, 1 };
+    PremultipliedImage rgba { 1, 1 };
     rgba.data[0] = 128;
     rgba.data[1] = 0;
     rgba.data[2] = 0;
     rgba.data[3] = 128;
 
     PremultipliedImage image = decodeImage(encodePNG(rgba));
-    EXPECT_EQ(64, image.data[0]);
+    EXPECT_EQ(128, image.data[0]);
     EXPECT_EQ(0, image.data[1]);
     EXPECT_EQ(0, image.data[2]);
     EXPECT_EQ(128, image.data[3]);
diff --git a/test/style/pending_resources.cpp b/test/style/pending_resources.cpp
index 58dc72937e..81452ffc7e 100644
--- a/test/style/pending_resources.cpp
+++ b/test/style/pending_resources.cpp
@@ -39,7 +39,7 @@ TEST_P(PendingResources, DeleteMapObjectWithPendingRequest) {
     const std::string style = util::read_file("test/fixtures/resources/style.json");
     map->setStyleJSON(style, ".");
 
-    map->renderStill([&endTest](std::exception_ptr, UnassociatedImage&&) {
+    map->renderStill([&endTest](std::exception_ptr, PremultipliedImage&&) {
         EXPECT_TRUE(false) << "Should never happen.";
     });