Support RGBA/BGRA Premul/Unpremul from SkPNGImageEncoder

src/images/transform_scanline.h

Index: src/images/transform_scanline.h
diff --git a/src/images/transform_scanline.h b/src/images/transform_scanline.h
index 4baf6b6c6be25cecbf150ef31509afd45203ae1a..5fd05ef8057b0b0d0e6cc1f32955c9c936e0dd78 100644
--- a/src/images/transform_scanline.h
+++ b/src/images/transform_scanline.h
@@ -20,15 +20,13 @@
  * Transform 'width' pixels from 'src' buffer into 'dst' buffer,
  * repacking color channel data as appropriate for the given transformation.
  */
-typedef void (*transform_scanline_proc)(const char* SK_RESTRICT src,
-                                        int width, char* SK_RESTRICT dst);
+typedef void (*transform_scanline_proc)(uint8_t* dst, const void* src, int width, int bpp);

[scroggo, 2016/09/12 17:16:50]

Just out of curiosity, why did you switch to uint8_t? (It seems like we are
generally inconsistent, and sometimes use char. Maybe one is better, but I do
want to make sure we don't change a particular use back and forth, even though
we'll probably have the bad choice used in various places.)

Also, I asked this elsewhere, but why the removal of SK_RESTRICT?

[msarett, 2016/09/12 19:39:20]

You're right, back to char and SK_RESTRICT.

 
 /**
  * Identity transformation: just copy bytes from src to dst.
  */
-static void transform_scanline_memcpy(const char* SK_RESTRICT src, int width,
-                                      char* SK_RESTRICT dst) {
-    memcpy(dst, src, width);
+static void transform_scanline_memcpy(uint8_t* dst, const void* src, int width, int bpp) {
+    memcpy(dst, src, width * bpp);
 }
 
 /**
@@ -36,9 +34,8 @@ static void transform_scanline_memcpy(const char* SK_RESTRICT src, int width,
  * Alpha channel data is not present in kRGB_565_Config format, so there is no
  * alpha channel data to preserve.
  */
-static void transform_scanline_565(const char* SK_RESTRICT src, int width,
-                                   char* SK_RESTRICT dst) {
-    const uint16_t* SK_RESTRICT srcP = (const uint16_t*)src;
+static void transform_scanline_565(uint8_t* dst, const void* src, int width, int) {
+    const uint16_t* srcP = (const uint16_t*)src;
     for (int i = 0; i < width; i++) {
         unsigned c = *srcP++;
         *dst++ = SkPacked16ToR32(c);
@@ -48,17 +45,30 @@ static void transform_scanline_565(const char* SK_RESTRICT src, int width,
 }
 
 /**
- * Transform from kARGB_8888_Config to 3-bytes-per-pixel RGB.
- * Alpha channel data, if any, is abandoned.
+ * Transform from kRGBA_8888_SkColorType to 3-bytes-per-pixel RGB.
+ * Alpha channel data is abandoned.
  */
-static void transform_scanline_888(const char* SK_RESTRICT src, int width,
-                                   char* SK_RESTRICT dst) {
-    const SkPMColor* SK_RESTRICT srcP = (const SkPMColor*)src;
+static void transform_scanline_RGB1(uint8_t* dst, const void* src, int width, int) {

[scroggo, 2016/09/12 17:16:50]

How'd you choose the "1" in the name? (I might be tempted to call it RGBX, since
we ignore the alpha.)

[msarett, 2016/09/12 19:39:20]

I think RGBX is a better name, switching.

+    const SkPMColor* srcP = (const SkPMColor*)src;
     for (int i = 0; i < width; i++) {
         SkPMColor c = *srcP++;
-        *dst++ = SkGetPackedR32(c);
-        *dst++ = SkGetPackedG32(c);
-        *dst++ = SkGetPackedB32(c);
+        *dst++ = (c >>  0) & 0xFF;
+        *dst++ = (c >>  8) & 0xFF;
+        *dst++ = (c >> 16) & 0xFF;
+    }
+}
+
+/**
+ * Transform from kBGRA_8888_SkColorType to 3-bytes-per-pixel RGB.
+ * Alpha channel data is abandoned.
+ */
+static void transform_scanline_BGR1(uint8_t* dst, const void* src, int width, int) {
+    const SkPMColor* srcP = (const SkPMColor*)src;
+    for (int i = 0; i < width; i++) {
+        SkPMColor c = *srcP++;

[scroggo, 2016/09/12 17:16:50]

One of these uses of SkPMColor is a lie, but I suppose we don't know which one
unless we look at the pm color macro...

[msarett, 2016/09/12 19:39:20]

Let's use uint32_t...

+        *dst++ = (c >> 16) & 0xFF;
+        *dst++ = (c >>  8) & 0xFF;
+        *dst++ = (c >>  0) & 0xFF;
     }
 }
 
@@ -66,9 +76,8 @@ static void transform_scanline_888(const char* SK_RESTRICT src, int width,
  * Transform from kARGB_4444_Config to 3-bytes-per-pixel RGB.
  * Alpha channel data, if any, is abandoned.
  */
-static void transform_scanline_444(const char* SK_RESTRICT src, int width,
-                                   char* SK_RESTRICT dst) {
-    const SkPMColor16* SK_RESTRICT srcP = (const SkPMColor16*)src;
+static void transform_scanline_444(uint8_t* dst, const void* src, int width, int) {
+    const SkPMColor16* srcP = (const SkPMColor16*)src;
     for (int i = 0; i < width; i++) {
         SkPMColor16 c = *srcP++;
         *dst++ = SkPacked4444ToR32(c);
@@ -78,22 +87,45 @@ static void transform_scanline_444(const char* SK_RESTRICT src, int width,
 }
 
 /**
- * Transform from kARGB_8888_Config to 4-bytes-per-pixel RGBA.
- * (This would be the identity transformation, except for byte-order and
- * scaling of RGB based on alpha channel).
+ * Transform from kPremul, kRGBA_8888_SkColorType to 4-bytes-per-pixel unpremultiplied RGBA.
+ */
+static void transform_scanline_rgbA(uint8_t* dst, const void* src, int width, int) {
+    const SkPMColor* srcP = (const SkPMColor*)src;
+    const SkUnPreMultiply::Scale* table = SkUnPreMultiply::GetScaleTable();
+
+    for (int i = 0; i < width; i++) {
+        SkPMColor c = *srcP++;
+        unsigned r = (c >>  0) & 0xFF;
+        unsigned g = (c >>  8) & 0xFF;
+        unsigned b = (c >> 16) & 0xFF;
+        unsigned a = (c >> 24) & 0xFF;
+
+        if (0 != a && 255 != a) {
+            SkUnPreMultiply::Scale scale = table[a];
+            r = SkUnPreMultiply::ApplyScale(scale, r);
+            g = SkUnPreMultiply::ApplyScale(scale, g);
+            b = SkUnPreMultiply::ApplyScale(scale, b);
+        }
+        *dst++ = r;
+        *dst++ = g;
+        *dst++ = b;
+        *dst++ = a;
+    }
+}
+
+/**
+ * Transform from kPremul, kBGRA_8888_SkColorType to 4-bytes-per-pixel unpremultiplied RGBA.
  */
-static void transform_scanline_8888(const char* SK_RESTRICT src, int width,
-                                    char* SK_RESTRICT dst) {
-    const SkPMColor* SK_RESTRICT srcP = (const SkPMColor*)src;
-    const SkUnPreMultiply::Scale* SK_RESTRICT table =
-                                              SkUnPreMultiply::GetScaleTable();
+static void transform_scanline_bgrA(uint8_t* dst, const void* src, int width, int) {
+    const SkPMColor* srcP = (const SkPMColor*)src;
+    const SkUnPreMultiply::Scale* table = SkUnPreMultiply::GetScaleTable();
 
     for (int i = 0; i < width; i++) {
         SkPMColor c = *srcP++;
-        unsigned a = SkGetPackedA32(c);
-        unsigned r = SkGetPackedR32(c);
-        unsigned g = SkGetPackedG32(c);
-        unsigned b = SkGetPackedB32(c);
+        unsigned r = (c >> 16) & 0xFF;

[scroggo, 2016/09/12 17:16:50]

Can we share code between these two methods (maybe with a template)? I think
only this block is different?

[msarett, 2016/09/12 19:39:20]

SGTM

+        unsigned g = (c >>  8) & 0xFF;
+        unsigned b = (c >>  0) & 0xFF;
+        unsigned a = (c >> 24) & 0xFF;
 
         if (0 != a && 255 != a) {
             SkUnPreMultiply::Scale scale = table[a];
@@ -109,14 +141,26 @@ static void transform_scanline_8888(const char* SK_RESTRICT src, int width,
 }
 
 /**
+ * Transform from kUnpremul, kBGRA_8888_SkColorType to 4-bytes-per-pixel unpremultiplied RGBA.
+ */
+static void transform_scanline_BGRA(uint8_t* dst, const void* src, int width, int) {
+    const SkPMColor* srcP = (const SkPMColor*)src;
+    for (int i = 0; i < width; i++) {
+        SkPMColor c = *srcP++;
+        *dst++ = (c >> 16) & 0xFF;
+        *dst++ = (c >>  8) & 0xFF;
+        *dst++ = (c >>  0) & 0xFF;
+        *dst++ = (c >> 24) & 0xFF;
+    }
+}
+
+/**
  * Transform from kARGB_8888_Config to 4-bytes-per-pixel RGBA,
  * with scaling of RGB based on alpha channel.
  */
-static void transform_scanline_4444(const char* SK_RESTRICT src, int width,
-                                    char* SK_RESTRICT dst) {
-    const SkPMColor16* SK_RESTRICT srcP = (const SkPMColor16*)src;
-    const SkUnPreMultiply::Scale* SK_RESTRICT table =
-                                              SkUnPreMultiply::GetScaleTable();
+static void transform_scanline_4444(uint8_t* dst, const void* src, int width, int) {
+    const SkPMColor16* srcP = (const SkPMColor16*)src;
+    const SkUnPreMultiply::Scale* table = SkUnPreMultiply::GetScaleTable();
 
     for (int i = 0; i < width; i++) {
         SkPMColor16 c = *srcP++;